Merge pull request #6834 from jlmitch5/searchFixes2

fix the scm_type search on projects lists and make search keys show labels instead of api values

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx/ui_next/src/components/Lookup/ProjectLookup.jsx

@@ -78,7 +78,7 @@ function ProjectLookup({
               },
               {
                 name: i18n._(t`Type`),
-                key: 'type',
+                key: 'scm_type',
                 options: [
                   [``, i18n._(t`Manual`)],
                   [`git`, i18n._(t`Git`)],

awx/ui_next/src/components/Search/Search.jsx

@@ -150,6 +150,16 @@ class Search extends React.Component {
       return paramsArr.filter(key => defaultParamsKeys.indexOf(key) === -1);
     };
 
+    const getLabelFromValue = (value, colKey) => {
+      const currentSearchColumn = columns.find(({ key }) => key === colKey);
+      if (currentSearchColumn?.options?.length) {
+        return currentSearchColumn.options.find(
+          ([optVal]) => optVal === value
+        )[1];
+      }
+      return value.toString();
+    };
+
     const getChipsByKey = () => {
       const queryParams = parseQueryString(qsConfig, location.search);
 
@@ -175,10 +185,16 @@ class Search extends React.Component {
 
         if (Array.isArray(queryParams[key])) {
           queryParams[key].forEach(val =>
-            queryParamsByKey[columnKey].chips.push(val.toString())
+            queryParamsByKey[columnKey].chips.push({
+              key: `${key}:${val}`,
+              node: getLabelFromValue(val, columnKey),
+            })
           );
         } else {
-          queryParamsByKey[columnKey].chips.push(queryParams[key].toString());
+          queryParamsByKey[columnKey].chips.push({
+            key: `${key}:${queryParams[key]}`,
+            node: getLabelFromValue(queryParams[key], columnKey),
+          });
         }
       });
 
@@ -215,8 +231,9 @@ class Search extends React.Component {
           ({ key, name, options, isBoolean, booleanLabels = {} }) => (
             <DataToolbarFilter
               chips={chipsByKey[key] ? chipsByKey[key].chips : []}
-              deleteChip={(unusedKey, val) => {
+              deleteChip={(unusedKey, chip) => {
-                onRemove(chipsByKey[key].key, val);
+                const [columnKey, ...value] = chip.key.split(':');
+                onRemove(columnKey, value.join(':'));
               }}
               categoryName={chipsByKey[key] ? chipsByKey[key].label : key}
               key={key}
@@ -231,7 +248,10 @@ class Search extends React.Component {
                     onSelect={(event, selection) =>
                       this.handleFilterDropdownSelect(key, event, selection)
                     }
-                    selections={chipsByKey[key].chips}
+                    selections={chipsByKey[key].chips.map(chip => {
+                      const [, ...value] = chip.key.split(':');
+                      return value.join(':');
+                    })}
                     isExpanded={isFilterDropdownOpen}
                     placeholderText={`Filter By ${name}`}
                   >

awx/ui_next/src/components/Search/Search.test.jsx

@@ -3,7 +3,9 @@ import {
   DataToolbar,
   DataToolbarContent,
 } from '@patternfly/react-core/dist/umd/experimental';
+import { createMemoryHistory } from 'history';
 import { mountWithContexts } from '@testUtils/enzymeHelpers';
+import { act } from 'react-dom/test-utils';
 import Search from './Search';
 
 describe('<Search />', () => {
@@ -141,4 +143,129 @@ describe('<Search />', () => {
     expect(onSearch).toHaveBeenCalledTimes(1);
     expect(onSearch).toBeCalledWith('name__icontains', 'test-321');
   });
+
+  test('filter keys are properly labeled', () => {
+    const columns = [
+      { name: 'Name', key: 'name', isDefault: true },
+      { name: 'Type', key: 'type', options: [['foo', 'Foo Bar!']] },
+      { name: 'Description', key: 'description' },
+    ];
+    const query =
+      '?organization.or__type=foo&organization.name=bar&item.page_size=10';
+    const history = createMemoryHistory({
+      initialEntries: [`/organizations/${query}`],
+    });
+    const wrapper = mountWithContexts(
+      <DataToolbar
+        id={`${QS_CONFIG.namespace}-list-toolbar`}
+        clearAllFilters={() => {}}
+        collapseListedFiltersBreakpoint="md"
+      >
+        <DataToolbarContent>
+          <Search qsConfig={QS_CONFIG} columns={columns} />
+        </DataToolbarContent>
+      </DataToolbar>,
+      { context: { router: { history } } }
+    );
+    const typeFilterWrapper = wrapper.find(
+      'DataToolbarFilter[categoryName="Type"]'
+    );
+    expect(typeFilterWrapper.prop('chips')[0].key).toEqual('or__type:foo');
+    const nameFilterWrapper = wrapper.find(
+      'DataToolbarFilter[categoryName="Name"]'
+    );
+    expect(nameFilterWrapper.prop('chips')[0].key).toEqual('name:bar');
+  });
+
+  test('should test handle remove of option-based key', async () => {
+    const qsConfigNew = {
+      namespace: 'item',
+      defaultParams: { page: 1, page_size: 5, order_by: '-type' },
+      integerFields: [],
+    };
+    const columns = [
+      {
+        name: 'type',
+        key: 'type',
+        options: [['foo', 'Foo Bar!']],
+        isDefault: true,
+      },
+    ];
+    const query = '?item.or__type=foo&item.page_size=10';
+    const history = createMemoryHistory({
+      initialEntries: [`/organizations/1/teams${query}`],
+    });
+    const onRemove = jest.fn();
+    const wrapper = mountWithContexts(
+      <DataToolbar
+        id={`${qsConfigNew.namespace}-list-toolbar`}
+        clearAllFilters={() => {}}
+        collapseListedFiltersBreakpoint="md"
+      >
+        <DataToolbarContent>
+          <Search
+            qsConfig={qsConfigNew}
+            columns={columns}
+            onRemove={onRemove}
+          />
+        </DataToolbarContent>
+      </DataToolbar>,
+      { context: { router: { history } } }
+    );
+    expect(history.location.search).toEqual(query);
+    // click remove button on chip
+    await act(async () => {
+      wrapper
+        .find('.pf-c-chip button[aria-label="close"]')
+        .at(0)
+        .simulate('click');
+    });
+    expect(onRemove).toBeCalledWith('or__type', 'foo');
+  });
+
+  test('should test handle remove of option-based with empty string value', async () => {
+    const qsConfigNew = {
+      namespace: 'item',
+      defaultParams: { page: 1, page_size: 5, order_by: '-type' },
+      integerFields: [],
+    };
+    const columns = [
+      {
+        name: 'type',
+        key: 'type',
+        options: [['', 'manual']],
+        isDefault: true,
+      },
+    ];
+    const query = '?item.or__type=&item.page_size=10';
+    const history = createMemoryHistory({
+      initialEntries: [`/organizations/1/teams${query}`],
+    });
+    const onRemove = jest.fn();
+    const wrapper = mountWithContexts(
+      <DataToolbar
+        id={`${qsConfigNew.namespace}-list-toolbar`}
+        clearAllFilters={() => {}}
+        collapseListedFiltersBreakpoint="md"
+      >
+        <DataToolbarContent>
+          <Search
+            qsConfig={qsConfigNew}
+            columns={columns}
+            onRemove={onRemove}
+          />
+        </DataToolbarContent>
+      </DataToolbar>,
+      { context: { router: { history } } }
+    );
+    expect(history.location.search).toEqual(query);
+    // click remove button on chip
+    await act(async () => {
+      wrapper
+        .find('.pf-c-chip button[aria-label="close"]')
+        .at(0)
+        .simulate('click');
+    });
+    expect(onRemove).toBeCalledWith('or__type', '');
+  });
 });

awx/ui_next/src/screens/Project/ProjectList/ProjectList.jsx

@@ -115,7 +115,7 @@ function ProjectList({ i18n }) {
               },
               {
                 name: i18n._(t`Type`),
-                key: 'type',
+                key: 'scm_type',
                 options: [
                   [``, i18n._(t`Manual`)],
                   [`git`, i18n._(t`Git`)],

awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/ProjectsList.jsx

@@ -71,8 +71,9 @@ function ProjectsList({ i18n, nodeResource, onUpdateNodeResource }) {
         },
         {
           name: i18n._(t`Type`),
-          key: 'type',
+          key: 'scm_type',
           options: [
+            [``, i18n._(t`Manual`)],
             [`git`, i18n._(t`Git`)],
             [`hg`, i18n._(t`Mercurial`)],
             [`svn`, i18n._(t`Subversion`)],

awx/ui_next/src/util/qs.js

@@ -208,7 +208,7 @@ function mergeParam(oldVal, newVal) {
   if (!newVal && newVal !== '') {
     return oldVal;
   }
-  if (!oldVal) {
+  if (!oldVal && oldVal !== '') {
     return newVal;
   }
   let merged;

awx/ui_next/src/util/qs.test.js

@@ -652,6 +652,28 @@ describe('qs (qs.js)', () => {
       });
     });
 
+    it('should not remove empty string values', () => {
+      const oldParams = {
+        foo: '',
+      };
+      const newParams = {
+        foo: 'two',
+      };
+      expect(mergeParams(oldParams, newParams)).toEqual({
+        foo: ['', 'two'],
+      });
+
+      const oldParams2 = {
+        foo: 'one',
+      };
+      const newParams2 = {
+        foo: '',
+      };
+      expect(mergeParams(oldParams2, newParams2)).toEqual({
+        foo: ['one', ''],
+      });
+    });
+
     it('should retain unaltered params', () => {
       const oldParams = {
         foo: 'one',