From 1a0b2dcc312ba44b3b3bffb2f010968375bdf25f Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 28 Jun 2024 11:42:03 +0200
Subject: [PATCH 01/34] Allow setting argoCD revision to PR git SHA

---
 internal/pkg/argocd/argocd.go           |  80 +++++++++++------
 internal/pkg/configuration/config.go    |  19 ++--
 internal/pkg/githubapi/github.go        | 110 +++++++++++++++++++++---
 internal/pkg/githubapi/github_test.go   |  87 +++++++++++++++++++
 templates/argoCD-diff-pr-comment.gotmpl |   8 +-
 5 files changed, 257 insertions(+), 47 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index dc746f7..ef4fe1b 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -6,6 +6,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -144,7 +145,9 @@ func getEnv(key, fallback string) string {
 	return fallback
 }
 
-func createArgoCdClient() (apiclient.Client, error) {
+func createArgoCdClients() (appClient application.ApplicationServiceClient, projClient projectpkg.ProjectServiceClient, settingClient settings.SettingsServiceClient, err error) {
+	var conn io.Closer
+
 	plaintext, _ := strconv.ParseBool(getEnv("ARGOCD_PLAINTEXT", "false"))
 	insecure, _ := strconv.ParseBool(getEnv("ARGOCD_INSECURE", "false"))
 
@@ -155,11 +158,27 @@ func createArgoCdClient() (apiclient.Client, error) {
 		Insecure:   insecure,
 	}
 
-	clientset, err := apiclient.NewClient(opts)
+	client, err := apiclient.NewClient(opts)
+	if err != nil {
+		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD API client: %v", err)
+	}
+
+	conn, appClient, err = client.NewApplicationClient()
 	if err != nil {
-		return nil, fmt.Errorf("Error creating ArgoCD API client: %v", err)
+		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD app client: %v", err)
 	}
-	return clientset, nil
+
+	conn, projClient, err = client.NewProjectClient()
+	if err != nil {
+		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD project client: %v", err)
+	}
+
+	conn, settingClient, err = client.NewSettingsClient()
+	if err != nil {
+		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD settings client: %v", err)
+	}
+	defer argoio.Close(conn)
+	return
 }
 
 // findArgocdAppBySHA1Label finds an ArgoCD application by the SHA1 label of the component path it's supposed to avoid performance issues with the "manifest-generate-paths" annotation method which requires pulling all ArgoCD applications(!) on every PR event.
@@ -231,6 +250,35 @@ func findArgocdAppByManifestPathAnnotation(ctx context.Context, componentPath st
 	return nil, fmt.Errorf("No ArgoCD application found with manifest-generate-paths annotation that matches %s(looked at repo %s, checked %v apps)	", componentPath, repo, len(allRepoApps.Items))
 }
 
+func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision string, repo string, useSHALabelForArgoDicovery bool) error {
+	var foundApp *argoappv1.Application
+	var err error
+	appClient, _, _, err := createArgoCdClients()
+	if useSHALabelForArgoDicovery {
+		foundApp, err = findArgocdAppBySHA1Label(ctx, componentPath, repo, appClient)
+	} else {
+		foundApp, err = findArgocdAppByManifestPathAnnotation(ctx, componentPath, repo, appClient)
+	}
+	if foundApp.Spec.Source.TargetRevision == revision {
+		log.Infof("App %s already has revision %s", foundApp.Name, revision)
+		return nil
+	}
+
+	foundApp.Spec.Source.TargetRevision = revision
+
+	_, err = appClient.UpdateSpec(ctx, &application.ApplicationUpdateSpecRequest{
+		Name:         &foundApp.Name,
+		Spec:         &foundApp.Spec,
+		AppNamespace: &foundApp.Namespace,
+	})
+
+	if err != nil {
+		return fmt.Errorf("Error setting app %s revision to  %s failed: %v", foundApp.Name, revision, err)
+	}
+
+	return nil
+}
+
 func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranch string, repo string, appClient application.ApplicationServiceClient, projClient projectpkg.ProjectServiceClient, argoSettings *settings.Settings, useSHALabelForArgoDicovery bool) (componentDiffResult DiffResult) {
 	componentDiffResult.ComponentPath = componentPath
 
@@ -313,32 +361,12 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st
 	hasComponentDiff = false
 	hasComponentDiffErrors = false
 	// env var should be centralized
-	client, err := createArgoCdClient()
-	if err != nil {
-		log.Errorf("Error creating ArgoCD client: %v", err)
-		return false, true, nil, err
-	}
-
-	conn, appClient, err := client.NewApplicationClient()
+	appClient, projClient, settingClient, err := createArgoCdClients()
 	if err != nil {
-		log.Errorf("Error creating ArgoCD app client: %v", err)
+		log.Errorf("Error creating ArgoCD clients: %v", err)
 		return false, true, nil, err
 	}
-	defer argoio.Close(conn)
-
-	conn, projClient, err := client.NewProjectClient()
-	if err != nil {
-		log.Errorf("Error creating ArgoCD project client: %v", err)
-		return false, true, nil, err
-	}
-	defer argoio.Close(conn)
 
-	conn, settingClient, err := client.NewSettingsClient()
-	if err != nil {
-		log.Errorf("Error creating ArgoCD settings client: %v", err)
-		return false, true, nil, err
-	}
-	defer argoio.Close(conn)
 	argoSettings, err := settingClient.Get(ctx, &settings.SettingsQuery{})
 	if err != nil {
 		log.Errorf("Error getting ArgoCD settings: %v", err)
diff --git a/internal/pkg/configuration/config.go b/internal/pkg/configuration/config.go
index f13738b..0d2bbc8 100644
--- a/internal/pkg/configuration/config.go
+++ b/internal/pkg/configuration/config.go
@@ -36,15 +36,16 @@ type Config struct {
 	PromotionPaths []PromotionPath `yaml:"promotionPaths"`
 
 	// Generic configuration
-	PromtionPrLables             []string               `yaml:"promtionPRlables"`
-	DryRunMode                   bool                   `yaml:"dryRunMode"`
-	AutoApprovePromotionPrs      bool                   `yaml:"autoApprovePromotionPrs"`
-	ToggleCommitStatus           map[string]string      `yaml:"toggleCommitStatus"`
-	WebhookEndpointRegexs        []WebhookEndpointRegex `yaml:"webhookEndpointRegexs"`
-	WhProxtSkipTLSVerifyUpstream bool                   `yaml:"whProxtSkipTLSVerifyUpstream"`
-	CommentArgocdDiffonPR        bool                   `yaml:"commentArgocdDiffonPR"`
-	AutoMergeNoDiffPRs           bool                   `yaml:"autoMergeNoDiffPRs"`
-	UseSHALabelForArgoDicovery   bool                   `yaml:"useSHALabelForArgoDicovery"`
+	PromtionPrLables                      []string               `yaml:"promtionPRlables"`
+	DryRunMode                            bool                   `yaml:"dryRunMode"`
+	AutoApprovePromotionPrs               bool                   `yaml:"autoApprovePromotionPrs"`
+	ToggleCommitStatus                    map[string]string      `yaml:"toggleCommitStatus"`
+	WebhookEndpointRegexs                 []WebhookEndpointRegex `yaml:"webhookEndpointRegexs"`
+	WhProxtSkipTLSVerifyUpstream          bool                   `yaml:"whProxtSkipTLSVerifyUpstream"`
+	CommentArgocdDiffonPR                 bool                   `yaml:"commentArgocdDiffonPR"`
+	AutoMergeNoDiffPRs                    bool                   `yaml:"autoMergeNoDiffPRs"`
+	AllowSyncArgoCDAppfromBranchPathRegex string                 `yaml:"allowSyncArgoCDAppfromBranchPathRegex"`
+	UseSHALabelForArgoDicovery            bool                   `yaml:"useSHALabelForArgoDicovery"`
 }
 
 func ParseConfigFromYaml(y string) (*Config, error) {
diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index bf75c41..57d6ea4 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -70,9 +70,10 @@ func (pm prMetadata) serialize() (string, error) {
 	return base64.StdEncoding.EncodeToString(pmJson), nil
 }
 
-func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPrClientDetails, mainGithubClientPair GhClientPair, approverGithubClientPair GhClientPair, ctx context.Context) {
+func (ghPrClientDetails *GhPrClientDetails) getPrMetadata(prBody string) {
+
 	prMetadataRegex := regexp.MustCompile(`<!--\|.*\|(.*)\|-->`)
-	serializedPrMetadata := prMetadataRegex.FindStringSubmatch(eventPayload.PullRequest.GetBody())
+	serializedPrMetadata := prMetadataRegex.FindStringSubmatch(prBody)
 	if len(serializedPrMetadata) == 2 {
 		if serializedPrMetadata[1] != "" {
 			ghPrClientDetails.PrLogger.Info("Found PR metadata")
@@ -83,6 +84,11 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 		}
 	}
 
+}
+
+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
 	wasCommitStatusSet := false
 
@@ -154,15 +160,34 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 				}
 			}
 
-			err, templateOutput := executeTemplate(ghPrClientDetails.PrLogger, "argoCdDiff", "argoCD-diff-pr-comment.gotmpl", diffOfChangedComponents)
-			if err != nil {
-				prHandleError = err
-				log.Errorf("Failed to generate ArgoCD diff comment template: err=%s\n", err)
-			}
-			err = commentPR(ghPrClientDetails, templateOutput)
-			if err != nil {
-				prHandleError = err
-				log.Errorf("Failed to comment ArgoCD diff: err=%s\n", err)
+			if len(diffOfChangedComponents) > 0 {
+
+				diffCommentData := struct {
+					diffOfChangedComponents []argocd.DiffResult
+					hasSyncableComponens    bool
+				}{
+					diffOfChangedComponents: diffOfChangedComponents,
+				}
+
+				for _, componentPath := range componentPathList {
+					if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
+						diffCommentData.hasSyncableComponens = true
+						break
+					}
+				}
+
+				err, templateOutput := executeTemplate(ghPrClientDetails.PrLogger, "argoCdDiff", "argoCD-diff-pr-comment.gotmpl", diffCommentData)
+				if err != nil {
+					prHandleError = err
+					log.Errorf("Failed to generate ArgoCD diff comment template: err=%s\n", err)
+				}
+				err = commentPR(ghPrClientDetails, templateOutput)
+				if err != nil {
+					prHandleError = err
+					log.Errorf("Failed to comment ArgoCD diff: err=%s\n", err)
+				}
+			} else {
+				ghPrClientDetails.PrLogger.Debugf("Diff not find affected ArogCD apps")
 			}
 		}
 		ghPrClientDetails.PrLogger.Infoln("Checking for Drift")
@@ -323,6 +348,36 @@ func handleEvent(eventPayloadInterface interface{}, mainGhClientCache *lru.Cache
 	}
 }
 
+func analyzeCommentUpdateCheckBox(newBody string, oldBody string, checkboxPattern string) (wasCheckedBefore bool, isCheckedNow bool) {
+	checkBoxRegex := regexp.MustCompile(checkboxPattern)
+	oldCheckBoxContent := checkBoxRegex.FindStringSubmatch(oldBody)
+	newCheckBoxContent := checkBoxRegex.FindStringSubmatch(newBody)
+
+	// I'm grabbing the second group of the regex, which is the checkbox content (either "x" or " ")
+	// The first element of the result is the whole match
+	if len(newCheckBoxContent) < 2 || len(oldCheckBoxContent) < 2 {
+		return false, false
+	}
+	if len(newCheckBoxContent) >= 2 {
+		if newCheckBoxContent[1] == "x" {
+			isCheckedNow = true
+		}
+	}
+
+	if len(oldCheckBoxContent) >= 2 {
+		if oldCheckBoxContent[1] == "x" {
+			wasCheckedBefore = true
+		}
+	}
+
+	return
+}
+
+func isSyncFromBranchAllowedForThisPath(allowedPathRegex string, path string) bool {
+	allowedPathsRegex := regexp.MustCompile(allowedPathRegex)
+	return allowedPathsRegex.MatchString(path)
+}
+
 func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueCommentEvent) error {
 	defaultBranch, _ := ghPrClientDetails.GetDefaultBranch()
 	config, err := GetInRepoConfig(ghPrClientDetails, defaultBranch)
@@ -332,6 +387,39 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 	// Comment events doesn't have Ref/SHA in payload, enriching the object:
 	_, _ = ghPrClientDetails.GetRef()
 	_, _ = ghPrClientDetails.GetSHA()
+
+	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
+	checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Issue.Body, checkboxPattern)
+	if !checkboxWaschecked && checkboxIsChecked {
+		ghPrClientDetails.PrLogger.Infof("Sync Checkbox was checked")
+		if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {
+			ghPrClientDetails.getPrMetadata(ce.Issue.GetBody()) // TODO is issue is not a PR but an actual issue, this will fail?
+
+			// Promotion PR have the list of paths to promote in the PR metadata
+			// For non promotion PR, we will generate the list of changed components based the PR changed files and the telefonistka configuration(sourcePath)
+			var componentPathList []string
+			if len(ghPrClientDetails.PrMetadata.PromotedPaths) > 0 {
+				componentPathList = ghPrClientDetails.PrMetadata.PromotedPaths
+			} else {
+				componentPathList, err = generateListOfChangedComponentPaths(ghPrClientDetails, config)
+				if err != nil {
+					ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)
+				}
+			}
+
+			for _, componentPath := range componentPathList {
+				if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
+					err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, ghPrClientDetails.PrSHA, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
+					if err != nil {
+						ghPrClientDetails.PrLogger.Errorf("Failed to sync ArgoCD app from branch: err=%s\n", err)
+					}
+				}
+
+			}
+		}
+	}
+
+	// I should probably deprecated this in part altogether.
 	for commentSubstring, commitStatusContext := range config.ToggleCommitStatus {
 		if strings.Contains(*ce.Comment.Body, "/"+commentSubstring) {
 			err := ghPrClientDetails.ToggleCommitStatus(commitStatusContext, *ce.Sender.Name)
diff --git a/internal/pkg/githubapi/github_test.go b/internal/pkg/githubapi/github_test.go
index 84b5c95..7786fce 100644
--- a/internal/pkg/githubapi/github_test.go
+++ b/internal/pkg/githubapi/github_test.go
@@ -62,3 +62,90 @@ func TestGenerateSafePromotionBranchNameLongTargets(t *testing.T) {
 		t.Errorf("Expected branch name to be less than 250 characters, got %d", len(result))
 	}
 }
+
+// Testing a case when a checkbox is marked
+func TestAnalyzeCommentUpdateCheckBoxChecked(t *testing.T) {
+	t.Parallel()
+	newBody := `This is a comment
+foobar
+- [x] <!-- check-slug-1 --> Description of checkbox
+foobar`
+
+	oldBody := `This is a comment
+foobar
+- [ ] <!-- check-slug-1 --> Description of checkbox
+foobar`
+	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`
+
+	wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(newBody, oldBody, checkboxPattern)
+	if !isCheckedNow {
+		t.Error("Expected isCheckedNow to be true")
+	}
+	if wasCheckedBefore {
+		t.Errorf("Expected wasCheckedBeforeto be false, actaully got %t", wasCheckedBefore)
+	}
+}
+
+// Testing a case when a checkbox is unmarked
+func TestAnalyzeCommentUpdateCheckBoxUnChecked(t *testing.T) {
+	t.Parallel()
+	newBody := `This is a comment
+foobar
+- [ ] <!-- check-slug-1 --> Description of checkbox
+foobar`
+
+	oldBody := `This is a comment
+foobar
+- [x] <!-- check-slug-1 --> Description of checkbox
+foobar`
+	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`
+
+	wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(newBody, oldBody, checkboxPattern)
+	if isCheckedNow {
+		t.Error("Expected isCheckedNow to be false")
+	}
+	if !wasCheckedBefore {
+		t.Error("Expected wasCheckedBeforeto be true")
+	}
+}
+
+// Testing a case when a checkbox isn't in the comment body
+func TestAnalyzeCommentUpdateCheckBoxNonRelevent(t *testing.T) {
+	t.Parallel()
+	newBody := `This is a comment
+foobar
+foobar`
+
+	oldBody := `This is a comment
+foobar2
+foobar2`
+	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`
+
+	wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(newBody, oldBody, checkboxPattern)
+	if isCheckedNow {
+		t.Error("Expected isCheckedNow to be false")
+	}
+	if wasCheckedBefore {
+		t.Error("Expected wasCheckedBeforeto be false")
+	}
+}
+
+func TestIsSyncFromBranchAllowedForThisPathTrue(t *testing.T) {
+	t.Parallel()
+	allowedPathRegex := `^workspace/.*$`
+	path := "workspace/app3"
+	result := isSyncFromBranchAllowedForThisPath(allowedPathRegex, path)
+	if !result {
+		t.Error("Expected result to be true")
+	}
+}
+
+func TestIsSyncFromBranchAllowedForThisPathFalse(t *testing.T) {
+	t.Parallel()
+	allowedPathRegex := `^workspace/.*$`
+	path := "clusters/prod/aws/eu-east-1/app3"
+	result := isSyncFromBranchAllowedForThisPath(allowedPathRegex, path)
+	if result {
+		t.Error("Expected result to be false")
+	}
+}
diff --git a/templates/argoCD-diff-pr-comment.gotmpl b/templates/argoCD-diff-pr-comment.gotmpl
index 7ac719b..47bb9ec 100644
--- a/templates/argoCD-diff-pr-comment.gotmpl
+++ b/templates/argoCD-diff-pr-comment.gotmpl
@@ -1,6 +1,6 @@
 {{define "argoCdDiff"}}
 Diff of ArgoCD applications:
-{{ range $appDiffResult := . }}
+{{ range $appDiffResult := .diffOfChangedComponents }}
 
 
 {{if $appDiffResult.DiffError }}
@@ -32,4 +32,10 @@ No diff 🤷
 {{- end }}
 
 {{- end }}
+
+{{- if .hasSyncableComponens }}
+- [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps to sync to current branch revision  
+{{- end}}
+
+
 {{- end }}

From d22f30a7715513f36afcb68769d0bb338ae38817 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 28 Jun 2024 12:58:17 +0200
Subject: [PATCH 02/34] Address linter issues

---
 internal/pkg/argocd/argocd.go         | 19 ++++++++++++-------
 internal/pkg/githubapi/github.go      |  5 -----
 internal/pkg/githubapi/github_test.go |  2 +-
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index ef4fe1b..de4e5b3 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -6,7 +6,6 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"io"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -146,8 +145,6 @@ func getEnv(key, fallback string) string {
 }
 
 func createArgoCdClients() (appClient application.ApplicationServiceClient, projClient projectpkg.ProjectServiceClient, settingClient settings.SettingsServiceClient, err error) {
-	var conn io.Closer
-
 	plaintext, _ := strconv.ParseBool(getEnv("ARGOCD_PLAINTEXT", "false"))
 	insecure, _ := strconv.ParseBool(getEnv("ARGOCD_INSECURE", "false"))
 
@@ -163,21 +160,23 @@ func createArgoCdClients() (appClient application.ApplicationServiceClient, proj
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD API client: %v", err)
 	}
 
-	conn, appClient, err = client.NewApplicationClient()
+	appClntConn, appClient, err := client.NewApplicationClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD app client: %v", err)
 	}
+	defer argoio.Close(appClntConn)
 
-	conn, projClient, err = client.NewProjectClient()
+	projClntConn, projClient, err := client.NewProjectClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD project client: %v", err)
 	}
+	defer argoio.Close(projClntConn)
 
-	conn, settingClient, err = client.NewSettingsClient()
+	setClntConn, settingClient, err := client.NewSettingsClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD settings client: %v", err)
 	}
-	defer argoio.Close(conn)
+	defer argoio.Close(setClntConn)
 	return
 }
 
@@ -254,11 +253,17 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	var foundApp *argoappv1.Application
 	var err error
 	appClient, _, _, err := createArgoCdClients()
+	if err != nil {
+		return fmt.Errorf("Error creating ArgoCD clients: %v", err)
+	}
 	if useSHALabelForArgoDicovery {
 		foundApp, err = findArgocdAppBySHA1Label(ctx, componentPath, repo, appClient)
 	} else {
 		foundApp, err = findArgocdAppByManifestPathAnnotation(ctx, componentPath, repo, appClient)
 	}
+	if err != nil {
+		return fmt.Errorf("Error finding ArgoCD application for component path %s: %v", componentPath, err)
+	}
 	if foundApp.Spec.Source.TargetRevision == revision {
 		log.Infof("App %s already has revision %s", foundApp.Name, revision)
 		return nil
diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 57d6ea4..1d66069 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -71,7 +71,6 @@ func (pm prMetadata) serialize() (string, error) {
 }
 
 func (ghPrClientDetails *GhPrClientDetails) getPrMetadata(prBody string) {
-
 	prMetadataRegex := regexp.MustCompile(`<!--\|.*\|(.*)\|-->`)
 	serializedPrMetadata := prMetadataRegex.FindStringSubmatch(prBody)
 	if len(serializedPrMetadata) == 2 {
@@ -83,11 +82,9 @@ func (ghPrClientDetails *GhPrClientDetails) getPrMetadata(prBody string) {
 			}
 		}
 	}
-
 }
 
 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
 	wasCommitStatusSet := false
@@ -161,7 +158,6 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 			}
 
 			if len(diffOfChangedComponents) > 0 {
-
 				diffCommentData := struct {
 					diffOfChangedComponents []argocd.DiffResult
 					hasSyncableComponens    bool
@@ -414,7 +410,6 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 						ghPrClientDetails.PrLogger.Errorf("Failed to sync ArgoCD app from branch: err=%s\n", err)
 					}
 				}
-
 			}
 		}
 	}
diff --git a/internal/pkg/githubapi/github_test.go b/internal/pkg/githubapi/github_test.go
index 7786fce..ebe7a2a 100644
--- a/internal/pkg/githubapi/github_test.go
+++ b/internal/pkg/githubapi/github_test.go
@@ -82,7 +82,7 @@ foobar`
 		t.Error("Expected isCheckedNow to be true")
 	}
 	if wasCheckedBefore {
-		t.Errorf("Expected wasCheckedBeforeto be false, actaully got %t", wasCheckedBefore)
+		t.Errorf("Expected wasCheckedBeforeto be false, got %t", wasCheckedBefore)
 	}
 }
 

From 26ad53a6bb50315c15f6a493f2ccaaa0b6b1d1fa Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 28 Jun 2024 13:04:46 +0200
Subject: [PATCH 03/34] Linter issue

---
 internal/pkg/argocd/argocd.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index de4e5b3..c00c930 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -276,7 +276,6 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		Spec:         &foundApp.Spec,
 		AppNamespace: &foundApp.Namespace,
 	})
-
 	if err != nil {
 		return fmt.Errorf("Error setting app %s revision to  %s failed: %v", foundApp.Name, revision, err)
 	}

From 0a550ecc1cafb26245db5d51d434688d1628e016 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 28 Jun 2024 14:56:22 +0200
Subject: [PATCH 04/34] debug defer issue

---
 internal/pkg/argocd/argocd.go | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index c00c930..dff28c0 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -20,7 +20,6 @@ import (
 	argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	argodiff "github.com/argoproj/argo-cd/v2/util/argo/diff"
 	"github.com/argoproj/argo-cd/v2/util/argo/normalizers"
-	argoio "github.com/argoproj/argo-cd/v2/util/io"
 	"github.com/argoproj/gitops-engine/pkg/sync/hook"
 	"github.com/google/go-cmp/cmp"
 	log "github.com/sirupsen/logrus"
@@ -160,23 +159,26 @@ func createArgoCdClients() (appClient application.ApplicationServiceClient, proj
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD API client: %v", err)
 	}
 
-	appClntConn, appClient, err := client.NewApplicationClient()
+	_, appClient, err = client.NewApplicationClient()
+	// appClntConn, appClient, err := client.NewApplicationClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD app client: %v", err)
 	}
-	defer argoio.Close(appClntConn)
+	// defer argoio.Close(appClntConn)
 
-	projClntConn, projClient, err := client.NewProjectClient()
+	_, projClient, err = client.NewProjectClient()
+	// projClntConn, projClient, err := client.NewProjectClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD project client: %v", err)
 	}
-	defer argoio.Close(projClntConn)
+	// defer argoio.Close(projClntConn)
 
-	setClntConn, settingClient, err := client.NewSettingsClient()
+	_, settingClient, err = client.NewSettingsClient()
+	// setClntConn, settingClient, err := client.NewSettingsClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD settings client: %v", err)
 	}
-	defer argoio.Close(setClntConn)
+	// defer argoio.Close(setClntConn)
 	return
 }
 

From 63325cd222fbff8ddc141948642402873f4ea338 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 28 Jun 2024 16:03:08 +0200
Subject: [PATCH 05/34] Adress "diffOfChangedComponents is an unexported field
 of struct" issue

---
 internal/pkg/githubapi/github.go        | 8 ++++----
 templates/argoCD-diff-pr-comment.gotmpl | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 1d66069..4639614 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -159,15 +159,15 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 
 			if len(diffOfChangedComponents) > 0 {
 				diffCommentData := struct {
-					diffOfChangedComponents []argocd.DiffResult
-					hasSyncableComponens    bool
+					DiffOfChangedComponents []argocd.DiffResult
+					HasSyncableComponens    bool
 				}{
-					diffOfChangedComponents: diffOfChangedComponents,
+					DiffOfChangedComponents: diffOfChangedComponents,
 				}
 
 				for _, componentPath := range componentPathList {
 					if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
-						diffCommentData.hasSyncableComponens = true
+						diffCommentData.HasSyncableComponens = true
 						break
 					}
 				}
diff --git a/templates/argoCD-diff-pr-comment.gotmpl b/templates/argoCD-diff-pr-comment.gotmpl
index 47bb9ec..9e34328 100644
--- a/templates/argoCD-diff-pr-comment.gotmpl
+++ b/templates/argoCD-diff-pr-comment.gotmpl
@@ -1,6 +1,6 @@
 {{define "argoCdDiff"}}
 Diff of ArgoCD applications:
-{{ range $appDiffResult := .diffOfChangedComponents }}
+{{ range $appDiffResult := .DiffOfChangedComponents }}
 
 
 {{if $appDiffResult.DiffError }}
@@ -33,7 +33,7 @@ No diff 🤷
 
 {{- end }}
 
-{{- if .hasSyncableComponens }}
+{{- if .HasSyncableComponens }}
 - [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps to sync to current branch revision  
 {{- end}}
 

From f9b06af524de180d737321d0e7aa641b5035186d Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 28 Jun 2024 16:39:01 +0200
Subject: [PATCH 06/34] Use comemnt sender login to check in event was
 triggered by Telefonistka itself. Comment.User.Login isn't relevant when
 editing a comment made by someone else

---
 internal/pkg/githubapi/github.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 4639614..3ce867f 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -322,7 +322,7 @@ func handleEvent(eventPayloadInterface interface{}, mainGhClientCache *lru.Cache
 			"repo":     *eventPayload.Repo.Owner.Login + "/" + *eventPayload.Repo.Name,
 			"prNumber": *eventPayload.Issue.Number,
 		})
-		if *eventPayload.Comment.User.Login != botIdentity {
+		if *eventPayload.Sender.Login != botIdentity {
 			ghPrClientDetails := GhPrClientDetails{
 				Ctx:          ctx,
 				GhClientPair: &mainGithubClientPair,

From 82225a36796fcc700fe906133791cd186452337c Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 28 Jun 2024 16:52:51 +0200
Subject: [PATCH 07/34] Use the correct field in the event payload

---
 internal/pkg/githubapi/github.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 3ce867f..36b4585 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -385,7 +385,7 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 	_, _ = ghPrClientDetails.GetSHA()
 
 	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
-	checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Issue.Body, checkboxPattern)
+	checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Changes.Body.From, checkboxPattern)
 	if !checkboxWaschecked && checkboxIsChecked {
 		ghPrClientDetails.PrLogger.Infof("Sync Checkbox was checked")
 		if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {

From b2ed2bc299cc2ea8d0c1b36eb8b13478bde4d10c Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Mon, 1 Jul 2024 09:31:35 +0200
Subject: [PATCH 08/34] fix some whitespace issue

---
 templates/argoCD-diff-pr-comment.gotmpl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/templates/argoCD-diff-pr-comment.gotmpl b/templates/argoCD-diff-pr-comment.gotmpl
index 9e34328..01dadb9 100644
--- a/templates/argoCD-diff-pr-comment.gotmpl
+++ b/templates/argoCD-diff-pr-comment.gotmpl
@@ -34,8 +34,10 @@ No diff 🤷
 {{- end }}
 
 {{- if .HasSyncableComponens }}
+
 - [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps to sync to current branch revision  
-{{- end}}
+
+{{ end}}
 
 
 {{- end }}

From e4cbcadbb6e088a2638158947c5c249e1cd5e0e3 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Mon, 1 Jul 2024 09:31:58 +0200
Subject: [PATCH 09/34] debug some panic

---
 internal/pkg/githubapi/github.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 36b4585..4358695 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -385,6 +385,8 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 	_, _ = ghPrClientDetails.GetSHA()
 
 	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
+	ghPrClientDetails.PrLogger.Debugf("=== old body: %s\n", *ce.Changes.Body.From)
+	ghPrClientDetails.PrLogger.Debugf("=== new body: %s\n", *ce.Comment.Body)
 	checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Changes.Body.From, checkboxPattern)
 	if !checkboxWaschecked && checkboxIsChecked {
 		ghPrClientDetails.PrLogger.Infof("Sync Checkbox was checked")

From 0c0d06f7746c4131c93bdb30179e21d68298e4eb Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Mon, 1 Jul 2024 10:24:46 +0200
Subject: [PATCH 10/34] Only check for checkbox event on **updates** on comment
 the bot created.

---
 internal/pkg/argocd/argocd.go    |  2 ++
 internal/pkg/githubapi/github.go | 56 +++++++++++++++++---------------
 2 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index dff28c0..c55fb36 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -280,6 +280,8 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	})
 	if err != nil {
 		return fmt.Errorf("Error setting app %s revision to  %s failed: %v", foundApp.Name, revision, err)
+	} else {
+		log.Infof("ArgoCD App %s revision set to %s", foundApp.Name, revision)
 	}
 
 	return nil
diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 4358695..499c361 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -322,6 +322,7 @@ func handleEvent(eventPayloadInterface interface{}, mainGhClientCache *lru.Cache
 			"repo":     *eventPayload.Repo.Owner.Login + "/" + *eventPayload.Repo.Name,
 			"prNumber": *eventPayload.Issue.Number,
 		})
+		// Ignore comment even sent by the bot (this is about who trigger the event not who wrote the comment)
 		if *eventPayload.Sender.Login != botIdentity {
 			ghPrClientDetails := GhPrClientDetails{
 				Ctx:          ctx,
@@ -333,7 +334,7 @@ func handleEvent(eventPayloadInterface interface{}, mainGhClientCache *lru.Cache
 				PrAuthor:     *eventPayload.Issue.User.Login,
 				PrLogger:     prLogger,
 			}
-			_ = handleCommentPrEvent(ghPrClientDetails, eventPayload)
+			_ = handleCommentPrEvent(ghPrClientDetails, eventPayload, botIdentity)
 		} else {
 			log.Debug("Ignoring self comment")
 		}
@@ -374,7 +375,7 @@ func isSyncFromBranchAllowedForThisPath(allowedPathRegex string, path string) bo
 	return allowedPathsRegex.MatchString(path)
 }
 
-func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueCommentEvent) error {
+func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueCommentEvent, botIdentity string) error {
 	defaultBranch, _ := ghPrClientDetails.GetDefaultBranch()
 	config, err := GetInRepoConfig(ghPrClientDetails, defaultBranch)
 	if err != nil {
@@ -384,39 +385,40 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 	_, _ = ghPrClientDetails.GetRef()
 	_, _ = ghPrClientDetails.GetSHA()
 
-	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
-	ghPrClientDetails.PrLogger.Debugf("=== old body: %s\n", *ce.Changes.Body.From)
-	ghPrClientDetails.PrLogger.Debugf("=== new body: %s\n", *ce.Comment.Body)
-	checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Changes.Body.From, checkboxPattern)
-	if !checkboxWaschecked && checkboxIsChecked {
-		ghPrClientDetails.PrLogger.Infof("Sync Checkbox was checked")
-		if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {
-			ghPrClientDetails.getPrMetadata(ce.Issue.GetBody()) // TODO is issue is not a PR but an actual issue, this will fail?
-
-			// Promotion PR have the list of paths to promote in the PR metadata
-			// For non promotion PR, we will generate the list of changed components based the PR changed files and the telefonistka configuration(sourcePath)
-			var componentPathList []string
-			if len(ghPrClientDetails.PrMetadata.PromotedPaths) > 0 {
-				componentPathList = ghPrClientDetails.PrMetadata.PromotedPaths
-			} else {
-				componentPathList, err = generateListOfChangedComponentPaths(ghPrClientDetails, config)
-				if err != nil {
-					ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)
+	// This part should only happen on edits of bot comments - this part is about catching checkbox changes
+	if *ce.Action == "edited" && *ce.Comment.User.Login == botIdentity {
+		checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
+		checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Changes.Body.From, checkboxPattern)
+		if !checkboxWaschecked && checkboxIsChecked {
+			ghPrClientDetails.PrLogger.Infof("Sync Checkbox was checked")
+			if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {
+				ghPrClientDetails.getPrMetadata(ce.Issue.GetBody()) // TODO is issue is not a PR but an actual issue, this will fail?
+
+				// Promotion PR have the list of paths to promote in the PR metadata
+				// For non promotion PR, we will generate the list of changed components based the PR changed files and the telefonistka configuration(sourcePath)
+				var componentPathList []string
+				if len(ghPrClientDetails.PrMetadata.PromotedPaths) > 0 {
+					componentPathList = ghPrClientDetails.PrMetadata.PromotedPaths
+				} else {
+					componentPathList, err = generateListOfChangedComponentPaths(ghPrClientDetails, config)
+					if err != nil {
+						ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)
+					}
 				}
-			}
 
-			for _, componentPath := range componentPathList {
-				if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
-					err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, ghPrClientDetails.PrSHA, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
-					if err != nil {
-						ghPrClientDetails.PrLogger.Errorf("Failed to sync ArgoCD app from branch: err=%s\n", err)
+				for _, componentPath := range componentPathList {
+					if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
+						err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, ghPrClientDetails.PrSHA, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
+						if err != nil {
+							ghPrClientDetails.PrLogger.Errorf("Failed to sync ArgoCD app from branch: err=%s\n", err)
+						}
 					}
 				}
 			}
 		}
 	}
 
-	// I should probably deprecated this in part altogether.
+	// I should probably deprecated this whole part altogether.
 	for commentSubstring, commitStatusContext := range config.ToggleCommitStatus {
 		if strings.Contains(*ce.Comment.Body, "/"+commentSubstring) {
 			err := ghPrClientDetails.ToggleCommitStatus(commitStatusContext, *ce.Sender.Name)

From 8cb4c1a6febf1fdddeaf97aa8d8a61b9223da7a5 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Mon, 1 Jul 2024 11:11:19 +0200
Subject: [PATCH 11/34] On PR merge - set apps TargetRef to HEAD

refacore: move some of the complexity in to generateListOfRelevantComponents to make stuff more DRY
---
 internal/pkg/githubapi/github.go    | 43 ++++++++++++++---------------
 internal/pkg/githubapi/promotion.go | 20 +++++++++-----
 2 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 499c361..be62089 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -115,18 +115,10 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 			ghPrClientDetails.PrLogger.Errorf("Failed to minimize stale comments: err=%s\n", err)
 		}
 		if config.CommentArgocdDiffonPR {
-			var componentPathList []string
-
-			// Promotion PR have the list of paths to promote in the PR metadata
-			// For non promotion PR, we will generate the list of changed components based the PR changed files and the telefonistka configuration(sourcePath)
-			if DoesPrHasLabel(*eventPayload, "promotion") {
-				componentPathList = ghPrClientDetails.PrMetadata.PromotedPaths
-			} else {
-				componentPathList, err = generateListOfChangedComponentPaths(ghPrClientDetails, config)
-				if err != nil {
-					prHandleError = err
-					ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)
-				}
+			componentPathList, err := generateListOfChangedComponentPaths(ghPrClientDetails, config)
+			if err != nil {
+				prHandleError = err
+				ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)
 			}
 
 			hasComponentDiff, hasComponentDiffErrors, diffOfChangedComponents, err := argocd.GenerateDiffOfChangedComponents(ctx, componentPathList, ghPrClientDetails.Ref, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
@@ -394,16 +386,9 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 			if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {
 				ghPrClientDetails.getPrMetadata(ce.Issue.GetBody()) // TODO is issue is not a PR but an actual issue, this will fail?
 
-				// Promotion PR have the list of paths to promote in the PR metadata
-				// For non promotion PR, we will generate the list of changed components based the PR changed files and the telefonistka configuration(sourcePath)
-				var componentPathList []string
-				if len(ghPrClientDetails.PrMetadata.PromotedPaths) > 0 {
-					componentPathList = ghPrClientDetails.PrMetadata.PromotedPaths
-				} else {
-					componentPathList, err = generateListOfChangedComponentPaths(ghPrClientDetails, config)
-					if err != nil {
-						ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)
-					}
+				componentPathList, err := generateListOfChangedComponentPaths(ghPrClientDetails, config)
+				if err != nil {
+					ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)
 				}
 
 				for _, componentPath := range componentPathList {
@@ -598,6 +583,20 @@ func handleMergedPrEvent(ghPrClientDetails GhPrClientDetails, prApproverGithubCl
 	} else {
 		commentPlanInPR(ghPrClientDetails, promotions)
 	}
+
+	if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {
+		componentPathList, err := generateListOfChangedComponentPaths(ghPrClientDetails, config)
+		for _, componentPath := range componentPathList {
+			if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
+				ghPrClientDetails.PrLogger.Errorf("Ensuring ArgoCD app %s is set to HEAD\n", componentPath)
+				err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, "HEAD", ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
+				if err != nil {
+					ghPrClientDetails.PrLogger.Errorf("Failed to set  ArgoCD app %s, to HEAD: err=%s\n", err)
+				}
+			}
+		}
+	}
+
 	return nil
 }
 
diff --git a/internal/pkg/githubapi/promotion.go b/internal/pkg/githubapi/promotion.go
index b9b013f..2bf73fd 100644
--- a/internal/pkg/githubapi/promotion.go
+++ b/internal/pkg/githubapi/promotion.go
@@ -170,14 +170,20 @@ type relevantComponent struct {
 	AutoMerge     bool
 }
 
-// This function basically turns the map with struct keys into a list of strings
 func generateListOfChangedComponentPaths(ghPrClientDetails GhPrClientDetails, config *cfg.Config) (changedComponentPaths []string, err error) {
-	relevantComponents, err := generateListOfRelevantComponents(ghPrClientDetails, config)
-	if err != nil {
-		return nil, err
-	}
-	for component := range relevantComponents {
-		changedComponentPaths = append(changedComponentPaths, component.SourcePath+component.ComponentName)
+
+	// If the PR has a list of promoted paths in the PR Telefonistika metadata(=is a promotion PR), we use that
+	if len(ghPrClientDetails.PrMetadata.PromotedPaths) > 0 {
+		changedComponentPaths = ghPrClientDetails.PrMetadata.PromotedPaths
+	} else {
+		// If not we will use in-repo config to geenrate it, and turns the map with struct keys into a list of strings
+		relevantComponents, err := generateListOfRelevantComponents(ghPrClientDetails, config)
+		if err != nil {
+			return nil, err
+		}
+		for component := range relevantComponents {
+			changedComponentPaths = append(changedComponentPaths, component.SourcePath+component.ComponentName)
+		}
 	}
 	return changedComponentPaths, nil
 }

From ea57c0e35ab997657f5742be031db98f5c14c938 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Mon, 1 Jul 2024 11:24:20 +0200
Subject: [PATCH 12/34] Linter issues

---
 internal/pkg/githubapi/github.go    | 18 +++++++++++-------
 internal/pkg/githubapi/promotion.go |  1 -
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index be62089..1b3ba77 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -586,18 +586,22 @@ func handleMergedPrEvent(ghPrClientDetails GhPrClientDetails, prApproverGithubCl
 
 	if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {
 		componentPathList, err := generateListOfChangedComponentPaths(ghPrClientDetails, config)
-		for _, componentPath := range componentPathList {
-			if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
-				ghPrClientDetails.PrLogger.Errorf("Ensuring ArgoCD app %s is set to HEAD\n", componentPath)
-				err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, "HEAD", ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
-				if err != nil {
-					ghPrClientDetails.PrLogger.Errorf("Failed to set  ArgoCD app %s, to HEAD: err=%s\n", err)
+		if err != nil {
+			ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components for setting ArgoCD app targetRef to HEAD: err=%s\n", err)
+		} else {
+			for _, componentPath := range componentPathList {
+				if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
+					ghPrClientDetails.PrLogger.Errorf("Ensuring ArgoCD app %s is set to HEAD\n", componentPath)
+					err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, "HEAD", ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
+					if err != nil {
+						ghPrClientDetails.PrLogger.Errorf("Failed to set ArgoCD app @  %s, to HEAD: err=%s\n", componentPath, err)
+					}
 				}
 			}
 		}
 	}
 
-	return nil
+	return err
 }
 
 // Creating a unique branch name based on the PR number, PR ref and the promotion target paths
diff --git a/internal/pkg/githubapi/promotion.go b/internal/pkg/githubapi/promotion.go
index 2bf73fd..d15a3f1 100644
--- a/internal/pkg/githubapi/promotion.go
+++ b/internal/pkg/githubapi/promotion.go
@@ -171,7 +171,6 @@ type relevantComponent struct {
 }
 
 func generateListOfChangedComponentPaths(ghPrClientDetails GhPrClientDetails, config *cfg.Config) (changedComponentPaths []string, err error) {
-
 	// If the PR has a list of promoted paths in the PR Telefonistika metadata(=is a promotion PR), we use that
 	if len(ghPrClientDetails.PrMetadata.PromotedPaths) > 0 {
 		changedComponentPaths = ghPrClientDetails.PrMetadata.PromotedPaths

From c54fbac65e313af5b6cb71f1dc145fb509c896bf Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Mon, 1 Jul 2024 14:01:44 +0200
Subject: [PATCH 13/34] Change method used to apply changes to ArgoCD apps

---
 internal/pkg/argocd/argocd.go | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index c55fb36..32f64ed 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -273,10 +273,14 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 
 	foundApp.Spec.Source.TargetRevision = revision
 
-	_, err = appClient.UpdateSpec(ctx, &application.ApplicationUpdateSpecRequest{
-		Name:         &foundApp.Name,
-		Spec:         &foundApp.Spec,
-		AppNamespace: &foundApp.Namespace,
+	// _, err = appClient.UpdateSpec(ctx, &application.ApplicationUpdateSpecRequest{
+	// Name:         &foundApp.Name,
+	// Spec:         &foundApp.Spec,
+	// AppNamespace: &foundApp.Namespace,
+	// })
+	_, err = appClient.Update(ctx, &application.ApplicationUpdateRequest{
+		Application: foundApp,
+		Project:     &foundApp.Spec.Project,
 	})
 	if err != nil {
 		return fmt.Errorf("Error setting app %s revision to  %s failed: %v", foundApp.Name, revision, err)
@@ -284,7 +288,7 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		log.Infof("ArgoCD App %s revision set to %s", foundApp.Name, revision)
 	}
 
-	return nil
+	return err
 }
 
 func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranch string, repo string, appClient application.ApplicationServiceClient, projClient projectpkg.ProjectServiceClient, argoSettings *settings.Settings, useSHALabelForArgoDicovery bool) (componentDiffResult DiffResult) {

From e20f5b566e5a9825b0a91bc4466b26ab6449122f Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Mon, 1 Jul 2024 14:52:53 +0200
Subject: [PATCH 14/34] Debug " application destination can't have both name
 and server defined" issue

---
 internal/pkg/argocd/argocd.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index 32f64ed..1d4f855 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -271,6 +271,7 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		return nil
 	}
 
+	log.Debugf("=== %v ===", foundApp.Spec.Destination)
 	foundApp.Spec.Source.TargetRevision = revision
 
 	// _, err = appClient.UpdateSpec(ctx, &application.ApplicationUpdateSpecRequest{

From 1e28e50a68a957b1ee8a2c494bf2aa9cfd72c0e2 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Mon, 1 Jul 2024 15:52:34 +0200
Subject: [PATCH 15/34] Use Patch method to be more surgical.

Try to avoid the "InvalidArgument desc = application destination spec
for foobar-plg-gcp-eu-west1-v1 is invalid: application destination can't
have both name and server defined:" error message
---
 internal/pkg/argocd/argocd.go | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index 1d4f855..cbc6028 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -272,16 +272,25 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	}
 
 	log.Debugf("=== %v ===", foundApp.Spec.Destination)
-	foundApp.Spec.Source.TargetRevision = revision
 
+	// foundApp.Spec.Source.TargetRevision = revision
 	// _, err = appClient.UpdateSpec(ctx, &application.ApplicationUpdateSpecRequest{
 	// Name:         &foundApp.Name,
 	// Spec:         &foundApp.Spec,
 	// AppNamespace: &foundApp.Namespace,
 	// })
-	_, err = appClient.Update(ctx, &application.ApplicationUpdateRequest{
-		Application: foundApp,
-		Project:     &foundApp.Spec.Project,
+
+	// _, err = appClient.Update(ctx, &application.ApplicationUpdateRequest{
+	// Application: foundApp,
+	// Project:     &foundApp.Spec.Project,
+	// })
+
+	patchType := "merge"
+	patch := fmt.Sprintf(`{"spec": {"source": {"targetRevision": "%s"}}}`, revision)
+	_, err = appClient.Patch(ctx, &application.ApplicationPatchRequest{
+		Name:      &foundApp.Name,
+		PatchType: &patchType,
+		Patch:     &patch,
 	})
 	if err != nil {
 		return fmt.Errorf("Error setting app %s revision to  %s failed: %v", foundApp.Name, revision, err)

From eb3394493997a55ea4d28e146a3f268e5e9c3256 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Tue, 2 Jul 2024 09:16:41 +0200
Subject: [PATCH 16/34] Remove old method for App update and debug message

---
 internal/pkg/argocd/argocd.go | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index cbc6028..a29f52a 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -271,20 +271,6 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		return nil
 	}
 
-	log.Debugf("=== %v ===", foundApp.Spec.Destination)
-
-	// foundApp.Spec.Source.TargetRevision = revision
-	// _, err = appClient.UpdateSpec(ctx, &application.ApplicationUpdateSpecRequest{
-	// Name:         &foundApp.Name,
-	// Spec:         &foundApp.Spec,
-	// AppNamespace: &foundApp.Namespace,
-	// })
-
-	// _, err = appClient.Update(ctx, &application.ApplicationUpdateRequest{
-	// Application: foundApp,
-	// Project:     &foundApp.Spec.Project,
-	// })
-
 	patchType := "merge"
 	patch := fmt.Sprintf(`{"spec": {"source": {"targetRevision": "%s"}}}`, revision)
 	_, err = appClient.Patch(ctx, &application.ApplicationPatchRequest{

From 80f34435e81488517c6d829392250a3f8c04c0ea Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Tue, 2 Jul 2024 10:47:51 +0200
Subject: [PATCH 17/34] Build patch from a structured object

---
 internal/pkg/argocd/argocd.go | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index a29f52a..f491383 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -271,15 +271,27 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		return nil
 	}
 
+	patchObject := argoappv1.Application{
+		Spec: argoappv1.ApplicationSpec{
+			Source: &argoappv1.ApplicationSource{
+				TargetRevision: revision,
+			},
+		},
+	}
+	patchJson, err := json.Marshal(patchObject)
+	if err != nil {
+		return fmt.Errorf("Error marshalling patch object: %v\n%v", err, patchObject)
+	}
+	patch := string(patchJson)
+
 	patchType := "merge"
-	patch := fmt.Sprintf(`{"spec": {"source": {"targetRevision": "%s"}}}`, revision)
 	_, err = appClient.Patch(ctx, &application.ApplicationPatchRequest{
 		Name:      &foundApp.Name,
 		PatchType: &patchType,
 		Patch:     &patch,
 	})
 	if err != nil {
-		return fmt.Errorf("Error setting app %s revision to  %s failed: %v", foundApp.Name, revision, err)
+		return fmt.Errorf("Error patching app %s revision to  %s failed: %v\n, patch: %v", foundApp.Name, revision, err, patch)
 	} else {
 		log.Infof("ArgoCD App %s revision set to %s", foundApp.Name, revision)
 	}

From c2358715202871e61bad1f53210717d40b07f46c Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Tue, 2 Jul 2024 11:14:10 +0200
Subject: [PATCH 18/34] Only allow sync for Open PRs

---
 internal/pkg/githubapi/github.go | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 1b3ba77..ae9c9b3 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -377,15 +377,14 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 	_, _ = ghPrClientDetails.GetRef()
 	_, _ = ghPrClientDetails.GetSHA()
 
-	// This part should only happen on edits of bot comments - this part is about catching checkbox changes
-	if *ce.Action == "edited" && *ce.Comment.User.Login == botIdentity {
+	// This part should only happen on edits of bot comments on open PRs (I'm not testing Issue vs PR as Telefonsitka only creates PRs at this point)
+	if *ce.Action == "edited" && *ce.Comment.User.Login == botIdentity && *ce.Issue.State == "open" {
 		checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
 		checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Changes.Body.From, checkboxPattern)
 		if !checkboxWaschecked && checkboxIsChecked {
 			ghPrClientDetails.PrLogger.Infof("Sync Checkbox was checked")
 			if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {
-				ghPrClientDetails.getPrMetadata(ce.Issue.GetBody()) // TODO is issue is not a PR but an actual issue, this will fail?
-
+				ghPrClientDetails.getPrMetadata(ce.Issue.GetBody())
 				componentPathList, err := generateListOfChangedComponentPaths(ghPrClientDetails, config)
 				if err != nil {
 					ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)

From 385a31b3de1d89bfc1439755233eeb2ea24c60a7 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Tue, 2 Jul 2024 11:48:19 +0200
Subject: [PATCH 19/34] Add Namespace to app patching, to ensure telefonistka
 doesn't try to creare the app in the "default" NS:

 level=error msg="Failed to sync ArgoCD app from branch: err=Error
 patching app foobar-plg-gcp-eu-west1-v1 revision to
 8b7e075554240ae773ab8532935fd2e6fd687582 failed: rpc error: code =
 PermissionDenied desc = permission denied: applications, create,
 default/foobar-plg-gcp-eu-west1-v1, sub: telefonistka
---
 internal/pkg/argocd/argocd.go | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index f491383..e19c58c 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -286,9 +286,10 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 
 	patchType := "merge"
 	_, err = appClient.Patch(ctx, &application.ApplicationPatchRequest{
-		Name:      &foundApp.Name,
-		PatchType: &patchType,
-		Patch:     &patch,
+		Name:         &foundApp.Name,
+		AppNamespace: &foundApp.Namespace,
+		PatchType:    &patchType,
+		Patch:        &patch,
 	})
 	if err != nil {
 		return fmt.Errorf("Error patching app %s revision to  %s failed: %v\n, patch: %v", foundApp.Name, revision, err, patch)

From 469270ce3017f44d85763ee16302f49f1efb7e68 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Tue, 2 Jul 2024 13:25:52 +0200
Subject: [PATCH 20/34] Add debug log line

---
 internal/pkg/argocd/argocd.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index e19c58c..07a3d3c 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -284,6 +284,8 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	}
 	patch := string(patchJson)
 
+	log.Debugf("Patching app %s/%s with: %s", foundApp.Namespace, foundApp.Name, patch)
+
 	patchType := "merge"
 	_, err = appClient.Patch(ctx, &application.ApplicationPatchRequest{
 		Name:         &foundApp.Name,

From 9db40f0a4c876fe88a6eac9283aba9fdd15c86df Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Tue, 2 Jul 2024 13:53:56 +0200
Subject: [PATCH 21/34] Explicitly set the NS

---
 internal/pkg/argocd/argocd.go | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index 07a3d3c..41f9566 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -23,6 +23,7 @@ import (
 	"github.com/argoproj/gitops-engine/pkg/sync/hook"
 	"github.com/google/go-cmp/cmp"
 	log "github.com/sirupsen/logrus"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -272,6 +273,9 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	}
 
 	patchObject := argoappv1.Application{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: foundApp.Namespace,
+		},
 		Spec: argoappv1.ApplicationSpec{
 			Source: &argoappv1.ApplicationSource{
 				TargetRevision: revision,
@@ -287,14 +291,14 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	log.Debugf("Patching app %s/%s with: %s", foundApp.Namespace, foundApp.Name, patch)
 
 	patchType := "merge"
-	_, err = appClient.Patch(ctx, &application.ApplicationPatchRequest{
+	patchedApp, err := appClient.Patch(ctx, &application.ApplicationPatchRequest{
 		Name:         &foundApp.Name,
 		AppNamespace: &foundApp.Namespace,
 		PatchType:    &patchType,
 		Patch:        &patch,
 	})
 	if err != nil {
-		return fmt.Errorf("Error patching app %s revision to  %s failed: %v\n, patch: %v", foundApp.Name, revision, err, patch)
+		return fmt.Errorf("Error patching app %s revision to  %s failed: %v\n, patch: %v\npatched app: %v\n", foundApp.Name, revision, err, patch, patchedApp)
 	} else {
 		log.Infof("ArgoCD App %s revision set to %s", foundApp.Name, revision)
 	}

From 779df8f90641addcd50165783e8fc4b72bcea4cb Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Tue, 2 Jul 2024 14:15:55 +0200
Subject: [PATCH 22/34] revert to old version using json tring

---
 internal/pkg/argocd/argocd.go | 41 +++++++++++++++++------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index 41f9566..e0c8f5d 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -23,7 +23,6 @@ import (
 	"github.com/argoproj/gitops-engine/pkg/sync/hook"
 	"github.com/google/go-cmp/cmp"
 	log "github.com/sirupsen/logrus"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -272,30 +271,30 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		return nil
 	}
 
-	patchObject := argoappv1.Application{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: foundApp.Namespace,
-		},
-		Spec: argoappv1.ApplicationSpec{
-			Source: &argoappv1.ApplicationSource{
-				TargetRevision: revision,
-			},
-		},
-	}
-	patchJson, err := json.Marshal(patchObject)
-	if err != nil {
-		return fmt.Errorf("Error marshalling patch object: %v\n%v", err, patchObject)
-	}
-	patch := string(patchJson)
-
+	// patchObject := argoappv1.Application{
+	// ObjectMeta: metav1.ObjectMeta{
+	// Namespace: foundApp.Namespace,
+	// },
+	// Spec: argoappv1.ApplicationSpec{
+	// Source: &argoappv1.ApplicationSource{
+	// TargetRevision: revision,
+	// },
+	// },
+	// }
+	// patchJson, err := json.Marshal(patchObject)
+	// if err != nil {
+	// return fmt.Errorf("Error marshalling patch object: %v\n%v", err, patchObject)
+	// }
+	// patch := string(patchJson)
+	patch := fmt.Sprintf(`{"spec": {"source": {"targetRevision": "%s"}}}`, revision)
 	log.Debugf("Patching app %s/%s with: %s", foundApp.Namespace, foundApp.Name, patch)
 
 	patchType := "merge"
 	patchedApp, err := appClient.Patch(ctx, &application.ApplicationPatchRequest{
-		Name:         &foundApp.Name,
-		AppNamespace: &foundApp.Namespace,
-		PatchType:    &patchType,
-		Patch:        &patch,
+		Name: &foundApp.Name,
+		// AppNamespace: &foundApp.Namespace,
+		PatchType: &patchType,
+		Patch:     &patch,
 	})
 	if err != nil {
 		return fmt.Errorf("Error patching app %s revision to  %s failed: %v\n, patch: %v\npatched app: %v\n", foundApp.Name, revision, err, patch, patchedApp)

From b69e0319705b951408198940763bc98002ea36af Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Tue, 2 Jul 2024 14:58:52 +0200
Subject: [PATCH 23/34] Add debug statements

---
 internal/pkg/argocd/argocd.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index e0c8f5d..578e971 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -347,6 +347,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc
 		return componentDiffResult
 	}
 	log.Debugf("Got (live)resources for app %s", app.Name)
+	log.Debugf("=== resources: %v ===", resources)
 
 	// Get the application manifests, these are the target state of the application objects, taken from the git repo, specificly from the PR branch.
 	diffOption := &DifferenceOption{}
@@ -363,6 +364,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc
 		return componentDiffResult
 	}
 	log.Debugf("Got manifests for app %s, revision %s", app.Name, prBranch)
+	log.Debugf("=== manifests: %v ===", manifests)
 	diffOption.res = manifests
 	diffOption.revision = prBranch
 

From 1f3ee4bc1392a2768f047820a9eb25db6dececcd Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Wed, 3 Jul 2024 09:32:47 +0200
Subject: [PATCH 24/34] Add more debug statements

---
 internal/pkg/githubapi/github.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index ae9c9b3..cfb8f62 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -126,7 +126,7 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 				prHandleError = err
 				ghPrClientDetails.PrLogger.Errorf("Failed to get ArgoCD diff information: err=%s\n", err)
 			} else {
-				ghPrClientDetails.PrLogger.Debugf("Successfully got ArgoCD diff\n")
+				ghPrClientDetails.PrLogger.Debugf("Successfully got ArgoCD diff vs git ref %s", ghPrClientDetails.Ref)
 				if !hasComponentDiffErrors && !hasComponentDiff {
 					ghPrClientDetails.PrLogger.Debugf("ArgoCD diff is empty, this PR will not change cluster state\n")
 					prLables, resp, err := ghPrClientDetails.GhClientPair.v3Client.Issues.AddLabelsToIssue(ghPrClientDetails.Ctx, ghPrClientDetails.Owner, ghPrClientDetails.Repo, *eventPayload.PullRequest.Number, []string{"noop"})
@@ -302,6 +302,7 @@ func handleEvent(eventPayloadInterface interface{}, mainGhClientCache *lru.Cache
 			PrSHA:        *eventPayload.PullRequest.Head.SHA,
 		}
 
+		log.Debugf("=== Ref is %s\n", ghPrClientDetails.Ref)
 		HandlePREvent(eventPayload, ghPrClientDetails, mainGithubClientPair, approverGithubClientPair, ctx)
 
 	case *github.IssueCommentEvent:

From 4d881260e17b6e3a5caf4cb9e0ab7eb1f29e04b8 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Wed, 3 Jul 2024 11:23:19 +0200
Subject: [PATCH 25/34] Use PR Branch HEAD instead of commit

---
 internal/pkg/argocd/argocd.go           | 6 ++++++
 internal/pkg/githubapi/github.go        | 4 +++-
 templates/argoCD-diff-pr-comment.gotmpl | 2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index 578e971..9c1dd08 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -340,6 +340,12 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc
 	log.Debugf("Got ArgoCD app %s", app.Name)
 	componentDiffResult.ArgoCdAppName = app.Name
 	componentDiffResult.ArgoCdAppURL = fmt.Sprintf("%s/applications/%s", argoSettings.URL, app.Name)
+
+	if app.Spec.Source.TargetRevision == prBranch {
+		componentDiffResult.DiffError = fmt.Errorf("App %s already has revision %s as Source Target Revision, skipping diff calculation", app.Name, prBranch)
+		return componentDiffResult
+	}
+
 	resources, err := appClient.ManagedResources(ctx, &application.ResourcesQuery{ApplicationName: &app.Name, AppNamespace: &app.Namespace})
 	if err != nil {
 		componentDiffResult.DiffError = err
diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index cfb8f62..e5b60e8 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -153,8 +153,10 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 				diffCommentData := struct {
 					DiffOfChangedComponents []argocd.DiffResult
 					HasSyncableComponens    bool
+					BranchName              string
 				}{
 					DiffOfChangedComponents: diffOfChangedComponents,
+					BranchName:              ghPrClientDetails.Ref,
 				}
 
 				for _, componentPath := range componentPathList {
@@ -393,7 +395,7 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 
 				for _, componentPath := range componentPathList {
 					if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
-						err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, ghPrClientDetails.PrSHA, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
+						err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, ghPrClientDetails.Ref, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
 						if err != nil {
 							ghPrClientDetails.PrLogger.Errorf("Failed to sync ArgoCD app from branch: err=%s\n", err)
 						}
diff --git a/templates/argoCD-diff-pr-comment.gotmpl b/templates/argoCD-diff-pr-comment.gotmpl
index 01dadb9..c8eacd2 100644
--- a/templates/argoCD-diff-pr-comment.gotmpl
+++ b/templates/argoCD-diff-pr-comment.gotmpl
@@ -35,7 +35,7 @@ No diff 🤷
 
 {{- if .HasSyncableComponens }}
 
-- [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps to sync to current branch revision  
+- [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps Target Revision to `{{ .BranchName }}`  
 
 {{ end}}
 

From c72229e2d7c494c78f8bffd728a9c37b5257ba3b Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Thu, 4 Jul 2024 11:45:20 +0200
Subject: [PATCH 26/34] Use an anonymous struct to create the patch Cleanup
 some debug lines and comments

---
 internal/pkg/argocd/argocd.go | 40 ++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index 9c1dd08..c1440c3 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -271,30 +271,28 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		return nil
 	}
 
-	// patchObject := argoappv1.Application{
-	// ObjectMeta: metav1.ObjectMeta{
-	// Namespace: foundApp.Namespace,
-	// },
-	// Spec: argoappv1.ApplicationSpec{
-	// Source: &argoappv1.ApplicationSource{
-	// TargetRevision: revision,
-	// },
-	// },
-	// }
-	// patchJson, err := json.Marshal(patchObject)
-	// if err != nil {
-	// return fmt.Errorf("Error marshalling patch object: %v\n%v", err, patchObject)
-	// }
-	// patch := string(patchJson)
-	patch := fmt.Sprintf(`{"spec": {"source": {"targetRevision": "%s"}}}`, revision)
+	patchObject := struct {
+		Spec struct {
+			Source struct {
+				TargetRevision string `json:"targetRevision"`
+			} `json:"source"`
+		} `json:"spec"`
+	}{}
+	patchObject.Spec.Source.TargetRevision = revision
+	patchJson, err := json.Marshal(patchObject)
+	if err != nil {
+		return fmt.Errorf("Error marshalling patch object: %v\n%v", err, patchObject)
+	}
+	patch := string(patchJson)
+	// patch := fmt.Sprintf(`{"spec": {"source": {"targetRevision": "%s"}}}`, revision)
 	log.Debugf("Patching app %s/%s with: %s", foundApp.Namespace, foundApp.Name, patch)
 
 	patchType := "merge"
 	patchedApp, err := appClient.Patch(ctx, &application.ApplicationPatchRequest{
-		Name: &foundApp.Name,
-		// AppNamespace: &foundApp.Namespace,
-		PatchType: &patchType,
-		Patch:     &patch,
+		Name:         &foundApp.Name,
+		AppNamespace: &foundApp.Namespace,
+		PatchType:    &patchType,
+		Patch:        &patch,
 	})
 	if err != nil {
 		return fmt.Errorf("Error patching app %s revision to  %s failed: %v\n, patch: %v\npatched app: %v\n", foundApp.Name, revision, err, patch, patchedApp)
@@ -353,7 +351,6 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc
 		return componentDiffResult
 	}
 	log.Debugf("Got (live)resources for app %s", app.Name)
-	log.Debugf("=== resources: %v ===", resources)
 
 	// Get the application manifests, these are the target state of the application objects, taken from the git repo, specificly from the PR branch.
 	diffOption := &DifferenceOption{}
@@ -370,7 +367,6 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc
 		return componentDiffResult
 	}
 	log.Debugf("Got manifests for app %s, revision %s", app.Name, prBranch)
-	log.Debugf("=== manifests: %v ===", manifests)
 	diffOption.res = manifests
 	diffOption.revision = prBranch
 

From e5872997d1dc3f5c8f46af26f43ea8a0b8db1a10 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Thu, 4 Jul 2024 13:13:32 +0200
Subject: [PATCH 27/34] Document new config key

---
 docs/installation.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/installation.md b/docs/installation.md
index cb727a4..a9f722f 100644
--- a/docs/installation.md
+++ b/docs/installation.md
@@ -126,6 +126,7 @@ Configuration keys:
 |`commentArgocdDiffonPR`| Uses ArgoCD API to calculate expected changes to k8s state and comment the resulting "diff" as comment in the PR. Requires ARGOCD_* environment variables, see below. |
 |`autoMergeNoDiffPRs`| if true, Telefonistka will **merge** promotion PRs that are not expected to change the target clusters. Requires `commentArgocdDiffonPR` and possibly `autoApprovePromotionPrs`(depending on repo branch protection rules)|
 |`useSHALabelForArgoDicovery`| The default method for discovering relevant ArgoCD applications (for a PR) relies on fetching all applications in the repo and checking the `argocd.argoproj.io/manifest-generate-paths` **annotation**, this might cause a performance issue on a repo with a large number of ArgoCD applications. The alternative is to add SHA1 of the application path as a  **label** and rely on ArgoCD server-side filtering, label name is `telefonistka.io/component-path-sha1`.|
+|`allowSyncArgoCDAppfromBranchPathRegex`| This controls which component(=ArgoCD apps) are allowed to be "applied" from a PR branch, by setting the ArgoCD application `Target Revision` to PR branch.|
 <!-- markdownlint-enable MD033 -->
 
 Example:
@@ -173,6 +174,7 @@ dryRunMode: true
 autoApprovePromotionPrs: true
 commentArgocdDiffonPR: true
 autoMergeNoDiffPRs: true
+allowSyncArgoCDAppfromBranchPathRegex: '^workspace/.*$'
 toggleCommitStatus:
   override-terrafrom-pipeline: "github-action-terraform"
 ```

From c93a0c07b96ed319bc9098fe063d218e62652c5a Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@gmail.com>
Date: Thu, 11 Jul 2024 10:56:57 +0200
Subject: [PATCH 28/34] Apply suggestions from code review

Co-authored-by: Hannes Gustafsson <hnnsgstfssn@gmail.com>
---
 internal/pkg/argocd/argocd.go    | 13 +++----------
 internal/pkg/githubapi/github.go |  2 +-
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index c1440c3..98f25c2 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -160,25 +160,19 @@ func createArgoCdClients() (appClient application.ApplicationServiceClient, proj
 	}
 
 	_, appClient, err = client.NewApplicationClient()
-	// appClntConn, appClient, err := client.NewApplicationClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD app client: %v", err)
 	}
-	// defer argoio.Close(appClntConn)
 
 	_, projClient, err = client.NewProjectClient()
-	// projClntConn, projClient, err := client.NewProjectClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD project client: %v", err)
 	}
-	// defer argoio.Close(projClntConn)
 
 	_, settingClient, err = client.NewSettingsClient()
-	// setClntConn, settingClient, err := client.NewSettingsClient()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD settings client: %v", err)
 	}
-	// defer argoio.Close(setClntConn)
 	return
 }
 
@@ -264,7 +258,7 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		foundApp, err = findArgocdAppByManifestPathAnnotation(ctx, componentPath, repo, appClient)
 	}
 	if err != nil {
-		return fmt.Errorf("Error finding ArgoCD application for component path %s: %v", componentPath, err)
+		return fmt.Errorf("error finding ArgoCD application for component path %s: %w", componentPath, err)
 	}
 	if foundApp.Spec.Source.TargetRevision == revision {
 		log.Infof("App %s already has revision %s", foundApp.Name, revision)
@@ -284,10 +278,9 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		return fmt.Errorf("Error marshalling patch object: %v\n%v", err, patchObject)
 	}
 	patch := string(patchJson)
-	// patch := fmt.Sprintf(`{"spec": {"source": {"targetRevision": "%s"}}}`, revision)
 	log.Debugf("Patching app %s/%s with: %s", foundApp.Namespace, foundApp.Name, patch)
 
-	patchType := "merge"
+	const patchType = "merge"
 	patchedApp, err := appClient.Patch(ctx, &application.ApplicationPatchRequest{
 		Name:         &foundApp.Name,
 		AppNamespace: &foundApp.Namespace,
@@ -295,7 +288,7 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		Patch:        &patch,
 	})
 	if err != nil {
-		return fmt.Errorf("Error patching app %s revision to  %s failed: %v\n, patch: %v\npatched app: %v\n", foundApp.Name, revision, err, patch, patchedApp)
+		return fmt.Errorf("revision patching failed: %w", err)
 	} else {
 		log.Infof("ArgoCD App %s revision set to %s", foundApp.Name, revision)
 	}
diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index e5b60e8..0b00aff 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -382,7 +382,7 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 
 	// This part should only happen on edits of bot comments on open PRs (I'm not testing Issue vs PR as Telefonsitka only creates PRs at this point)
 	if *ce.Action == "edited" && *ce.Comment.User.Login == botIdentity && *ce.Issue.State == "open" {
-		checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
+		const checkboxPattern = `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
 		checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Changes.Body.From, checkboxPattern)
 		if !checkboxWaschecked && checkboxIsChecked {
 			ghPrClientDetails.PrLogger.Infof("Sync Checkbox was checked")

From 6a131d39d12cb997ecdf2b4d94e77e2aa0f19995 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Thu, 11 Jul 2024 11:34:04 +0200
Subject: [PATCH 29/34] Use "%w" for warapped errors Removed unneeded JSON
 marshel error handeling(it should never fail) Switched back Const to String -
 I need a pointer for it

---
 internal/pkg/argocd/argocd.go | 37 ++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index 98f25c2..81766ef 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -50,7 +50,7 @@ type DiffResult struct {
 func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj *argoappv1.AppProject, resources *application.ManagedResourcesResponse, argoSettings *settings.Settings, diffOptions *DifferenceOption) (foundDiffs bool, diffElements []DiffElement, err error) {
 	liveObjs, err := cmdutil.LiveObjects(resources.Items)
 	if err != nil {
-		return false, nil, fmt.Errorf("Failed to get live objects: %v", err)
+		return false, nil, fmt.Errorf("Failed to get live objects: %w", err)
 	}
 
 	items := make([]objKeyLiveTarget, 0)
@@ -58,17 +58,17 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj
 	for _, mfst := range diffOptions.res.Manifests {
 		obj, err := argoappv1.UnmarshalToUnstructured(mfst)
 		if err != nil {
-			return false, nil, fmt.Errorf("Failed to unmarshal manifest: %v", err)
+			return false, nil, fmt.Errorf("Failed to unmarshal manifest: %w", err)
 		}
 		unstructureds = append(unstructureds, obj)
 	}
 	groupedObjs, err := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace)
 	if err != nil {
-		return false, nil, fmt.Errorf("Failed to group objects by key: %v", err)
+		return false, nil, fmt.Errorf("Failed to group objects by key: %w", err)
 	}
 	items, err = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace), app.Spec.Destination.Namespace)
 	if err != nil {
-		return false, nil, fmt.Errorf("Failed to group objects for diff: %v", err)
+		return false, nil, fmt.Errorf("Failed to group objects for diff: %w", err)
 	}
 
 	for _, item := range items {
@@ -90,11 +90,11 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj
 			WithNoCache().
 			Build()
 		if err != nil {
-			return false, nil, fmt.Errorf("Failed to build diff config: %v", err)
+			return false, nil, fmt.Errorf("Failed to build diff config: %w", err)
 		}
 		diffRes, err := argodiff.StateDiff(item.live, item.target, diffConfig)
 		if err != nil {
-			return false, nil, fmt.Errorf("Failed to diff objects: %v", err)
+			return false, nil, fmt.Errorf("Failed to diff objects: %w", err)
 		}
 
 		if diffRes.Modified || item.target == nil || item.live == nil {
@@ -110,7 +110,7 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj
 				live = item.live
 				err = json.Unmarshal(diffRes.PredictedLive, target)
 				if err != nil {
-					return false, nil, fmt.Errorf("Failed to unmarshal predicted live object: %v", err)
+					return false, nil, fmt.Errorf("Failed to unmarshal predicted live object: %w", err)
 				}
 			} else {
 				live = item.live
@@ -122,7 +122,7 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj
 
 			diffElement.Diff, err = diffLiveVsTargetObject(live, target)
 			if err != nil {
-				return false, nil, fmt.Errorf("Failed to diff live objects: %v", err)
+				return false, nil, fmt.Errorf("Failed to diff live objects: %w", err)
 			}
 		}
 		diffElements = append(diffElements, diffElement)
@@ -156,22 +156,22 @@ func createArgoCdClients() (appClient application.ApplicationServiceClient, proj
 
 	client, err := apiclient.NewClient(opts)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD API client: %v", err)
+		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD API client: %w", err)
 	}
 
 	_, appClient, err = client.NewApplicationClient()
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD app client: %v", err)
+		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD app client: %w", err)
 	}
 
 	_, projClient, err = client.NewProjectClient()
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD project client: %v", err)
+		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD project client: %w", err)
 	}
 
 	_, settingClient, err = client.NewSettingsClient()
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD settings client: %v", err)
+		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD settings client: %w", err)
 	}
 	return
 }
@@ -192,7 +192,7 @@ func findArgocdAppBySHA1Label(ctx context.Context, componentPath string, repo st
 	}
 	foundApps, err := appClient.List(ctx, &appLabelQuery)
 	if err != nil {
-		return nil, fmt.Errorf("Error listing ArgoCD applications: %v", err)
+		return nil, fmt.Errorf("Error listing ArgoCD applications: %w", err)
 	}
 	if len(foundApps.Items) == 0 {
 		return nil, fmt.Errorf("No ArgoCD application found for component path sha1 %s(repo %s), used this label selector: %s", componentPathSha1, repo, labelSelector)
@@ -250,7 +250,7 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	var err error
 	appClient, _, _, err := createArgoCdClients()
 	if err != nil {
-		return fmt.Errorf("Error creating ArgoCD clients: %v", err)
+		return fmt.Errorf("Error creating ArgoCD clients: %w", err)
 	}
 	if useSHALabelForArgoDicovery {
 		foundApp, err = findArgocdAppBySHA1Label(ctx, componentPath, repo, appClient)
@@ -273,15 +273,12 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 		} `json:"spec"`
 	}{}
 	patchObject.Spec.Source.TargetRevision = revision
-	patchJson, err := json.Marshal(patchObject)
-	if err != nil {
-		return fmt.Errorf("Error marshalling patch object: %v\n%v", err, patchObject)
-	}
+	patchJson, _ := json.Marshal(patchObject)
 	patch := string(patchJson)
 	log.Debugf("Patching app %s/%s with: %s", foundApp.Namespace, foundApp.Name, patch)
 
-	const patchType = "merge"
-	patchedApp, err := appClient.Patch(ctx, &application.ApplicationPatchRequest{
+	patchType := "merge"
+	_, err = appClient.Patch(ctx, &application.ApplicationPatchRequest{
 		Name:         &foundApp.Name,
 		AppNamespace: &foundApp.Namespace,
 		PatchType:    &patchType,

From 47e002bc8db548591e33c841f74cfb86e0ca6bb8 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Thu, 11 Jul 2024 15:57:30 +0200
Subject: [PATCH 30/34] Use table tests

---
 internal/pkg/githubapi/github_test.go | 146 ++++++++++++++------------
 1 file changed, 77 insertions(+), 69 deletions(-)

diff --git a/internal/pkg/githubapi/github_test.go b/internal/pkg/githubapi/github_test.go
index ebe7a2a..1b89aae 100644
--- a/internal/pkg/githubapi/github_test.go
+++ b/internal/pkg/githubapi/github_test.go
@@ -63,89 +63,97 @@ func TestGenerateSafePromotionBranchNameLongTargets(t *testing.T) {
 	}
 }
 
-// Testing a case when a checkbox is marked
-func TestAnalyzeCommentUpdateCheckBoxChecked(t *testing.T) {
+func TestAnalyzeCommentUpdateCheckBox(t *testing.T) {
 	t.Parallel()
-	newBody := `This is a comment
+	tests := map[string]struct {
+		newBody                  string
+		oldBody                  string
+		checkboxPattern          string
+		expectedWasCheckedBefore bool
+		expectedIsCheckedNow     bool
+	}{
+		"Checkbox is marked": {
+			oldBody: `This is a comment
+foobar
+- [ ] <!-- check-slug-1 --> Description of checkbox
+foobar`,
+			newBody: `This is a comment
 foobar
 - [x] <!-- check-slug-1 --> Description of checkbox
-foobar`
-
-	oldBody := `This is a comment
+foobar`,
+			checkboxPattern:          `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`,
+			expectedWasCheckedBefore: false,
+			expectedIsCheckedNow:     true,
+		},
+		"Checkbox is unmarked": {
+			oldBody: `This is a comment
 foobar
-- [ ] <!-- check-slug-1 --> Description of checkbox
-foobar`
-	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`
-
-	wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(newBody, oldBody, checkboxPattern)
-	if !isCheckedNow {
-		t.Error("Expected isCheckedNow to be true")
-	}
-	if wasCheckedBefore {
-		t.Errorf("Expected wasCheckedBeforeto be false, got %t", wasCheckedBefore)
-	}
-}
-
-// Testing a case when a checkbox is unmarked
-func TestAnalyzeCommentUpdateCheckBoxUnChecked(t *testing.T) {
-	t.Parallel()
-	newBody := `This is a comment
+- [x] <!-- check-slug-1 --> Description of checkbox
+foobar`,
+			newBody: `This is a comment
 foobar
 - [ ] <!-- check-slug-1 --> Description of checkbox
-foobar`
-
-	oldBody := `This is a comment
+foobar`,
+			checkboxPattern:          `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`,
+			expectedWasCheckedBefore: true,
+			expectedIsCheckedNow:     false,
+		},
+		"Checkbox isn't in comment body": {
+			oldBody: `This is a comment
 foobar
-- [x] <!-- check-slug-1 --> Description of checkbox
-foobar`
-	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`
-
-	wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(newBody, oldBody, checkboxPattern)
-	if isCheckedNow {
-		t.Error("Expected isCheckedNow to be false")
-	}
-	if !wasCheckedBefore {
-		t.Error("Expected wasCheckedBeforeto be true")
-	}
-}
-
-// Testing a case when a checkbox isn't in the comment body
-func TestAnalyzeCommentUpdateCheckBoxNonRelevent(t *testing.T) {
-	t.Parallel()
-	newBody := `This is a comment
+foobar`,
+			newBody: `This is a comment
 foobar
-foobar`
-
-	oldBody := `This is a comment
-foobar2
-foobar2`
-	checkboxPattern := `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`
-
-	wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(newBody, oldBody, checkboxPattern)
-	if isCheckedNow {
-		t.Error("Expected isCheckedNow to be false")
+foobar`,
+			checkboxPattern:          `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`,
+			expectedWasCheckedBefore: false,
+			expectedIsCheckedNow:     false,
+		},
 	}
-	if wasCheckedBefore {
-		t.Error("Expected wasCheckedBeforeto be false")
+	for name, tc := range tests {
+		tc := tc // capture range variable
+		name := name
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(tc.newBody, tc.oldBody, tc.checkboxPattern)
+			if isCheckedNow != tc.expectedIsCheckedNow {
+				t.Errorf("%s: Expected isCheckedNow to be %v, got %v", name, tc.expectedIsCheckedNow, isCheckedNow)
+			}
+			if wasCheckedBefore != tc.expectedWasCheckedBefore {
+				t.Errorf("%s: Expected wasCheckedBeforeto to be %v, got %v", name, tc.expectedWasCheckedBefore, wasCheckedBefore)
+			}
+		})
 	}
 }
 
-func TestIsSyncFromBranchAllowedForThisPathTrue(t *testing.T) {
+func TestIsSyncFromBranchAllowedForThisPath(t *testing.T) {
 	t.Parallel()
-	allowedPathRegex := `^workspace/.*$`
-	path := "workspace/app3"
-	result := isSyncFromBranchAllowedForThisPath(allowedPathRegex, path)
-	if !result {
-		t.Error("Expected result to be true")
+	tests := map[string]struct {
+		allowedPathRegex string
+		path             string
+		expectedResult   bool
+	}{
+		"Path is allowed": {
+			allowedPathRegex: `^workspace/.*$`,
+			path:             "workspace/app3",
+			expectedResult:   true,
+		},
+		"Path is not allowed": {
+			allowedPathRegex: `^workspace/.*$`,
+			path:             "clusters/prod/aws/eu-east-1/app3",
+			expectedResult:   false,
+		},
 	}
-}
 
-func TestIsSyncFromBranchAllowedForThisPathFalse(t *testing.T) {
-	t.Parallel()
-	allowedPathRegex := `^workspace/.*$`
-	path := "clusters/prod/aws/eu-east-1/app3"
-	result := isSyncFromBranchAllowedForThisPath(allowedPathRegex, path)
-	if result {
-		t.Error("Expected result to be false")
+	for name, tc := range tests {
+		tc := tc // capture range variable
+		name := name
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			result := isSyncFromBranchAllowedForThisPath(tc.allowedPathRegex, tc.path)
+			if result != tc.expectedResult {
+				t.Errorf("%s: Expected result to be %v, got %v", name, tc.expectedResult, result)
+			}
+		})
 	}
 }

From efbe16e59859c55d2f780a0ff5d9d91ad7f69e82 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Thu, 11 Jul 2024 16:54:29 +0200
Subject: [PATCH 31/34] Address PR comments

---
 internal/pkg/githubapi/github.go    | 18 +++++++++---------
 internal/pkg/githubapi/promotion.go | 19 ++++++++++---------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 0b00aff..17d2d37 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -405,7 +405,8 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 		}
 	}
 
-	// I should probably deprecated this whole part altogether.
+	// I should probably deprecated this whole part altogether - it was designed to solve a *very* specific problem that is probably no longer relevant with GitHub Rulesets
+	// The only reason I'm keeping it is that I don't have a clear feature depreciation policy and if I do remove it should be in a distinct PR
 	for commentSubstring, commitStatusContext := range config.ToggleCommitStatus {
 		if strings.Contains(*ce.Comment.Body, "/"+commentSubstring) {
 			err := ghPrClientDetails.ToggleCommitStatus(commitStatusContext, *ce.Sender.Name)
@@ -590,14 +591,13 @@ func handleMergedPrEvent(ghPrClientDetails GhPrClientDetails, prApproverGithubCl
 		componentPathList, err := generateListOfChangedComponentPaths(ghPrClientDetails, config)
 		if err != nil {
 			ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components for setting ArgoCD app targetRef to HEAD: err=%s\n", err)
-		} else {
-			for _, componentPath := range componentPathList {
-				if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
-					ghPrClientDetails.PrLogger.Errorf("Ensuring ArgoCD app %s is set to HEAD\n", componentPath)
-					err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, "HEAD", ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
-					if err != nil {
-						ghPrClientDetails.PrLogger.Errorf("Failed to set ArgoCD app @  %s, to HEAD: err=%s\n", componentPath, err)
-					}
+		}
+		for _, componentPath := range componentPathList {
+			if isSyncFromBranchAllowedForThisPath(config.AllowSyncArgoCDAppfromBranchPathRegex, componentPath) {
+				ghPrClientDetails.PrLogger.Infof("Ensuring ArgoCD app %s is set to HEAD\n", componentPath)
+				err := argocd.SetArgoCDAppRevision(ghPrClientDetails.Ctx, componentPath, "HEAD", ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
+				if err != nil {
+					ghPrClientDetails.PrLogger.Errorf("Failed to set ArgoCD app @  %s, to HEAD: err=%s\n", componentPath, err)
 				}
 			}
 		}
diff --git a/internal/pkg/githubapi/promotion.go b/internal/pkg/githubapi/promotion.go
index d15a3f1..84f1e19 100644
--- a/internal/pkg/githubapi/promotion.go
+++ b/internal/pkg/githubapi/promotion.go
@@ -174,15 +174,16 @@ func generateListOfChangedComponentPaths(ghPrClientDetails GhPrClientDetails, co
 	// If the PR has a list of promoted paths in the PR Telefonistika metadata(=is a promotion PR), we use that
 	if len(ghPrClientDetails.PrMetadata.PromotedPaths) > 0 {
 		changedComponentPaths = ghPrClientDetails.PrMetadata.PromotedPaths
-	} else {
-		// If not we will use in-repo config to geenrate it, and turns the map with struct keys into a list of strings
-		relevantComponents, err := generateListOfRelevantComponents(ghPrClientDetails, config)
-		if err != nil {
-			return nil, err
-		}
-		for component := range relevantComponents {
-			changedComponentPaths = append(changedComponentPaths, component.SourcePath+component.ComponentName)
-		}
+		return changedComponentPaths, nil
+	}
+
+	// If not we will use in-repo config to generate it, and turns the map with struct keys into a list of strings
+	relevantComponents, err := generateListOfRelevantComponents(ghPrClientDetails, config)
+	if err != nil {
+		return nil, err
+	}
+	for component := range relevantComponents {
+		changedComponentPaths = append(changedComponentPaths, component.SourcePath+component.ComponentName)
 	}
 	return changedComponentPaths, nil
 }

From 80601dd5416e043054f39ab7bcfc1dece92bf321 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 12 Jul 2024 10:34:32 +0200
Subject: [PATCH 32/34] Group ArgoCD clients in a struct per Arthur's PR
 comment

---
 internal/pkg/argocd/argocd.go | 36 ++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go
index 81766ef..8d1b1e9 100644
--- a/internal/pkg/argocd/argocd.go
+++ b/internal/pkg/argocd/argocd.go
@@ -26,6 +26,12 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
+type argoCdClients struct {
+	app     application.ApplicationServiceClient
+	project projectpkg.ProjectServiceClient
+	setting settings.SettingsServiceClient
+}
+
 // DiffElement struct to store diff element details, this represents a single k8s object
 type DiffElement struct {
 	ObjectGroup     string
@@ -143,7 +149,7 @@ func getEnv(key, fallback string) string {
 	return fallback
 }
 
-func createArgoCdClients() (appClient application.ApplicationServiceClient, projClient projectpkg.ProjectServiceClient, settingClient settings.SettingsServiceClient, err error) {
+func createArgoCdClients() (ac argoCdClients, err error) {
 	plaintext, _ := strconv.ParseBool(getEnv("ARGOCD_PLAINTEXT", "false"))
 	insecure, _ := strconv.ParseBool(getEnv("ARGOCD_INSECURE", "false"))
 
@@ -156,22 +162,22 @@ func createArgoCdClients() (appClient application.ApplicationServiceClient, proj
 
 	client, err := apiclient.NewClient(opts)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD API client: %w", err)
+		return ac, fmt.Errorf("Error creating ArgoCD API client: %w", err)
 	}
 
-	_, appClient, err = client.NewApplicationClient()
+	_, ac.app, err = client.NewApplicationClient()
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD app client: %w", err)
+		return ac, fmt.Errorf("Error creating ArgoCD app client: %w", err)
 	}
 
-	_, projClient, err = client.NewProjectClient()
+	_, ac.project, err = client.NewProjectClient()
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD project client: %w", err)
+		return ac, fmt.Errorf("Error creating ArgoCD project client: %w", err)
 	}
 
-	_, settingClient, err = client.NewSettingsClient()
+	_, ac.setting, err = client.NewSettingsClient()
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("Error creating ArgoCD settings client: %w", err)
+		return ac, fmt.Errorf("Error creating ArgoCD settings client: %w", err)
 	}
 	return
 }
@@ -248,14 +254,14 @@ func findArgocdAppByManifestPathAnnotation(ctx context.Context, componentPath st
 func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision string, repo string, useSHALabelForArgoDicovery bool) error {
 	var foundApp *argoappv1.Application
 	var err error
-	appClient, _, _, err := createArgoCdClients()
+	ac, err := createArgoCdClients()
 	if err != nil {
 		return fmt.Errorf("Error creating ArgoCD clients: %w", err)
 	}
 	if useSHALabelForArgoDicovery {
-		foundApp, err = findArgocdAppBySHA1Label(ctx, componentPath, repo, appClient)
+		foundApp, err = findArgocdAppBySHA1Label(ctx, componentPath, repo, ac.app)
 	} else {
-		foundApp, err = findArgocdAppByManifestPathAnnotation(ctx, componentPath, repo, appClient)
+		foundApp, err = findArgocdAppByManifestPathAnnotation(ctx, componentPath, repo, ac.app)
 	}
 	if err != nil {
 		return fmt.Errorf("error finding ArgoCD application for component path %s: %w", componentPath, err)
@@ -278,7 +284,7 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	log.Debugf("Patching app %s/%s with: %s", foundApp.Namespace, foundApp.Name, patch)
 
 	patchType := "merge"
-	_, err = appClient.Patch(ctx, &application.ApplicationPatchRequest{
+	_, err = ac.app.Patch(ctx, &application.ApplicationPatchRequest{
 		Name:         &foundApp.Name,
 		AppNamespace: &foundApp.Namespace,
 		PatchType:    &patchType,
@@ -381,13 +387,13 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st
 	hasComponentDiff = false
 	hasComponentDiffErrors = false
 	// env var should be centralized
-	appClient, projClient, settingClient, err := createArgoCdClients()
+	ac, err := createArgoCdClients()
 	if err != nil {
 		log.Errorf("Error creating ArgoCD clients: %v", err)
 		return false, true, nil, err
 	}
 
-	argoSettings, err := settingClient.Get(ctx, &settings.SettingsQuery{})
+	argoSettings, err := ac.setting.Get(ctx, &settings.SettingsQuery{})
 	if err != nil {
 		log.Errorf("Error getting ArgoCD settings: %v", err)
 		return false, true, nil, err
@@ -395,7 +401,7 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st
 
 	log.Debugf("Checking ArgoCD diff for components: %v", componentPathList)
 	for _, componentPath := range componentPathList {
-		currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, appClient, projClient, argoSettings, useSHALabelForArgoDicovery)
+		currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, ac.app, ac.project, argoSettings, useSHALabelForArgoDicovery)
 		if currentDiffResult.DiffError != nil {
 			log.Errorf("Error generating diff for component %s: %v", componentPath, currentDiffResult.DiffError)
 			hasComponentDiffErrors = true

From 0a7eee262f64fef0207d5dc5063a65f53a0311a9 Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 12 Jul 2024 11:10:29 +0200
Subject: [PATCH 33/34] Move the Regex pattern in analyzeCommentUpdateCheckBox
 (while keeping the identifier paramatize)

This addresses a PR analyzeCommentUpdateCheckBox
---
 internal/pkg/githubapi/github.go      |  9 +++++----
 internal/pkg/githubapi/github_test.go | 10 +++++-----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index 17d2d37..a6fb57d 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -317,7 +317,7 @@ func handleEvent(eventPayloadInterface interface{}, mainGhClientCache *lru.Cache
 			"repo":     *eventPayload.Repo.Owner.Login + "/" + *eventPayload.Repo.Name,
 			"prNumber": *eventPayload.Issue.Number,
 		})
-		// Ignore comment even sent by the bot (this is about who trigger the event not who wrote the comment)
+		// Ignore comment events sent by the bot (this is about who trigger the event not who wrote the comment)
 		if *eventPayload.Sender.Login != botIdentity {
 			ghPrClientDetails := GhPrClientDetails{
 				Ctx:          ctx,
@@ -340,7 +340,8 @@ func handleEvent(eventPayloadInterface interface{}, mainGhClientCache *lru.Cache
 	}
 }
 
-func analyzeCommentUpdateCheckBox(newBody string, oldBody string, checkboxPattern string) (wasCheckedBefore bool, isCheckedNow bool) {
+func analyzeCommentUpdateCheckBox(newBody string, oldBody string, checkboxIdentifier string) (wasCheckedBefore bool, isCheckedNow bool) {
+	checkboxPattern := fmt.Sprintf(`(?m)^\s*-\s*\[(.)\]\s*<!-- %s -->.*$`, checkboxIdentifier)
 	checkBoxRegex := regexp.MustCompile(checkboxPattern)
 	oldCheckBoxContent := checkBoxRegex.FindStringSubmatch(oldBody)
 	newCheckBoxContent := checkBoxRegex.FindStringSubmatch(newBody)
@@ -382,8 +383,8 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 
 	// This part should only happen on edits of bot comments on open PRs (I'm not testing Issue vs PR as Telefonsitka only creates PRs at this point)
 	if *ce.Action == "edited" && *ce.Comment.User.Login == botIdentity && *ce.Issue.State == "open" {
-		const checkboxPattern = `(?m)^\s*-\s*\[(.)\]\s*<!-- telefonistka-argocd-branch-sync -->.*$`
-		checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Changes.Body.From, checkboxPattern)
+		const checkboxIdentifier = "telefonistka-argocd-branch-sync"
+		checkboxWaschecked, checkboxIsChecked := analyzeCommentUpdateCheckBox(*ce.Comment.Body, *ce.Changes.Body.From, checkboxIdentifier)
 		if !checkboxWaschecked && checkboxIsChecked {
 			ghPrClientDetails.PrLogger.Infof("Sync Checkbox was checked")
 			if config.AllowSyncArgoCDAppfromBranchPathRegex != "" {
diff --git a/internal/pkg/githubapi/github_test.go b/internal/pkg/githubapi/github_test.go
index 1b89aae..d363487 100644
--- a/internal/pkg/githubapi/github_test.go
+++ b/internal/pkg/githubapi/github_test.go
@@ -68,7 +68,7 @@ func TestAnalyzeCommentUpdateCheckBox(t *testing.T) {
 	tests := map[string]struct {
 		newBody                  string
 		oldBody                  string
-		checkboxPattern          string
+		checkboxIdentifier       string
 		expectedWasCheckedBefore bool
 		expectedIsCheckedNow     bool
 	}{
@@ -81,7 +81,7 @@ foobar`,
 foobar
 - [x] <!-- check-slug-1 --> Description of checkbox
 foobar`,
-			checkboxPattern:          `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`,
+			checkboxIdentifier:       "check-slug-1",
 			expectedWasCheckedBefore: false,
 			expectedIsCheckedNow:     true,
 		},
@@ -94,7 +94,7 @@ foobar`,
 foobar
 - [ ] <!-- check-slug-1 --> Description of checkbox
 foobar`,
-			checkboxPattern:          `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`,
+			checkboxIdentifier:       "check-slug-1",
 			expectedWasCheckedBefore: true,
 			expectedIsCheckedNow:     false,
 		},
@@ -105,7 +105,7 @@ foobar`,
 			newBody: `This is a comment
 foobar
 foobar`,
-			checkboxPattern:          `(?m)^\s*-\s*\[(.)\]\s*<!-- check-slug-1 -->.*$`,
+			checkboxIdentifier:       "check-slug-1",
 			expectedWasCheckedBefore: false,
 			expectedIsCheckedNow:     false,
 		},
@@ -115,7 +115,7 @@ foobar`,
 		name := name
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(tc.newBody, tc.oldBody, tc.checkboxPattern)
+			wasCheckedBefore, isCheckedNow := analyzeCommentUpdateCheckBox(tc.newBody, tc.oldBody, tc.checkboxIdentifier)
 			if isCheckedNow != tc.expectedIsCheckedNow {
 				t.Errorf("%s: Expected isCheckedNow to be %v, got %v", name, tc.expectedIsCheckedNow, isCheckedNow)
 			}

From 3834ddde81ae6da971e8a6302f6c4e91fe95ff2c Mon Sep 17 00:00:00 2001
From: Oded Ben Ozer <oded.benozer@commercetools.com>
Date: Fri, 12 Jul 2024 11:13:58 +0200
Subject: [PATCH 34/34] Improve log message to address PR comment

---
 internal/pkg/githubapi/github.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go
index a6fb57d..e49758a 100644
--- a/internal/pkg/githubapi/github.go
+++ b/internal/pkg/githubapi/github.go
@@ -126,7 +126,7 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 				prHandleError = err
 				ghPrClientDetails.PrLogger.Errorf("Failed to get ArgoCD diff information: err=%s\n", err)
 			} else {
-				ghPrClientDetails.PrLogger.Debugf("Successfully got ArgoCD diff vs git ref %s", ghPrClientDetails.Ref)
+				ghPrClientDetails.PrLogger.Debugf("Successfully got ArgoCD diff(comparing live objects against objects rendered form git ref %s)", ghPrClientDetails.Ref)
 				if !hasComponentDiffErrors && !hasComponentDiff {
 					ghPrClientDetails.PrLogger.Debugf("ArgoCD diff is empty, this PR will not change cluster state\n")
 					prLables, resp, err := ghPrClientDetails.GhClientPair.v3Client.Issues.AddLabelsToIssue(ghPrClientDetails.Ctx, ghPrClientDetails.Owner, ghPrClientDetails.Repo, *eventPayload.PullRequest.Number, []string{"noop"})