From 52c6cb8779d5cba75f574fe178da113a84875a68 Mon Sep 17 00:00:00 2001
From: Hidetake Iwata <int128@gmail.com>
Date: Sun, 4 Feb 2024 18:44:01 +0900
Subject: [PATCH] Improve comment and deployment status description

---
 internal/notification/healthcomment.go    |  6 ++----
 internal/notification/healthdeployment.go |  8 ++++----
 internal/notification/phasecomment.go     | 12 ++++++++---
 internal/notification/phasedeployment.go  | 25 ++++++++++++++++++++++-
 4 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/internal/notification/healthcomment.go b/internal/notification/healthcomment.go
index 3c52b237..5a7fcf7f 100644
--- a/internal/notification/healthcomment.go
+++ b/internal/notification/healthcomment.go
@@ -52,16 +52,14 @@ func generateCommentBodyOnHealthChanged(app argocdv1alpha1.Application, argocdUR
 	argocdApplicationURL := fmt.Sprintf("%s/applications/%s", argocdURL, app.Name)
 	switch app.Status.Health.Status {
 	case health.HealthStatusHealthy:
-		return fmt.Sprintf("## %s %s: [%s](%s)\nDeployed %s",
-			":white_check_mark:",
+		return fmt.Sprintf(":white_check_mark: %s [%s](%s) at %s",
 			app.Status.Health.Status,
 			app.Name,
 			argocdApplicationURL,
 			sourceRevision.Revision,
 		)
 	case health.HealthStatusDegraded:
-		return fmt.Sprintf("## %s %s: [%s](%s)\nError while deploying %s:\n%s",
-			":x:",
+		return fmt.Sprintf("## :x: %s [%s](%s) at %s:\n%s",
 			app.Status.Health.Status,
 			app.Name,
 			argocdApplicationURL,
diff --git a/internal/notification/healthdeployment.go b/internal/notification/healthdeployment.go
index 76e5963c..2400fc19 100644
--- a/internal/notification/healthdeployment.go
+++ b/internal/notification/healthdeployment.go
@@ -57,10 +57,10 @@ func generateDeploymentStatusOnHealthChanged(app argocdv1alpha1.Application, arg
 
 func generateDeploymentStatusDescriptionOnHealthChanged(app argocdv1alpha1.Application) string {
 	var b strings.Builder
-	b.WriteString(fmt.Sprintf("%s:\n%s\n",
-		app.Status.Health.Status,
-		app.Status.Health.Message,
-	))
+	b.WriteString(fmt.Sprintf("%s:\n", app.Status.Health.Status))
+	if app.Status.Health.Message != "" {
+		b.WriteString(fmt.Sprintf("%s\n", app.Status.Health.Message))
+	}
 	for _, r := range app.Status.Resources {
 		if r.Health == nil {
 			continue
diff --git a/internal/notification/phasecomment.go b/internal/notification/phasecomment.go
index 007f7216..87bf40da 100644
--- a/internal/notification/phasecomment.go
+++ b/internal/notification/phasecomment.go
@@ -61,9 +61,15 @@ func generateCommentBodyOnPhaseChanged(app argocdv1alpha1.Application, argocdURL
 		return fmt.Sprintf(":warning: Syncing [%s](%s) to %s", app.Name, argocdApplicationURL, sourceRevision.Revision)
 	case synccommon.OperationSucceeded:
 		return fmt.Sprintf(":white_check_mark: Synced [%s](%s) to %s", app.Name, argocdApplicationURL, sourceRevision.Revision)
-	case synccommon.OperationFailed, synccommon.OperationError:
-		return fmt.Sprintf("## :x: Sync %s: [%s](%s)\nError while syncing to %s:\n%s",
-			phase,
+	case synccommon.OperationFailed:
+		return fmt.Sprintf("## :x: Failed to sync [%s](%s) to %s\n%s",
+			app.Name,
+			argocdApplicationURL,
+			sourceRevision.Revision,
+			generateCommentResourcesOnPhaseChanged(app.Status.OperationState.SyncResult),
+		)
+	case synccommon.OperationError:
+		return fmt.Sprintf("## :x: Sync error [%s](%s) at %s\n%s",
 			app.Name,
 			argocdApplicationURL,
 			sourceRevision.Revision,
diff --git a/internal/notification/phasedeployment.go b/internal/notification/phasedeployment.go
index 9a8d5ba9..026ddf99 100644
--- a/internal/notification/phasedeployment.go
+++ b/internal/notification/phasedeployment.go
@@ -3,6 +3,7 @@ package notification
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	argocdv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	synccommon "github.com/argoproj/gitops-engine/pkg/sync/common"
@@ -44,7 +45,7 @@ func generateDeploymentStatusOnPhaseChanged(app argocdv1alpha1.Application, argo
 		GitHubDeployment: *deployment,
 		GitHubDeploymentStatus: github.DeploymentStatus{
 			LogURL:      fmt.Sprintf("%s/applications/%s", argocdURL, app.Name),
-			Description: trimDescription(fmt.Sprintf("%s:\n%s", phase, app.Status.OperationState.Message)),
+			Description: trimDescription(generateDeploymentStatusDescriptionOnPhaseChanged(app)),
 		},
 	}
 	if len(app.Status.Summary.ExternalURLs) > 0 {
@@ -66,3 +67,25 @@ func generateDeploymentStatusOnPhaseChanged(app argocdv1alpha1.Application, argo
 	}
 	return nil
 }
+
+func generateDeploymentStatusDescriptionOnPhaseChanged(app argocdv1alpha1.Application) string {
+	phase := argocd.GetSyncOperationPhase(app)
+	if phase == "" {
+		return ""
+	}
+	syncResult := app.Status.OperationState.SyncResult
+	if syncResult == nil {
+		return ""
+	}
+
+	var b strings.Builder
+	b.WriteString(fmt.Sprintf("%s:\n", phase))
+	for _, r := range syncResult.Resources {
+		namespacedName := r.Namespace + "/" + r.Name
+		switch r.Status {
+		case synccommon.ResultCodeSyncFailed:
+			b.WriteString(fmt.Sprintf("%s: %s\n", namespacedName, r.Message))
+		}
+	}
+	return b.String()
+}