From 49ccc008ae5781f5791a8515ed416282f07e21da Mon Sep 17 00:00:00 2001
From: Arthur Shvarts <arthur.shvarts@commercetools.com>
Date: Mon, 28 Oct 2024 10:00:49 -0400
Subject: [PATCH] Unique targetPaths in PR Body comment.

We were returning a slice of targetPaths that matches the promotionPaths,
this was creating duplicates in the pr body, like in this PR:
https://github.com/commercetools/k8s-gitops/pull/1004

This PR return a unique list of targetPaths.
---
 internal/pkg/githubapi/github.go              | 18 ++++++++----
 internal/pkg/githubapi/github_test.go         | 28 ++++++++++++++++++-
 .../pr_body_multi_component.golden.md         |  4 +++
 3 files changed, 44 insertions(+), 6 deletions(-)
 create mode 100644 internal/pkg/githubapi/testdata/pr_body_multi_component.golden.md

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 892f7956..745c6fb6 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -1127,7 +1127,7 @@ func prBody(keys []int, newPrMetadata prMetadata, newPrBody string) string {
 
 	for i, k := range keys {
 		sp = newPrMetadata.PreviousPromotionMetadata[k].SourcePath
-		x := identifyCommonPaths(newPrMetadata.PromotedPaths, newPrMetadata.PreviousPromotionMetadata[k].TargetPaths)
+		x := uniqueCommonPaths(newPrMetadata.PromotedPaths, newPrMetadata.PreviousPromotionMetadata[k].TargetPaths)
 		tp = strings.Join(x, fmt.Sprintf("`  \n%s`", strings.Repeat(mkTab, i+1)))
 		newPrBody = newPrBody + fmt.Sprintf("%s↘️  #%d  `%s` ➡️  \n%s`%s`  \n", strings.Repeat(mkTab, i), k, sp, strings.Repeat(mkTab, i+1), tp)
 	}
@@ -1135,13 +1135,14 @@ func prBody(keys []int, newPrMetadata prMetadata, newPrBody string) string {
 	return newPrBody
 }
 
-// identifyCommonPaths takes a slice of promotion paths and target paths and
+// uniqueCommonPaths takes a slice of promotion paths and target paths and
 // returns a slice containing paths in common.
-func identifyCommonPaths(promotionPaths []string, targetPaths []string) []string {
+func uniqueCommonPaths(promotionPaths []string, targetPaths []string) []string {
 	if (len(promotionPaths) == 0) || (len(targetPaths) == 0) {
 		return nil
 	}
-	var commonPaths []string
+
+	uniqueCommonPaths := make(map[string]bool)
 	for _, pp := range promotionPaths {
 		if pp == "" {
 			continue
@@ -1153,11 +1154,18 @@ func identifyCommonPaths(promotionPaths []string, targetPaths []string) []string
 			// strings.HasPrefix is used to check that the target path and promotion path match instead of
 			// using 'pp ==  tp' because the promotion path is targetPath + component.
 			if strings.HasPrefix(pp, tp) {
-				commonPaths = append(commonPaths, tp)
+				if _, ok := uniqueCommonPaths[tp]; !ok {
+					uniqueCommonPaths[tp] = true
+				}
 			}
 		}
 	}
 
+	var commonPaths []string
+	for path := range uniqueCommonPaths {
+		commonPaths = append(commonPaths, path)
+	}
+
 	return commonPaths
 }
 
diff --git a/internal/pkg/githubapi/github_test.go b/internal/pkg/githubapi/github_test.go
index 46445281..c9dd026a 100644
--- a/internal/pkg/githubapi/github_test.go
+++ b/internal/pkg/githubapi/github_test.go
@@ -262,6 +262,32 @@ func TestPrBody(t *testing.T) {
 	assert.Equal(t, string(expectedPrBody), newPrBody)
 }
 
+func TestPrBodyMultiComponent(t *testing.T) {
+	t.Parallel()
+	keys := []int{1, 2}
+	newPrMetadata := prMetadata{
+		// note: "targetPath3" is missing from the list of promoted paths, so it should not
+		// be included in the new PR body.
+		PromotedPaths: []string{"targetPath1/component1", "targetPath1/component2", "targetPath2/component1"},
+		PreviousPromotionMetadata: map[int]promotionInstanceMetaData{
+			1: {
+				SourcePath:  "sourcePath1",
+				TargetPaths: []string{"targetPath1"},
+			},
+			2: {
+				SourcePath:  "sourcePath2",
+				TargetPaths: []string{"targetPath2"},
+			},
+		},
+	}
+	newPrBody := prBody(keys, newPrMetadata, "")
+	expectedPrBody, err := os.ReadFile("testdata/pr_body_multi_component.golden.md")
+	if err != nil {
+		t.Fatalf("Error loading golden file: %s", err)
+	}
+	assert.Equal(t, string(expectedPrBody), newPrBody)
+}
+
 func TestGhPrClientDetailsGetBlameURLPrefix(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
@@ -474,7 +500,7 @@ func Test_identifyCommonPaths(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			got := identifyCommonPaths(tt.args.promoPaths, tt.args.targetPaths)
+			got := uniqueCommonPaths(tt.args.promoPaths, tt.args.targetPaths)
 			assert.Equal(t, got, tt.want)
 		})
 	}
diff --git a/internal/pkg/githubapi/testdata/pr_body_multi_component.golden.md b/internal/pkg/githubapi/testdata/pr_body_multi_component.golden.md
new file mode 100644
index 00000000..5294cc7b
--- /dev/null
+++ b/internal/pkg/githubapi/testdata/pr_body_multi_component.golden.md
@@ -0,0 +1,4 @@
+↘️  #1  `sourcePath1` ➡️  
+&nbsp;&nbsp;&nbsp;&nbsp;`targetPath1`  
+&nbsp;&nbsp;&nbsp;&nbsp;↘️  #2  `sourcePath2` ➡️  
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;`targetPath2`