🐛 Fixed the Issue.Move() field injection (#328)

ctreminiom · Oct 13, 2024 · 3fd17a6 · 3fd17a6
1 parent e5bee47
commit 3fd17a6
Show file tree

Hide file tree

Showing 9 changed files with 142 additions and 227 deletions.
diff --git a/jira/internal/issue_impl_adf.go b/jira/internal/issue_impl_adf.go
@@ -369,62 +369,33 @@ func (i *internalIssueADFServiceImpl) Move(ctx context.Context, issueKeyOrID, tr
 		return nil, model.ErrNoTransitionID
 	}
 
-	payloadUpdated := make(map[string]interface{})
-	payloadUpdated["transition"] = map[string]interface{}{"id": transitionID}
+	payload := map[string]interface{}{"transition": map[string]interface{}{"id": transitionID}}
 
-	// Process logic only if the transition options are provided
 	if options != nil {
-
 		if options.Fields == nil {
 			return nil, model.ErrNoIssueScheme
 		}
 
-		withCustomFields := options.CustomFields != nil
-		withOperations := options.Operations != nil
-
-		var err error
-
-		// Executed when the customfields and operations are provided
-		if withCustomFields && withOperations {
-
-			payloadUpdated, err = options.Fields.MergeCustomFields(options.CustomFields)
-			if err != nil {
-				return nil, err
-			}
-
-			payloadWithOperations, err := options.Fields.MergeOperations(options.Operations)
-			if err != nil {
-				return nil, err
-			}
-
-			if err = mergo.Map(&payloadUpdated, &payloadWithOperations, mergo.WithOverride); err != nil {
-				return nil, err
-			}
+		// Merge the customfields and operations
+		payloadWithFields, err := options.Fields.MergeCustomFields(options.CustomFields)
+		if err != nil {
+			return nil, err
 		}
-
-		// Executed when only the customfields are provided, but not the operations
-		if withCustomFields && !withOperations {
-
-			payloadUpdated, err = options.Fields.MergeCustomFields(options.CustomFields)
-			if err != nil {
-				return nil, err
-			}
+		if err = mergo.Map(&payload, &payloadWithFields, mergo.WithOverride); err != nil {
+			return nil, err
 		}
 
-		// Executed when only the operations are provided, but not the customfields
-		if withOperations && !withCustomFields {
-
-			payloadUpdated, err = options.Fields.MergeOperations(options.Operations)
-			if err != nil {
-				return nil, err
-			}
+		payloadWithOperation, err := options.Fields.MergeOperations(options.Operations)
+		if err != nil {
+			return nil, err
+		}
+		if err = mergo.Map(&payload, &payloadWithOperation, mergo.WithOverride); err != nil {
+			return nil, err
 		}
-
 	}
 
 	endpoint := fmt.Sprintf("rest/api/%v/issue/%v/transitions", i.version, issueKeyOrID)
-
-	request, err := i.c.NewRequest(ctx, http.MethodPost, endpoint, "", payloadUpdated)
+	request, err := i.c.NewRequest(ctx, http.MethodPost, endpoint, "", payload)
 	if err != nil {
 		return nil, err
 	}

diff --git a/jira/internal/issue_impl_adf_test.go b/jira/internal/issue_impl_adf_test.go
@@ -3,6 +3,7 @@ package internal
 import (
 	"context"
 	"errors"
+	"github.com/stretchr/testify/mock"
 	"net/http"
 	"testing"
 
@@ -1007,38 +1008,57 @@ func Test_internalIssueADFServiceImpl_Get(t *testing.T) {
 
 func Test_internalIssueADFServiceImpl_Move(t *testing.T) {
 
+	/*
+		"customfield_10042": 1000.2222,
+		"customfield_10052": [
+			{
+				"name": "jira-administrators"
+			},
+			{
+				"name": "jira-administrators-system"
+			}
+		]
+	*/
 	customFieldsMocked := &model.CustomFields{}
-
-	err := customFieldsMocked.Groups("customfield_10052", []string{"jira-administrators", "jira-administrators-system"})
-	if err != nil {
+	if err := customFieldsMocked.Groups("customfield_10052", []string{"jira-administrators", "jira-administrators-system"}); err != nil {
 		t.Fatal(err)
 	}
-
-	err = customFieldsMocked.Number("customfield_10042", 1000.2222)
-	if err != nil {
+	if err := customFieldsMocked.Number("customfield_10042", 1000.2222); err != nil {
 		t.Fatal(err)
 	}
 
+	/*
+		 "update": {
+			"labels": [
+				{
+					"remove": "triaged"
+				}
+			]
+		}
+	*/
 	operationsMocked := &model.UpdateOperations{}
-	err = operationsMocked.AddArrayOperation("labels", map[string]string{
-		"triaged": "remove",
-	})
+	if err := operationsMocked.AddArrayOperation("labels", map[string]string{"triaged": "remove"}); err != nil {
+		t.Fatal(err)
+	}
 
 	expectedPayloadWithCustomFieldsAndOperations := map[string]interface{}{
-
 		"fields": map[string]interface{}{
 			"customfield_10042": 1000.2222,
 			"customfield_10052": []map[string]interface{}{map[string]interface{}{
 				"name": "jira-administrators"}, map[string]interface{}{
 				"name": "jira-administrators-system"}},
 
-			"issuetype": map[string]interface{}{"name": "Story"},
-			"project":   map[string]interface{}{"id": "10000"},
-			"summary":   "New summary test"},
+			"issuetype":  map[string]interface{}{"name": "Story"},
+			"project":    map[string]interface{}{"id": "10000"},
+			"resolution": map[string]interface{}{"name": "Done"},
+			"summary":    "New summary test"},
 
 		"update": map[string]interface{}{
 			"labels": []map[string]interface{}{map[string]interface{}{
-				"remove": "triaged"}}}}
+				"remove": "triaged"}}},
+
+		"transition": map[string]interface{}{"id": "10001"},
+	}
 
 	expectedPayloadWithCustomfields := map[string]interface{}{
 		"fields": map[string]interface{}{
@@ -1049,7 +1069,9 @@ func Test_internalIssueADFServiceImpl_Move(t *testing.T) {
 
 			"issuetype": map[string]interface{}{"name": "Story"},
 			"project":   map[string]interface{}{"id": "10000"},
-			"summary":   "New summary test"}}
+			"summary":   "New summary test"},
+		"transition": map[string]interface{}{"id": "10001"},
+	}
 
 	expectedPayloadWithOperations := map[string]interface{}{
 		"fields": map[string]interface{}{
@@ -1059,14 +1081,12 @@ func Test_internalIssueADFServiceImpl_Move(t *testing.T) {
 
 		"update": map[string]interface{}{
 			"labels": []map[string]interface{}{map[string]interface{}{
-				"remove": "triaged"}}}}
+				"remove": "triaged"}}},
+		"transition": map[string]interface{}{"id": "10001"},
+	}
 
 	expectedPayloadWithNoOptions := map[string]interface{}{"transition": map[string]interface{}{"id": "10001"}}
 
-	if err != nil {
-		t.Fatal(err)
-	}
-
 	type fields struct {
 		c       service.Connector
 		version string
@@ -1099,6 +1119,9 @@ func Test_internalIssueADFServiceImpl_Move(t *testing.T) {
 							Summary:   "New summary test",
 							Project:   &model.ProjectScheme{ID: "10000"},
 							IssueType: &model.IssueTypeScheme{Name: "Story"},
+							Resolution: &model.ResolutionScheme{
+								Name: "Done",
+							},
 						},
 					},
 					CustomFields: customFieldsMocked,
@@ -1332,7 +1355,7 @@ func Test_internalIssueADFServiceImpl_Move(t *testing.T) {
 					http.MethodPost,
 					"rest/api/3/issue/DUMMY-1/transitions",
 					"",
-					expectedPayloadWithCustomFieldsAndOperations).
+					mock.Anything).
 					Return(&http.Request{}, errors.New("error, unable to create the http request"))
 
 				fields.c = client

diff --git a/jira/internal/issue_impl_rich_text.go b/jira/internal/issue_impl_rich_text.go
@@ -369,62 +369,33 @@ func (i *internalRichTextServiceImpl) Move(ctx context.Context, issueKeyOrID, tr
 		return nil, model.ErrNoTransitionID
 	}
 
-	payloadUpdated := make(map[string]interface{})
-	payloadUpdated["transition"] = map[string]interface{}{"id": transitionID}
+	payload := map[string]interface{}{"transition": map[string]interface{}{"id": transitionID}}
 
-	// Process logic only if the transition options are provided
 	if options != nil {
-
 		if options.Fields == nil {
 			return nil, model.ErrNoIssueScheme
 		}
 
-		withCustomFields := options.CustomFields != nil
-		withOperations := options.Operations != nil
-
-		var err error
-
-		// Executed when the customfields and operations are provided
-		if withCustomFields && withOperations {
-
-			payloadUpdated, err = options.Fields.MergeCustomFields(options.CustomFields)
-			if err != nil {
-				return nil, err
-			}
-
-			payloadWithOperations, err := options.Fields.MergeOperations(options.Operations)
-			if err != nil {
-				return nil, err
-			}
-
-			if err = mergo.Map(&payloadUpdated, &payloadWithOperations, mergo.WithOverride); err != nil {
-				return nil, err
-			}
+		// Merge the customfields and operations
+		payloadWithFields, err := options.Fields.MergeCustomFields(options.CustomFields)
+		if err != nil {
+			return nil, err
 		}
-
-		// Executed when only the customfields are provided, but not the operations
-		if withCustomFields && !withOperations {
-
-			payloadUpdated, err = options.Fields.MergeCustomFields(options.CustomFields)
-			if err != nil {
-				return nil, err
-			}
+		if err = mergo.Map(&payload, &payloadWithFields, mergo.WithOverride); err != nil {
+			return nil, err
 		}
 
-		// Executed when only the operations are provided, but not the customfields
-		if withOperations && !withCustomFields {
-
-			payloadUpdated, err = options.Fields.MergeOperations(options.Operations)
-			if err != nil {
-				return nil, err
-			}
+		payloadWithOperation, err := options.Fields.MergeOperations(options.Operations)
+		if err != nil {
+			return nil, err
+		}
+		if err = mergo.Map(&payload, &payloadWithOperation, mergo.WithOverride); err != nil {
+			return nil, err
 		}
-
 	}
 
 	endpoint := fmt.Sprintf("rest/api/%v/issue/%v/transitions", i.version, issueKeyOrID)
-
-	request, err := i.c.NewRequest(ctx, http.MethodPost, endpoint, "", payloadUpdated)
+	request, err := i.c.NewRequest(ctx, http.MethodPost, endpoint, "", payload)
 	if err != nil {
 		return nil, err
 	}