Avoid displaying the "Sync from PR Branch" checkbox for new applicati…

…ons.

match its new usage
* Create a new function to check if all application in a PR are new.
* Write tests for new function
* Rename HasSyncableComponents to DisplaySyncBranchCheckBox to better

internal/pkg/githubapi/github.go

@@ -34,10 +34,10 @@ const (
 )
 
 type DiffCommentData struct {
-	DiffOfChangedComponents []argocd.DiffResult
-	HasSyncableComponents   bool
-	BranchName              string
-	Header                  string
+	DiffOfChangedComponents   []argocd.DiffResult
+	DisplaySyncBranchCheckBox bool
+	BranchName                string
+	Header                    string
 }
 
 type promotionInstanceMetaData struct {
@@ -104,6 +104,30 @@ func (ghPrClientDetails *GhPrClientDetails) getBlameURLPrefix() string {
 	return fmt.Sprintf("%s/%s/%s/blame", githubHost, ghPrClientDetails.Owner, ghPrClientDetails.Repo)
 }
 
+// This function is used to check if we should display the sync branch checkbox in the PR comment.
+// This depends on the allowSyncfromBranchPathRegex configuration and whether the component is new or not.
+func shouldSyncBranchCheckBoxBeDisplayed(componentPathList []string, allowSyncfromBranchPathRegex string, diffOfChangedComponents []argocd.DiffResult) bool {
+	var displaySyncBranchCheckBox bool
+
+out:
+	for _, componentPath := range componentPathList {
+		if isSyncFromBranchAllowedForThisPath(allowSyncfromBranchPathRegex, componentPath) {
+			// Here we are checking if the syncable component is not a new app that was temporarily created.
+			// We don't support syncing new apps from branches
+			for _, diffOfChangedComponent := range diffOfChangedComponents {
+				if diffOfChangedComponent.ComponentPath == componentPath {
+					if !diffOfChangedComponent.AppWasTemporarilyCreated {
+						displaySyncBranchCheckBox = true
+						break out
+					}
+				}
+			}
+		}
+	}
+
+	return displaySyncBranchCheckBox
+}
+
 func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPrClientDetails, mainGithubClientPair GhClientPair, approverGithubClientPair GhClientPair, ctx context.Context) {
 	ghPrClientDetails.getPrMetadata(eventPayload.PullRequest.GetBody())
 	// wasCommitStatusSet and the placement of SetCommitStatus in the flow is used to ensure an API call is only made where it needed
@@ -192,12 +216,8 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 					BranchName:              ghPrClientDetails.Ref,
 				}
 
-				for _, componentPath := range componentPathList {
-					if isSyncFromBranchAllowedForThisPath(config.Argocd.AllowSyncfromBranchPathRegex, componentPath) {
-						diffCommentData.HasSyncableComponents = true
-						break
-					}
-				}
+				diffCommentData.DisplaySyncBranchCheckBox = shouldSyncBranchCheckBoxBeDisplayed(componentPathList, config.Argocd.AllowSyncfromBranchPathRegex, diffOfChangedComponents)
+
 				comments, err := generateArgoCdDiffComments(diffCommentData, githubCommentMaxSize)
 				if err != nil {
 					prHandleError = err

internal/pkg/githubapi/github_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/wayfair-incubator/telefonistka/internal/pkg/argocd"
 )
 
 func TestGenerateSafePromotionBranchName(t *testing.T) {
@@ -289,3 +290,56 @@ func TestGhPrClientDetailsGetBlameURLPrefix(t *testing.T) {
 		assert.Equal(t, tc.ExpectURL, blameURLPrefix)
 	}
 }
+
+func TestShouldSyncBranchCheckBoxBeDisplayed(t *testing.T) {
+	t.Parallel()
+	tests := map[string]struct {
+		componentPathList            []string
+		allowSyncfromBranchPathRegex string
+		diffOfChangedComponents      []argocd.DiffResult
+		expected                     bool
+	}{
+		"New App": {
+			componentPathList:            []string{"workspace/app1"},
+			allowSyncfromBranchPathRegex: `^workspace/.*$`,
+			diffOfChangedComponents: []argocd.DiffResult{
+				{
+					AppWasTemporarilyCreated: true,
+					ComponentPath:            "workspace/app1",
+				},
+			},
+			expected: false,
+		},
+		"Existing App": {
+			componentPathList:            []string{"workspace/app1"},
+			allowSyncfromBranchPathRegex: `^workspace/.*$`,
+			diffOfChangedComponents: []argocd.DiffResult{
+				{
+					AppWasTemporarilyCreated: false,
+					ComponentPath:            "workspace/app1",
+				},
+			},
+			expected: true,
+		},
+		"Mixed New and Existing Apps": {
+			componentPathList:            []string{"workspace/app1", "workspace/app2"},
+			allowSyncfromBranchPathRegex: `^workspace/.*$`,
+			diffOfChangedComponents: []argocd.DiffResult{
+				{
+					AppWasTemporarilyCreated: false,
+					ComponentPath:            "workspace/app1",
+				},
+				{
+					AppWasTemporarilyCreated: true,
+					ComponentPath:            "workspace/app2",
+				},
+			},
+			expected: true,
+		},
+	}
+
+	for i, tc := range tests {
+		result := shouldSyncBranchCheckBoxBeDisplayed(tc.componentPathList, tc.allowSyncfromBranchPathRegex, tc.diffOfChangedComponents)
+		assert.Equal(t, tc.expected, result, i)
+	}
+}

internal/pkg/githubapi/testdata/diff_comment_data_test.json

@@ -74,7 +74,7 @@
       "AppWasTemporarilyCreated": false
     }
   ],
-  "HasSyncableComponents": false,
+  "DisplaySyncBranchCheckBox": false,
   "BranchName": "promotions/284-simulate-error-5c159151017f",
   "Header": ""
 }

templates/argoCD-diff-pr-comment-concise.gotmpl

@@ -37,7 +37,7 @@ It will not be present in ArgoCD UI for more than a few seconds and it can not b
 
 {{- end }}
 
-{{- if .HasSyncableComponents }}
+{{- if .DisplaySyncBranchCheckBox }}
 
 - [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps Target Revision to `{{ .BranchName }}`  
 

templates/argoCD-diff-pr-comment.gotmpl

@@ -47,7 +47,7 @@ It will not be present in ArgoCD UI for more than a few seconds and it can not b
 
 {{- end }}
 
-{{- if .HasSyncableComponents }}
+{{- if .DisplaySyncBranchCheckBox }}
 
 - [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps Target Revision to `{{ .BranchName }}`