Merge branch '13539-disable-license-check-text-field' into 'master'

Disable editing of the `License-Check` approval rule name.

See merge request gitlab-org/gitlab-ee!16149

ee/app/assets/javascripts/approvals/components/rule_form.vue

@@ -8,6 +8,8 @@ import ApproversSelect from './approvers_select.vue';
 import { TYPE_USER, TYPE_GROUP, TYPE_HIDDEN_GROUPS } from '../constants';
 
 const DEFAULT_NAME = 'Default';
+const DEFAULT_NAME_FOR_LICENSE_REPORT = 'License-Check';
+const READONLY_NAMES = [DEFAULT_NAME_FOR_LICENSE_REPORT];
 
 export default {
   components: {
@@ -103,6 +105,12 @@ export default {
         this.settings.allowMultiRule && this.isFallback && !this.name && !this.approvers.length
       );
     },
+    isPersisted() {
+      return this.initRule && this.initRule.id;
+    },
+    isNameDisabled() {
+      return this.isPersisted && READONLY_NAMES.includes(this.name);
+    },
     removeHiddenGroups() {
       return this.containsHiddenGroups && !this.approversByType[TYPE_HIDDEN_GROUPS];
     },
@@ -226,6 +234,7 @@ export default {
           <input
             v-model="name"
             :class="{ 'is-invalid': validation.name }"
+            :disabled="isNameDisabled"
             class="form-control"
             name="name"
             type="text"

ee/app/models/approval_project_rule.rb

@@ -12,6 +12,7 @@ class ApprovalProjectRule < ApplicationRecord
   }
 
   alias_method :code_owner, :code_owner?
+  validate :validate_default_license_report_name, on: :update, if: :report_approver?
 
   validates :name, uniqueness: { scope: :project_id }
 
@@ -46,4 +47,11 @@ class ApprovalProjectRule < ApplicationRecord
         report_type: report_type
       )
   end
+
+  def validate_default_license_report_name
+    return unless name_changed?
+    return unless name_was == ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT
+
+    errors.add(:name, _("cannot be modified"))
+  end
 end

ee/changelogs/unreleased/13539-disable-license-check-text-field.yml

+---
+title: Disable editing of the 'License-Check' approval rule name
+merge_request: 16149
+author:
+type: added

ee/spec/frontend/approvals/components/modal_rule_create_spec.js

+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import Vuex from 'vuex';
+import { createStoreOptions } from 'ee/approvals/stores';
+import projectSettingsModule from 'ee/approvals/stores/modules/project_settings';
+import RuleForm from 'ee/approvals/components/rule_form.vue';
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
+describe('RuleForm', () => {
+  const exampleRule = {
+    id: 1,
+    name: 'Example',
+    approvalsRequired: 0,
+    users: [],
+    groups: [],
+  };
+  let wrapper;
+  let store;
+
+  const findNameInput = () => wrapper.find('input[name=name]');
+  const createStore = () => createStoreOptions(projectSettingsModule(), { projectId: '7' });
+  const createComponent = (vuexStore, props = {}) =>
+    shallowMount(localVue.extend(RuleForm), {
+      propsData: {
+        ...props,
+      },
+      store: new Vuex.Store(vuexStore),
+      localVue,
+      sync: false,
+    });
+
+  beforeEach(() => {
+    store = createStore();
+  });
+
+  afterEach(() => {
+    if (wrapper) wrapper.destroy();
+  });
+
+  describe('isNameDisabled', () => {
+    beforeEach(() => {
+      store.state.settings.allowMultiRule = true;
+    });
+
+    describe('When creating the License-Check rule', () => {
+      beforeEach(() => {
+        wrapper = createComponent(store, {
+          initRule: { ...exampleRule, ...{ id: null, name: 'License-Check' } },
+        });
+      });
+
+      it('does not disable the name text field', () => {
+        expect(findNameInput().attributes('disabled')).toBe(undefined);
+      });
+    });
+
+    describe('When creating any other rule', () => {
+      beforeEach(() => {
+        wrapper = createComponent(store, {
+          initRule: { ...exampleRule, ...{ id: null, name: 'QA' } },
+        });
+      });
+
+      it('does not disable the name text field', () => {
+        expect(findNameInput().attributes('disabled')).toBe(undefined);
+      });
+    });
+
+    describe('When editing the License-Check rule', () => {
+      beforeEach(() => {
+        wrapper = createComponent(store, {
+          initRule: { ...exampleRule, ...{ id: 1, name: 'License-Check' } },
+        });
+      });
+
+      it('disables the name text field', () => {
+        expect(findNameInput().attributes('disabled')).toBe('disabled');
+      });
+    });
+
+    describe('When editing any other rule', () => {
+      beforeEach(() => {
+        wrapper = createComponent(store, {
+          initRule: { ...exampleRule, ...{ id: 1, name: 'QA' } },
+        });
+      });
+
+      it('does not disable the name text field', () => {
+        expect(findNameInput().attributes('disabled')).toBe(undefined);
+      });
+    });
+  });
+});

ee/spec/models/approval_project_rule_spec.rb

@@ -88,4 +88,39 @@ describe ApprovalProjectRule do
       end
     end
   end
+
+  describe "validation" do
+    let(:project_approval_rule) { create(:approval_project_rule) }
+    let(:license_compliance_rule) { create(:approval_project_rule, :license_management) }
+    let(:vulnerability_check_rule) { create(:approval_project_rule, :security) }
+
+    context "when creating a new rule" do
+      specify { expect(project_approval_rule).to be_valid }
+      specify { expect(license_compliance_rule).to be_valid }
+      specify { expect(vulnerability_check_rule).to be_valid }
+    end
+
+    context "when attempting to edit the name of the rule" do
+      subject { project_approval_rule }
+
+      before do
+        subject.name = SecureRandom.uuid
+      end
+
+      specify { expect(subject).to be_valid }
+
+      context "with a `License-Check` rule" do
+        subject { license_compliance_rule }
+
+        specify { expect(subject).not_to be_valid }
+        specify { expect { subject.valid? }.to change { subject.errors[:name].present? } }
+      end
+
+      context "with a `Vulnerability-Check` rule" do
+        subject { vulnerability_check_rule }
+
+        specify { expect(subject).to be_valid }
+      end
+    end
+  end
 end

locale/gitlab.pot

@@ -18148,6 +18148,9 @@ msgstr ""
 msgid "cannot be in the same project"
 msgstr ""
 
+msgid "cannot be modified"
+msgstr ""
+
 msgid "cannot block others"
 msgstr ""