ERM-3390 Show columns: Name column should not be in the list of unsel… (

#1366) * ERM-3390 Show columns: Name column should not be in the list of unselectable columns * exclude 'name' column from toggle menu * ERM-3390 Show columns: Name column should not be in the list of unselectable columns * exclude 'name' column from toggle menu * fix test * fix more tests * test: Minor test tweaks (not really relevant) * test: Added route testing useEffect redirect (WIP... can be improved) * test: Commented out now useless tests * test: Fixed manual mock usage * test: Add mock for useEresourcesEnabled --------- Co-authored-by: EthanFreestone <[email protected]> Co-authored-by: Ethan Freestone <[email protected]>
folio-org · Nov 11, 2024 · e91e1b1 · e91e1b1
1 parent 1dd4346
commit e91e1b1
Show file tree

Hide file tree

Showing 15 changed files with 169 additions and 120 deletions.
diff --git a/src/components/AgreementSections/CoveredEResourcesList/CoveredEResourcesList.test.js b/src/components/AgreementSections/CoveredEResourcesList/CoveredEResourcesList.test.js
@@ -1,90 +1,85 @@
-
 import { waitFor } from '@folio/jest-config-stripes/testing-library/react';
+
 import { Button, Dropdown, MultiColumnList, renderWithIntl } from '@folio/stripes-erm-testing';
-import { MemoryRouter } from 'react-router-dom';
+
 import translationsProperties from '../../../../test/helpers';
 import CoveredEResourcesList from './CoveredEResourcesList';
 import agreement from './testResources';
 
-jest.mock('../../IfEResourcesEnabled', () => ({ children }) => {
-  return typeof children === 'function' ? children({ isEnabled: true }) : children;
-});
-
-// Use manual mocks set up in hooks/__mocks__ folder
-jest.mock('../../../hooks');
+// Use manual mocks set up in hooks/__mocks__ folder (small correction to the way this was done before)
+jest.mock('../../../hooks/useAgreementsSettings');
+jest.mock('../../../hooks/useEresourcesEnabled');
 
 const handlers = {
   onFilterEResources: jest.fn(),
   onExportEResourcesAsJSON: jest.fn().mockImplementation(() => Promise.resolve()),
   onExportEResourcesAsKBART: jest.fn().mockImplementation(() => Promise.resolve()),
 };
 
+// TODO there are 2 warnings when running this test... contentData ends up as a boolean not an array --
+// maybe needs mocking fetchMultiplePages
+// received NaN for the children attribute within PrevNextPagination, better mocking needed there too?
 describe('CoveredEResourcesList', () => {
-  beforeEach(() => {
-    renderWithIntl(
-      <MemoryRouter>
+  beforeEach(async () => {
+    // This is blowing my mind... I do not understand why this needs to be wrapped in a waitFor
+    await waitFor(() => {
+      renderWithIntl(
         <CoveredEResourcesList
           agreement={agreement}
           eresourcesFilterPath="current"
           {...handlers}
-        />
-      </MemoryRouter>,
-      translationsProperties
-    );
-  });
-
-  test('renders the expected filter buttons', async () => {
-    await Button('Current').exists();
-    await Button('Future').exists();
-    await Button('Dropped').exists();
-    await Button('All').exists();
+        />,
+        translationsProperties
+      );
+    });
   });
 
-  test('clicking the filter buttons should call the onFilterEResources callback', async () => {
-    await waitFor(async () => {
-      await Button('Future').click();
+  describe.each(['Future', 'Current', 'Dropped', 'All'])('%s filter button', (filterLabel) => {
+    test(`renders the ${filterLabel} button`, async () => {
+      await waitFor(async () => {
+        await Button(filterLabel).exists();
+      });
     });
 
-    await waitFor(async () => {
-      expect(handlers.onFilterEResources.mock.calls.length).toBe(1);
-    });
-
-    await waitFor(async () => {
-      await Button('Dropped').click();
-    });
+    describe(`clicking the ${filterLabel} button`, () => {
+      beforeEach(async () => {
+        handlers.onFilterEResources.mockClear();
 
-    await waitFor(async () => {
-      expect(handlers.onFilterEResources.mock.calls.length).toBe(2);
-    });
+        expect(handlers.onFilterEResources.mock.calls.length).toBe(0);
 
-    await waitFor(async () => {
-      await Button('All').click();
-    });
+        await waitFor(async () => {
+          await Button(filterLabel).click();
+        });
+      });
 
-    await waitFor(async () => {
-      expect(handlers.onFilterEResources.mock.calls.length).toBe(3);
+      test('onFilterEResources callback called', async () => {
+        await waitFor(async () => {
+          expect(handlers.onFilterEResources.mock.calls.length).toBe(1);
+        });
+      });
     });
   });
 
   test('renders the Export dropdown', async () => {
-    await Dropdown('Export as...').exists();
-  });
-
-  test('choosing the dropdown options', async () => {
     await waitFor(async () => {
-      await Dropdown('Export as...').choose('JSON');
+      await Dropdown('Export as...').exists();
     });
+  });
 
-    await waitFor(async () => {
-      expect(handlers.onExportEResourcesAsJSON.mock.calls.length).toBe(1);
+  describe.each([
+    { dropdownChoice: 'JSON', mockHandler: handlers.onExportEResourcesAsJSON },
+    { dropdownChoice: 'KBART', mockHandler: handlers.onExportEResourcesAsKBART }
+  ])('choosing export as $dropdownChoice', ({ dropdownChoice, mockHandler }) => {
+    beforeEach(async () => {
+      await waitFor(async () => {
+        await Dropdown('Export as...').choose(dropdownChoice);
+      });
     });
 
-    await waitFor(async () => {
-      await Dropdown('Export as...').choose('KBART');
-    });
-
-    await waitFor(async () => {
-      expect(handlers.onExportEResourcesAsKBART.mock.calls.length).toBe(1);
+    test('correct onExportResources handler called', async () => {
+      await waitFor(async () => {
+        expect(mockHandler.mock.calls.length).toBe(1);
+      });
     });
   });
 
@@ -100,4 +95,3 @@ describe('CoveredEResourcesList', () => {
     await MultiColumnList({ columns: ['Name', 'eISSN/ISSN', 'Platform', 'Package', 'Coverage', ' ', 'Access start', 'Access end'] }).exists();
   });
 });
-
diff --git a/src/components/AgreementSections/Lines/Lines.js b/src/components/AgreementSections/Lines/Lines.js
@@ -71,6 +71,7 @@ const Lines = ({
             </Button>
             <ColumnManagerMenu
               columnMapping={LINE_LISTING_COLUMN_MAPPING}
+              excludeColumns={['name']}
               prefix="line-listing"
               toggleColumn={toggleColumn}
               visibleColumns={visibleColumns}

diff --git a/src/components/views/AgreementLines/AgreementLines.js b/src/components/views/AgreementLines/AgreementLines.js
@@ -90,6 +90,7 @@ const AgreementLines = ({
     return (
       <ColumnManagerMenu
         columnMapping={AGREEMENT_LINES_COLUMN_MAPPING}
+        excludeColumns={['name']}
         prefix="agreement-lines"
         toggleColumn={toggleColumn}
         visibleColumns={visibleColumns}

diff --git a/src/components/views/Agreements/Agreements.js b/src/components/views/Agreements/Agreements.js
@@ -142,6 +142,7 @@ const Agreements = ({
         </IfPermission>
         <ColumnManagerMenu
           columnMapping={AGREEMENTS_COLUMN_MAPPING}
+          excludeColumns={['name']}
           prefix="agreements-list"
           toggleColumn={toggleColumn}
           visibleColumns={visibleColumns}

diff --git a/src/hooks/__mocks__/index.js b/src/hooks/__mocks__/index.js
diff --git a/src/hooks/__mocks__/useAgreementsSettings.js b/src/hooks/__mocks__/useAgreementsSettings.js
@@ -0,0 +1,11 @@
+module.exports = jest.fn(() => {
+  return (
+    {
+      parsedSettings: {
+        pageSize: {
+          agreementEresources: 12
+        }
+      }
+    }
+  );
+});
diff --git a/src/hooks/__mocks__/useEresourcesEnabled.js b/src/hooks/__mocks__/useEresourcesEnabled.js
@@ -0,0 +1 @@
+module.exports = jest.fn(() => true);
diff --git a/src/routes/AgreementLinesRoute/AgreementLinesRoute.test.js b/src/routes/AgreementLinesRoute/AgreementLinesRoute.test.js
@@ -39,7 +39,10 @@ describe('AgreementLinesRoute', () => {
       expect(getByTestId('agreementLines')).toBeInTheDocument();
     });
 
-    describe('re-rendering the route', () => { // makes sure that we hit the componentDidUpdate block
+    // TODO we should actually be _properly_ testing the useEffect, see AgreementsRoute example
+    // using memory router to render with props which force it to call history.push mock and measuring that mock output
+
+    /* describe('re-rendering the route', () => { // makes sure that we hit the componentDidUpdate block
       beforeEach(() => {
         renderWithIntl(
           <MemoryRouter>
@@ -51,9 +54,10 @@ describe('AgreementLinesRoute', () => {
       });
 
       test('renders the agreementLines component', () => {
-        const { getByTestId } = renderComponent;
-        expect(getByTestId('agreementLines')).toBeInTheDocument();
+        const { getAllByTestId } = renderComponent;
+        const agreementLinesElements = getAllByTestId('agreementLines');
+        expect(agreementLinesElements.length).toBeGreaterThan(0);
       });
-    });
+    }); */
   });
 });
diff --git a/src/routes/AgreementViewRoute/AgreementViewRoute.test.js b/src/routes/AgreementViewRoute/AgreementViewRoute.test.js
@@ -115,7 +115,7 @@ jest.mock('../../components/views/Agreement', () => {
 });
 
 const data = {
-  history:{
+  history: {
     push: historyPushMock,
   },
   location,
@@ -137,8 +137,9 @@ describe('AgreementViewRoute', () => {
     });
 
     test('renders the Agreement component', () => {
-      const { getByText } = renderComponent;
-      expect(getByText('Agreement')).toBeInTheDocument();
+      const { getAllByText } = renderComponent;
+      const agreementElements = getAllByText('Agreement');
+      expect(agreementElements.length).toBeGreaterThan(0);
     });
 
     test('renders the AgreementLineButton', () => {
@@ -198,8 +199,9 @@ describe('AgreementViewRoute', () => {
       });
 
       test('renders the Agreements component', () => {
-        const { getByText } = renderComponent;
-        expect(getByText('Agreement')).toBeInTheDocument();
+        const { getAllByText } = renderComponent;
+        const agreementElements = getAllByText('Agreement');
+        expect(agreementElements.length).toBeGreaterThan(0);
       });
     });
   });

diff --git a/src/routes/AgreementsRoute/AgreementsRoute.test.js b/src/routes/AgreementsRoute/AgreementsRoute.test.js
@@ -1,23 +1,33 @@
 
-import { renderWithIntl } from '@folio/stripes-erm-testing';
+import { useQuery } from 'react-query';
 import { MemoryRouter } from 'react-router-dom';
 
+import { renderWithIntl } from '@folio/stripes-erm-testing';
 import { useStripes } from '@folio/stripes/core';
+
+
 import translationsProperties from '../../../test/helpers';
 import AgreementsRoute from './AgreementsRoute';
 import mockRefdata from '../../../test/jest/refdata';
 
+
 jest.mock('../../hooks', () => ({
   ...jest.requireActual('../../hooks'),
   useAgreementsSettings: jest.fn(() => ({ data: { configs: [{}] } })),
   useAgreementsRefdata: () => mockRefdata,
 }));
 
+jest.mock('../../components/views/Agreements', () => () => <div>Agreements view component</div>);
+
+
+const mockPush = jest.fn();
 const routeProps = {
   history: {
-    push: () => jest.fn()
+    push: mockPush
+  },
+  location: {
+    search: '?thisIsAFakeSearch'
   },
-  location: {},
   match: {
     params: {},
   },
@@ -36,25 +46,35 @@ describe('AgreementsRoute', () => {
     });
 
     test('renders the agreements component', () => {
-      const { getByTestId } = renderComponent;
-      expect(getByTestId('agreements')).toBeInTheDocument();
+      const { getByText } = renderComponent;
+      expect(getByText('Agreements view component')).toBeInTheDocument();
+    });
+
+
+    // TODO also test with multiple agreements
+    test('did not redirect to view pane', () => {
+      expect(mockPush.mock.calls.length).toBe(0);
+    });
+  });
+
+  // EXAMPLE testing redirect in a route Test
+  describe('rendering the route with single agreement', () => {
+    beforeEach(() => {
+      mockPush.mockClear();
+      useQuery.mockImplementation(() => ({
+        data: { results: [{ title: 'fakeAgreement', id: 'fakeId' }], totalRecords: 1 }
+      }));
+
+      renderWithIntl(
+        <MemoryRouter>
+          <AgreementsRoute {...routeProps} />
+        </MemoryRouter>,
+        translationsProperties
+      );
     });
 
-    describe('re-rendering the route', () => { // makes sure that we hit the componentDidUpdate block
-      beforeEach(() => {
-        renderWithIntl(
-          <MemoryRouter>
-            <AgreementsRoute {...routeProps} />
-          </MemoryRouter>,
-          translationsProperties,
-          renderComponent.rerender
-        );
-      });
-
-      test('renders the agreements component', () => {
-        const { getByTestId } = renderComponent;
-        expect(getByTestId('agreements')).toBeInTheDocument();
-      });
+    test('redirected to view pane', () => {
+      expect(mockPush.mock.calls[0][0]).toEqual('/erm/agreements/fakeId?thisIsAFakeSearch');
     });
   });
 

diff --git a/src/routes/EResourceViewRoute/EResourceViewRoute.test.js b/src/routes/EResourceViewRoute/EResourceViewRoute.test.js
@@ -122,8 +122,8 @@ jest.mock('../../components/views/EResource', () => {
 
 const data = {
   handlers,
-  isSuppressFromDiscoveryEnabled: () => {},
-  history:{
+  isSuppressFromDiscoveryEnabled: () => { },
+  history: {
     push: historyPushMock,
   },
   location: {
@@ -157,8 +157,9 @@ describe('EResourceViewRoute', () => {
     });
 
     test('renders the EResource component', () => {
-      const { getByText } = renderComponent;
-      expect(getByText('EResource')).toBeInTheDocument();
+      const { getAllByText } = renderComponent;
+      const eresourceElements = getAllByText('EResource');
+      expect(eresourceElements.length).toBeGreaterThan(0);
     });
 
     test('triggers the CloseButton callback', async () => {
@@ -231,7 +232,9 @@ describe('EResourceViewRoute', () => {
       expect(getByText('ToggleTagsButton')).toBeInTheDocument();
     });
 
-    describe('re-rendering the route', () => { // makes sure that we hit the componentDidUpdate block
+    // TODO we should actually be _properly_ testing the useEffect, see AgreementsRoute example
+    // using memory router to render with props which force it to call history.push mock and measuring that mock output
+    /* describe('re-rendering the route', () => { // makes sure that we hit the componentDidUpdate block
       beforeEach(() => {
         renderWithIntl(
           <MemoryRouter>
@@ -243,9 +246,10 @@ describe('EResourceViewRoute', () => {
       });
 
       test('renders the EResource component', () => {
-        const { getByText } = renderComponent;
-        expect(getByText('EResource')).toBeInTheDocument();
+        const { getAllByText } = renderComponent;
+        const eresourceElements = getAllByText('EResource');
+        expect(eresourceElements.length).toBeGreaterThan(0);
       });
-    });
+    }); */
   });
 });