Merge branch '408676-adjust-label-close' into 'master'

Remove label close button when lock on merge

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



Merged-by: Miguel Rincon <mrincon@gitlab.com>
Reviewed-by: Simon Knox <simon@gitlab.com>
Reviewed-by: Miguel Rincon <mrincon@gitlab.com>
Reviewed-by: Donald Cook <dcook@gitlab.com>
Co-authored-by: Donald Cook <dcook@gitlab.com>
Co-authored-by: Brett Walker <bwalker@gitlab.com>

app/assets/javascripts/issuable/issuable_label_selector.js

 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
 import createDefaultClient from '~/lib/graphql';
+import { parseBoolean } from '~/lib/utils/common_utils';
 import { VARIANT_EMBEDDED } from '~/sidebar/components/labels/labels_select_widget/constants';
 import { WORKSPACE_PROJECT } from '~/issues/constants';
 import IssuableLabelSelector from '~/vue_shared/issuable/create/components/issuable_label_selector.vue';
@@ -25,6 +26,7 @@ export default () => {
     issuableType,
     labelsFilterBasePath,
     labelsManagePath,
+    supportsLockOnMerge,
   } = el.dataset;
 
   return new Vue({
@@ -46,6 +48,7 @@ export default () => {
       variant: VARIANT_EMBEDDED,
       workspaceType: WORKSPACE_PROJECT,
       toggleAttrs: { 'data-testid': 'issuable-label-dropdown' },
+      supportsLockOnMerge: parseBoolean(supportsLockOnMerge),
     },
     render(createElement) {
       return createElement(IssuableLabelSelector);

app/assets/javascripts/sidebar/components/labels/labels_select_widget/dropdown_value.vue

@@ -27,6 +27,11 @@ export default {
       type: Boolean,
       required: true,
     },
+    supportsLockOnMerge: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     labelsFilterBasePath: {
       type: String,
       required: true,
@@ -67,6 +72,12 @@ export default {
     scopedLabel(label) {
       return this.allowScopedLabels && isScopedLabel(label);
     },
+    isLabelLocked(label) {
+      return label.lockOnMerge && this.supportsLockOnMerge;
+    },
+    showCloseButton(label) {
+      return this.allowLabelRemove && !this.isLabelLocked(label);
+    },
     removeLabel(labelId) {
       this.$emit('onLabelRemove', labelId);
     },
@@ -115,7 +126,7 @@ export default {
         :background-color="label.color"
         :target="labelFilterUrl(label)"
         :scoped="scopedLabel(label)"
-        :show-close-button="allowLabelRemove"
+        :show-close-button="showCloseButton(label)"
         :disabled="disableLabels"
         tooltip-placement="top"
         @close="removeLabel(label.id)"

app/assets/javascripts/sidebar/components/labels/labels_select_widget/embedded_labels_list.vue

@@ -22,6 +22,11 @@ export default {
       type: Boolean,
       required: true,
     },
+    supportsLockOnMerge: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     labelsFilterBasePath: {
       type: String,
       required: true,
@@ -42,9 +47,15 @@ export default {
 
       return `${basePath}?${filterParam}[]=${encodeURIComponent(title)}`;
     },
-    showScopedLabel(label) {
+    scopedLabel(label) {
       return this.allowScopedLabels && isScopedLabel(label);
     },
+    isLabelLocked(label) {
+      return label.lockOnMerge && this.supportsLockOnMerge;
+    },
+    showCloseButton(label) {
+      return this.allowLabelRemove && !this.isLabelLocked(label);
+    },
     removeLabel(labelId) {
       this.$emit('onLabelRemove', labelId);
     },
@@ -63,8 +74,8 @@ export default {
       :description="label.description"
       :background-color="label.color"
       :target="buildFilterUrl(label)"
-      :scoped="showScopedLabel(label)"
-      :show-close-button="allowLabelRemove"
+      :scoped="scopedLabel(label)"
+      :show-close-button="showCloseButton(label)"
       :disabled="disabled"
       tooltip-placement="top"
       @close="removeLabel(label.id)"

app/assets/javascripts/sidebar/components/labels/labels_select_widget/graphql/merge_request_labels.query.graphql

@@ -8,6 +8,7 @@ query mergeRequestLabels($fullPath: ID!, $iid: String!) {
       labels {
         nodes {
           ...Label
+          lockOnMerge
         }
       }
     }

app/assets/javascripts/sidebar/components/labels/labels_select_widget/labels_select_root.vue

@@ -52,6 +52,11 @@ export default {
       required: false,
       default: false,
     },
+    supportsLockOnMerge: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     showEmbeddedLabelsList: {
       type: Boolean,
       required: false,
@@ -151,6 +156,9 @@ export default {
     isLabelListEnabled() {
       return this.showEmbeddedLabelsList && isDropdownVariantEmbedded(this.variant);
     },
+    isLockOnMergeSupported() {
+      return this.enforceLockedLabelsOnMerge;
+    },
   },
   apollo: {
     issuable: {
@@ -376,6 +384,7 @@ export default {
             :disable-labels="labelsSelectInProgress"
             :selected-labels="issuableLabels"
             :allow-label-remove="allowLabelRemove"
+            :supports-lock-on-merge="isLockOnMergeSupported"
             :labels-filter-base-path="labelsFilterBasePath"
             :labels-filter-param="labelsFilterParam"
             @onLabelRemove="handleLabelRemove"
@@ -389,6 +398,7 @@ export default {
             :disable-labels="labelsSelectInProgress"
             :selected-labels="issuableLabels"
             :allow-label-remove="allowLabelRemove"
+            :supports-lock-on-merge="isLockOnMergeSupported"
             :labels-filter-base-path="labelsFilterBasePath"
             :labels-filter-param="labelsFilterParam"
             class="gl-mb-2"
@@ -440,6 +450,7 @@ export default {
         :disabled="labelsSelectInProgress"
         :selected-labels="issuableLabels"
         :allow-label-remove="allowLabelRemove"
+        :supports-lock-on-merge="isLockOnMergeSupported"
         :labels-filter-base-path="labelsFilterBasePath"
         :labels-filter-param="labelsFilterParam"
         @onLabelRemove="handleLabelRemove"

app/assets/javascripts/sidebar/mount_sidebar.js

@@ -355,6 +355,7 @@ export function mountSidebarLabelsWidget() {
           workspaceType: WORKSPACE_PROJECT,
           attrWorkspacePath: el.dataset.projectPath,
           labelCreateType: WORKSPACE_PROJECT,
+          enforceLockedLabelsOnMerge: gon.features.enforceLockedLabelsOnMerge,
         },
         class: ['block labels js-labels-block'],
         scopedSlots: {

app/assets/javascripts/vue_shared/issuable/create/components/issuable_label_selector.vue

@@ -18,6 +18,7 @@ export default {
     'initialLabels',
     'issuableType',
     'labelType',
+    'supportsLockOnMerge',
     'variant',
     'workspaceType',
   ],
@@ -76,6 +77,7 @@ export default {
           :issuable-type="issuableType"
           :label-create-type="labelType"
           :selected-labels="selectedLabels"
+          :supports-lock-on-merge="supportsLockOnMerge"
           @updateSelectedLabels="handleUpdateSelectedLabels"
           @onLabelRemove="handleLabelRemove"
         >

app/controllers/projects/merge_requests/application_controller.rb

@@ -4,6 +4,9 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
   before_action :check_merge_requests_available!
   before_action :merge_request
   before_action :authorize_read_merge_request!
+  before_action do
+    push_force_frontend_feature_flag(:enforce_locked_labels_on_merge, project&.supports_lock_on_merge?)
+  end
 
   feature_category :code_review_workflow
 

app/helpers/issuables_helper.rb

@@ -248,7 +248,8 @@ def issuable_label_selector_data(project, issuable)
         title: label.title,
         description: label.description,
         color: label.color,
-        text_color: label.text_color
+        text_color: label.text_color,
+        lock_on_merge: label.lock_on_merge
       }
     end
 
@@ -265,7 +266,8 @@ def issuable_label_selector_data(project, issuable)
       initial_labels: initial_labels.to_json,
       issuable_type: issuable.issuable_type,
       labels_filter_base_path: filter_base_path,
-      labels_manage_path: project_labels_path(project)
+      labels_manage_path: project_labels_path(project),
+      supports_lock_on_merge: issuable.supports_lock_on_merge?.to_s
     }
   end
 

spec/frontend/sidebar/components/labels/labels_select_widget/dropdown_value_spec.js

@@ -3,14 +3,15 @@ import { shallowMount } from '@vue/test-utils';
 
 import DropdownValue from '~/sidebar/components/labels/labels_select_widget/dropdown_value.vue';
 
-import { mockRegularLabel, mockScopedLabel } from './mock_data';
+import { mockRegularLabel, mockScopedLabel, mockLockedLabel } from './mock_data';
 
 describe('DropdownValue', () => {
   let wrapper;
 
   const findAllLabels = () => wrapper.findAllComponents(GlLabel);
-  const findRegularLabel = () => findAllLabels().at(1);
+  const findRegularLabel = () => findAllLabels().at(2);
   const findScopedLabel = () => findAllLabels().at(0);
+  const findLockedLabel = () => findAllLabels().at(1);
   const findWrapper = () => wrapper.find('[data-testid="value-wrapper"]');
   const findEmptyPlaceholder = () => wrapper.find('[data-testid="empty-placeholder"]');
 
@@ -18,7 +19,7 @@ describe('DropdownValue', () => {
     wrapper = shallowMount(DropdownValue, {
       slots,
       propsData: {
-        selectedLabels: [mockRegularLabel, mockScopedLabel],
+        selectedLabels: [mockLockedLabel, mockRegularLabel, mockScopedLabel],
         allowLabelRemove: true,
         labelsFilterBasePath: '/gitlab-org/my-project/issues',
         labelsFilterParam: 'label_name',
@@ -69,8 +70,8 @@ describe('DropdownValue', () => {
       expect(findEmptyPlaceholder().exists()).toBe(false);
     });
 
-    it('renders a list of two labels', () => {
-      expect(findAllLabels().length).toBe(2);
+    it('renders a list of three labels', () => {
+      expect(findAllLabels().length).toBe(3);
     });
 
     it('passes correct props to the regular label', () => {
@@ -96,5 +97,19 @@ describe('DropdownValue', () => {
       wrapper.find('.sidebar-collapsed-icon').trigger('click');
       expect(wrapper.emitted('onCollapsedValueClick')).toEqual([[]]);
     });
+
+    it('does not show close button if label is locked', () => {
+      createComponent({
+        supportsLockOnMerge: true,
+      });
+      expect(findLockedLabel().props('showCloseButton')).toBe(false);
+    });
+
+    it('shows close button if label is not locked', () => {
+      createComponent({
+        supportsLockOnMerge: true,
+      });
+      expect(findRegularLabel().props('showCloseButton')).toBe(true);
+    });
   });
 });

spec/frontend/sidebar/components/labels/labels_select_widget/embedded_labels_list_spec.js

 import { GlLabel } from '@gitlab/ui';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import EmbeddedLabelsList from '~/sidebar/components/labels/labels_select_widget/embedded_labels_list.vue';
-import { mockRegularLabel, mockScopedLabel } from './mock_data';
+import { mockRegularLabel, mockScopedLabel, mockLockedLabel } from './mock_data';
 
 describe('EmbeddedLabelsList', () => {
   let wrapper;
@@ -13,12 +13,13 @@ describe('EmbeddedLabelsList', () => {
       .at(0);
   const findRegularLabel = () => findLabelByTitle(mockRegularLabel.title);
   const findScopedLabel = () => findLabelByTitle(mockScopedLabel.title);
+  const findLockedLabel = () => findLabelByTitle(mockLockedLabel.title);
 
   const createComponent = (props = {}, slots = {}) => {
     wrapper = shallowMountExtended(EmbeddedLabelsList, {
       slots,
       propsData: {
-        selectedLabels: [mockRegularLabel, mockScopedLabel],
+        selectedLabels: [mockRegularLabel, mockScopedLabel, mockLockedLabel],
         allowLabelRemove: true,
         labelsFilterBasePath: '/gitlab-org/my-project/issues',
         labelsFilterParam: 'label_name',
@@ -47,8 +48,8 @@ describe('EmbeddedLabelsList', () => {
       createComponent();
     });
 
-    it('renders a list of two labels', () => {
-      expect(findAllLabels()).toHaveLength(2);
+    it('renders a list of three labels', () => {
+      expect(findAllLabels()).toHaveLength(3);
     });
 
     it('passes correct props to the regular label', () => {
@@ -69,5 +70,12 @@ describe('EmbeddedLabelsList', () => {
       findRegularLabel().vm.$emit('close');
       expect(wrapper.emitted('onLabelRemove')).toStrictEqual([[mockRegularLabel.id]]);
     });
+
+    it('does not show close button if label is locked', () => {
+      createComponent({
+        supportsLockOnMerge: true,
+      });
+      expect(findLockedLabel().props('showCloseButton')).toBe(false);
+    });
   });
 });

spec/frontend/sidebar/components/labels/labels_select_widget/mock_data.js

@@ -14,6 +14,15 @@ export const mockScopedLabel = {
   textColor: '#FFFFFF',
 };
 
+export const mockLockedLabel = {
+  id: 30,
+  title: 'Bar Label',
+  description: 'Bar',
+  color: '#DADA55',
+  textColor: '#FFFFFF',
+  lockOnMerge: true,
+};
+
 export const mockLabels = [
   mockRegularLabel,
   mockScopedLabel,

spec/frontend/vue_shared/issuable/create/components/issuable_label_selector_spec.js

@@ -17,6 +17,7 @@ const labelsFilterBasePath = '/labels-filter-base-path';
 const initialLabels = [];
 const issuableType = 'issue';
 const labelType = WORKSPACE_PROJECT;
+const supportsLockOnMerge = false;
 const variant = VARIANT_EMBEDDED;
 const workspaceType = WORKSPACE_PROJECT;
 
@@ -37,6 +38,7 @@ describe('IssuableLabelSelector', () => {
         initialLabels,
         issuableType,
         labelType,
+        supportsLockOnMerge,
         variant,
         workspaceType,
         ...injectedProps,

spec/helpers/issuables_helper_spec.rb

@@ -600,7 +600,8 @@
           initial_labels: '[]',
           issuable_type: issuable.issuable_type,
           labels_filter_base_path: project_issues_path(project),
-          labels_manage_path: project_labels_path(project)
+          labels_manage_path: project_labels_path(project),
+          supports_lock_on_merge: issuable.supports_lock_on_merge?.to_s
         })
       end
     end
@@ -620,7 +621,8 @@
             title: label.title,
             description: label.description,
             color: label.color,
-            text_color: label.text_color
+            text_color: label.text_color,
+            lock_on_merge: label.lock_on_merge
           },
           {
             __typename: "Label",
@@ -628,7 +630,8 @@
             title: label2.title,
             description: label2.description,
             color: label2.color,
-            text_color: label2.text_color
+            text_color: label2.text_color,
+            lock_on_merge: label.lock_on_merge
           }
         ]
 
@@ -638,7 +641,8 @@
           initial_labels: initial_labels.to_json,
           issuable_type: issuable.issuable_type,
           labels_filter_base_path: project_merge_requests_path(project),
-          labels_manage_path: project_labels_path(project)
+          labels_manage_path: project_labels_path(project),
+          supports_lock_on_merge: issuable.supports_lock_on_merge?.to_s
         })
       end
     end

spec/support/shared_contexts/merge_request_edit_shared_context.rb

@@ -5,7 +5,7 @@
   let(:user2)       { create(:user) }
   let!(:milestone)   { create(:milestone, project: target_project) }
   let!(:label)       { create(:label, project: target_project) }
-  let!(:label2)      { create(:label, project: target_project) }
+  let!(:label2)      { create(:label, project: target_project, lock_on_merge: true) }
   let(:target_project) { create(:project, :public, :repository) }
   let(:source_project) { target_project }
   let(:merge_request) do

spec/support/shared_examples/features/editable_merge_request_shared_examples.rb

@@ -54,6 +54,8 @@
       page.within '.labels' do
         expect(page).to have_content label.title
         expect(page).to have_content label2.title
+
+        expect(page).to have_selector("[data-testid='close-icon']", count: 1)
       end
     end
   end