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

Fix the ResponderRequest input/output structures #328

Merged
merged 1 commit into from
Sep 1, 2021

Conversation

CerealBoy
Copy link
Contributor

The doc for incidents/{id}/responder_requests is mis-leading, in that the description, example, and formatted doc each show a different structure. This morning I worked with support to actually determine the necessary structure here, so have updated the input and output structure here to resolve this.

For any implementation of this previously the structure of the code will require updating, but as this was already broken this should be fine?

Currently:

{
  "requester_id": "identifier",
  "message": "Help has been requested",
  "responder_request_targets": [
    {
      "type": "escalation_policy",
      "id": "identifier"
    }
  ]
}

Should be:

{
  "requester_id": "identifier",
  "message": "Help has been requested",
  "responder_request_targets": [
    {
      "responder_request_target": {
        "type": "escalation_policy",
        "id": "identifier"
      }
    }
  ]
}

@asabitov
Copy link

asabitov commented May 4, 2021

Ah, that makes sense, thought it is not really natural.
fruits: [apple:{color,red}, pear:{color,green}]

Copy link

@asabitov asabitov left a comment

Choose a reason for hiding this comment

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

Well, as long as it works :)

@theckman
Copy link
Collaborator

theckman commented May 4, 2021

#233 does basically the same thing, merging it was blocked waiting on v1.4.0 release (since this is a breaking change). If that PR isn't updated relatively soon we'll move forward with reviewing this one.

@theckman theckman added the breaking change This PR contains a breaking change label May 4, 2021
@CerealBoy
Copy link
Contributor Author

#233 does basically the same thing, merging it was blocked waiting on v1.4.0 release (since this is a breaking change). If that PR isn't updated relatively soon we'll move forward with reviewing this one.

Oh, I should've checked! Happy for this to be closed in favour of the other 👍

@theckman theckman added this to the v1.5.0 milestone May 8, 2021
@theckman theckman linked an issue May 16, 2021 that may be closed by this pull request
@CerealBoy
Copy link
Contributor Author

@theckman I wanted to check-in to see if there's been any update on when this can be brought in?

@theckman
Copy link
Collaborator

theckman commented Jun 27, 2021

@CerealBoy So, I've enough access in this repo to review and merge the PRs of others, but not myself. I'm waiting for PagerDuty staff to review #325 so I can merge it, and then I can merge external PRs that are going to go into v1.5.0 (this being one of them).

@CerealBoy
Copy link
Contributor Author

That's great, thank you very much for the update! Hopefully they get back soon 🤞

Copy link
Collaborator

@theckman theckman left a comment

Choose a reason for hiding this comment

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

Alright! v1.5.0 merge window has opened, so I'll get this brought-in to be included in that release once it's cut. No timeline for that just yet, but I hope to get it all hammered out within the coming weeks.

@theckman theckman merged commit 340f6d5 into PagerDuty:master Sep 1, 2021
@theckman
Copy link
Collaborator

theckman commented Sep 1, 2021

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This PR contains a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improper unmarshalling
3 participants