Merge branch 'kp-fix-epic-token-render-and-filtering' into 'master'

Fix EpicToken rendering and filtering

See merge request gitlab-org/gitlab!61898

app/assets/javascripts/issues_list/components/issues_list_app.vue

@@ -288,6 +288,7 @@ export default {
           icon: 'epic',
           token: EpicToken,
           unique: true,
+          idProperty: 'id',
           fetchEpics: this.fetchEpics,
         });
       }
@@ -320,13 +321,23 @@ export default {
       );
     },
     urlParams() {
+      const filterParams = {
+        ...this.urlFilterParams,
+      };
+
+      if (filterParams.epic_id) {
+        filterParams.epic_id = encodeURIComponent(filterParams.epic_id);
+      } else if (filterParams['not[epic_id]']) {
+        filterParams['not[epic_id]'] = encodeURIComponent(filterParams['not[epic_id]']);
+      }
+
       return {
         due_date: this.dueDateFilter,
         page: this.page,
         search: this.searchQuery,
         state: this.state,
         ...urlSortParams[this.sortKey],
-        ...this.urlFilterParams,
+        ...filterParams,
       };
     },
   },
@@ -358,7 +369,7 @@ export default {
     fetchEmojis(search) {
       return this.fetchWithCache(this.autocompleteAwardEmojisPath, 'emojis', 'name', search);
     },
-    async fetchEpics(search) {
+    async fetchEpics({ search }) {
       const epics = await this.fetchWithCache(this.groupEpicsPath, 'epics');
       if (!search) {
         return epics.slice(0, MAX_LIST_SIZE);
@@ -387,6 +398,16 @@ export default {
 
       this.isLoading = true;
 
+      const filterParams = {
+        ...this.apiFilterParams,
+      };
+
+      if (filterParams.epic_id) {
+        filterParams.epic_id = filterParams.epic_id.split('::&').pop();
+      } else if (filterParams['not[epic_id]']) {
+        filterParams['not[epic_id]'] = filterParams['not[epic_id]'].split('::&').pop();
+      }
+
       return axios
         .get(this.endpoint, {
           params: {
@@ -397,7 +418,7 @@ export default {
             state: this.state,
             with_labels_details: true,
             ...apiSortParams[this.sortKey],
-            ...this.apiFilterParams,
+            ...filterParams,
           },
         })
         .then(({ data, headers }) => {

app/assets/javascripts/vue_shared/components/filtered_search_bar/tokens/epic_token.vue

@@ -11,6 +11,7 @@ import { __ } from '~/locale';
 import { DEBOUNCE_DELAY, DEFAULT_NONE_ANY } from '../constants';
 
 export default {
+  separator: '::&',
   components: {
     GlDropdownDivider,
     GlFilteredSearchToken,
@@ -34,17 +35,35 @@ export default {
     };
   },
   computed: {
+    idProperty() {
+      return this.config.idProperty || 'iid';
+    },
     currentValue() {
-      return Number(this.value.data);
+      const epicIid = Number(this.value.data);
+      if (epicIid) {
+        return epicIid;
+      }
+      return this.value.data;
     },
     defaultEpics() {
       return this.config.defaultEpics || DEFAULT_NONE_ANY;
     },
-    idProperty() {
-      return this.config.idProperty || 'id';
-    },
     activeEpic() {
+      if (this.currentValue && this.epics.length) {
+        // Check if current value is an epic ID.
+        if (typeof this.currentValue === 'number') {
           return this.epics.find((epic) => epic[this.idProperty] === this.currentValue);
+        }
+
+        // Current value is a string.
+        const [groupPath, idProperty] = this.currentValue?.split('::&');
+        return this.epics.find(
+          (epic) =>
+            epic.group_full_path === groupPath &&
+            epic[this.idProperty] === parseInt(idProperty, 10),
+        );
+      }
+      return null;
     },
   },
   watch: {
@@ -58,10 +77,10 @@ export default {
     },
   },
   methods: {
-    fetchEpicsBySearchTerm(searchTerm = '') {
+    fetchEpicsBySearchTerm({ epicPath = '', search = '' }) {
       this.loading = true;
       this.config
-        .fetchEpics(searchTerm)
+        .fetchEpics({ epicPath, search })
         .then((response) => {
           this.epics = Array.isArray(response) ? response : response.data;
         })
@@ -71,11 +90,21 @@ export default {
         });
     },
     searchEpics: debounce(function debouncedSearch({ data }) {
-      this.fetchEpicsBySearchTerm(data);
+      let epicPath = this.activeEpic?.web_url;
+
+      // When user visits the page with token value already included in filters
+      // We don't have any information about selected token except for its
+      // group path and iid joined by separator, so we need to manually
+      // compose epic path from it.
+      if (data.includes(this.$options.separator)) {
+        const [groupPath, epicIid] = data.split(this.$options.separator);
+        epicPath = `/groups/${groupPath}/-/epics/${epicIid}`;
+      }
+      this.fetchEpicsBySearchTerm({ epicPath, search: data });
     }, DEBOUNCE_DELAY),
 
     getEpicDisplayText(epic) {
-      return `${epic.title}::&${epic[this.idProperty]}`;
+      return `${epic.title}${this.$options.separator}${epic.iid}`;
     },
   },
 };
@@ -104,8 +133,8 @@ export default {
       <template v-else>
         <gl-filtered-search-suggestion
           v-for="epic in epics"
-          :key="epic[idProperty]"
-          :value="String(epic[idProperty])"
+          :key="epic.id"
+          :value="`${epic.group_full_path}::&${epic[idProperty]}`"
         >
           {{ epic.title }}
         </gl-filtered-search-suggestion>

ee/app/assets/javascripts/epics_list/components/epics_list_root.vue

@@ -207,7 +207,7 @@ export default {
     :current-tab="currentState"
     :tab-counts="epicsCount"
     :search-input-placeholder="__('Search or filter results...')"
-    :search-tokens="getFilteredSearchTokens()"
+    :search-tokens="getFilteredSearchTokens({ supportsEpic: false })"
     :sort-options="$options.EpicsSortOptions"
     :initial-filter-value="getFilteredSearchValue()"
     :initial-sort-by="sortedBy"

ee/app/assets/javascripts/roadmap/mixins/filtered_search_mixin.js

@@ -36,13 +36,13 @@ export default {
         milestone_title: milestoneTitle,
         confidential,
         my_reaction_emoji: myReactionEmoji,
-        epic_iid: epicIid && Number(epicIid),
+        epic_iid: epicIid,
         search,
       };
     },
   },
   methods: {
-    getFilteredSearchTokens() {
+    getFilteredSearchTokens({ supportsEpic = true } = {}) {
       const tokens = [
         {
           type: 'author_username',
@@ -113,7 +113,10 @@ export default {
             { icon: 'eye', value: false, title: __('No') },
           ],
         },
-        {
+      ];
+
+      if (supportsEpic) {
+        tokens.push({
           type: 'epic_iid',
           icon: 'epic',
           title: __('Epic'),
@@ -121,16 +124,28 @@ export default {
           symbol: '&',
           token: EpicToken,
           operators: OPERATOR_IS_ONLY,
-          idProperty: 'iid',
           defaultEpics: [],
-          fetchEpics: (search = '') => {
-            const number = Number(search);
-            return !search || Number.isNaN(number)
-              ? axios.get(this.listEpicsPath, { params: { search } })
-              : axios.get(joinPaths(this.listEpicsPath, search)).then(({ data }) => [data]);
-          },
+          fetchEpics: ({ epicPath = '', search = '' }) => {
+            const epicId = Number(search) || null;
+
+            // No search criteria or path has been provided, fetch all epics.
+            if (!epicPath && !search) {
+              return axios.get(this.listEpicsPath);
+            } else if (epicPath) {
+              // Just epicPath has been provided, fetch a specific epic.
+              return axios.get(epicPath).then(({ data }) => [data]);
+            } else if (!epicPath && epicId) {
+              // Exact epic ID provided, fetch the epic.
+              return axios
+                .get(joinPaths(this.listEpicsPath, String(epicId)))
+                .then(({ data }) => [data]);
+            }
+
+            // Search for an epic.
+            return axios.get(this.listEpicsPath, { params: { search } });
           },
-      ];
+        });
+      }
 
       if (gon.current_user_id) {
         // Appending to tokens only when logged-in

ee/app/assets/javascripts/roadmap/roadmap_bundle.js

@@ -79,7 +79,7 @@ export default () => {
         }),
 
         ...(rawFilterParams.epicIid && {
-          epicIid: parseInt(rawFilterParams.epicIid, 10),
+          epicIid: rawFilterParams.epicIid,
         }),
       };
       const timeframe = getTimeframeForPreset(

ee/app/assets/javascripts/roadmap/store/actions.js

@@ -47,7 +47,7 @@ const fetchGroupEpics = (
     };
 
     if (filterParams?.epicIid) {
-      variables.iid = filterParams.epicIid;
+      variables.iid = filterParams.epicIid.split('::&').pop();
     }
   }
 

ee/app/serializers/epic_entity.rb

@@ -9,6 +9,9 @@ class EpicEntity < IssuableEntity
   expose :group_full_name do |epic|
     epic.group.full_name
   end
+  expose :group_full_path do |epic|
+    epic.group.full_path
+  end
 
   expose :start_date
   expose :start_date_is_fixed?, as: :start_date_is_fixed

ee/spec/fixtures/api/schemas/entities/epic.json

@@ -22,6 +22,7 @@
     "labels": { "type": ["array", "null"] },
     "group_name": { "type": "string" },
     "group_full_name": { "type": "string" },
+    "group_full_path": { "type": "string" },
     "current_user": {
       "can_create_note": { "type": "boolean" }
     },
@@ -49,6 +50,7 @@
     "labels",
     "group_name",
     "group_full_name",
+    "group_full_path",
     "current_user",
     "create_note_path",
     "preview_note_path"

ee/spec/frontend/epics_list/components/epics_list_root_spec.js

@@ -170,6 +170,7 @@ describe('EpicsListRoot', () => {
     const getIssuableList = () => wrapper.find(IssuableList);
 
     it('renders issuable-list component', async () => {
+      jest.spyOn(wrapper.vm, 'getFilteredSearchTokens');
       wrapper.setData({
         filterParams: {
           search: 'foo',
@@ -192,6 +193,10 @@ describe('EpicsListRoot', () => {
         issuableSymbol: '&',
         recentSearchesStorageKey: 'epics',
       });
+
+      expect(wrapper.vm.getFilteredSearchTokens).toHaveBeenCalledWith({
+        supportsEpic: false,
+      });
     });
 
     it.each`

ee/spec/frontend/roadmap/components/roadmap_filters_spec.js

@@ -218,7 +218,6 @@ describe('RoadmapFilters', () => {
           symbol: '&',
           token: EpicToken,
           operators,
-          idProperty: 'iid',
           defaultEpics: [],
           fetchEpics: expect.any(Function),
         },

ee/spec/serializers/epic_entity_spec.rb

@@ -16,6 +16,6 @@ RSpec.describe EpicEntity do
   end
 
   it 'has epic specific attributes' do
-    expect(subject).to include(:start_date, :end_date, :group_id, :group_name, :group_full_name, :web_url)
+    expect(subject).to include(:start_date, :end_date, :group_id, :group_name, :group_full_name, :group_full_path, :web_url)
   end
 end

spec/frontend/issues_list/mock_data.js

@@ -21,8 +21,8 @@ export const locationSearch = [
   'confidential=no',
   'iteration_title=season:+%234',
   'not[iteration_title]=season:+%2320',
-  'epic_id=12',
-  'not[epic_id]=34',
+  'epic_id=gitlab-org%3A%3A%2612',
+  'not[epic_id]=gitlab-org%3A%3A%2634',
   'weight=1',
   'not[weight]=3',
 ].join('&');
@@ -53,8 +53,8 @@ export const filteredTokens = [
   { type: 'confidential', value: { data: 'no', operator: OPERATOR_IS } },
   { type: 'iteration', value: { data: 'season: #4', operator: OPERATOR_IS } },
   { type: 'iteration', value: { data: 'season: #20', operator: OPERATOR_IS_NOT } },
-  { type: 'epic_id', value: { data: '12', operator: OPERATOR_IS } },
-  { type: 'epic_id', value: { data: '34', operator: OPERATOR_IS_NOT } },
+  { type: 'epic_id', value: { data: 'gitlab-org::&12', operator: OPERATOR_IS } },
+  { type: 'epic_id', value: { data: 'gitlab-org::&34', operator: OPERATOR_IS_NOT } },
   { type: 'weight', value: { data: '1', operator: OPERATOR_IS } },
   { type: 'weight', value: { data: '3', operator: OPERATOR_IS_NOT } },
   { type: 'filtered-search-term', value: { data: 'find' } },
@@ -84,7 +84,7 @@ export const apiParams = {
   iteration_title: 'season: #4',
   'not[iteration_title]': 'season: #20',
   epic_id: '12',
-  'not[epic_id]': '34',
+  'not[epic_id]': 'gitlab-org::&34',
   weight: '1',
   'not[weight]': '3',
 };
@@ -111,8 +111,8 @@ export const urlParams = {
   confidential: 'no',
   iteration_title: 'season: #4',
   'not[iteration_title]': 'season: #20',
-  epic_id: '12',
-  'not[epic_id]': '34',
+  epic_id: 'gitlab-org%3A%3A%2612',
+  'not[epic_id]': 'gitlab-org::&34',
   weight: '1',
   'not[weight]': '3',
 };

spec/frontend/issues_list/utils_spec.js

@@ -82,7 +82,10 @@ describe('getFilterTokens', () => {
 
 describe('convertToParams', () => {
   it('returns api params given filtered tokens', () => {
-    expect(convertToParams(filteredTokens, API_PARAM)).toEqual(apiParams);
+    expect(convertToParams(filteredTokens, API_PARAM)).toEqual({
+      ...apiParams,
+      epic_id: 'gitlab-org::&12',
+    });
   });
 
   it('returns api params given filtered tokens with special values', () => {
@@ -92,7 +95,10 @@ describe('convertToParams', () => {
   });
 
   it('returns url params given filtered tokens', () => {
-    expect(convertToParams(filteredTokens, URL_PARAM)).toEqual(urlParams);
+    expect(convertToParams(filteredTokens, URL_PARAM)).toEqual({
+      ...urlParams,
+      epic_id: 'gitlab-org::&12',
+    });
   });
 
   it('returns url params given filtered tokens with special values', () => {

spec/frontend/vue_shared/components/filtered_search_bar/mock_data.js

@@ -65,8 +65,8 @@ export const mockMilestones = [
 ];
 
 export const mockEpics = [
-  { iid: 1, id: 1, title: 'Foo' },
-  { iid: 2, id: 2, title: 'Bar' },
+  { iid: 1, id: 1, title: 'Foo', group_full_path: 'gitlab-org' },
+  { iid: 2, id: 2, title: 'Bar', group_full_path: 'gitlab-org/design' },
 ];
 
 export const mockEmoji1 = {

spec/frontend/vue_shared/components/filtered_search_bar/tokens/epic_token_spec.js

@@ -67,18 +67,6 @@ describe('EpicToken', () => {
 
       await wrapper.vm.$nextTick();
     });
-
-    describe('activeEpic', () => {
-      it('returns object for currently present `value.data`', async () => {
-        wrapper.setProps({
-          value: { data: `${mockEpics[0].iid}` },
-        });
-
-        await wrapper.vm.$nextTick();
-
-        expect(wrapper.vm.activeEpic).toEqual(mockEpics[0]);
-      });
-    });
   });
 
   describe('methods', () => {
@@ -86,9 +74,12 @@ describe('EpicToken', () => {
       it('calls `config.fetchEpics` with provided searchTerm param', () => {
         jest.spyOn(wrapper.vm.config, 'fetchEpics');
 
-        wrapper.vm.fetchEpicsBySearchTerm('foo');
+        wrapper.vm.fetchEpicsBySearchTerm({ search: 'foo' });
 
-        expect(wrapper.vm.config.fetchEpics).toHaveBeenCalledWith('foo');
+        expect(wrapper.vm.config.fetchEpics).toHaveBeenCalledWith({
+          epicPath: '',
+          search: 'foo',
+        });
       });
 
       it('sets response to `epics` when request is successful', async () => {
@@ -96,7 +87,7 @@ describe('EpicToken', () => {
           data: mockEpics,
         });
 
-        wrapper.vm.fetchEpicsBySearchTerm();
+        wrapper.vm.fetchEpicsBySearchTerm({});
 
         await waitForPromises();
 
@@ -106,7 +97,7 @@ describe('EpicToken', () => {
       it('calls `createFlash` with flash error message when request fails', async () => {
         jest.spyOn(wrapper.vm.config, 'fetchEpics').mockRejectedValue({});
 
-        wrapper.vm.fetchEpicsBySearchTerm('foo');
+        wrapper.vm.fetchEpicsBySearchTerm({ search: 'foo' });
 
         await waitForPromises();
 
@@ -118,7 +109,7 @@ describe('EpicToken', () => {
       it('sets `loading` to false when request completes', async () => {
         jest.spyOn(wrapper.vm.config, 'fetchEpics').mockRejectedValue({});
 
-        wrapper.vm.fetchEpicsBySearchTerm('foo');
+        wrapper.vm.fetchEpicsBySearchTerm({ search: 'foo' });
 
         await waitForPromises();
 
@@ -128,9 +119,11 @@ describe('EpicToken', () => {
   });
 
   describe('template', () => {
+    const getTokenValueEl = () => wrapper.findAllComponents(GlFilteredSearchTokenSegment).at(2);
+
     beforeEach(async () => {
       wrapper = createComponent({
-        value: { data: `${mockEpics[0].iid}` },
+        value: { data: `${mockEpics[0].group_full_path}::&${mockEpics[0].iid}` },
         data: { epics: mockEpics },
       });
 
@@ -147,5 +140,19 @@ describe('EpicToken', () => {
       expect(tokenSegments).toHaveLength(3);
       expect(tokenSegments.at(2).text()).toBe(`${mockEpics[0].title}::&${mockEpics[0].iid}`);
     });
+
+    it.each`
+      value                                                      | valueType   | tokenValueString
+      ${`${mockEpics[0].group_full_path}::&${mockEpics[0].iid}`} | ${'string'} | ${`${mockEpics[0].title}::&${mockEpics[0].iid}`}
+      ${`${mockEpics[1].group_full_path}::&${mockEpics[1].iid}`} | ${'number'} | ${`${mockEpics[1].title}::&${mockEpics[1].iid}`}
+    `('renders token item when selection is a $valueType', async ({ value, tokenValueString }) => {
+      wrapper.setProps({
+        value: { data: value },
+      });
+
+      await wrapper.vm.$nextTick();
+
+      expect(getTokenValueEl().text()).toBe(tokenValueString);
+    });
   });
 });