Repository: PagerDuty/go-pagerduty
Pull request #358 (2021-09-04 01:38:21)

Remove returned *http.Response from business svc related methods
This is a breaking change.

This is one of the changes that will need to be made to complete issue #305,
which is being done in a minor release even though it does contain breaking
changes.

The internal implementation details of how this was implemented meant it never
worked in a way that consumers could use, because the body was always empty.
Seeing as this API never worked and we do not wish to support it, we are going
to remove it so that anyone who may have depended on it, but missed that it was
broken, can be made aware.

If someone wanted to capture the full response of these API calls, they can now
do so using the the functionality added in #325 (4f01c5befe6bde8e812f8acf42546764d58bf7c8).


Patch:
diff --git a/business_service.go b/business_service.go
index 5275ab6..b8df9e0 100644
--- a/business_service.go
+++ b/business_service.go
@@ -102,41 +102,31 @@ func (c *Client) ListBusinessServicesPaginated(ctx context.Context, o ListBusine
 // CreateBusinessService creates a new business service.
 //
 // Deprecated: Use CreateBusinessServiceWithContext instead
-func (c *Client) CreateBusinessService(b *BusinessService) (*BusinessService, *http.Response, error) {
-	return c.createBusinessServiceWithContext(context.Background(), b)
+func (c *Client) CreateBusinessService(b *BusinessService) (*BusinessService, error) {
+	return c.CreateBusinessServiceWithContext(context.Background(), b)
 }
 
 // CreateBusinessServiceWithContext creates a new business service.
 func (c *Client) CreateBusinessServiceWithContext(ctx context.Context, b *BusinessService) (*BusinessService, error) {
-	bs, _, err := c.createBusinessServiceWithContext(ctx, b)
-	return bs, err
-}
-
-func (c *Client) createBusinessServiceWithContext(ctx context.Context, b *BusinessService) (*BusinessService, *http.Response, error) {
 	d := map[string]*BusinessService{
 		"business_service": b,
 	}
 
 	resp, err := c.post(ctx, "/business_services", d, nil)
-	return getBusinessServiceFromResponse(c, resp, err)
+	return getBusinessServiceFromResponse(resp, err, c.decodeJSON)
 }
 
 // GetBusinessService gets details about a business service.
 //
 // Deprecated: Use GetBusinessServiceWithContext instead.
-func (c *Client) GetBusinessService(id string) (*BusinessService, *http.Response, error) {
-	return c.getBusinessServiceWithContext(context.Background(), id)
+func (c *Client) GetBusinessService(id string) (*BusinessService, error) {
+	return c.GetBusinessServiceWithContext(context.Background(), id)
 }
 
 // GetBusinessServiceWithContext gets details about a business service.
 func (c *Client) GetBusinessServiceWithContext(ctx context.Context, id string) (*BusinessService, error) {
-	bs, _, err := c.getBusinessServiceWithContext(ctx, id)
-	return bs, err
-}
-
-func (c *Client) getBusinessServiceWithContext(ctx context.Context, id string) (*BusinessService, *http.Response, error) {
 	resp, err := c.get(ctx, "/business_services/"+id)
-	return getBusinessServiceFromResponse(c, resp, err)
+	return getBusinessServiceFromResponse(resp, err, c.decodeJSON)
 }
 
 // DeleteBusinessService deletes a business_service.
@@ -155,17 +145,12 @@ func (c *Client) DeleteBusinessServiceWithContext(ctx context.Context, id string
 // UpdateBusinessService updates a business_service.
 //
 // Deprecated: Use UpdateBusinessServiceWithContext instead.
-func (c *Client) UpdateBusinessService(b *BusinessService) (*BusinessService, *http.Response, error) {
-	return c.updateBusinessServiceWithContext(context.Background(), b)
+func (c *Client) UpdateBusinessService(b *BusinessService) (*BusinessService, error) {
+	return c.UpdateBusinessServiceWithContext(context.Background(), b)
 }
 
 // UpdateBusinessServiceWithContext updates a business_service.
 func (c *Client) UpdateBusinessServiceWithContext(ctx context.Context, b *BusinessService) (*BusinessService, error) {
-	bs, _, err := c.updateBusinessServiceWithContext(ctx, b)
-	return bs, err
-}
-
-func (c *Client) updateBusinessServiceWithContext(ctx context.Context, b *BusinessService) (*BusinessService, *http.Response, error) {
 	id := b.ID
 	b.ID = ""
 
@@ -174,25 +159,25 @@ func (c *Client) updateBusinessServiceWithContext(ctx context.Context, b *Busine
 	}
 
 	resp, err := c.put(ctx, "/business_services/"+id, d, nil)
-	return getBusinessServiceFromResponse(c, resp, err)
+	return getBusinessServiceFromResponse(resp, err, c.decodeJSON)
 }
 
-func getBusinessServiceFromResponse(c *Client, resp *http.Response, err error) (*BusinessService, *http.Response, error) {
+func getBusinessServiceFromResponse(resp *http.Response, err error, decodeFn func(*http.Response, interface{}) error) (*BusinessService, error) {
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	var target map[string]BusinessService
-	if dErr := c.decodeJSON(resp, &target); dErr != nil {
-		return nil, nil, fmt.Errorf("Could not decode JSON response: %v", dErr)
+	if dErr := decodeFn(resp, &target); dErr != nil {
+		return nil, fmt.Errorf("Could not decode JSON response: %v", dErr)
 	}
 
 	const rootNode = "business_service"
 
 	t, nodeOK := target[rootNode]
 	if !nodeOK {
-		return nil, nil, fmt.Errorf("JSON response does not have %s field", rootNode)
+		return nil, fmt.Errorf("JSON response does not have %s field", rootNode)
 	}
 
-	return &t, resp, nil
+	return &t, nil
 }


Test patch:
diff --git a/business_service_test.go b/business_service_test.go
index b05ca35..60cbb19 100644
--- a/business_service_test.go
+++ b/business_service_test.go
@@ -51,7 +51,7 @@ func TestBusinessService_Create(t *testing.T) {
 	input := &BusinessService{
 		Name: "foo",
 	}
-	res, _, err := client.CreateBusinessService(input)
+	res, err := client.CreateBusinessService(input)
 
 	want := &BusinessService{
 		ID:   "1",
@@ -77,7 +77,7 @@ func TestBusinessService_Get(t *testing.T) {
 	client := defaultTestClient(server.URL, "foo")
 	ruleSetID := "1"
 
-	res, _, err := client.GetBusinessService(ruleSetID)
+	res, err := client.GetBusinessService(ruleSetID)
 
 	want := &BusinessService{
 		ID:   "1",
@@ -117,7 +117,7 @@ func TestBusinessService_Update(t *testing.T) {
 		ID:   "1",
 		Name: "foo",
 	}
-	res, _, err := client.UpdateBusinessService(input)
+	res, err := client.UpdateBusinessService(input)
 
 	want := &BusinessService{
 		ID:   "1",