From 9203dd16af6dc39381ce01289d24ca3b9617d6cd Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Date: Tue, 17 Dec 2024 02:31:06 -0500
Subject: [PATCH] chore(server): simplify project validation logic (#21191)

* chore(server): simplify project validation logic

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* improve tests

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---
 server/project/project.go      | 36 ++++++++++++----------------------
 server/project/project_test.go |  3 +++
 2 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/server/project/project.go b/server/project/project.go
index 62487b268a705..cac913715b623 100644
--- a/server/project/project.go
+++ b/server/project/project.go
@@ -398,43 +398,31 @@ func (s *Server) Update(ctx context.Context, q *project.ProjectUpdateRequest) (*
 		return nil, err
 	}
 
-	var srcValidatedApps []v1alpha1.Application
-	var dstValidatedApps []v1alpha1.Application
 	getProjectClusters := func(project string) ([]*v1alpha1.Cluster, error) {
 		return s.db.GetProjectClusters(ctx, project)
 	}
 
-	for _, a := range argo.FilterByProjects(appsList.Items, []string{q.Project.Name}) {
-		if oldProj.IsSourcePermitted(a.Spec.GetSource()) {
-			srcValidatedApps = append(srcValidatedApps, a)
-		}
-
-		dstPermitted, err := oldProj.IsDestinationPermitted(a.Spec.Destination, getProjectClusters)
-		if err != nil {
-			return nil, err
-		}
-
-		if dstPermitted {
-			dstValidatedApps = append(dstValidatedApps, a)
-		}
-	}
-
 	invalidSrcCount := 0
 	invalidDstCount := 0
 
-	for _, a := range srcValidatedApps {
-		if !q.Project.IsSourcePermitted(a.Spec.GetSource()) {
+	for _, a := range argo.FilterByProjects(appsList.Items, []string{q.Project.Name}) {
+		if oldProj.IsSourcePermitted(a.Spec.GetSource()) && !q.Project.IsSourcePermitted(a.Spec.GetSource()) {
 			invalidSrcCount++
 		}
-	}
-	for _, a := range dstValidatedApps {
-		dstPermitted, err := q.Project.IsDestinationPermitted(a.Spec.Destination, getProjectClusters)
+
+		dstPermitted, err := oldProj.IsDestinationPermitted(a.Spec.Destination, getProjectClusters)
 		if err != nil {
 			return nil, err
 		}
 
-		if !dstPermitted {
-			invalidDstCount++
+		if dstPermitted {
+			dstPermitted, err := q.Project.IsDestinationPermitted(a.Spec.Destination, getProjectClusters)
+			if err != nil {
+				return nil, err
+			}
+			if !dstPermitted {
+				invalidDstCount++
+			}
 		}
 	}
 
diff --git a/server/project/project_test.go b/server/project/project_test.go
index 96e9beed30a54..d4d9c3e40e4eb 100644
--- a/server/project/project_test.go
+++ b/server/project/project_test.go
@@ -196,6 +196,7 @@ func TestProjectServer(t *testing.T) {
 		require.Error(t, err)
 		statusCode, _ := status.FromError(err)
 		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
+		assert.Equal(t, "as a result of project update 1 applications destination became invalid", statusCode.Message())
 	})
 
 	t.Run("TestRemoveSourceSuccessful", func(t *testing.T) {
@@ -232,6 +233,7 @@ func TestProjectServer(t *testing.T) {
 		require.Error(t, err)
 		statusCode, _ := status.FromError(err)
 		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
+		assert.Equal(t, "as a result of project update 1 applications source became invalid", statusCode.Message())
 	})
 
 	t.Run("TestRemoveSourceUsedByAppSuccessfulIfPermittedByAnotherSrc", func(t *testing.T) {
@@ -318,6 +320,7 @@ func TestProjectServer(t *testing.T) {
 		require.Error(t, err)
 		statusCode, _ := status.FromError(err)
 		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
+		assert.Equal(t, "project is referenced by 1 applications", statusCode.Message())
 	})
 
 	// configure a user named "admin" which is denied by default