Merge branch '280781-fix-iteration-wildcard-filter-board' into 'master'

Fix negated iteration wildcard filtering for boards [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!62801

app/assets/javascripts/boards/boards_util.js

 import { sortBy, cloneDeep } from 'lodash';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
-import { ListType, NOT_FILTER, AssigneeIdParamValues } from './constants';
+import { ListType } from './constants';
 
 export function getMilestone() {
   return null;
@@ -175,45 +175,106 @@ export function isListDraggable(list) {
   return list.listType !== ListType.backlog && list.listType !== ListType.closed;
 }
 
-export function transformNotFilters(filters) {
-  return Object.keys(filters)
-    .filter((key) => key.startsWith(NOT_FILTER))
-    .reduce((obj, key) => {
-      return {
-        ...obj,
-        [key.substring(4, key.length - 1)]: filters[key],
-      };
-    }, {});
-}
+export const FiltersInfo = {
+  assigneeUsername: {
+    negatedSupport: true,
+  },
+  assigneeId: {
+    // assigneeId should be renamed to assigneeWildcardId.
+    // Classic boards used 'assigneeId'
+    remap: () => 'assigneeWildcardId',
+  },
+  assigneeWildcardId: {
+    negatedSupport: false,
+    transform: (val) => val.toUpperCase(),
+  },
+  authorUsername: {
+    negatedSupport: true,
+  },
+  labelName: {
+    negatedSupport: true,
+  },
+  milestoneTitle: {
+    negatedSupport: true,
+  },
+  myReactionEmoji: {
+    negatedSupport: true,
+  },
+  releaseTag: {
+    negatedSupport: true,
+  },
+  search: {
+    negatedSupport: false,
+  },
+};
 
-export function getSupportedParams(filters, supportedFilters) {
-  return supportedFilters.reduce((acc, f) => {
-    /**
-     * TODO the API endpoint for the classic boards
-     * accepts assignee wildcard value as 'assigneeId' param -
-     * while the GraphQL query accepts the value in 'assigneWildcardId' field.
-     * Once we deprecate the classics boards,
-     * we should change the filtered search bar to use 'asssigneeWildcardId' as a token name.
+/**
+ * @param {Object} filters - ex. { search: "foobar", "not[authorUsername]": "root", }
+ * @returns {Object} - ex. [ ["search", "foobar", false], ["authorUsername", "root", true], ]
  */
-    if (f === 'assigneeId' && filters[f]) {
-      return AssigneeIdParamValues.includes(filters[f])
+const parseFilters = (filters) => {
+  /* eslint-disable-next-line @gitlab/require-i18n-strings */
+  const isNegated = (x) => x.startsWith('not[') && x.endsWith(']');
+
+  return Object.entries(filters).map(([k, v]) => {
+    const isNot = isNegated(k);
+    const filterKey = isNot ? k.slice(4, -1) : k;
+
+    return [filterKey, v, isNot];
+  });
+};
+
+/**
+ * Returns an object of filter key/value pairs used as variables in GraphQL requests.
+ * (warning: filter values are not validated)
+ *
+ * @param {Object} objParam.filters - filters extracted from url params. ex. { search: "foobar", "not[authorUsername]": "root", }
+ * @param {string} objParam.issuableType - issuable type e.g., issue.
+ * @param {Object} objParam.filterInfo - data on filters such as how to transform filter value, if filter can be negated, etc.
+ * @param {Object} objParam.filterFields - data on what filters are available for given issuableType (based on GraphQL schema)
+ */
+export const filterVariables = ({ filters, issuableType, filterInfo, filterFields }) =>
+  parseFilters(filters)
+    .map(([k, v, negated]) => {
+      // for legacy reasons, some filters need to be renamed to correct GraphQL fields.
+      const remapAvailable = filterInfo[k]?.remap;
+      const remappedKey = remapAvailable ? filterInfo[k].remap(k, v) : k;
+
+      return [remappedKey, v, negated];
+    })
+    .filter(([k, , negated]) => {
+      // remove unsupported filters (+ check if the filters support negation)
+      const supported = filterFields[issuableType].includes(k);
+      if (supported) {
+        return negated ? filterInfo[k].negatedSupport : true;
+      }
+
+      return false;
+    })
+    .map(([k, v, negated]) => {
+      // if the filter value needs a special transformation, apply it (e.g., capitalization)
+      const transform = filterInfo[k]?.transform;
+      const newVal = transform ? transform(v) : v;
+
+      return [k, newVal, negated];
+    })
+    .reduce(
+      (acc, [k, v, negated]) => {
+        return negated
           ? {
               ...acc,
-            assigneeWildcardId: filters[f].toUpperCase(),
-          }
-        : acc;
+              not: {
+                ...acc.not,
+                [k]: v,
+              },
             }
-
-    if (filters[f]) {
-      return {
+          : {
               ...acc,
-        [f]: filters[f],
+              [k]: v,
             };
-    }
-
-    return acc;
-  }, {});
-}
+      },
+      { not: {} },
+    );
 
 // EE-specific feature. Find the implementation in the `ee/`-folder
 export function transformBoardConfig() {
@@ -228,5 +289,4 @@ export default {
   fullLabelId,
   fullIterationId,
   isListDraggable,
-  transformNotFilters,
 };

app/assets/javascripts/boards/constants.js

@@ -9,17 +9,6 @@ import updateBoardListMutation from './graphql/board_list_update.mutation.graphq
 import issueSetSubscriptionMutation from './graphql/issue_set_subscription.mutation.graphql';
 import issueSetTitleMutation from './graphql/issue_set_title.mutation.graphql';
 
-export const SupportedFilters = [
-  'assigneeUsername',
-  'authorUsername',
-  'labelName',
-  'milestoneTitle',
-  'releaseTag',
-  'search',
-  'myReactionEmoji',
-  'assigneeId',
-];
-
 /* eslint-disable-next-line @gitlab/require-i18n-strings */
 export const AssigneeIdParamValues = ['Any', 'None'];
 
@@ -60,8 +49,6 @@ export const inactiveId = 0;
 export const ISSUABLE = 'issuable';
 export const LIST = 'list';
 
-export const NOT_FILTER = 'not[';
-
 export const flashAnimationDuration = 2000;
 
 export const listsQuery = {
@@ -106,6 +93,19 @@ export const subscriptionQueries = {
   },
 };
 
+export const FilterFields = {
+  [issuableTypes.issue]: [
+    'assigneeUsername',
+    'assigneeWildcardId',
+    'authorUsername',
+    'labelName',
+    'milestoneTitle',
+    'myReactionEmoji',
+    'releaseTag',
+    'search',
+  ],
+};
+
 export default {
   BoardType,
   ListType,

app/assets/javascripts/boards/stores/actions.js

@@ -7,11 +7,11 @@ import {
   ISSUABLE,
   titleQueries,
   subscriptionQueries,
-  SupportedFilters,
   deleteListQueries,
   listsQuery,
   updateListQueries,
   issuableTypes,
+  FilterFields,
 } from 'ee_else_ce/boards/constants';
 import createBoardListMutation from 'ee_else_ce/boards/graphql/board_list_create.mutation.graphql';
 import issueMoveListMutation from 'ee_else_ce/boards/graphql/issue_move_list.mutation.graphql';
@@ -26,10 +26,10 @@ import {
   formatIssue,
   formatIssueInput,
   updateListPosition,
-  transformNotFilters,
   moveItemListHelper,
   getMoveData,
-  getSupportedParams,
+  FiltersInfo,
+  filterVariables,
 } from '../boards_util';
 import boardLabelsQuery from '../graphql/board_labels.query.graphql';
 import groupProjectsQuery from '../graphql/group_projects.query.graphql';
@@ -60,13 +60,16 @@ export default {
     dispatch('setActiveId', { id: inactiveId, sidebarType: '' });
   },
 
-  setFilters: ({ commit }, filters) => {
-    const filterParams = {
-      ...getSupportedParams(filters, SupportedFilters),
-      not: transformNotFilters(filters),
-    };
-
-    commit(types.SET_FILTERS, filterParams);
+  setFilters: ({ commit, state: { issuableType } }, filters) => {
+    commit(
+      types.SET_FILTERS,
+      filterVariables({
+        filters,
+        issuableType,
+        filterInfo: FiltersInfo,
+        filterFields: FilterFields,
+      }),
+    );
   },
 
   performSearch({ dispatch }) {

ee/app/assets/javascripts/boards/boards_util.js

 import { sortBy } from 'lodash';
+import { FiltersInfo as FiltersInfoCE } from '~/boards/boards_util';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import { urlParamsToObject } from '~/lib/utils/common_utils';
 import { objectToQuery } from '~/lib/utils/url_utility';
@@ -10,6 +11,7 @@ import {
   MilestoneIDs,
   WeightFilterType,
   WeightIDs,
+  EpicFilterType,
 } from './constants';
 
 export function getMilestone({ milestone }) {
@@ -134,6 +136,40 @@ export function transformBoardConfig(boardConfig) {
   return updatedFilterPath;
 }
 
+export const FiltersInfo = {
+  ...FiltersInfoCE,
+  epicId: {
+    negatedSupport: true,
+    transform: (epicId) => fullEpicId(epicId),
+    // epic_id should be renamed to epicWildcardId when ANY or NONE is the value
+    remap: (k, v) => (v === EpicFilterType.any || v === EpicFilterType.none ? 'epicWildcardId' : k),
+  },
+  epicWildcardId: {
+    negatedSupport: false,
+    transform: (val) => val.toUpperCase(),
+  },
+  iterationId: {
+    negatedSupport: true,
+    remap: (k, v) =>
+      // iteration_id should be renamed to iterationWildcardId when CURRENT is the value
+      v === IterationFilterType.any ||
+      v === IterationFilterType.none ||
+      v === IterationFilterType.current
+        ? 'iterationWildcardId'
+        : k,
+  },
+  iterationTitle: {
+    negatedSupport: true,
+  },
+  iterationWildcardId: {
+    negatedSupport: true,
+    transform: (val) => val.toUpperCase(),
+  },
+  weight: {
+    negatedSupport: true,
+  },
+};
+
 export default {
   getMilestone,
   fullEpicId,

ee/app/assets/javascripts/boards/constants.js

 /* eslint-disable import/export */
-import { issuableTypes } from '~/boards/constants';
+import { issuableTypes, FilterFields as FilterFieldsCE } from '~/boards/constants';
 import destroyBoardListMutation from '~/boards/graphql/board_list_destroy.mutation.graphql';
 import updateBoardListMutation from '~/boards/graphql/board_list_update.mutation.graphql';
 
@@ -22,7 +22,18 @@ export const EpicFilterType = {
   none: 'None',
 };
 
-export const SupportedFiltersEE = ['epicId', 'iterationTitle', 'weight'];
+export const FilterFields = {
+  [issuableTypes.issue]: [
+    ...FilterFieldsCE[issuableTypes.issue],
+    'epicId',
+    'epicWildcardId',
+    'weight',
+    'iterationId',
+    'iterationTitle',
+    'iterationWildcardId',
+  ],
+  [issuableTypes.epic]: ['authorUsername', 'labelName', 'search', 'myReactionEmoji'],
+};
 
 export const IterationFilterType = {
   any: 'Any',

ee/app/assets/javascripts/boards/stores/actions.js

@@ -2,11 +2,10 @@ import {
   formatListIssues,
   formatListsPageInfo,
   fullBoardId,
-  transformNotFilters,
   getMoveData,
-  getSupportedParams,
+  filterVariables,
 } from '~/boards/boards_util';
-import { BoardType, SupportedFilters } from '~/boards/constants';
+import { BoardType } from '~/boards/constants';
 import eventHub from '~/boards/eventhub';
 import listsIssuesQuery from '~/boards/graphql/lists_issues.query.graphql';
 import actionsCE, { gqlClient } from '~/boards/stores/actions';
@@ -24,14 +23,10 @@ import {
   fullEpicBoardId,
   formatListEpics,
   formatEpicListsPageInfo,
+  FiltersInfo,
 } from '../boards_util';
 
-import {
-  EpicFilterType,
-  IterationFilterType,
-  GroupByParamType,
-  SupportedFiltersEE,
-} from '../constants';
+import { EpicFilterType, GroupByParamType, FilterFields } from '../constants';
 import epicQuery from '../graphql/epic.query.graphql';
 import createEpicBoardListMutation from '../graphql/epic_board_list_create.mutation.graphql';
 import epicMoveListMutation from '../graphql/epic_move_list.mutation.graphql';
@@ -107,35 +102,20 @@ export { gqlClient };
 export default {
   ...actionsCE,
 
-  setFilters: ({ commit, dispatch, getters }, filters) => {
-    const supportedFilters = [...SupportedFilters, ...SupportedFiltersEE];
-    const filterParams = getSupportedParams(filters, supportedFilters);
-
-    filterParams.not = transformNotFilters(filters);
-
+  setFilters: ({ commit, dispatch, state: { issuableType } }, filters) => {
     if (filters.groupBy === GroupByParamType.epic) {
       dispatch('setEpicSwimlanes');
     }
 
-    if (filterParams.epicId === EpicFilterType.any || filterParams.epicId === EpicFilterType.none) {
-      filterParams.epicWildcardId = filterParams.epicId.toUpperCase();
-      filterParams.epicId = undefined;
-    } else if (filterParams.epicId) {
-      filterParams.epicId = fullEpicId(filterParams.epicId);
-    }
-    if (!getters.isEpicBoard && filterParams.not.epicId) {
-      filterParams.not.epicId = fullEpicId(filterParams.not.epicId);
-    }
-
-    if (
-      filters.iterationId === IterationFilterType.any ||
-      filters.iterationId === IterationFilterType.none ||
-      filters.iterationId === IterationFilterType.current
-    ) {
-      filterParams.iterationWildcardId = filters.iterationId.toUpperCase();
-    }
-
-    commit(types.SET_FILTERS, filterParams);
+    commit(
+      types.SET_FILTERS,
+      filterVariables({
+        filters,
+        issuableType,
+        filterInfo: FiltersInfo,
+        filterFields: FilterFields,
+      }),
+    );
   },
 
   performSearch({ dispatch, getters }) {

ee/spec/features/issues/filtered_search/filter_issues_by_iteration_spec.rb

@@ -79,9 +79,6 @@ RSpec.describe 'Filter issues by iteration', :js do
 
       context 'when filtering by negated iteration' do
         before do
-          # iterationWildCardId is not yet supported by graphQL https://gitlab.com/gitlab-org/gitlab/-/issues/300115
-          stub_feature_flags(graphql_board_lists: false)
-
           visit page_path
 
           page.within('.filtered-search-wrapper') do
@@ -171,8 +168,6 @@ RSpec.describe 'Filter issues by iteration', :js do
     end
 
     before do
-      # iterationWildCardId is not yet supported by graphQL https://gitlab.com/gitlab-org/gitlab/-/issues/300115
-      stub_feature_flags(graphql_board_lists: false)
       sign_in user
     end
 

ee/spec/frontend/boards/stores/actions_spec.js

@@ -41,62 +41,60 @@ afterEach(() => {
 });
 
 describe('setFilters', () => {
-  it('should commit mutation SET_FILTERS, updates epicId with global id', () => {
-    const state = {
-      filters: {},
-    };
-
-    const filters = { labelName: 'label', epicId: 1 };
-    const updatedFilters = { labelName: 'label', epicId: 'gid://gitlab/Epic/1', not: {} };
+  let state;
 
-    return testAction(
-      actions.setFilters,
-      filters,
-      state,
-      [{ type: types.SET_FILTERS, payload: updatedFilters }],
-      [],
-    );
-  });
-
-  it('should commit mutation SET_FILTERS, updates epicWildcardId', () => {
-    const state = {
+  beforeEach(() => {
+    state = {
       filters: {},
+      issuableType: issuableTypes.issue,
     };
-
-    const filters = { labelName: 'label', epicId: 'None' };
-    const updatedFilters = { labelName: 'label', epicWildcardId: 'NONE', not: {} };
-
-    return testAction(
-      actions.setFilters,
-      filters,
-      state,
-      [{ type: types.SET_FILTERS, payload: updatedFilters }],
-      [],
-    );
   });
 
-  it('should commit mutation SET_FILTERS, updates iterationWildcardId', () => {
-    const state = {
-      filters: {},
-    };
-
-    const filters = { labelName: 'label', iterationId: 'None' };
-    const updatedFilters = { labelName: 'label', iterationWildcardId: 'NONE', not: {} };
-
-    return testAction(
+  it.each([
+    [
+      'with correct EE filters as payload',
+      {
+        filters: { weight: 3, 'not[iterationId]': 1 },
+        filterVariables: {
+          weight: 3,
+          not: {
+            iterationId: 1,
+          },
+        },
+      },
+    ],
+    [
+      'and update epicId with global id',
+      {
+        filters: { epicId: 1 },
+        filterVariables: { epicId: 'gid://gitlab/Epic/1', not: {} },
+      },
+    ],
+    [
+      "and use 'epicWildcardId' as filter variable when epic wildcard is used",
+      {
+        filters: { epicId: 'None' },
+        filterVariables: { epicWildcardId: 'NONE', not: {} },
+      },
+    ],
+    [
+      "and use 'iterationWildcardId' as filter variable when iteration wildcard is used",
+      {
+        filters: { iterationId: 'None' },
+        filterVariables: { iterationWildcardId: 'NONE', not: {} },
+      },
+    ],
+  ])('should commit mutation SET_FILTERS %s', (_, { filters, filterVariables }) => {
+    testAction(
       actions.setFilters,
       filters,
       state,
-      [{ type: types.SET_FILTERS, payload: updatedFilters }],
+      [{ type: types.SET_FILTERS, payload: filterVariables }],
       [],
     );
   });
 
   it('should commit mutation SET_FILTERS, dispatches setEpicSwimlanes action if filters contain groupBy epic', () => {
-    const state = {
-      filters: {},
-    };
-
     const filters = { labelName: 'label', epicId: 1, groupBy: 'epic' };
     const updatedFilters = { labelName: 'label', epicId: 'gid://gitlab/Epic/1', not: {} };
 

spec/frontend/boards/boards_util_spec.js

-import { transformNotFilters } from '~/boards/boards_util';
+import { filterVariables } from '~/boards/boards_util';
 
-describe('transformNotFilters', () => {
-  const filters = {
-    'not[labelName]': ['label'],
-    'not[assigneeUsername]': 'assignee',
-  };
-
-  it('formats not filters, transforms epicId to fullEpicId', () => {
-    const result = transformNotFilters(filters);
-
-    expect(result).toEqual({
-      labelName: ['label'],
-      assigneeUsername: 'assignee',
+describe('filterVariables', () => {
+  it.each([
+    [
+      'correctly processes array filter values',
+      {
+        filters: {
+          'not[filterA]': ['val1', 'val2'],
+        },
+        expected: {
+          not: {
+            filterA: ['val1', 'val2'],
+          },
+        },
+      },
+    ],
+    [
+      "renames a filter if 'remap' method is available",
+      {
+        filters: {
+          filterD: 'some value',
+        },
+        expected: {
+          filterA: 'some value',
+          not: {},
+        },
+      },
+    ],
+    [
+      'correctly processes a negated filter that supports negation',
+      {
+        filters: {
+          'not[filterA]': 'some value 1',
+          'not[filterB]': 'some value 2',
+        },
+        expected: {
+          not: {
+            filterA: 'some value 1',
+          },
+        },
+      },
+    ],
+    [
+      'correctly removes an unsupported filter depending on issuableType',
+      {
+        issuableType: 'epic',
+        filters: {
+          filterA: 'some value 1',
+          filterE: 'some value 2',
+        },
+        expected: {
+          filterE: 'some value 2',
+          not: {},
+        },
+      },
+    ],
+    [
+      'applies a transform when the filter value needs to be modified',
+      {
+        filters: {
+          filterC: 'abc',
+          'not[filterC]': 'def',
+        },
+        expected: {
+          filterC: 'ABC',
+          not: {
+            filterC: 'DEF',
+          },
+        },
+      },
+    ],
+  ])('%s', (_, { filters, issuableType = 'issue', expected }) => {
+    const result = filterVariables({
+      filters,
+      issuableType,
+      filterInfo: {
+        filterA: {
+          negatedSupport: true,
+        },
+        filterB: {
+          negatedSupport: false,
+        },
+        filterC: {
+          negatedSupport: true,
+          transform: (val) => val.toUpperCase(),
+        },
+        filterD: {
+          remap: () => 'filterA',
+        },
+        filterE: {
+          negatedSupport: true,
+        },
+      },
+      filterFields: {
+        issue: ['filterA', 'filterB', 'filterC', 'filterD'],
+        epic: ['filterE'],
+      },
     });
+
+    expect(result).toEqual(expected);
   });
 });

spec/frontend/boards/components/board_filtered_search_spec.js

@@ -105,9 +105,9 @@ describe('BoardFilteredSearch', () => {
     beforeEach(() => {
       store = createStore();
 
-      jest.spyOn(store, 'dispatch');
-
       createComponent();
+
+      jest.spyOn(wrapper.vm, 'performSearch').mockImplementation();
     });
 
     it('sets the url params to the correct results', async () => {

spec/frontend/boards/stores/actions_spec.js

@@ -70,27 +70,28 @@ describe('setFilters', () => {
     [
       'with correct filters as payload',
       {
-        filters: { labelName: 'label' },
-        updatedFilters: { labelName: 'label', not: {} },
+        filters: { labelName: 'label', foobar: 'not-a-filter', search: 'quick brown fox' },
+        filterVariables: { labelName: 'label', search: 'quick brown fox', not: {} },
       },
     ],
     [
-      'and updates assigneeWildcardId',
+      "and use 'assigneeWildcardId' as filter variable for 'assigneId' param",
       {
         filters: { assigneeId: 'None' },
-        updatedFilters: { assigneeWildcardId: 'NONE', not: {} },
+        filterVariables: { assigneeWildcardId: 'NONE', not: {} },
       },
     ],
-  ])('should commit mutation SET_FILTERS %s', (_, { filters, updatedFilters }) => {
+  ])('should commit mutation SET_FILTERS %s', (_, { filters, filterVariables }) => {
     const state = {
       filters: {},
+      issuableType: issuableTypes.issue,
     };
 
     testAction(
       actions.setFilters,
       filters,
       state,
-      [{ type: types.SET_FILTERS, payload: updatedFilters }],
+      [{ type: types.SET_FILTERS, payload: filterVariables }],
       [],
     );
   });