Implement the Incident endpoint for ResponderRequest #196
Conversation
|
Cool! Thank you for submitting this pull request! I'll leave my comments inline! |
incident.go
Outdated
| @@ -380,4 +380,46 @@ type ListAlertResponse struct { | |||
| Alerts []Alert `json:"alerts,omitempty"` | |||
| } | |||
|
|
|||
| /* TODO: Manage Alerts, Get Alert, Create Status Updates, Create Responder Request, */ | |||
| // CreateIncidentResponderRequestResponse | |||
| type CreateIncidentResponderRequestResponse struct { | |||
There was a problem hiding this comment.
I think we'll go with a shorter name. Will you remove all the CreateIncidents and start the names at ResponderRequest? So, this struct would be ResponderRequestResponse.
incident.go
Outdated
|
|
||
| data["message"] = o.Message | ||
| data["request_id"] = o.RequesterID | ||
| data["responder_request_targets"] = o.Targets |
There was a problem hiding this comment.
Rather than piece out each field in the payload object, will you create a ResponderRequestPayload struct with json tags and add that to the ResponderRequestOptions?
incident_test.go
Outdated
| id := "1" | ||
| mux.HandleFunc("/incidents/"+id+"/responder_requests", func(w http.ResponseWriter, r *http.Request) { | ||
| testMethod(t, r, "POST") | ||
| w.Write([]byte(`{"requester_id": "PL1JMK5", "message": "Help", "responder_request_targets": [{"responder_request_target":{"id":"PJ25ZYX","type":"user_reference"}}]}`)) |
There was a problem hiding this comment.
This is supposed to match the JSON of the response you're expecting. Can you change it to reflect the response?
…del the response, fix tests to actually model the data properly, use the data structures as a part of the request processing where possible
|
Hi @stmcallister, thank you very much for the feedback, I've made the changes as you've requested. The only difference I have is to use the Looking at the original commit, I can't work out how I managed to think the response payload looked the way it did considering the docs are completely different, so I've made a bunch of changes to better model the expected response data from this request. |
|
Looks good! Thanks for the feature and for making those changes @CerealBoy ! |
Was hoping to use this functionality, but it lacks in the API here, so adding it in. Based on the docs this looks to be correct?
I wasn't entirely sure about the test case either, whether there is a way to provide better response content from the API (or have it generated).