Merge branch '495518-add-errors-to-security-policy-project-created-subscription' into 'master'

Add errors to SecurityPolicyProjectCreated subscription

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167422



Merged-by: James Nutt <jnutt@gitlab.com>
Approved-by: Eva Kadlecová <ekadlecova@gitlab.com>
Approved-by: James Nutt <jnutt@gitlab.com>
Reviewed-by: Andy Schoenen <asoiron@gitlab.com>
Reviewed-by: Eva Kadlecová <ekadlecova@gitlab.com>
Co-authored-by: Andy Soiron <asoiron@gitlab.com>

doc/api/graphql/reference/index.md

@@ -29540,7 +29540,8 @@ Response of security policy creation.
 | Name | Type | Description |
 | ---- | ---- | ----------- |
-| <a id="policyprojectcreatederrormessage"></a>`errorMessage` | [`String`](#string) | Error message in case status is :error. |
+| <a id="policyprojectcreatederrormessage"></a>`errorMessage` **{warning-solid}** | [`String`](#string) | **Deprecated** in GitLab 17.5. Use errors instead. |
+| <a id="policyprojectcreatederrors"></a>`errors` | [`[String!]`](#string) | Error messages in case status is :error. |
 | <a id="policyprojectcreatedproject"></a>`project` | [`Project`](#project) | Security Policy Project that was created. |
 | <a id="policyprojectcreatedstatus"></a>`status` | [`PolicyProjectCreatedStatus`](#policyprojectcreatedstatus) | Status of the creation of the security policy project. |

ee/app/graphql/ee/graphql_triggers.rb

@@ -54,11 +54,13 @@ def self.workflow_events_updated(checkpoint)
           checkpoint)
       end
 
-      def self.security_policy_project_created(container, status, security_policy_project, error_message)
+      def self.security_policy_project_created(container, status, security_policy_project, errors)
+        error_message = errors.any? ? errors.join(' ') : nil
+
         ::GitlabSchema.subscriptions.trigger(
           :security_policy_project_created,
           { full_path: container.full_path },
-          { status: status, error_message: error_message, project: security_policy_project }
+          { status: status, errors: errors, error_message: error_message, project: security_policy_project }
         )
       end
     end

ee/app/graphql/subscriptions/security/policy_project_created.rb

@@ -21,6 +21,7 @@ def update(_)
         {
           project: object[:project],
           status: object[:status],
+          errors: object[:errors],
           error_message: object[:error_message]
         }
       end

ee/app/graphql/types/gitlab_subscriptions/security/policy_project_created.rb

@@ -15,9 +15,14 @@ class PolicyProjectCreated < ::Types::BaseObject
         field :status, Types::GitlabSubscriptions::Security::PolicyProjectCreatedStatusEnum,
           description: 'Status of the creation of the security policy project.'
 
+        field :errors, [GraphQL::Types::String],
+          null: true,
+          description: 'Error messages in case status is :error.'
+
         field :error_message, GraphQL::Types::String,
           null: true,
-          description: 'Error message in case status is :error.'
+          description: 'Error messages in case status is :error.',
+          deprecated: { milestone: '17.5', reason: 'Use errors instead' }
       end
       # rubocop:enable Graphql/AuthorizeTypes
     end

ee/app/workers/security/create_security_policy_project_worker.rb

@@ -20,7 +20,7 @@ def perform(project_or_group_path, current_user_id)
       end
 
       if errors.any?
-        error(errors.join(' '), container)
+        error(errors, container)
 
         return
       end
@@ -33,18 +33,18 @@ def perform(project_or_group_path, current_user_id)
         container,
         service_result[:status],
         service_result[:policy_project],
-        service_result[:message]
+        [service_result[:message]].compact
       )
     end
 
     private
 
-    def error(message, container = nil)
+    def error(messages, container = nil)
       GraphqlTriggers.security_policy_project_created(
         container,
         :error,
         nil,
-        message
+        messages
       )
     end
   end

ee/spec/graphql/graphql_triggers_spec.rb

@@ -156,22 +156,36 @@
 
   describe '.security_policy_project_created' do
     subject(:trigger) do
-      described_class.security_policy_project_created(container, status, security_policy_project, error_message)
+      described_class.security_policy_project_created(container, status, security_policy_project, errors)
     end
 
     let_it_be(:container) { create(:project) }
     let_it_be(:security_policy_project) { create(:project) }
     let(:status) { :success }
-    let(:error_message) { nil }
+    let(:errors) { [] }
 
     it 'triggers the subscription' do
       expect(GitlabSchema.subscriptions).to receive(:trigger).with(
         :security_policy_project_created,
         { full_path: container.full_path },
-        { status: status, project: security_policy_project, error_message: error_message }
+        { status: status, project: security_policy_project, errors: errors, error_message: nil }
       )
 
       trigger
     end
+
+    context 'with errors' do
+      let(:errors) { %w[error1 error2] }
+
+      it 'triggers the subscription with errors' do
+        expect(GitlabSchema.subscriptions).to receive(:trigger).with(
+          :security_policy_project_created,
+          { full_path: container.full_path },
+          { status: status, project: security_policy_project, errors: errors, error_message: 'error1 error2' }
+        )
+
+        trigger
+      end
+    end
   end
 end

ee/spec/graphql/subscriptions/security/policy_project_created_spec.rb

@@ -13,7 +13,7 @@
   let(:current_user) { nil }
   let(:subscribe) { security_policy_project_created_subscription(project, current_user) }
   let(:status) { :success }
-  let(:error_message) { nil }
+  let(:errors) { [] }
 
   let(:security_policy_project_created) do
     graphql_dig_at(graphql_data(response[:result]), :securityPolicyProjectCreated)
@@ -32,7 +32,7 @@
 
   subject(:response) do
     subscription_response do
-      GraphqlTriggers.security_policy_project_created(project, status, security_policy_project, error_message)
+      GraphqlTriggers.security_policy_project_created(project, status, security_policy_project, errors)
     end
   end
 
@@ -49,7 +49,8 @@
       created_response = security_policy_project_created
 
       expect(created_response['project']).to be_nil
-      expect(created_response['error_message']).to eq(nil)
+      expect(created_response['errors']).to eq([])
+      expect(created_response['errorMessage']).to eq(nil)
       expect(created_response['status']).to eq('SUCCESS')
     end
 
@@ -62,20 +63,22 @@
         created_response = security_policy_project_created
 
         expect(created_response['project']['name']).to eq(security_policy_project.name)
-        expect(created_response['error_message']).to eq(nil)
+        expect(created_response['errors']).to eq([])
+        expect(created_response['errorMessage']).to eq(nil)
         expect(created_response['status']).to eq('SUCCESS')
       end
 
-      context 'and there is an error_message' do
+      context 'and there is an error' do
         let_it_be(:security_policy_project) { nil }
 
-        let(:error_message) { 'Error' }
+        let(:errors) { ['Error'] }
         let(:status) { :error }
 
         it 'receives the error message' do
           created_response = security_policy_project_created
 
           expect(created_response['project']).to eq(nil)
+          expect(created_response['errors']).to contain_exactly('Error')
           expect(created_response['errorMessage']).to eq('Error')
           expect(created_response['status']).to eq('ERROR')
         end

ee/spec/support/helpers/graphql/subscriptions/security/policy_project_created/helper.rb

@@ -30,6 +30,7 @@ def security_policy_project_created_subscription_query(container)
                     name
                   }
                   status
+                  errors
                   errorMessage
                 }
               }

ee/spec/workers/security/create_security_policy_project_worker_spec.rb

@@ -45,7 +45,7 @@
       end
 
       expect(GraphqlTriggers).to receive(:security_policy_project_created).with(
-        project, :success, "a policy project", nil)
+        project, :success, "a policy project", [])
 
       run_worker
     end
@@ -63,7 +63,7 @@
         end
 
         expect(GraphqlTriggers).to receive(:security_policy_project_created).with(
-          project, :error, nil, 'Security Policy project already exists.'
+          project, :error, nil, ['Security Policy project already exists.']
         )
 
         run_worker
@@ -77,7 +77,7 @@
 
       it 'triggers the subscription with an error' do
         expect(GraphqlTriggers).to receive(:security_policy_project_created).with(
-          project, :error, nil, 'User not found.'
+          project, :error, nil, ['User not found.']
         )
 
         run_worker
@@ -91,7 +91,7 @@
 
       it 'triggers the subscription with an error' do
         expect(GraphqlTriggers).to receive(:security_policy_project_created).with(
-          nil, :error, nil, 'Group or project not found.'
+          nil, :error, nil, ['Group or project not found.']
         )
 
         run_worker
@@ -106,7 +106,7 @@
 
       it 'triggers the subscription with an error' do
         expect(GraphqlTriggers).to receive(:security_policy_project_created).with(
-          nil, :error, nil, 'Group or project not found. User not found.'
+          nil, :error, nil, ['Group or project not found.', 'User not found.']
         )
 
         run_worker