Merge branch...

Merge branch '340813-prevent-sending-a-labels-update-mutation-if-labels-list-is-not-changed' into 'master'

Prevent sending a labels update mutation if labels list is not changed

See merge request gitlab-org/gitlab!71740

app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents.vue

@@ -3,7 +3,7 @@ import { GlButton, GlDropdown, GlDropdownItem, GlLink } from '@gitlab/ui';
 import { __, s__, sprintf } from '~/locale';
 import DropdownContentsCreateView from './dropdown_contents_create_view.vue';
 import DropdownContentsLabelsView from './dropdown_contents_labels_view.vue';
-import { isDropdownVariantStandalone } from './utils';
+import { isDropdownVariantStandalone, isDropdownVariantSidebar } from './utils';
 
 export default {
   components: {
@@ -52,11 +52,17 @@ export default {
       type: String,
       required: true,
     },
+    isVisible: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   data() {
     return {
       showDropdownContentsCreateView: false,
       localSelectedLabels: [...this.selectedLabels],
+      isDirty: false,
     };
   },
   computed: {
@@ -87,6 +93,26 @@ export default {
       return isDropdownVariantStandalone(this.variant);
     },
   },
+  watch: {
+    localSelectedLabels: {
+      handler() {
+        this.isDirty = true;
+      },
+      deep: true,
+    },
+    isVisible(newVal) {
+      if (newVal) {
+        this.$refs.dropdown.show();
+        this.isDirty = false;
+      } else {
+        this.$refs.dropdown.hide();
+        this.setLabels();
+      }
+    },
+    selectedLabels(newVal) {
+      this.localSelectedLabels = newVal;
+    },
+  },
   methods: {
     toggleDropdownContentsCreateView() {
       this.showDropdownContentsCreateView = !this.showDropdownContentsCreateView;
@@ -98,12 +124,16 @@ export default {
         this.$refs.dropdown.$refs.dropdown.$_popper.scheduleUpdate();
       }
     },
-    showDropdown() {
-      this.$refs.dropdown.show();
-    },
-    closeDropdown() {
+    setLabels() {
+      if (!this.isDirty) {
+        return;
+      }
       this.$emit('setLabels', this.localSelectedLabels);
-      this.$refs.dropdown.hide();
+    },
+    handleDropdownHide() {
+      if (!isDropdownVariantSidebar(this.variant)) {
+        this.setLabels();
+      }
     },
   },
 };
@@ -115,7 +145,7 @@ export default {
     :text="buttonText"
     class="gl-w-full gl-mt-2"
     data-qa-selector="labels_dropdown_content"
-    @hide="$emit('setLabels', localSelectedLabels)"
+    @hide="handleDropdownHide"
   >
     <template #header>
       <div
@@ -141,7 +171,7 @@ export default {
           class="dropdown-header-button gl-p-0!"
           icon="close"
           data-testid="close-button"
-          @click="closeDropdown"
+          @click="$emit('closeDropdown')"
         />
       </div>
     </template>

app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view.vue

@@ -50,6 +50,7 @@ export default {
     return {
       searchKey: '',
       labels: [],
+      isVisible: false,
     };
   },
   apollo: {
@@ -64,13 +65,13 @@ export default {
         };
       },
       skip() {
-        return this.searchKey.length === 1;
+        return this.searchKey.length === 1 || !this.isVisible;
       },
       update: (data) => data.workspace?.labels?.nodes || [],
       async result() {
         if (this.$refs.searchInput) {
           await this.$nextTick;
-          this.focusInputField();
+          this.$refs.searchInput.focusInput();
         }
       },
       error() {
@@ -150,7 +151,8 @@ export default {
     setSearchKey(value) {
       this.searchKey = value;
     },
-    focusInputField() {
+    onDropdownAppear() {
+      this.isVisible = true;
       this.$refs.searchInput.focusInput();
     },
   },
@@ -158,7 +160,7 @@ export default {
 </script>
 
 <template>
-  <gl-intersection-observer @appear="focusInputField">
+  <gl-intersection-observer @appear="onDropdownAppear">
     <gl-dropdown-form class="labels-select-contents-list js-labels-list">
       <gl-search-box-by-type
         ref="searchInput"

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

@@ -136,14 +136,14 @@ export default {
   methods: {
     handleDropdownClose(labels) {
       this.$emit('updateSelectedLabels', labels);
+      this.collapseEditableItem();
+    },
+    collapseEditableItem() {
       this.$refs.editable?.collapse();
     },
     handleCollapsedValueClick() {
       this.$emit('toggleCollapse');
     },
-    showDropdownContents() {
-      this.$refs.dropdownContents.showDropdown();
-    },
     isDropdownVariantSidebar,
     isDropdownVariantStandalone,
     isDropdownVariantEmbedded,
@@ -170,7 +170,6 @@ export default {
         :title="__('Labels')"
         :loading="isLoading"
         :can-edit="allowLabelEdit"
-        @open="showDropdownContents"
       >
         <template #collapsed>
           <dropdown-value
@@ -184,7 +183,7 @@ export default {
             <slot></slot>
           </dropdown-value>
         </template>
-        <template #default>
+        <template #default="{ edit }">
           <dropdown-value
             :disable-labels="labelsSelectInProgress"
             :selected-labels="issuableLabels"
@@ -197,7 +196,6 @@ export default {
             <slot></slot>
           </dropdown-value>
           <dropdown-contents
-            ref="dropdownContents"
             :dropdown-button-text="dropdownButtonText"
             :allow-multiselect="allowMultiselect"
             :labels-list-title="labelsListTitle"
@@ -207,7 +205,9 @@ export default {
             :selected-labels="selectedLabels"
             :variant="variant"
             :issuable-type="issuableType"
+            :is-visible="edit"
             @setLabels="handleDropdownClose"
+            @closeDropdown="collapseEditableItem"
           />
         </template>
       </sidebar-editable-item>

spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view_spec.js

-import { GlLoadingIcon, GlSearchBoxByType, GlDropdownItem } from '@gitlab/ui';
+import {
+  GlLoadingIcon,
+  GlSearchBoxByType,
+  GlDropdownItem,
+  GlIntersectionObserver,
+} from '@gitlab/ui';
 import { shallowMount, createLocalVue } from '@vue/test-utils';
 import { nextTick } from 'vue';
 import VueApollo from 'vue-apollo';
@@ -68,23 +73,31 @@ describe('DropdownContentsLabelsView', () => {
   const findSearchInput = () => wrapper.findComponent(GlSearchBoxByType);
   const findLabels = () => wrapper.findAllComponents(LabelItem);
   const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
+  const findObserver = () => wrapper.findComponent(GlIntersectionObserver);
 
   const findLabelsList = () => wrapper.find('[data-testid="labels-list"]');
   const findNoResultsMessage = () => wrapper.find('[data-testid="no-results"]');
 
+  async function makeObserverAppear() {
+    await findObserver().vm.$emit('appear');
+  }
+
   describe('when loading labels', () => {
     it('renders disabled search input field', async () => {
       createComponent();
+      await makeObserverAppear();
       expect(findSearchInput().props('disabled')).toBe(true);
     });
 
     it('renders loading icon', async () => {
       createComponent();
+      await makeObserverAppear();
       expect(findLoadingIcon().exists()).toBe(true);
     });
 
     it('does not render labels list', async () => {
       createComponent();
+      await makeObserverAppear();
       expect(findLabelsList().exists()).toBe(false);
     });
   });
@@ -92,6 +105,7 @@ describe('DropdownContentsLabelsView', () => {
   describe('when labels are loaded', () => {
     beforeEach(async () => {
       createComponent();
+      await makeObserverAppear();
       await waitForPromises();
     });
 
@@ -121,6 +135,7 @@ describe('DropdownContentsLabelsView', () => {
         },
       }),
     });
+    await makeObserverAppear();
     findSearchInput().vm.$emit('input', '123');
     await waitForPromises();
     await nextTick();
@@ -130,6 +145,7 @@ describe('DropdownContentsLabelsView', () => {
 
   it('calls `createFlash` when fetching labels failed', async () => {
     createComponent({ queryHandler: jest.fn().mockRejectedValue('Houston, we have a problem!') });
+    await makeObserverAppear();
     jest.advanceTimersByTime(DEFAULT_DEBOUNCE_AND_THROTTLE_MS);
     await waitForPromises();
 
@@ -138,9 +154,17 @@ describe('DropdownContentsLabelsView', () => {
 
   it('emits an `input` event on label click', async () => {
     createComponent();
+    await makeObserverAppear();
     await waitForPromises();
     findFirstLabel().trigger('click');
 
     expect(wrapper.emitted('input')[0][0]).toEqual(expect.arrayContaining(localSelectedLabels));
   });
+
+  it('does not trigger query when component did not appear', () => {
+    createComponent();
+    expect(findLoadingIcon().exists()).toBe(false);
+    expect(findLabelsList().exists()).toBe(false);
+    expect(successfulQueryHandler).not.toHaveBeenCalled();
+  });
 });

spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_spec.js

@@ -19,6 +19,7 @@ const GlDropdownStub = {
   `,
   methods: {
     show: showDropdown,
+    hide: jest.fn(),
   },
 };
 
@@ -68,18 +69,52 @@ describe('DropdownContent', () => {
   const findCreateLabelButton = () => wrapper.find('[data-testid="create-label-button"]');
   const findGoBackButton = () => wrapper.find('[data-testid="go-back-button"]');
 
-  it('emits `show` for dropdown on call showDropdown', () => {
+  it('calls dropdown `show` method on `isVisible` prop change', async () => {
     createComponent();
-    wrapper.vm.showDropdown();
+    await wrapper.setProps({
+      isVisible: true,
+    });
 
     expect(findDropdown().emitted('show')).toBeUndefined();
   });
 
-  it('emits `setLabels` event on dropdown hide', () => {
+  it('does not emit `setLabels` event on dropdown hide if labels did not change', () => {
     createComponent();
     findDropdown().vm.$emit('hide');
 
-    expect(wrapper.emitted('setLabels')).toEqual([[mockLabels]]);
+    expect(wrapper.emitted('setLabels')).toBeUndefined();
+  });
+
+  it('emits `setLabels` event on dropdown hide if labels changed on non-sidebar widget', async () => {
+    createComponent({ props: { variant: DropdownVariant.Standalone } });
+    const updatedLabel = {
+      id: 28,
+      title: 'Bug',
+      description: 'Label for bugs',
+      color: '#FF0000',
+      textColor: '#FFFFFF',
+    };
+    findLabelsView().vm.$emit('input', [updatedLabel]);
+    await nextTick();
+    findDropdown().vm.$emit('hide');
+
+    expect(wrapper.emitted('setLabels')).toEqual([[[updatedLabel]]]);
+  });
+
+  it('emits `setLabels` event on visibility change if labels changed on sidebar widget', async () => {
+    createComponent({ props: { variant: DropdownVariant.Standalone, isVisible: true } });
+    const updatedLabel = {
+      id: 28,
+      title: 'Bug',
+      description: 'Label for bugs',
+      color: '#FF0000',
+      textColor: '#FFFFFF',
+    };
+    findLabelsView().vm.$emit('input', [updatedLabel]);
+    wrapper.setProps({ isVisible: false });
+    await nextTick();
+
+    expect(wrapper.emitted('setLabels')).toEqual([[[updatedLabel]]]);
   });
 
   it('does not render header on standalone variant', () => {

spec/frontend/vue_shared/components/sidebar/labels_select_widget/labels_select_root_spec.js

@@ -27,8 +27,6 @@ describe('LabelsSelectRoot', () => {
   const findDropdownValue = () => wrapper.findComponent(DropdownValue);
   const findDropdownContents = () => wrapper.findComponent(DropdownContents);
 
-  const expandDropdown = () => wrapper.vm.$refs.editable.expand();
-
   const createComponent = ({
     config = mockConfig,
     slots = {},
@@ -125,23 +123,11 @@ describe('LabelsSelectRoot', () => {
     });
   });
 
-  it('emits `updateSelectedLabels` event on dropdown contents `setLabels` event if there are labels to update', async () => {
+  it('emits `updateSelectedLabels` event on dropdown contents `setLabels` event', async () => {
     const label = { id: 'gid://gitlab/ProjectLabel/1' };
     createComponent();
-    wrapper.vm.$refs.dropdownContents = {
-      showDropdown: jest.fn(),
-    };
-    const showSpy = jest.spyOn(wrapper.vm.$refs.dropdownContents, 'showDropdown');
-    findDropdownContents().vm.$refs.dropdown = {
-      show: jest.fn(),
-    };
-    await waitForPromises();
-
-    expandDropdown();
-    await nextTick();
 
     findDropdownContents().vm.$emit('setLabels', [label]);
     expect(wrapper.emitted('updateSelectedLabels')).toEqual([[[label]]]);
-    expect(showSpy).toHaveBeenCalled();
   });
 });