Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding DeletedAt variable for deleted user from an EP #425

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

priyankasekhar
Copy link

Adding the 'deleted_at' variable for a user. This will return the date/time a user was deleted from an escalation policy and is returned by /oncalls Pagerduty API today.

@priyankasekhar priyankasekhar changed the title Update client.go Adding DeletedAt variable for deleted user from an EP Feb 16, 2022
Copy link
Collaborator

@theckman theckman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you point me to where PagerDuty documents the deleted_at field for the API call you're talking about? I checked here, but did not see it: https://developer.pagerduty.com/api-reference/b3A6Mjc0ODE2Mw-list-all-of-the-on-calls. We only add support for things that PagerDuty has publicly documented.

Separate from that, I don't believe where we are trying to add this field is the right place as not all objects include deleted_at.

client.go Outdated
@@ -51,6 +51,7 @@ type APIObject struct {
Summary string `json:"summary,omitempty"`
Self string `json:"self,omitempty"`
HTMLURL string `json:"html_url,omitempty"`
DeletedAt string `json:"deleted_at,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@priyankasekhar as the majority of API endpoints do not return this field, I don't think it's appropriate to place it on that type.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theckman You're right, ideally when all the users part of an escalation policy are active in Pagerduty, the deleted_at variable is not present in the response. It's only returned when a user is marked 'XYZ -inactive'. Their documentation doesn't really reference to this variable. But here's an example of such a scenario:
Screen Shot 2022-02-16 at 3 44 50 PM

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it'll make more sense to add this variable here: https://github.com/PagerDuty/go-pagerduty/blob/master/user.go#L38 ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the documentation they shared with me on "what happens when a user is deleted and that user is currently on an escalation policy". Although It doesn't mention the deleted_at variable : Gaps in Escalation Policies and Scheduling Settings.

Additionally, if you navigate to their API documentation for the Get an escalation policy endpoint, and provide an escalation policy ID for an escalation policy with inactive users you will see that the response contains the deleted_at variable.

Does this help at all?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that it makes sense to add it to the User object, as I'm not sure I've seen deleted_at get mentioned for users anywhere else. It'd be helpful to know where-all PagerDuty can return the deleted_at field.

That being visible in the JSON output doesn't help, unfortunately. Per policy set on this project by PagerDuty employees, the field needs to be listed in the documentation for it to be supported by this package.

I don't personally have an inside way to ask for it to be fixed, and generally requires someone reach out to support@pagerduty.com to ask they do it.

DeletedAt variable only shows up against a deleted user marked as -inactive and still part of an escalation policy
@theckman theckman added the undocumented feature This PR or issue is trying to make use of a feature not yet documented by PagerDuty label Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
undocumented feature This PR or issue is trying to make use of a feature not yet documented by PagerDuty
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants