From 2f47dfc62321b7edfd19b3ce38483ee967294d8f Mon Sep 17 00:00:00 2001 From: Tim Heckman Date: Sat, 15 May 2021 23:54:39 -0700 Subject: [PATCH] Fix overlapping struct fields & last golint errors 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 --- event.go | 4 ++-- incident.go | 1 - incident_test.go | 33 +++++++++++++++++++++++++-------- service.go | 1 - user.go | 1 - 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/event.go b/event.go index c2438a6d..d2241f0a 100644 --- a/event.go +++ b/event.go @@ -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 @@ -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 { diff --git a/incident.go b/incident.go index 418f9d4f..500a80df 100644 --- a/incident.go +++ b/incident.go @@ -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"` diff --git a/incident_test.go b/incident_test.go index ae98b7f8..5b2da752 100644 --- a/incident_test.go +++ b/incident_test.go @@ -24,7 +24,9 @@ func TestIncident_List(t *testing.T) { APIListObject: listObj, Incidents: []Incident{ { - Id: "1", + APIObject: APIObject{ + ID: "1", + }, }, }, } @@ -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", } @@ -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", }, @@ -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{ @@ -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{ { @@ -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 { @@ -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) @@ -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", diff --git a/service.go b/service.go index 9281de23..3767c4fb 100644 --- a/service.go +++ b/service.go @@ -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"` } diff --git a/user.go b/user.go index 60331a5d..f8e12d51 100644 --- a/user.go +++ b/user.go @@ -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"`