Merge branch 'ss/rm-limit' into 'master'

Add remove limit button to limit

See merge request gitlab-org/gitlab!22552

ee/app/assets/javascripts/boards/components/board_settings_sidebar.vue

@@ -9,7 +9,7 @@ import {
   GlLink,
 } from '@gitlab/ui';
 import { mapActions, mapState } from 'vuex';
-import { __ } from '~/locale';
+import { __, n__ } from '~/locale';
 import autofocusonshow from '~/vue_shared/directives/autofocusonshow';
 import boardsStoreEE from '../stores/boards_store_ee';
 import flash from '~/flash';
@@ -17,7 +17,7 @@ import flash from '~/flash';
 // NOTE: need to revisit how we handle headerHeight, because we have so many different header and footer options.
 export default {
   headerHeight: process.env.NODE_ENV === 'development' ? '75px' : '40px',
-  listSettingsText: __('List Settings'),
+  listSettingsText: __('List settings'),
   assignee: 'assignee',
   milestone: 'milestone',
   label: 'label',
@@ -26,7 +26,9 @@ export default {
   labelAssigneeText: __('Assignee'),
   editLinkText: __('Edit'),
   noneText: __('None'),
-  wipLimitText: __('Work in Progress Limit'),
+  wipLimitText: __('Work in progress Limit'),
+  removeLimitText: __('Remove limit'),
+  inputPlaceholderText: __('Enter number of issues'),
   components: {
     GlDrawer,
     GlLabel,
@@ -42,7 +44,7 @@ export default {
   data() {
     return {
       edit: false,
-      currentWipLimit: 0,
+      currentWipLimit: null,
       updating: false,
     };
   },
@@ -53,7 +55,6 @@ export default {
         Warning: Though a computed property it is not reactive because we are
         referencing a List Model class. Reactivity only applies to plain JS objects
       */
-
       return boardsStoreEE.store.state.lists.find(({ id }) => id === this.activeListId);
     },
     isSidebarOpen() {
@@ -68,10 +69,14 @@ export default {
     activeListAssignee() {
       return this.activeList.assignee;
     },
+    wipLimitTypeText() {
+      return n__('%d issue', '%d issues', this.activeList.maxIssueCount);
+    },
+    wipLimitIsSet() {
+      return this.activeList.maxIssueCount !== 0;
+    },
     activeListWipLimit() {
-      return this.activeList.maxIssueCount === 0
-        ? this.$options.noneText
-        : this.activeList.maxIssueCount;
+      return this.activeList.maxIssueCount === 0 ? this.$options.noneText : this.wipLimitTypeText;
     },
     boardListType() {
       return this.activeList.type || null;
@@ -101,22 +106,24 @@ export default {
     },
     showInput() {
       this.edit = true;
-      this.currentWipLimit = this.activeList.maxIssueCount;
+      this.currentWipLimit =
+        this.activeList.maxIssueCount > 0 ? this.activeList.maxIssueCount : null;
     },
     resetStateAfterUpdate() {
       this.edit = false;
       this.updating = false;
-      this.currentWipLimit = 0;
+      this.currentWipLimit = null;
     },
     offFocus() {
-      if (this.currentWipLimit !== this.activeList.maxIssueCount) {
+      if (this.currentWipLimit !== this.activeList.maxIssueCount && this.currentWipLimit !== null) {
         this.updating = true;
-        // NOTE: Need a ref to activeListId in case the user closes the drawer.
+        // need to reassign bc were clearing the ref in resetStateAfterUpdate.
+        const wipLimit = this.currentWipLimit;
         const id = this.activeListId;
 
         this.updateListWipLimit({ maxIssueCount: this.currentWipLimit, id })
-          .then(({ config }) => {
-            boardsStoreEE.setMaxIssueCountOnList(id, JSON.parse(config.data).list.max_issue_count);
+          .then(() => {
+            boardsStoreEE.setMaxIssueCountOnList(id, wipLimit);
             this.resetStateAfterUpdate();
           })
           .catch(() => {
@@ -128,6 +135,25 @@ export default {
         this.edit = false;
       }
     },
+    clearWipLimit() {
+      this.updateListWipLimit({ maxIssueCount: 0, id: this.activeListId })
+        .then(() => {
+          boardsStoreEE.setMaxIssueCountOnList(this.activeListId, 0);
+          this.resetStateAfterUpdate();
+        })
+        .catch(() => {
+          this.resetStateAfterUpdate();
+          this.setActiveListId(0);
+          flash(__('Something went wrong while updating your list settings'));
+        });
+    },
+    handleWipLimitChange(wipLimit) {
+      if (wipLimit === '') {
+        this.currentWipLimit = null;
+      } else {
+        this.currentWipLimit = Number(wipLimit);
+      }
+    },
     onEnter() {
       this.offFocus();
     },
@@ -169,25 +195,39 @@ export default {
           }}</gl-link>
         </template>
       </div>
-      <div class="d-flex justify-content-between">
-        <div>
-          <label>{{ $options.wipLimitText }}</label>
-          <gl-form-input
-            v-if="edit"
-            v-model.number="currentWipLimit"
-            v-autofocusonshow
-            :disabled="updating"
-            type="number"
-            min="0"
-            trim
-            @keydown.enter.native="onEnter"
-            @blur="offFocus"
-          />
-          <p v-else class="js-wip-limit bold">{{ activeListWipLimit }}</p>
+      <div class="d-flex justify-content-between flex-column">
+        <div class="d-flex justify-content-between align-items-center mb-2">
+          <label class="m-0">{{ $options.wipLimitText }}</label>
+          <gl-button
+            class="js-edit-button h-100 border-0 gl-line-height-14 text-dark"
+            variant="link"
+            @click="showInput"
+            >{{ $options.editLinkText }}</gl-button
+          >
+        </div>
+        <gl-form-input
+          v-if="edit"
+          v-autofocusonshow
+          :value="currentWipLimit"
+          :disabled="updating"
+          :placeholder="$options.inputPlaceholderText"
+          trim
+          @input="handleWipLimitChange"
+          @keydown.enter.native="onEnter"
+          @blur="offFocus"
+        />
+        <div v-else class="d-flex align-items-center">
+          <p class="js-wip-limit bold m-0 text-secondary">{{ activeListWipLimit }}</p>
+          <template v-if="wipLimitIsSet">
+            <span class="m-1">-</span>
+            <gl-button
+              class="js-remove-limit h-100 border-0 gl-line-height-14 text-secondary"
+              variant="link"
+              @click="clearWipLimit"
+              >{{ $options.removeLimitText }}</gl-button
+            >
+          </template>
         </div>
-        <gl-button class="h-100 border-0 gl-line-height-14" variant="link" @click="showInput">
-          {{ $options.editLinkText }}
-        </gl-button>
       </div>
     </template>
   </gl-drawer>

ee/app/views/shared/boards/components/_list_settings.html.haml

@@ -2,9 +2,9 @@
   %gl-button.no-drag.rounded-right{ type: "button",
     "@click" => "openSidebarSettings",
     "v-if" => "isSettingsShown",
-    "aria-label" => _("List Settings"),
+    "aria-label" => _("List settings"),
     "ref" => "settingsBtn",
-    "title" => _("List Settings") }
+    "title" => _("List settings") }
     = sprite_icon("settings")
   %gl-tooltip{ ":target" => "() => $refs.settingsBtn" }
-    = _("List Settings")
+    = _("List settings")

ee/spec/features/boards/boards_spec.rb

@@ -204,14 +204,14 @@ describe 'issue boards', :js do
       end
 
       it 'shows the list settings button' do
-        expect(page).to have_selector(:button, "List Settings")
+        expect(page).to have_selector(:button, "List settings")
         expect(page).not_to have_selector(".js-board-settings-sidebar")
       end
 
       context 'when settings button is clicked' do
         it 'shows the board list settings sidebar' do
           page.within(find(".board:nth-child(2)")) do
-            click_button('List Settings')
+            click_button('List settings')
           end
 
           expect(page.find('.js-board-settings-sidebar').find('.gl-label-text')).to have_text("Brount")
@@ -221,7 +221,31 @@ describe 'issue boards', :js do
       context 'when boards setting sidebar is open' do
         before do
           page.within(find(".board:nth-child(2)")) do
-            click_button('List Settings')
+            click_button('List settings')
+          end
+        end
+
+        context "when user clicks Remove Limit" do
+          before do
+            max_issue_count = 2
+            page.within(find('.js-board-settings-sidebar')) do
+              click_button("Edit")
+
+              find('input').set(max_issue_count)
+            end
+
+            # Off click
+            find('body').click
+
+            wait_for_requests
+          end
+
+          it "sets max issue count to zero" do
+            page.find('.js-remove-limit').click
+
+            wait_for_requests
+
+            expect(page.find('.js-wip-limit')).to have_text("None")
           end
         end
 
@@ -242,7 +266,7 @@ describe 'issue boards', :js do
             wait_for_requests
 
             page.within(find(".board:nth-child(2)")) do
-              click_button('List Settings')
+              click_button('List settings')
             end
 
             expect(page.find('.js-wip-limit')).to have_text(max_issue_count)
@@ -308,7 +332,7 @@ describe 'issue boards', :js do
       end
 
       it 'does not show the list settings button' do
-        expect(page).to have_no_selector(:button, "List Settings")
+        expect(page).to have_no_selector(:button, "List settings")
         expect(page).not_to have_selector(".js-board-settings-sidebar")
       end
     end

ee/spec/frontend/boards/components/__snapshots__/board_settings_sidebar_spec.js.snap

@@ -2,9 +2,9 @@
 
 exports[`BoardSettingsSideBar when activeList is present when activeListWipLimit is 0 renders "None" in the block 1`] = `"None"`;
 
-exports[`BoardSettingsSideBar when activeList is present when activeListWipLimit is greater than 0 it renders 1 1`] = `"1"`;
+exports[`BoardSettingsSideBar when activeList is present when activeListWipLimit is greater than 0 it renders 1 1`] = `"1 issue"`;
 
-exports[`BoardSettingsSideBar when activeList is present when activeListWipLimit is greater than 0 it renders 11 1`] = `"11"`;
+exports[`BoardSettingsSideBar when activeList is present when activeListWipLimit is greater than 0 it renders 11 1`] = `"11 issues"`;
 
 exports[`BoardSettingsSideBar when activeList is present when activeListWipLimit is greater than 0 when list type is "assignee" renders the correct list type text 1`] = `"Assignee"`;
 

ee/spec/frontend/boards/components/board_settings_sidebar_spec.js

@@ -3,14 +3,7 @@ import MockAdapter from 'axios-mock-adapter';
 import axios from 'axios';
 import Vuex from 'vuex';
 import { shallowMount, createLocalVue } from '@vue/test-utils';
-import {
-  GlDrawer,
-  GlLabel,
-  GlButton,
-  GlFormInput,
-  GlAvatarLink,
-  GlAvatarLabeled,
-} from '@gitlab/ui';
+import { GlDrawer, GlLabel, GlFormInput, GlAvatarLink, GlAvatarLabeled } from '@gitlab/ui';
 import BoardSettingsSidebar from 'ee/boards/components/board_settings_sidebar.vue';
 import boardsStore from 'ee_else_ce/boards/stores/boards_store_ee';
 import getters from 'ee_else_ce/boards/stores/getters';
@@ -32,8 +25,9 @@ describe('BoardSettingsSideBar', () => {
   const labelTitle = 'test';
   const labelColor = '#FFFF';
   const listId = 1;
+  const currentWipLimit = 1; // Needs to be other than null to trigger requests.
 
-  const createComponent = (state = {}, actions = {}, localState = {}) => {
+  const createComponent = (state = { activeListId: 0 }, actions = {}, localState = {}) => {
     storeActions = actions;
 
     const store = new Vuex.Store({
@@ -88,23 +82,19 @@ describe('BoardSettingsSideBar', () => {
     });
 
     describe('on close', () => {
-      it('calls closeSidebar', done => {
+      it('calls closeSidebar', () => {
         const spy = jest.fn();
-        createComponent({}, { setActiveListId: spy });
+        createComponent({ activeListId: 0 }, { setActiveListId: spy });
 
         wrapper.find(GlDrawer).vm.$emit('close');
 
-        return wrapper.vm
-          .$nextTick()
-          .then(() => {
-            expect(storeActions.setActiveListId).toHaveBeenCalledWith(
-              expect.anything(),
-              0,
-              undefined,
-            );
-          })
-          .then(done)
-          .catch(done.fail);
+        return wrapper.vm.$nextTick().then(() => {
+          expect(storeActions.setActiveListId).toHaveBeenCalledWith(
+            expect.anything(),
+            0,
+            undefined,
+          );
+        });
       });
     });
 
@@ -307,45 +297,80 @@ describe('BoardSettingsSideBar', () => {
         list_type: 'label',
       });
 
-      createComponent({ activeListId: listId });
+      createComponent({ activeListId: listId }, { updateListWipLimit: () => {} });
     });
 
-    it('renders an input', done => {
-      wrapper.find(GlButton).vm.$emit('click');
+    it('renders an input', () => {
+      wrapper.find('.js-edit-button').vm.$emit('click');
 
-      return wrapper.vm
-        .$nextTick()
-        .then(() => {
-          expect(wrapper.find(GlFormInput).exists()).toBe(true);
-        })
-        .then(done)
-        .catch(done.fail);
+      return wrapper.vm.$nextTick().then(() => {
+        expect(wrapper.find(GlFormInput).exists()).toBe(true);
+      });
     });
 
-    it('does not render current wipLimit text', done => {
-      wrapper.find(GlButton).vm.$emit('click');
+    it('does not render current wipLimit text', () => {
+      wrapper.find('.js-edit-button').vm.$emit('click');
 
-      return wrapper.vm
-        .$nextTick()
-        .then(() => {
-          expect(wrapper.find('.js-wip-limit').exists()).toBe(false);
-        })
-        .then(done)
-        .catch(done.fail);
+      return wrapper.vm.$nextTick().then(() => {
+        expect(wrapper.find('.js-wip-limit').exists()).toBe(false);
+      });
     });
 
-    it('sets wipLimit to be the value of list.maxIssueCount', done => {
-      expect(wrapper.vm.currentWipLimit).toEqual(0);
+    it('sets wipLimit to be the value of list.maxIssueCount', () => {
+      expect(wrapper.vm.currentWipLimit).toEqual(null);
 
-      wrapper.find(GlButton).vm.$emit('click');
+      wrapper.find('.js-edit-button').vm.$emit('click');
 
-      return wrapper.vm
-        .$nextTick()
-        .then(() => {
-          expect(wrapper.vm.currentWipLimit).toBe(4);
-        })
-        .then(done)
-        .catch(done.fail);
+      return wrapper.vm.$nextTick().then(() => {
+        expect(wrapper.vm.currentWipLimit).toBe(4);
+      });
+    });
+  });
+
+  describe('remove limit', () => {
+    describe('when wipLimit is set', () => {
+      beforeEach(() => {
+        mock = new MockAdapter(axios);
+        boardsStore.store.addList({
+          id: listId,
+          label: { title: labelTitle, color: labelColor },
+          max_issue_count: 4,
+          list_type: 'label',
+        });
+        const spy = jest.fn().mockResolvedValue({
+          config: { data: JSON.stringify({ list: { max_issue_count: 0 } }) },
+        });
+
+        createComponent({ activeListId: listId }, { updateListWipLimit: spy });
+      });
+
+      it('resets wipLimit to 0', () => {
+        expect(wrapper.vm.activeList.maxIssueCount).toEqual(4);
+
+        wrapper.find('.js-remove-limit').vm.$emit('click');
+
+        return wrapper.vm.$nextTick().then(() => {
+          expect(wrapper.vm.activeList.maxIssueCount).toEqual(0);
+        });
+      });
+    });
+
+    describe('when wipLimit is not set', () => {
+      beforeEach(() => {
+        mock = new MockAdapter(axios);
+        boardsStore.store.addList({
+          id: listId,
+          label: { title: labelTitle, color: labelColor },
+          max_issue_count: 0,
+          list_type: 'label',
+        });
+
+        createComponent({ activeListId: listId }, { updateListWipLimit: () => {} });
+      });
+
+      it('does not render the remove limit button', () => {
+        expect(wrapper.find('.js-remove-limit').exists()).toBe(false);
+      });
     });
   });
 
@@ -374,9 +399,13 @@ describe('BoardSettingsSideBar', () => {
       describe(`when blur is triggered by ${blurMethod}`, () => {
         it('calls updateListWipLimit', () => {
           const spy = jest.fn().mockResolvedValue({
-            config: { data: JSON.stringify({ list: { max_issue_count: 'hello' } }) },
+            config: { data: JSON.stringify({ list: { max_issue_count: '4' } }) },
           });
-          createComponent({ activeListId: 1 }, { updateListWipLimit: spy }, { edit: true });
+          createComponent(
+            { activeListId: 1 },
+            { updateListWipLimit: spy },
+            { edit: true, currentWipLimit },
+          );
 
           triggerBlur(blurMethod);
 
@@ -387,11 +416,7 @@ describe('BoardSettingsSideBar', () => {
 
         describe('when component wipLimit and List.maxIssueCount are equal', () => {
           it('doesnt call updateListWipLimit', () => {
-            const spy = jest.fn(() =>
-              Promise.resolve({
-                config: { data: JSON.stringify({ list: { max_issue_count: 0 } }) },
-              }),
-            );
+            const spy = jest.fn().mockResolvedValue({});
             createComponent(
               { activeListId: 1 },
               { updateListWipLimit: spy },
@@ -406,16 +431,33 @@ describe('BoardSettingsSideBar', () => {
           });
         });
 
+        describe('when currentWipLimit is null', () => {
+          it('doesnt call updateListWipLimit', () => {
+            const spy = jest.fn().mockResolvedValue({});
+            createComponent(
+              { activeListId: 1 },
+              { updateListWipLimit: spy },
+              { edit: true, currentWipLimit: null },
+            );
+
+            triggerBlur(blurMethod);
+
+            return wrapper.vm.$nextTick().then(() => {
+              expect(spy).toHaveBeenCalledTimes(0);
+            });
+          });
+        });
+
         describe('when response is successful', () => {
           const maxIssueCount = 11;
 
           beforeEach(() => {
-            const spy = jest.fn(() =>
-              Promise.resolve({
-                config: { data: JSON.stringify({ list: { max_issue_count: maxIssueCount } }) },
-              }),
+            const spy = jest.fn().mockResolvedValue({});
+            createComponent(
+              { activeListId: 1 },
+              { updateListWipLimit: spy },
+              { edit: true, currentWipLimit: maxIssueCount },
             );
-            createComponent({ activeListId: 1 }, { updateListWipLimit: spy }, { edit: true });
 
             triggerBlur(blurMethod);
 
@@ -445,7 +487,7 @@ describe('BoardSettingsSideBar', () => {
             createComponent(
               { activeListId: 1 },
               { updateListWipLimit: spy, setActiveListId: () => {} },
-              { edit: true },
+              { edit: true, currentWipLimit },
             );
 
             triggerBlur(blurMethod);

locale/gitlab.pot

@@ -7164,6 +7164,9 @@ msgstr ""
 msgid "Enter new AWS Secret Access Key"
 msgstr ""
 
+msgid "Enter number of issues"
+msgstr ""
+
 msgid "Enter the issue description"
 msgstr ""
 
@@ -11292,9 +11295,6 @@ msgstr ""
 msgid "List"
 msgstr ""
 
-msgid "List Settings"
-msgstr ""
-
 msgid "List Your Gitea Repositories"
 msgstr ""
 
@@ -11304,6 +11304,9 @@ msgstr ""
 msgid "List of IPs and CIDRs of allowed secondary nodes. Comma-separated, e.g. \"1.1.1.1, 2.2.2.0/24\""
 msgstr ""
 
+msgid "List settings"
+msgstr ""
+
 msgid "List the merge requests that must be merged before this one."
 msgstr ""
 
@@ -15610,6 +15613,9 @@ msgstr ""
 msgid "Remove group"
 msgstr ""
 
+msgid "Remove limit"
+msgstr ""
+
 msgid "Remove milestone"
 msgstr ""
 
@@ -21439,7 +21445,7 @@ msgstr ""
 msgid "Withdraw Access Request"
 msgstr ""
 
-msgid "Work in Progress Limit"
+msgid "Work in progress Limit"
 msgstr ""
 
 msgid "Workflow Help"