Validate that all structure formats and method signatures work with PagerDuty API #389
Closed
26 tasks done
Assignees
Milestone
Comments
theckman
added a commit
that referenced
this issue
Nov 16, 2021
This change includes the first few fixes while reviewing the repo for #389, and does include one breaking change. In `change_events.go`, there were a few optional fields that were not set to `omitempty`, which means we were rendering them in the JSON even when not set. This also reorders the `ChangeEventPayload` so that the `Summary` field is at the top, since it's the only required field. In `escalation_policy.go`, the `EscalationPolicy` type looked to have a field on it that indicated if repeating the notification rules was enabled. However, this field is not documented and when looking at the actual API response no such field was present. Instead, the field to indicate the number of loops is set to `0` when there is no repeating of the rules. The removal of this field is the breaking change, although hopefully nobody was using it. :) Updates #389
theckman
added a commit
that referenced
this issue
Nov 25, 2021
While looking for API shape mismatches for #389, I stumbled upon a major issue with our pagination parameters across the entire package. This fixes that discrepency, but is done via a pretty intrusive breaking change by removing the embedded `APIListObject` from all `List*Options` types, replacing them with three distinct fields: `Limit`, `Offset`, and `Total`. For the traditional API pagination, there is a `total` URL query parameter that tells the API to include the total count of records in the response. For the query parameter it's a bool flag, with a value of `true` telling the API to include the count. When the response comes back from the API, there is a `total` field in the JSON that is the total count of items in the collection. This value is only set if that `total` query parameter is set to `true` because it's an expensive operation that PagerDuty recommends you don't enable. The problem is we tried to reuse the `APIListObject` struct type for both the pagination URL query parameters, and the pagination fields in the JSON response body. The issue with doing that is for requests the `Total` field needs to be a bool, but within the `APIListObject` it's a `uint` because it was really only designed to be used for response body parsing. So since we're intending to remove the embedded struct types in v2 (#318), it feels like the best course of action here is to move forward with that plan for these paginated query parameter struct types so that we can conform to the PagerDuty API spec. Related to #318 Related to #389
theckman
added a commit
that referenced
this issue
Nov 25, 2021
While looking for API shape mismatches for #389, I stumbled upon a major issue with our pagination parameters across the entire package. This fixes that discrepency, but is done via a pretty intrusive breaking change by removing the embedded `APIListObject` from all `List*Options` types, replacing them with three distinct fields: `Limit`, `Offset`, and `Total`. For the traditional API pagination, there is a `total` URL query parameter that tells the API to include the total count of records in the response. For the query parameter it's a bool flag, with a value of `true` telling the API to include the count. When the response comes back from the API, there is a `total` field in the JSON that is the total count of items in the collection. This value is only set if that `total` query parameter is set to `true` because it's an expensive operation that PagerDuty recommends you don't enable. The problem is we tried to reuse the `APIListObject` struct type for both the pagination URL query parameters, and the pagination fields in the JSON response body. The issue with doing that is for requests the `Total` field needs to be a bool, but within the `APIListObject` it's a `uint` because it was really only designed to be used for response body parsing. So since we're intending to remove the embedded struct types in v2 (#318), it feels like the best course of action here is to move forward with that plan for these paginated query parameter struct types so that we can conform to the PagerDuty API spec. Related to #318 Related to #389
theckman
added a commit
that referenced
this issue
Jan 16, 2022
theckman
added a commit
that referenced
this issue
Jan 16, 2022
theckman
added a commit
that referenced
this issue
Jan 16, 2022
theckman
added a commit
that referenced
this issue
Jan 18, 2022
theckman
added a commit
that referenced
this issue
Jan 18, 2022
theckman
added a commit
that referenced
this issue
Jan 18, 2022
theckman
added a commit
that referenced
this issue
Jan 18, 2022
theckman
added a commit
that referenced
this issue
Jan 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We're making
v1.5.0a special release, where we introduce breaking changes to fix issues within this package that probably never worked since being merged. Since we've fixed so many things so far, we should go over the entire package to make sure there are no outstanding issues.Files to check:
The text was updated successfully, but these errors were encountered: