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.

Sign up for GitHub

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

Jump to bottom

Adding DeletedAt variable for deleted user from an EP #425

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

Adding DeletedAt variable for deleted user from an EP #425

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebeb75d
Update client.go
priyankasekhar Feb 16, 2022
b72e56b
Removing DeletedAt variable from client.go
priyankasekhar Feb 17, 2022
b84dc56
Adding DeletedAt variable to user.go
priyankasekhar Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@priyankasekhar priyankasekhar Feb 17, 2022 •
edited
Loading

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 [email protected] to ask they do it.

}

// APIListObject are the fields used to control pagination when listing objects.
Expand Down