Merge branch 'allow-removing-policy-project' into 'master'

Allow removing policy project

See merge request gitlab-org/gitlab!56356

ee/app/controllers/projects/security/policies_controller.rb

@@ -22,7 +22,7 @@ module Projects
         result = ::Security::Orchestration::AssignService.new(project, nil, policy_project_id: policy_project_params[:policy_project_id]).execute
 
         if result.success?
-          flash[:notice] = _('Successfull')
+          flash[:notice] = _('Operation completed')
         else
           flash[:alert] = result.message
         end

ee/app/models/security/orchestration_policy_configuration.rb

@@ -14,7 +14,7 @@ module Security
     belongs_to :security_policy_management_project, class_name: 'Project', foreign_key: 'security_policy_management_project_id'
 
     validates :project, presence: true, uniqueness: true
-    validates :security_policy_management_project, presence: true, uniqueness: true
+    validates :security_policy_management_project, presence: true
 
     def enabled?
       ::Feature.enabled?(:security_orchestration_policies_configuration, project)

ee/app/services/security/orchestration/assign_service.rb

@@ -9,7 +9,7 @@ module Security
         return success if res
 
       rescue ActiveRecord::RecordNotFound => _
-        error(_('Policy project doesn\'t exists'))
+        error(_('Policy project doesn\'t exist'))
       rescue ActiveRecord::RecordInvalid => _
         error(_('Couldn\'t assign policy to project'))
       end
@@ -17,6 +17,10 @@ module Security
       private
 
       def create_or_update_security_policy_configuration
+        if policy_project_id.blank? && has_existing_policy?
+          return unassign_policy_project
+        end
+
         policy_project = Project.find(policy_project_id)
 
         if has_existing_policy?
@@ -30,6 +34,10 @@ module Security
         end
       end
 
+      def unassign_policy_project
+        project.security_orchestration_policy_configuration.delete
+      end
+
       def success
         ServiceResponse.success(payload: { policy_project: policy_project_id })
       end

ee/app/views/layouts/nav/sidebar/_project_security_link.html.haml

@@ -44,7 +44,7 @@
           = link_to project_threat_monitoring_path(@project), title: _('Threat Monitoring') do
             %span= _('Threat Monitoring')
 
-      - if project_nav_tab?(:security_orchestration_policies)
+      - if project_nav_tab?(:security_orchestration_policies) && Feature.enabled?(:security_orchestration_policies_configuration, @project)
         = nav_link(controller: ['projects/security/policies']) do
           = link_to project_security_policy_path(@project), title: _('Scan Policies') do
             %span= _('Scan Policies')

ee/spec/models/security/orchestration_policy_configuration_spec.rb

@@ -21,7 +21,6 @@ RSpec.describe Security::OrchestrationPolicyConfiguration do
     it { is_expected.to validate_presence_of(:security_policy_management_project) }
 
     it { is_expected.to validate_uniqueness_of(:project) }
-    it { is_expected.to validate_uniqueness_of(:security_policy_management_project) }
   end
 
   describe '#enabled?' do

ee/spec/requests/projects/security/policies_controller_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe Projects::Security::PoliciesController, type: :request do
     it 'returns error message for invalid input' do
       post assign_project_security_policy_url(project), params: { orchestration: { policy_project_id: nil } }
 
-      expect(flash[:alert]).to eq 'Policy project doesn\'t exists'
+      expect(flash[:alert]).to eq 'Policy project doesn\'t exist'
     end
   end
 end

ee/spec/services/security/orchestration/assign_service_spec.rb

@@ -9,32 +9,50 @@ RSpec.describe Security::Orchestration::AssignService do
   let_it_be(:new_policy_project) { create(:project) }
 
   describe '#execute' do
-    subject(:service) { described_class.new(project, nil, policy_project_id: policy_project.id).execute }
+    subject(:service) do
+      described_class.new(project, nil, policy_project_id: policy_project.id).execute
+    end
+
+    before do
+      service
+    end
 
     it 'assigns policy project to project' do
       expect(service).to be_success
-      expect(project.security_orchestration_policy_configuration.security_policy_management_project_id).to eq(policy_project.id)
+      expect(
+        project.security_orchestration_policy_configuration.security_policy_management_project_id
+      ).to eq(policy_project.id)
     end
 
     it 'updates project with new policy project' do
-      service
-
-      repeated_service = described_class.new(project, nil, policy_project_id: new_policy_project.id).execute
+      repeated_service =
+        described_class.new(project, nil, policy_project_id: new_policy_project.id).execute
 
       expect(repeated_service).to be_success
-      expect(project.security_orchestration_policy_configuration.security_policy_management_project_id).to eq(new_policy_project.id)
+      expect(
+        project.security_orchestration_policy_configuration.security_policy_management_project_id
+      ).to eq(new_policy_project.id)
     end
 
-    it 'returns error when same policy is assigned to different projects' do
-      service
+    it 'assigns same policy to different projects' do
+      repeated_service =
+        described_class.new(another_project, nil, policy_project_id: policy_project.id).execute
+      expect(repeated_service).to be_success
+    end
 
-      repeated_service = described_class.new(another_project, nil, policy_project_id: policy_project.id).execute
-      expect(repeated_service).to be_error
+    it 'unassigns project' do
+      expect { described_class.new(project, nil, policy_project_id: nil).execute }.to change {
+        project.reload.security_orchestration_policy_configuration
+      }.to(nil)
     end
 
     it 'returns error when db has problem' do
       dbl_error = double('ActiveRecord')
-      dbl = double('Security::OrchestrationPolicyConfiguration', security_orchestration_policy_configuration: dbl_error)
+      dbl =
+        double(
+          'Security::OrchestrationPolicyConfiguration',
+          security_orchestration_policy_configuration: dbl_error
+        )
 
       allow(dbl_error).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
 
@@ -43,7 +61,8 @@ RSpec.describe Security::Orchestration::AssignService do
         allow(instance).to receive(:project).and_return(dbl)
       end
 
-      repeated_service = described_class.new(project, nil, policy_project_id: new_policy_project.id).execute
+      repeated_service =
+        described_class.new(project, nil, policy_project_id: new_policy_project.id).execute
 
       expect(repeated_service).to be_error
     end

locale/gitlab.pot

@@ -21580,6 +21580,9 @@ msgstr ""
 msgid "Opens in a new window"
 msgstr ""
 
+msgid "Operation completed"
+msgstr ""
+
 msgid "Operation failed. Check pod logs for %{pod_name} for more details."
 msgstr ""
 
@@ -22951,7 +22954,7 @@ msgstr ""
 msgid "Point to any links you like: documentation, built binaries, or other related materials. These can be internal or external links from your GitLab instance. Duplicate URLs are not allowed."
 msgstr ""
 
-msgid "Policy project doesn't exists"
+msgid "Policy project doesn't exist"
 msgstr ""
 
 msgid "Popularity"
@@ -29216,9 +29219,6 @@ msgstr ""
 msgid "Successful purchase image"
 msgstr ""
 
-msgid "Successfull"
-msgstr ""
-
 msgid "Successfully activated"
 msgstr ""