Fix ResponderRequest regression #452
Conversation
incident.go
Outdated
| } | ||
|
|
||
| // ResponderRequestTargets is a wrapper for a ResponderRequestTarget. | ||
| type ResponderRequestTargets struct { |
There was a problem hiding this comment.
I know this is a little strange but could we call this something like ResponderRequestTargetWrapper? Calling it ResponderRequestTargets [plural] is a little strange because it isn't actually a list or collection of any kind...
There was a problem hiding this comment.
@ChuckCrawford Thanks so much for taking a look at this! And I totally agree - ResponderRequestTargetWrapper is both clearer and more accurate (I had noticed that false pluralization as well, but I chickened out of changing the name and just left it as it was named in its last working state). Edit made, and squashed the commits to keep the history tidier.
There was a problem hiding this comment.
@ehlerorngard : would you mind having a look at the one small comment I left? Then this should be good to go.
This provides a fix for issue #451 where ResponderRequest has been broken (requests are returned a 2100 Not Found).
The PR also omits
incident_respondersfrom the request when empty, as it's not required/useful.I fixed the related test in
incident_test.goand tested successfully both with that test and my own test which actually sent the successful POSTs to the/incidents/{id}/responder_requestsendpoint.