Sd 770 fixup unique target paths and multi promotion

internal/pkg/githubapi/github.go

@@ -2,6 +2,7 @@ package githubapi
 
 import (
 	"bytes"
+	"cmp"
 	"context"
 	"crypto/sha1" //nolint:gosec // G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec), this is not a cryptographic use case
 	"encoding/base64"
@@ -14,6 +15,7 @@ import (
 	"path"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"sort"
 	"strings"
 	"text/template"
@@ -1126,14 +1128,17 @@ func generatePromotionPrBody(ghPrClientDetails GhPrClientDetails, components str
 
 	newPrMetadata.PromotedPaths = maps.Keys(promotion.ComputedSyncPaths)
 
+	promotionSkipPaths := getPromotionSkipPaths(promotion)
+
 	newPrBody = fmt.Sprintf("Promotion path(%s):\n\n", components)
 
 	keys := make([]int, 0)
 	for k := range newPrMetadata.PreviousPromotionMetadata {
 		keys = append(keys, k)
 	}
 	sort.Ints(keys)
-	newPrBody = prBody(keys, newPrMetadata, newPrBody)
+
+	newPrBody = prBody(keys, newPrMetadata, newPrBody, promotionSkipPaths)
 
 	prMetadataString, _ := newPrMetadata.serialize()
 
@@ -1142,45 +1147,86 @@ func generatePromotionPrBody(ghPrClientDetails GhPrClientDetails, components str
 	return newPrBody
 }
 
-func prBody(keys []int, newPrMetadata prMetadata, newPrBody string) string {
+// getPromotionSkipPaths returns a map of paths that are marked as skipped for this promotion
+// when we have multiple components, we are going to use the component that has the fewest skip paths
+func getPromotionSkipPaths(promotion PromotionInstance) map[string]bool {
+	perComponentSkippedTargetPaths := promotion.Metadata.PerComponentSkippedTargetPaths
+	promotionSkipPaths := map[string]bool{}
+
+	if len(perComponentSkippedTargetPaths) == 0 {
+		return promotionSkipPaths
+	}
+
+	// if any promoted component is not in the perComponentSkippedTargetPaths
+	// then that means we have a component that is promoted to all paths,
+	// therefore, we return an empty promotionSkipPaths map to signify that
+	// there are no paths that are skipped for this promotion
+	for _, component := range promotion.Metadata.ComponentNames {
+		if _, ok := perComponentSkippedTargetPaths[component]; !ok {
+			return promotionSkipPaths
+		}
+	}
+
+	// if we have one or more components then we are just going to
+	// user the component that has the fewest skipPaths when
+	// generating the promotion prBody. This way the promotion
+	// body will error on the side of informing the user
+	// of more promotion paths, rather than leaving some out.
+	skipCounts := map[string]int{}
+	for component, paths := range perComponentSkippedTargetPaths {
+		skipCounts[component] = len(paths)
+	}
+
+	skipPaths := maps.Keys(skipCounts)
+	slices.SortFunc(skipPaths, func(a, b string) int {
+		return cmp.Compare(skipCounts[a], skipCounts[b])
+	})
+
+	componentWithFewestSkippedPaths := skipPaths[0]
+	for _, p := range perComponentSkippedTargetPaths[componentWithFewestSkippedPaths] {
+		promotionSkipPaths[p] = true
+	}
+
+	return promotionSkipPaths
+}
+
+func prBody(keys []int, newPrMetadata prMetadata, newPrBody string, promotionSkipPaths map[string]bool) string {
 	const mkTab = "    "
 	sp := ""
 	tp := ""
 
 	for i, k := range keys {
 		sp = newPrMetadata.PreviousPromotionMetadata[k].SourcePath
-		x := identifyCommonPaths(newPrMetadata.PromotedPaths, newPrMetadata.PreviousPromotionMetadata[k].TargetPaths)
+		x := filterSkipPaths(newPrMetadata.PreviousPromotionMetadata[k].TargetPaths, promotionSkipPaths)
+		// sort the paths so that we have a predictable order for tests and better readability for users
+		sort.Strings(x)
 		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)
 	}
 
 	return newPrBody
 }
 
-// identifyCommonPaths takes a slice of promotion paths and target paths and
-// returns a slice containing paths in common.
-func identifyCommonPaths(promotionPaths []string, targetPaths []string) []string {
-	if (len(promotionPaths) == 0) || (len(targetPaths) == 0) {
-		return nil
-	}
-	var commonPaths []string
-	for _, pp := range promotionPaths {
-		if pp == "" {
-			continue
+// filterSkipPaths filters out the paths that are marked as skipped
+func filterSkipPaths(targetPaths []string, promotionSkipPaths map[string]bool) []string {
+	pathSkip := make(map[string]bool)
+	for _, targetPath := range targetPaths {
+		if _, ok := promotionSkipPaths[targetPath]; ok {
+			pathSkip[targetPath] = true
+		} else {
+			pathSkip[targetPath] = false
 		}
-		for _, tp := range targetPaths {
-			if tp == "" {
-				continue
-			}
-			// 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)
-			}
+	}
+
+	var paths []string
+
+	for path, skip := range pathSkip {
+		if !skip {
+			paths = append(paths, path)
 		}
 	}
 
-	return commonPaths
+	return paths
 }
 
 func createPrObject(ghPrClientDetails GhPrClientDetails, newBranchRef string, newPrTitle string, newPrBody string, defaultBranch string, assignee string) (*github.PullRequest, error) {

internal/pkg/githubapi/github_test.go

@@ -235,6 +235,7 @@ func readJSONFromFile(t *testing.T, filename string, data interface{}) {
 func TestPrBody(t *testing.T) {
 	t.Parallel()
 	keys := []int{1, 2, 3}
+	promotionSkipPaths := map[string]bool{"targetPath3": true}
 	newPrMetadata := prMetadata{
 		// note: "targetPath3" is missing from the list of promoted paths, so it should not
 		// be included in the new PR body.
@@ -254,14 +255,41 @@ func TestPrBody(t *testing.T) {
 			},
 		},
 	}
-	newPrBody := prBody(keys, newPrMetadata, "")
+	newPrBody := prBody(keys, newPrMetadata, "", promotionSkipPaths)
 	expectedPrBody, err := os.ReadFile("testdata/pr_body.golden.md")
 	if err != nil {
 		t.Fatalf("Error loading golden file: %s", err)
 	}
 	assert.Equal(t, string(expectedPrBody), newPrBody)
 }
 
+func TestPrBodyMultiComponent(t *testing.T) {
+	t.Parallel()
+	keys := []int{1, 2}
+	promotionSkipPaths := map[string]bool{}
+	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, "", promotionSkipPaths)
+	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 {
@@ -408,71 +436,66 @@ func TestCommitStatusTargetURL(t *testing.T) {
 	}
 }
 
-func Test_identifyCommonPaths(t *testing.T) {
+func Test_getPromotionSkipPaths(t *testing.T) {
 	t.Parallel()
 	type args struct {
-		promoPaths  []string
-		targetPaths []string
+		promotion PromotionInstance
 	}
 	tests := []struct {
 		name string
 		args args
-		want []string
+		want map[string]bool
 	}{
 		{
-			name: "same paths",
-			args: args{
-				promoPaths:  []string{"path1/component/path", "path2/component/path", "path3/component/path"},
-				targetPaths: []string{"path1", "path2", "path3"},
-			},
-			want: []string{"path1", "path2", "path3"},
-		},
-		{
-			name: "paths1 is empty",
+			name: "No skip paths",
 			args: args{
-				promoPaths:  []string{},
-				targetPaths: []string{"path1", "path2", "path3"},
+				promotion: PromotionInstance{
+					Metadata: PromotionInstanceMetaData{
+						PerComponentSkippedTargetPaths: map[string][]string{},
+					},
+				},
 			},
-			want: nil,
+			want: map[string]bool{},
 		},
 		{
-			name: "paths2 is empty",
+			name: "one skip path",
 			args: args{
-				promoPaths:  []string{"path1/component/some", "path2/some/other", "path3"},
-				targetPaths: []string{},
+				promotion: PromotionInstance{
+					Metadata: PromotionInstanceMetaData{
+						PerComponentSkippedTargetPaths: map[string][]string{
+							"component1": {"targetPath1", "targetPath2"},
+						},
+					},
+				},
 			},
-			want: nil,
-		},
-		{
-			name: "paths2 missing elements",
-			args: args{
-				promoPaths:  []string{"path1", "path2", "path3"},
-				targetPaths: []string{""},
+			want: map[string]bool{
+				"targetPath1": true,
+				"targetPath2": true,
 			},
-			want: nil,
 		},
 		{
-			name: "path1 missing elements",
+			name: "multiple skip path",
 			args: args{
-				promoPaths:  []string{""},
-				targetPaths: []string{"path1", "path2"},
+				promotion: PromotionInstance{
+					Metadata: PromotionInstanceMetaData{
+						PerComponentSkippedTargetPaths: map[string][]string{
+							"component1": {"targetPath1", "targetPath2", "targetPath3"},
+							"component2": {"targetPath3"},
+							"component3": {"targetPath1", "targetPath2"},
+						},
+					},
+				},
 			},
-			want: nil,
-		},
-		{
-			name: "path1 and path2 common elements",
-			args: args{
-				promoPaths:  []string{"path1/component/path", "path3/component/also"},
-				targetPaths: []string{"path1", "path2", "path3"},
+			want: map[string]bool{
+				"targetPath3": true,
 			},
-			want: []string{"path1", "path3"},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			got := identifyCommonPaths(tt.args.promoPaths, tt.args.targetPaths)
-			assert.Equal(t, got, tt.want)
+			got := getPromotionSkipPaths(tt.args.promotion)
+			assert.Equal(t, tt.want, got)
 		})
 	}
 }

internal/pkg/githubapi/testdata/pr_body_multi_component.golden.md

@@ -0,0 +1,4 @@
+↘️  #1  `sourcePath1` ➡️  
+    `targetPath1`  
+    ↘️  #2  `sourcePath2` ➡️  
+        `targetPath2`