Skip to content

Commit

Permalink
Fix overlapping struct fields & last golint errors
Browse files Browse the repository at this point in the history
There have been a few issues filed over the past few years, relative to
unexpected or unreliable behavior in the client, due to overlapping struct
fields because of an embedded type or conflicting JSON field tags. This could
make it appear like some fields are unset, because `encoding/json` has put the
value in the overlapped field and not the one the programmer is accessing.
Unfortunately, fixing these issues are technically a breaking API change and so
we waited to completely fix them until now.

In the spirit of making the struct fields consistently named, this also updates
the `HttpStatus` field in the `EventResponse` struct to be `HTTPStatus`. This is
purely cosmetic, but feels appropriate to do now since the removal if the `Id`
field makes it the last one.

Fixes #218
Fixes #316
Fixes #268
  • Loading branch information
theckman committed May 16, 2021
1 parent e23b94c commit 2f47dfc
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 13 deletions.
4 changes: 2 additions & 2 deletions event.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type EventResponse struct {
Status string `json:"status"`
Message string `json:"message"`
IncidentKey string `json:"incident_key"`
HttpStatus int
HTTPStatus int
}

// CreateEvent sends PagerDuty an event to trigger, acknowledge, or resolve a
Expand Down Expand Up @@ -55,7 +55,7 @@ func CreateEventWithHTTPClient(e Event, client HTTPClient) (*EventResponse, erro
defer func() { _ = resp.Body.Close() }() // explicitly discard error

if resp.StatusCode != http.StatusOK {
return &EventResponse{HttpStatus: resp.StatusCode}, fmt.Errorf("HTTP Status Code: %d", resp.StatusCode)
return &EventResponse{HTTPStatus: resp.StatusCode}, fmt.Errorf("HTTP Status Code: %d", resp.StatusCode)
}
var eventResponse EventResponse
if err := json.NewDecoder(resp.Body).Decode(&eventResponse); err != nil {
Expand Down
1 change: 0 additions & 1 deletion incident.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ type Incident struct {
Priority *Priority `json:"priority,omitempty"`
Urgency string `json:"urgency,omitempty"`
Status string `json:"status,omitempty"`
Id string `json:"id,omitempty"`
ResolveReason ResolveReason `json:"resolve_reason,omitempty"`
AlertCounts AlertCounts `json:"alert_counts,omitempty"`
Body IncidentBody `json:"body,omitempty"`
Expand Down
33 changes: 25 additions & 8 deletions incident_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ func TestIncident_List(t *testing.T) {
APIListObject: listObj,
Incidents: []Incident{
{
Id: "1",
APIObject: APIObject{
ID: "1",
},
},
},
}
Expand Down Expand Up @@ -54,8 +56,10 @@ func TestIncident_Create(t *testing.T) {
res, err := client.CreateIncident(from, input)

want := &Incident{
APIObject: APIObject{
ID: "1",
},
Title: "foo",
Id: "1",
Urgency: "low",
}

Expand Down Expand Up @@ -89,7 +93,9 @@ func TestIncident_Manage_status(t *testing.T) {
APIListObject: listObj,
Incidents: []Incident{
{
Id: "1",
APIObject: APIObject{
ID: "1",
},
Title: "foo",
Status: "acknowledged",
},
Expand Down Expand Up @@ -129,7 +135,9 @@ func TestIncident_Manage_priority(t *testing.T) {
APIListObject: listObj,
Incidents: []Incident{
{
Id: "1",
APIObject: APIObject{
ID: "1",
},
Title: "foo",
Priority: &Priority{
APIObject: APIObject{
Expand Down Expand Up @@ -184,7 +192,9 @@ func TestIncident_Manage_assignments(t *testing.T) {
APIListObject: listObj,
Incidents: []Incident{
{
Id: "1",
APIObject: APIObject{
ID: "1",
},
Title: "foo",
Assignments: []Assignment{
{
Expand Down Expand Up @@ -222,7 +232,12 @@ func TestIncident_Merge(t *testing.T) {
from := "foo@bar.com"

input := []MergeIncidentsOptions{{ID: "2", Type: "incident"}}
want := &Incident{Id: "1", Title: "foo"}
want := &Incident{
APIObject: APIObject{
ID: "1",
},
Title: "foo",
}

res, err := client.MergeIncidents(from, "1", input)
if err != nil {
Expand All @@ -245,7 +260,7 @@ func TestIncident_Get(t *testing.T) {
id := "1"
res, err := client.GetIncident(id)

want := &Incident{Id: "1"}
want := &Incident{APIObject: APIObject{ID: "1"}}

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -433,7 +448,9 @@ func TestIncident_SnoozeIncidentWithResponse(t *testing.T) {
res, err := client.SnoozeIncidentWithResponse(id, duration)

want := &Incident{
Id: "1",
APIObject: APIObject{
ID: "1",
},
PendingActions: []PendingAction{
{
Type: "unacknowledge",
Expand Down
1 change: 0 additions & 1 deletion service.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ type Integration struct {
Service *APIObject `json:"service,omitempty"`
CreatedAt string `json:"created_at,omitempty"`
Vendor *APIObject `json:"vendor,omitempty"`
Type string `json:"type,omitempty"`
IntegrationKey string `json:"integration_key,omitempty"`
IntegrationEmail string `json:"integration_email,omitempty"`
}
Expand Down
1 change: 0 additions & 1 deletion user.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ type NotificationRule struct {
// User is a member of a PagerDuty account that has the ability to interact with incidents and other data on the account.
type User struct {
APIObject
Type string `json:"type"`
Name string `json:"name"`
Summary string `json:"summary"`
Email string `json:"email"`
Expand Down

0 comments on commit 2f47dfc

Please sign in to comment.