chore(server): simplify project validation logic (#21191)

* chore(server): simplify project validation logic

Signed-off-by: Michael Crenshaw <[email protected]>

* improve tests

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>

Author: crenshaw-dev

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++
+			}
 		}
 	}
 

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