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

Add json field incidents_responders to Incident struct #365

Merged
merged 2 commits into from
Jan 16, 2022
Merged

Add json field incidents_responders to Incident struct #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d115d39
Add json field incidents_responders to Incident struct
sostakas Sep 23, 2021
fab90ea
Ensure incidents_responders field is being passed within the request …
sostakas Sep 28, 2021
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 incident.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type Incident struct {
Body IncidentBody `json:"body,omitempty"`
IsMergeable bool `json:"is_mergeable,omitempty"`
ConferenceBridge *ConferenceBridge `json:"conference_bridge,omitempty"`
IncidentResponders []IncidentResponders `json:"incidents_responders,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you implement a test that ensures this field is being passed along in the JSON within the request body?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theckman Thanks for comment. I added the test. It is not documented but you can check response body using Try it. This field is must to test if additional responders were added successfully (if you want to stick to go-pagerduty and do not want to create your own http requests)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for that context. The policy set by PagerDuty on this library is that only fields that are explicitly listed in the documentation are merged in.

I'll label this PR as being focused on an undocumented feature, and try to engage with PagerDuty to see if they'd be willing/able to document the field(s). I'm not an employee, so I'll probably end up filing a support ticket to make the request to have the documentation updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theckman Thank you!

}

// ListIncidentsResponse is the response structure when calling the ListIncident API endpoint.
Expand Down
4 changes: 2 additions & 2 deletions incident_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,15 @@ func TestIncident_Get(t *testing.T) {

mux.HandleFunc("/incidents/1", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
_, _ = w.Write([]byte(`{"incident": {"id": "1"}}`))
_, _ = w.Write([]byte(`{"incident": {"id": "1", "incidents_responders": [{"incident": {"id": "1"}}]}}`))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for doing this. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theckman 👍

})

client := defaultTestClient(server.URL, "foo")

id := "1"
res, err := client.GetIncident(id)

want := &Incident{Id: "1"}
want := &Incident{Id: "1", IncidentResponders: []IncidentResponders{{Incident: APIObject{ID: "1"}}}}

if err != nil {
t.Fatal(err)
Expand Down