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

Handle Incident Log Entries of User Notification #201

Closed

Conversation

joez-tkpd
Copy link

@joez-tkpd joez-tkpd commented Feb 19, 2020

Add User struct on LogEntries

Because on Incident's Log Entries /incidents/{id}/log_entries when "type": "notify_log_entry"
the payload doesn't contain any information about the notified user except on user object

additionally add Service on LogEntries
additionally Channel also missed Notification object

@stmcallister
Copy link
Contributor

Thanks for submitting this PR! As a rule, we're not adding support for any undocumented API features. And, as of right now, none of these fields are documented in the PagerDuty API Reference. I'm working with the engineering team that owns this endpoint to see if we can change that. Will keep you posted.

Until then, the Service and User fields should probably be of type APIObject as that type more accurately represents the objects on the log_entries endpoint.

@obl1v1us
Copy link

Hi @stmcallister ! I'm also interested in retrieving User data from the LogEntries. Has there been any progress getting this added to the library? Or are we still waiting on having the Pagerduty API reference updated before adding this change?

@theckman
Copy link
Collaborator

@joez-tkpd @obl1v1us I'm not a PD employee so I don't have any inside baseball, but it seems like the API documentation still has not been updated to include these fields:

https://developer.pagerduty.com/api-reference/reference/REST/openapiv3.json/paths/~1incidents~1%7Bid%7D~1log_entries/get

This branch is also conflicting with master and cannot be merged in its current form so we would need to get it updated, and it may also want to be merged with #264 as it seems like the Channel type (which is what's conflicting) is not correct in master.

@stmcallister can you speak to the documentation status?

@theckman theckman added awaiting refresh branch / PR is out of date open question there is an open question on the issue / PR undocumented feature This PR or issue is trying to make use of a feature not yet documented by PagerDuty labels Feb 20, 2021
@theckman
Copy link
Collaborator

@joez-tkpd with #264 merged, some of what you're trying to accomplish is now possible. The Channel type has a Raw field on it that's a map[string]interface{} of all the fields (except for type) sent via JSON. You should be able to start using this once we release v1.4.0.

I'll leave the question about the API documentation for Scott.

@theckman
Copy link
Collaborator

@joez-tkpd I'm thinking of raising a support ticket to PagerDuty to ask about the user field, but I am not able to find a log entry on my development account that presents that field.

Do you have an example log entry where the user field is sent by PagerDuty?

@theckman
Copy link
Collaborator

Ah I found it.

@stmcallister
Copy link
Contributor

Hey everyone! Sorry for taking so long on this one. After trying to get one of the engineering teams to update the PagerDuty API Docs to match this PR I took matters into my own hands and figured out how to do it myself. The PR has been approved and should be merged in the coming days. Thanks for your patience!

@stmcallister
Copy link
Contributor

Log Entry Docs are updated.

@theckman
Copy link
Collaborator

theckman commented Nov 9, 2021

@joez-tkpd this will be part of v1.5.0 when released (merged via #377).

@joez-tkpd joez-tkpd deleted the handle_log_entry_notify_user branch August 4, 2022 04:18
@joez-tkpd joez-tkpd restored the handle_log_entry_notify_user branch August 4, 2022 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting refresh branch / PR is out of date open question there is an open question on the issue / PR 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.

4 participants