From b89cbb1799b86be140f26e709d8e7b230c9f9e98 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Thu, 18 Jan 2024 13:28:53 -0700 Subject: [PATCH 01/21] MMT-3492: Saving progress on table component --- .../src/js/components/DraftList/DraftList.jsx | 174 ++++++------------ static/src/js/components/Table/Table.jsx | 79 ++++++++ .../components/Table/__tests__/Table.test.js | 117 ++++++++++++ .../ManagePage/__tests__/ManagePage.test.js | 8 +- 4 files changed, 259 insertions(+), 119 deletions(-) create mode 100644 static/src/js/components/Table/Table.jsx create mode 100644 static/src/js/components/Table/__tests__/Table.test.js diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index a544b4a40..bd13ab0fb 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -6,9 +6,8 @@ import { FaFileDownload } from 'react-icons/fa' import pluralize from 'pluralize' import Col from 'react-bootstrap/Col' import Placeholder from 'react-bootstrap/Placeholder' -import PlaceholderButton from 'react-bootstrap/PlaceholderButton' import Row from 'react-bootstrap/Row' -import Table from 'react-bootstrap/Table' +import Table from '../Table/Table' import useAppContext from '../../hooks/useAppContext' import useDraftsQuery from '../../hooks/useDraftsQuery' @@ -18,7 +17,6 @@ import constructDownloadableFile from '../../utils/constructDownloadableFile' import Button from '../Button/Button' import ErrorBanner from '../ErrorBanner/ErrorBanner' -import For from '../For/For' import Page from '../Page/Page' import { DOWNLOAD_DRAFT } from '../../operations/queries/getDownloadDraft' @@ -53,6 +51,59 @@ const DraftList = ({ draftType }) => { }) } + // Building a Table using Data in items + const renderRows = items.length + ? (items.map((item) => { + const { conceptId, revisionDate, previewMetadata: { name, longName } } = item + const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` + + return ( + + + + {name || ''} + + + + {longName || ''} + + + {new Date(revisionDate).toISOString().split('T')[0]} + + +
+ +
+ + + ) + }) + ) + : ( + [ + + + No + {' '} + {draftType} + {' '} + drafts exist for the provider + {' '} + {providerId} + . + + + ] + ) + return ( { ) } - - - - - - - - - - - - - { - loading - ? ( - - { - (item, i) => ( - - - - - - - ) - } - - ) - : ( - - - - ) - } - > - { - ( - { - conceptId, - previewMetadata: { - name, - longName - }, - revisionDate - } - ) => { - const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` - - return ( - - - - - - - ) - } - } - - ) - } - -
Short NameEntry TitleLast ModifiedActions
- - - - - - - - - - - - - - - -
- No - {' '} - {draftType} - {' '} - drafts exist for the provider - {' '} - {providerId} - . -
- - {name || ''} - - - {longName || ''} - - {new Date(revisionDate).toISOString().split('T')[0]} - -
- -
-
+ ) } diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx new file mode 100644 index 000000000..5333a93b7 --- /dev/null +++ b/static/src/js/components/Table/Table.jsx @@ -0,0 +1,79 @@ +import React from 'react' +import PropTypes from 'prop-types' +import BootstrapTable from 'react-bootstrap/Table' +import Placeholder from 'react-bootstrap/Placeholder' +import PlaceholderButton from 'react-bootstrap/PlaceholderButton' + +import For from '../For/For' + +/** + * CustomArrayFieldTemplate + * @typedef {Object} Table + * @property {Boolean} loading A value provided by whichever useQuery is being used in parent component + * @property {Object[]} headers A list of header titles for a table + * @property {Array} renderRows An array of formatted rows with data already populated + */ + +const Table = ({ + headers, loading, renderRows +}) => { + const renderHeaders = headers.map((header) => ( + + )) + + return ( + + + + {renderHeaders} + + + + { + (loading) ? ( + + { + (item, i) => ( + + + + + + + ) + } + + ) : (renderRows) + } + + + ) +} + +Table.defaultProps = { + loading: null +} + +Table.propTypes = { + headers: PropTypes.arrayOf(PropTypes.string).isRequired, + loading: PropTypes.bool, + renderRows: PropTypes.arrayOf(PropTypes.element).isRequired +} + +export default Table diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js new file mode 100644 index 000000000..c5f854127 --- /dev/null +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -0,0 +1,117 @@ +import React from 'react' +import { render, screen } from '@testing-library/react' +import { FaStar } from 'react-icons/fa' +import { Link } from 'react-router-dom' +import userEvent from '@testing-library/user-event' + +import Table from '../Table' + +const setup = (overrideProps = {}) => { + const props = { + headers: [ + 'column 1', + 'column 2' + ], + loading: false, + renderRows: [ + { + key: 'tools/TD1200000093-MMT_2', + children: [ + , + + ] + }, + { + key: 'tools/TD1200000094-MMT_2', + children: [ + , + + ] + } + ], + ...overrideProps + } + + render( +
{header}
+ + + + + + + + + + + + + + + +
+ + Sample Text + + + More Sample Text + + + Sample Text + + + More Sample Text +
+ ) + + return { + props, + user: userEvent.setup() + } +} + +describe('Table', () => { + describe('when the table component is passed markup and data', () => { + test('renders filled table', () => { + setup() + + expect(screen.getByText('column 1')).toBeInTheDocument() + }) + }) + + // Describe('when the button with an icon', () => { + // beforeEach(() => { + // FaStar.mockImplementation( + // jest.requireActual('react-icons/fa').FaStar + // ) + // }) + + // test('renders the button', () => { + // setup({ + // Icon: FaStar + // }) + + // expect(FaStar).toHaveBeenCalledTimes(1) + // }) + // }) + + // describe('when the button is clicked', () => { + // test('executes onClick callback', async () => { + // const { props, user } = setup() + + // await user.click(screen.getByRole('button')) + + // expect(props.onClick).toHaveBeenCalledTimes(1) + // }) + // }) + + // describe('when set the specified size and variant to the button', () => { + // test('applies to the button', () => { + // setup({ + // size: 'lg', + // variant: 'danger' + // }) + + // const button = screen.getByRole('button') + + // expect(button).toHaveClass('btn-lg') + // expect(button).toHaveClass('btn-danger') + // }) + // }) + + // describe('when set naked prop is true to the button', () => { + // test('applies naked style to the button', () => { + // setup({ + // naked: true + // }) + + // const button = screen.getByRole('button') + + // expect(button).toHaveClass('button--naked') + // }) + // }) +}) diff --git a/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js b/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js index aaf699d00..0b93046f4 100644 --- a/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js +++ b/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js @@ -39,7 +39,7 @@ const setup = (overrideMocks) => { items: [ { conceptId: 'TD1000000000-TESTPROV', - revisionDate: '2023-11-30 00:00:00', + revisionDate: '2024-01-18T16:42:00.111Z', previewMetadata: { __typename: 'Tool', name: 'Tool Draft 1 Name', @@ -48,7 +48,7 @@ const setup = (overrideMocks) => { }, { conceptId: 'TD1000000001-TESTPROV', - revisionDate: '2023-11-30 00:00:00', + revisionDate: '2024-01-18T16:42:00.111Z', previewMetadata: { __typename: 'Tool', name: 'Tool Draft 2 Name', @@ -167,10 +167,10 @@ describe('ManagePage component', () => { expect(draftLinks.length).toEqual(2) expect(within(draftLinks[0]).queryByText('Tool Draft 1 Name')).toBeInTheDocument() expect(within(draftLinks[0]).queryByText('Tool Draft 1 Long Name')).toBeInTheDocument() - expect(within(draftLinks[0]).queryByText('11/30/2023, 24:00:00')).toBeInTheDocument() + expect(within(draftLinks[0]).queryByText('1/18/2024, 09:42:00')).toBeInTheDocument() expect(within(draftLinks[1]).queryByText('Tool Draft 2 Name')).toBeInTheDocument() expect(within(draftLinks[1]).queryByText('Tool Draft 2 Long Name')).toBeInTheDocument() - expect(within(draftLinks[1]).queryByText('11/30/2023, 24:00:00')).toBeInTheDocument() + expect(within(draftLinks[1]).queryByText('1/18/2024, 09:42:00')).toBeInTheDocument() }) }) }) From 10d76a51fbef05d0b18e578814f7292bfe10fec9 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Fri, 19 Jan 2024 11:50:37 -0700 Subject: [PATCH 02/21] MMT-3492: Adjusting RenderRows --- .../src/js/components/DraftList/DraftList.jsx | 77 +++++-------- static/src/js/components/Table/Table.jsx | 106 ++++++++++++------ .../components/Table/__tests__/Table.test.js | 38 ++----- static/src/js/utils/stringifyCircularJSON.js | 15 +++ 4 files changed, 127 insertions(+), 109 deletions(-) create mode 100644 static/src/js/utils/stringifyCircularJSON.js diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index bd13ab0fb..b347006a5 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -29,6 +29,8 @@ const DraftList = ({ draftType }) => { const { drafts, error, loading } = useDraftsQuery({ draftType }) const { count, items = [] } = drafts + const noDraftsError = `No ${draftType} drafts exist for the provider ${providerId}` + const [downloadDraft] = useLazyQuery(DOWNLOAD_DRAFT, { onCompleted: (getDraftData) => { const { draft: fetchedDraft } = getDraftData @@ -52,57 +54,36 @@ const DraftList = ({ draftType }) => { } // Building a Table using Data in items - const renderRows = items.length - ? (items.map((item) => { - const { conceptId, revisionDate, previewMetadata: { name, longName } } = item - const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` + const renderRows = (items.map((item) => { + const { conceptId, revisionDate, previewMetadata: { name, longName } } = item + const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` - return ( - - - - - - - ) - }) - ) - : ( + return ( [ - - - + + {name || ''} + , + + {longName || ''} + , + + {new Date(revisionDate).toISOString().split('T')[0]} + , +
+ +
] ) + }) + ) return ( { }
- - {name || ''} - - - {longName || ''} - - {new Date(revisionDate).toISOString().split('T')[0]} - -
- -
-
- No - {' '} - {draftType} - {' '} - drafts exist for the provider - {' '} - {providerId} - . -
) diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index 5333a93b7..93bab5691 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -5,22 +5,83 @@ import Placeholder from 'react-bootstrap/Placeholder' import PlaceholderButton from 'react-bootstrap/PlaceholderButton' import For from '../For/For' +import stringifyCircularJSON from '../../utils/stringifyCircularJSON' /** - * CustomArrayFieldTemplate + * Table * @typedef {Object} Table * @property {Boolean} loading A value provided by whichever useQuery is being used in parent component * @property {Object[]} headers A list of header titles for a table * @property {Array} renderRows An array of formatted rows with data already populated + * @property {String} error A string with a specific error for table */ const Table = ({ - headers, loading, renderRows + headers, classNames, loading, renderRows, error }) => { const renderHeaders = headers.map((header) => ( )) + const content = [] + + if (loading) { + content.push( + + { + (item, i) => ( + + + + + + + ) + } + + ) + } else if (renderRows.length > 0) { + const rowData = renderRows.map((row) => { + const rowKey = stringifyCircularJSON(row) + + return ( + + { + row.map((cell, index) => { + const cellKey = stringifyCircularJSON(cell) + + return ( + + ) + }) + } + + ) + }) + + content.push(rowData) + } else { + content.push() + } + return ( @@ -29,51 +90,24 @@ const Table = ({ - { - (loading) ? ( - - { - (item, i) => ( - - - - - - - ) - } - - ) : (renderRows) - } + {content} ) } Table.defaultProps = { - loading: null + classNames: [], + loading: null, + error: 'No Data to Display' } Table.propTypes = { headers: PropTypes.arrayOf(PropTypes.string).isRequired, + classNames: PropTypes.arrayOf(PropTypes.string), loading: PropTypes.bool, - renderRows: PropTypes.arrayOf(PropTypes.element).isRequired + renderRows: PropTypes.node.isRequired, + error: PropTypes.string } export default Table diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js index c5f854127..f3036199c 100644 --- a/static/src/js/components/Table/__tests__/Table.test.js +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -14,32 +14,18 @@ const setup = (overrideProps = {}) => { ], loading: false, renderRows: [ - { - key: 'tools/TD1200000093-MMT_2', - children: [ - , - - ] - }, - { - key: 'tools/TD1200000094-MMT_2', - children: [ - , - - ] - } + + Sample Text + , + + More Sample Text + , + + Sample Text + , + + More Sample Text + ], ...overrideProps } diff --git a/static/src/js/utils/stringifyCircularJSON.js b/static/src/js/utils/stringifyCircularJSON.js new file mode 100644 index 000000000..78632cdda --- /dev/null +++ b/static/src/js/utils/stringifyCircularJSON.js @@ -0,0 +1,15 @@ +const stringifyCircularJSON = (obj) => { + const seen = new WeakSet() + + return JSON.stringify(obj, (k, v) => { + if (v !== null && typeof v === 'object') { + if (seen.has(v)) return + seen.add(v) + } + + // eslint-disable-next-line consistent-return + return v + }) +} + +export default stringifyCircularJSON From 205f9cc29c8d89d5f9fc898f921e1ee3720c9d15 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Fri, 19 Jan 2024 14:13:50 -0700 Subject: [PATCH 03/21] MMT-3492: Finish writing tests --- .../components/Table/__tests__/Table.test.js | 99 +++++++------------ 1 file changed, 37 insertions(+), 62 deletions(-) diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js index f3036199c..3f0db8ee5 100644 --- a/static/src/js/components/Table/__tests__/Table.test.js +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -1,7 +1,5 @@ import React from 'react' import { render, screen } from '@testing-library/react' -import { FaStar } from 'react-icons/fa' -import { Link } from 'react-router-dom' import userEvent from '@testing-library/user-event' import Table from '../Table' @@ -14,18 +12,22 @@ const setup = (overrideProps = {}) => { ], loading: false, renderRows: [ - - Sample Text - , - - More Sample Text - , - - Sample Text - , - - More Sample Text - + [ + + Row 1 Cell 1 + , + + Row 1 Cell 2 + + ], + [ + + Row 2 Cell 1 + , + + Row 2 Cell 2 + + ] ], ...overrideProps } @@ -46,58 +48,31 @@ describe('Table', () => { setup() expect(screen.getByText('column 1')).toBeInTheDocument() + expect(screen.getByRole('table')).toBeInTheDocument() + expect(screen.getByText('Row 2 Cell 2')).toBeInTheDocument() }) }) - // Describe('when the button with an icon', () => { - // beforeEach(() => { - // FaStar.mockImplementation( - // jest.requireActual('react-icons/fa').FaStar - // ) - // }) + describe('when the table component is passed a custom error mesage with no data', () => { + test('renders custom error message', () => { + setup({ + renderRows: [], + error: 'Custom Error Message' + }) - // test('renders the button', () => { - // setup({ - // Icon: FaStar - // }) - - // expect(FaStar).toHaveBeenCalledTimes(1) - // }) - // }) - - // describe('when the button is clicked', () => { - // test('executes onClick callback', async () => { - // const { props, user } = setup() - - // await user.click(screen.getByRole('button')) - - // expect(props.onClick).toHaveBeenCalledTimes(1) - // }) - // }) - - // describe('when set the specified size and variant to the button', () => { - // test('applies to the button', () => { - // setup({ - // size: 'lg', - // variant: 'danger' - // }) - - // const button = screen.getByRole('button') - - // expect(button).toHaveClass('btn-lg') - // expect(button).toHaveClass('btn-danger') - // }) - // }) - - // describe('when set naked prop is true to the button', () => { - // test('applies naked style to the button', () => { - // setup({ - // naked: true - // }) + expect(screen.getByText('Custom Error Message')).toBeInTheDocument() + }) + }) - // const button = screen.getByRole('button') + describe('when the table component is passed loading:true', () => { + test('renders loading screen', () => { + setup({ + loading: true + }) - // expect(button).toHaveClass('button--naked') - // }) - // }) + const buttons = screen.queryAllByRole('button', { className: 'btn-sm col-10 placeholder placeholder-sm btn btn-secondary' }) + expect(screen.getByRole('table', { className: 'table table-striped' })).toBeInTheDocument() + expect(buttons).toHaveLength(5) + }) + }) }) From 9f15d69808450a232b6b198cf1cda28142f5f3df Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Mon, 22 Jan 2024 11:33:07 -0700 Subject: [PATCH 04/21] MMT-3492: Addressing PR Comments --- .../src/js/components/DraftList/DraftList.jsx | 4 +- .../DraftList/__tests__/DraftList.test.js | 4 +- static/src/js/components/Table/Table.jsx | 58 +++++++++---------- .../components/Table/__tests__/Table.test.js | 7 +-- .../__tests__/stringifyCircularJSON.test.js | 18 ++++++ static/src/js/utils/stringifyCircularJSON.js | 7 +++ 6 files changed, 59 insertions(+), 39 deletions(-) create mode 100644 static/src/js/utils/__tests__/stringifyCircularJSON.test.js diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index b347006a5..4e8f52438 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -54,7 +54,7 @@ const DraftList = ({ draftType }) => { } // Building a Table using Data in items - const renderRows = (items.map((item) => { + const data = (items.map((item) => { const { conceptId, revisionDate, previewMetadata: { name, longName } } = item const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` @@ -140,7 +140,7 @@ const DraftList = ({ draftType }) => { headers={['Short Name', 'Entry Title', 'Last Modified', 'Actions']} classNames={['col-md-4', 'col-md-4', 'col-auto', 'col-auto']} loading={loading} - renderRows={renderRows} + data={data} error={noDraftsError} /> diff --git a/static/src/js/components/DraftList/__tests__/DraftList.test.js b/static/src/js/components/DraftList/__tests__/DraftList.test.js index d6cc7bc1a..86b22c445 100644 --- a/static/src/js/components/DraftList/__tests__/DraftList.test.js +++ b/static/src/js/components/DraftList/__tests__/DraftList.test.js @@ -169,7 +169,7 @@ describe('DraftList', () => { const rows = screen.getAllByRole('row') expect(within(rows[1]).getByRole('cell', { - name: 'No Tool drafts exist for the provider MMT_2 .' + name: 'No Tool drafts exist for the provider MMT_2' })).toBeInTheDocument() }) }) @@ -183,7 +183,7 @@ describe('DraftList', () => { setup() - expect(Placeholder).toHaveBeenCalledTimes(21) + expect(Placeholder).toHaveBeenCalledTimes(17) }) }) diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index 93bab5691..8a92381b1 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -2,7 +2,6 @@ import React from 'react' import PropTypes from 'prop-types' import BootstrapTable from 'react-bootstrap/Table' import Placeholder from 'react-bootstrap/Placeholder' -import PlaceholderButton from 'react-bootstrap/PlaceholderButton' import For from '../For/For' import stringifyCircularJSON from '../../utils/stringifyCircularJSON' @@ -12,12 +11,12 @@ import stringifyCircularJSON from '../../utils/stringifyCircularJSON' * @typedef {Object} Table * @property {Boolean} loading A value provided by whichever useQuery is being used in parent component * @property {Object[]} headers A list of header titles for a table - * @property {Array} renderRows An array of formatted rows with data already populated + * @property {Array} data An array of formatted rows with data already populated * @property {String} error A string with a specific error for table */ const Table = ({ - headers, classNames, loading, renderRows, error + headers, classNames, loading, data, error }) => { const renderHeaders = headers.map((header) => ( @@ -27,37 +26,34 @@ const Table = ({ if (loading) { content.push( - + { - (item, i) => ( - - - - - - - ) + (item, i) => { + const trKey = headers[i] + + return ( + + { + headers.map((index) => { + const tdKey = `${trKey}_${index}` + + return ( + + ) + }) + } + + ) + } } ) - } else if (renderRows.length > 0) { - const rowData = renderRows.map((row) => { + } else if (data.length > 0) { + const rowData = data.map((row) => { const rowKey = stringifyCircularJSON(row) return ( @@ -106,7 +102,7 @@ Table.propTypes = { headers: PropTypes.arrayOf(PropTypes.string).isRequired, classNames: PropTypes.arrayOf(PropTypes.string), loading: PropTypes.bool, - renderRows: PropTypes.node.isRequired, + data: PropTypes.node.isRequired, error: PropTypes.string } diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js index 3f0db8ee5..ad9305ffa 100644 --- a/static/src/js/components/Table/__tests__/Table.test.js +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -11,7 +11,7 @@ const setup = (overrideProps = {}) => { 'column 2' ], loading: false, - renderRows: [ + data: [ [ Row 1 Cell 1 @@ -56,7 +56,7 @@ describe('Table', () => { describe('when the table component is passed a custom error mesage with no data', () => { test('renders custom error message', () => { setup({ - renderRows: [], + data: [], error: 'Custom Error Message' }) @@ -70,9 +70,8 @@ describe('Table', () => { loading: true }) - const buttons = screen.queryAllByRole('button', { className: 'btn-sm col-10 placeholder placeholder-sm btn btn-secondary' }) expect(screen.getByRole('table', { className: 'table table-striped' })).toBeInTheDocument() - expect(buttons).toHaveLength(5) + expect(screen.queryAllByRole('cell', { className: 'col-md-4' })).toHaveLength(4) }) }) }) diff --git a/static/src/js/utils/__tests__/stringifyCircularJSON.test.js b/static/src/js/utils/__tests__/stringifyCircularJSON.test.js new file mode 100644 index 000000000..ded207cf8 --- /dev/null +++ b/static/src/js/utils/__tests__/stringifyCircularJSON.test.js @@ -0,0 +1,18 @@ +import stringifyCircularJSON from '../stringifyCircularJSON' + +const circularObject = { + name: 'Jane Doe' +} + +circularObject.address = { + city: 'Circuit City', + owner: circularObject +} + +describe('stringifyCircularJSON', () => { + describe('when passed a circular JSON reference', () => { + test('returns a usable string', () => { + expect(stringifyCircularJSON(circularObject)).toEqual('{"name":"Jane Doe","address":{"city":"Circuit City"}}') + }) + }) +}) diff --git a/static/src/js/utils/stringifyCircularJSON.js b/static/src/js/utils/stringifyCircularJSON.js index 78632cdda..70af4392c 100644 --- a/static/src/js/utils/stringifyCircularJSON.js +++ b/static/src/js/utils/stringifyCircularJSON.js @@ -1,3 +1,10 @@ +/** + * This returns a usable string when JSON.stringify can't be used due to + * a circular reference. + * @param {Object} obj The circular JSON to be modified + * @returns {string} stringified circular JSON + */ + const stringifyCircularJSON = (obj) => { const seen = new WeakSet() From 4a3f8db6e2e0def877e037ddf62abc57c5dbb430 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Tue, 23 Jan 2024 10:29:55 -0700 Subject: [PATCH 05/21] MMT-3492: Fixing 'data' structure --- .../src/js/components/DraftList/DraftList.jsx | 64 ++++++++++++------- .../js/components/Pagination/Pagination.jsx | 10 +++ static/src/js/components/Table/Table.jsx | 18 +++--- .../components/Table/__tests__/Table.test.js | 39 ++++++----- .../__tests__/stringifyCircularJSON.test.js | 18 ------ static/src/js/utils/stringifyCircularJSON.js | 22 ------- 6 files changed, 85 insertions(+), 86 deletions(-) create mode 100644 static/src/js/components/Pagination/Pagination.jsx delete mode 100644 static/src/js/utils/__tests__/stringifyCircularJSON.test.js delete mode 100644 static/src/js/utils/stringifyCircularJSON.js diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 4e8f52438..511e9c5f9 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -59,28 +59,48 @@ const DraftList = ({ draftType }) => { const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` return ( - [ - - {name || ''} - , - - {longName || ''} - , - - {new Date(revisionDate).toISOString().split('T')[0]} - , -
- -
- ] + { + key: `${conceptId}`, + cells: + [ + { + value: + ( + + {name || ''} + + ) + }, + { + value: + ( + longName || '' + ) + }, + { + value: + ( + new Date(revisionDate).toISOString().split('T')[0] + ) + }, + { + value: + ( +
+ +
+ ) + } + ] + } ) }) ) diff --git a/static/src/js/components/Pagination/Pagination.jsx b/static/src/js/components/Pagination/Pagination.jsx new file mode 100644 index 000000000..86f29699c --- /dev/null +++ b/static/src/js/components/Pagination/Pagination.jsx @@ -0,0 +1,10 @@ +import React from 'react' + +const Pagination = () => { + console.log('rendered') + return ( +

Pagination

+ ) +} + +export default Pagination diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index 8a92381b1..f832221bb 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -4,7 +4,6 @@ import BootstrapTable from 'react-bootstrap/Table' import Placeholder from 'react-bootstrap/Placeholder' import For from '../For/For' -import stringifyCircularJSON from '../../utils/stringifyCircularJSON' /** * Table @@ -54,17 +53,17 @@ const Table = ({ ) } else if (data.length > 0) { const rowData = data.map((row) => { - const rowKey = stringifyCircularJSON(row) - + const { cells, key } = row + const rowKey = key return ( { - row.map((cell, index) => { - const cellKey = stringifyCircularJSON(cell) - + cells.map((cell, index) => { + const cellKey = `${rowKey}_${headers[index]}` + const { value } = cell return ( ) }) @@ -102,7 +101,10 @@ Table.propTypes = { headers: PropTypes.arrayOf(PropTypes.string).isRequired, classNames: PropTypes.arrayOf(PropTypes.string), loading: PropTypes.bool, - data: PropTypes.node.isRequired, + data: PropTypes.arrayOf(PropTypes.shape({ + key: PropTypes.string, + cells: PropTypes.arrayOf(PropTypes.shape({})) + })).isRequired, error: PropTypes.string } diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js index ad9305ffa..c20ab43a0 100644 --- a/static/src/js/components/Table/__tests__/Table.test.js +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -12,22 +12,29 @@ const setup = (overrideProps = {}) => { ], loading: false, data: [ - [ - - Row 1 Cell 1 - , - - Row 1 Cell 2 - - ], - [ - - Row 2 Cell 1 - , - - Row 2 Cell 2 - - ] + { + key: 'conceptId001', + cells: [ + { + value: ('Row 1 Cell 1') + }, + { + value: ('Row 1 Cell 2') + } + ] + }, + { + key: 'conceptId002', + cells: [ + { + value: ('Row 2 Cell 1') + }, + { + value: ('Row 2 Cell 2') + } + ] + } + ], ...overrideProps } diff --git a/static/src/js/utils/__tests__/stringifyCircularJSON.test.js b/static/src/js/utils/__tests__/stringifyCircularJSON.test.js deleted file mode 100644 index ded207cf8..000000000 --- a/static/src/js/utils/__tests__/stringifyCircularJSON.test.js +++ /dev/null @@ -1,18 +0,0 @@ -import stringifyCircularJSON from '../stringifyCircularJSON' - -const circularObject = { - name: 'Jane Doe' -} - -circularObject.address = { - city: 'Circuit City', - owner: circularObject -} - -describe('stringifyCircularJSON', () => { - describe('when passed a circular JSON reference', () => { - test('returns a usable string', () => { - expect(stringifyCircularJSON(circularObject)).toEqual('{"name":"Jane Doe","address":{"city":"Circuit City"}}') - }) - }) -}) diff --git a/static/src/js/utils/stringifyCircularJSON.js b/static/src/js/utils/stringifyCircularJSON.js deleted file mode 100644 index 70af4392c..000000000 --- a/static/src/js/utils/stringifyCircularJSON.js +++ /dev/null @@ -1,22 +0,0 @@ -/** - * This returns a usable string when JSON.stringify can't be used due to - * a circular reference. - * @param {Object} obj The circular JSON to be modified - * @returns {string} stringified circular JSON - */ - -const stringifyCircularJSON = (obj) => { - const seen = new WeakSet() - - return JSON.stringify(obj, (k, v) => { - if (v !== null && typeof v === 'object') { - if (seen.has(v)) return - seen.add(v) - } - - // eslint-disable-next-line consistent-return - return v - }) -} - -export default stringifyCircularJSON From 4a6d7ac74d51b42cb50f1ac86572cd05f394fd62 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Wed, 24 Jan 2024 08:30:59 -0700 Subject: [PATCH 06/21] MMT-3561: Adding Pagination to Table --- static.config.json | 3 +- static/src/js/components/Table/Table.jsx | 161 +++++++++++++++++++++-- 2 files changed, 150 insertions(+), 14 deletions(-) diff --git a/static.config.json b/static.config.json index 37b7d07fc..9daef5968 100644 --- a/static.config.json +++ b/static.config.json @@ -3,7 +3,8 @@ "apiHost": "http://localhost:4001/dev", "graphQlHost": "http://localhost:3013/dev/api", "cmrHost": "http://localhost:4000", - "version": "development" + "version": "development", + "defaultPageSize": 2 }, "ummVersions": { "ummC": "1.17.3", diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index f832221bb..52d5f04b6 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -1,10 +1,15 @@ -import React from 'react' +import React, { useEffect, useState } from 'react' import PropTypes from 'prop-types' +import commafy from 'commafy-anything' import BootstrapTable from 'react-bootstrap/Table' +import Pagination from 'react-bootstrap/Pagination' import Placeholder from 'react-bootstrap/Placeholder' +import config from '../../../../../static.config.json' import For from '../For/For' +const { application } = config +const { defaultPageSize } = application /** * Table * @typedef {Object} Table @@ -17,10 +22,133 @@ import For from '../For/For' const Table = ({ headers, classNames, loading, data, error }) => { + const [rowCount, setRowCount] = useState(0) + const [pageNum, setPageNum] = useState(1) + + // Start and end index of the current page of rows + const startIndex = (pageNum - 1) * defaultPageSize + const endIndex = Math.min(startIndex + defaultPageSize, rowCount) + + // Current page of rows + const pagedRows = data ? data.slice(startIndex, endIndex) : [] + + const lastPageNum = parseInt(Math.ceil(rowCount / defaultPageSize), 10) + + // // Does this provider have enough Rows to page? We hide the pagination buttons if not + const hasPages = rowCount > defaultPageSize + + useEffect(() => { + if (!loading) { + setRowCount(data.length); + } + }, [loading, data]); + const renderHeaders = headers.map((header) => ( )) + const defaultPaginationStyles = { + minWidth: '2.5rem', + textAlign: 'center' + } + + const generatePaginationItems = () => { + // Only show 3 pages, the current page and one before or after (within the valid range of pages) + const pages = [pageNum - 1, pageNum, pageNum + 1] + .filter((page) => page >= 1 && page <= lastPageNum) + + const returnItems = [] + + // If the first page is not 1, add the pagination item for page 1 + if (pages[0] !== 1) { + returnItems.push( + setPageNum(1)} + active={pageNum === 1} + data-testid="provider-holdings__pagination-button--1" + style={defaultPaginationStyles} + > + {1} + + ) + + // And if the first page is not 2, add an ellipsis + if (pages[0] !== 2) { + returnItems.push( + + ) + } + } + + pages.forEach((page) => { + returnItems.push( + setPageNum(page)} + active={page === pageNum} + data-testid={`provider-holdings__pagination-button--${page}`} + style={defaultPaginationStyles} + > + {commafy(page, { thousands: true })} + + ) + }) + + // If the last page is not lastPageNum, add the pagination item for the lastPageNum + if (pages[pages.length - 1] !== lastPageNum) { + // And if the last page is not lastPageNum - 1, add an ellipsis + if (pages[pages.length - 1] !== lastPageNum - 1) { + returnItems.push( + + ) + } + + returnItems.push( + setPageNum(lastPageNum)} + active={pageNum === lastPageNum} + data-testid={`provider-holdings__pagination-button--${lastPageNum}`} + style={defaultPaginationStyles} + > + {commafy(lastPageNum, { thousands: true })} + + ) + } + + return returnItems + } + + const pagination = ( + + setPageNum(pageNum - 1)} + disabled={pageNum <= 1} + data-testid="provider-holdings__pagination-button--previous" + style={defaultPaginationStyles} + /> + + {generatePaginationItems()} + + setPageNum(pageNum + 1)} + disabled={pageNum >= lastPageNum} + data-testid="provider-holdings__pagination-button--next" + style={defaultPaginationStyles} + /> + + ) + + const content = [] if (loading) { @@ -51,8 +179,8 @@ const Table = ({ } ) - } else if (data.length > 0) { - const rowData = data.map((row) => { + } else if (rowCount > 0) { + const rowData = pagedRows.map((row) => { const { cells, key } = row const rowKey = key return ( @@ -77,17 +205,24 @@ const Table = ({ content.push() } + console.log(pagedRows) + console.log(`rowCount: ${rowCount}, defaultPageSize: ${defaultPageSize}, hasPages: ${hasPages}`) return ( - - - - {renderHeaders} - - - - {content} - - +
+ +
+ + {renderHeaders} + + + + {content} + + +
+ {hasPages && pagination} +
+ ) } From e003d4ad3c66214f8c788804b0c27f9f5b46666d Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Wed, 24 Jan 2024 09:02:16 -0700 Subject: [PATCH 07/21] MMT-3492: eslint error fixes --- static/src/js/components/DraftList/DraftList.jsx | 2 +- static/src/js/components/Pagination/Pagination.jsx | 10 ---------- static/src/js/components/Table/Table.jsx | 2 ++ 3 files changed, 3 insertions(+), 11 deletions(-) delete mode 100644 static/src/js/components/Pagination/Pagination.jsx diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 511e9c5f9..5a4595848 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -60,7 +60,7 @@ const DraftList = ({ draftType }) => { return ( { - key: `${conceptId}`, + key: conceptId, cells: [ { diff --git a/static/src/js/components/Pagination/Pagination.jsx b/static/src/js/components/Pagination/Pagination.jsx deleted file mode 100644 index 86f29699c..000000000 --- a/static/src/js/components/Pagination/Pagination.jsx +++ /dev/null @@ -1,10 +0,0 @@ -import React from 'react' - -const Pagination = () => { - console.log('rendered') - return ( -

Pagination

- ) -} - -export default Pagination diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index f832221bb..edec3619b 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -55,12 +55,14 @@ const Table = ({ const rowData = data.map((row) => { const { cells, key } = row const rowKey = key + return ( { cells.map((cell, index) => { const cellKey = `${rowKey}_${headers[index]}` const { value } = cell + return (
{header}
+ + + + + + + + + + + + + + + +
+ {cell} +
{error}
- - - - - - - - - - - - - - - -
- - Sample Text - - - More Sample Text - - - Sample Text - - - More Sample Text - {header}
- - - - - - - - - - - - - - - -
+ + + +
- {cell} + {value} {header}
{error}
{value} From 0a2ba219f807ffb398d9473a3846bcd5ea170e90 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Wed, 24 Jan 2024 11:10:07 -0700 Subject: [PATCH 08/21] MMT-3492: reverting changes to ManagePage --- .../src/js/pages/ManagePage/__tests__/ManagePage.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js b/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js index 0b93046f4..aaf699d00 100644 --- a/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js +++ b/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js @@ -39,7 +39,7 @@ const setup = (overrideMocks) => { items: [ { conceptId: 'TD1000000000-TESTPROV', - revisionDate: '2024-01-18T16:42:00.111Z', + revisionDate: '2023-11-30 00:00:00', previewMetadata: { __typename: 'Tool', name: 'Tool Draft 1 Name', @@ -48,7 +48,7 @@ const setup = (overrideMocks) => { }, { conceptId: 'TD1000000001-TESTPROV', - revisionDate: '2024-01-18T16:42:00.111Z', + revisionDate: '2023-11-30 00:00:00', previewMetadata: { __typename: 'Tool', name: 'Tool Draft 2 Name', @@ -167,10 +167,10 @@ describe('ManagePage component', () => { expect(draftLinks.length).toEqual(2) expect(within(draftLinks[0]).queryByText('Tool Draft 1 Name')).toBeInTheDocument() expect(within(draftLinks[0]).queryByText('Tool Draft 1 Long Name')).toBeInTheDocument() - expect(within(draftLinks[0]).queryByText('1/18/2024, 09:42:00')).toBeInTheDocument() + expect(within(draftLinks[0]).queryByText('11/30/2023, 24:00:00')).toBeInTheDocument() expect(within(draftLinks[1]).queryByText('Tool Draft 2 Name')).toBeInTheDocument() expect(within(draftLinks[1]).queryByText('Tool Draft 2 Long Name')).toBeInTheDocument() - expect(within(draftLinks[1]).queryByText('1/18/2024, 09:42:00')).toBeInTheDocument() + expect(within(draftLinks[1]).queryByText('11/30/2023, 24:00:00')).toBeInTheDocument() }) }) }) From 3fa0aecbc401528aa9d61fc1d5a09fab64fcabea Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Thu, 25 Jan 2024 14:18:08 -0700 Subject: [PATCH 09/21] MMT-3561: Adding Pagination to Table --- static.config.json | 2 +- static/src/css/vendor/bootstrap/index.scss | 2 +- .../src/js/components/DraftList/DraftList.jsx | 17 +-- .../js/components/Pagination/Pagination.jsx | 10 -- static/src/js/components/Table/Table.jsx | 115 +++++++++------ .../components/Table/__tests__/Table.test.js | 131 +++++++++++++++++- 6 files changed, 204 insertions(+), 73 deletions(-) delete mode 100644 static/src/js/components/Pagination/Pagination.jsx diff --git a/static.config.json b/static.config.json index 9daef5968..0fe8af417 100644 --- a/static.config.json +++ b/static.config.json @@ -4,7 +4,7 @@ "graphQlHost": "http://localhost:3013/dev/api", "cmrHost": "http://localhost:4000", "version": "development", - "defaultPageSize": 2 + "defaultPageSize": 20 }, "ummVersions": { "ummC": "1.17.3", diff --git a/static/src/css/vendor/bootstrap/index.scss b/static/src/css/vendor/bootstrap/index.scss index 8e81a0731..484111fff 100644 --- a/static/src/css/vendor/bootstrap/index.scss +++ b/static/src/css/vendor/bootstrap/index.scss @@ -47,7 +47,7 @@ // @import "bootstrap/scss/card"; @import "bootstrap/scss/breadcrumb"; @import "bootstrap/scss/accordion"; -// @import "bootstrap/scss/pagination"; +@import "bootstrap/scss/pagination"; @import "bootstrap/scss/badge"; @import "bootstrap/scss/alert"; // @import "bootstrap/scss/progress"; diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 5a4595848..2b54cf49d 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -3,7 +3,6 @@ import PropTypes from 'prop-types' import { Link, useParams } from 'react-router-dom' import { useLazyQuery } from '@apollo/client' import { FaFileDownload } from 'react-icons/fa' -import pluralize from 'pluralize' import Col from 'react-bootstrap/Col' import Placeholder from 'react-bootstrap/Placeholder' import Row from 'react-bootstrap/Row' @@ -135,26 +134,13 @@ const DraftList = ({ draftType }) => { <> { loading - ? ( + && ( ) - : ( - - Showing - {' '} - {count > 0 && 'all'} - {' '} - {count} - {' '} - {draftType} - {' '} - {pluralize('Draft', count)} - - ) } { loading={loading} data={data} error={noDraftsError} + count={count} /> ) diff --git a/static/src/js/components/Pagination/Pagination.jsx b/static/src/js/components/Pagination/Pagination.jsx deleted file mode 100644 index 86f29699c..000000000 --- a/static/src/js/components/Pagination/Pagination.jsx +++ /dev/null @@ -1,10 +0,0 @@ -import React from 'react' - -const Pagination = () => { - console.log('rendered') - return ( -

Pagination

- ) -} - -export default Pagination diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index 52df26236..df58898c4 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -1,8 +1,10 @@ import React, { useEffect, useState } from 'react' import PropTypes from 'prop-types' -import commafy from 'commafy-anything' import BootstrapTable from 'react-bootstrap/Table' import Pagination from 'react-bootstrap/Pagination' +import Row from 'react-bootstrap/Row' +import Col from 'react-bootstrap/Col' +import Container from 'react-bootstrap/Container' import Placeholder from 'react-bootstrap/Placeholder' import config from '../../../../../static.config.json' @@ -20,10 +22,11 @@ const { defaultPageSize } = application */ const Table = ({ - headers, classNames, loading, data, error + headers, classNames, loading, data, error, count }) => { const [rowCount, setRowCount] = useState(0) const [pageNum, setPageNum] = useState(1) + const [currentCount, setCurrentCount] = useState(defaultPageSize) // Start and end index of the current page of rows const startIndex = (pageNum - 1) * defaultPageSize @@ -37,6 +40,11 @@ const Table = ({ // // Does this provider have enough Rows to page? We hide the pagination buttons if not const hasPages = rowCount > defaultPageSize + const defaultPaginationStyles = { + minWidth: '2.5rem', + textAlign: 'center' + } + useEffect(() => { if (!loading) { setRowCount(data.length) @@ -47,9 +55,9 @@ const Table = ({ )) - const defaultPaginationStyles = { - minWidth: '2.5rem', - textAlign: 'center' + const handleItemClick = (currentPage) => { + setPageNum(currentPage) + setCurrentCount(currentPage * defaultPageSize) } const generatePaginationItems = () => { @@ -64,9 +72,8 @@ const Table = ({ returnItems.push( setPageNum(1)} + onClick={() => handleItemClick(1)} active={pageNum === 1} - data-testid="provider-holdings__pagination-button--1" style={defaultPaginationStyles} > {1} @@ -79,7 +86,6 @@ const Table = ({ ) } @@ -89,12 +95,11 @@ const Table = ({ returnItems.push( setPageNum(page)} + onClick={() => handleItemClick(page)} active={page === pageNum} - data-testid={`provider-holdings__pagination-button--${page}`} style={defaultPaginationStyles} > - {commafy(page, { thousands: true })} + {page} ) }) @@ -107,7 +112,6 @@ const Table = ({ ) } @@ -115,12 +119,11 @@ const Table = ({ returnItems.push( setPageNum(lastPageNum)} + onClick={() => handleItemClick(lastPageNum)} active={pageNum === lastPageNum} - data-testid={`provider-holdings__pagination-button--${lastPageNum}`} style={defaultPaginationStyles} > - {commafy(lastPageNum, { thousands: true })} + {lastPageNum} ) } @@ -128,22 +131,26 @@ const Table = ({ return returnItems } + const handlePageChange = (direction) => { + const newPageNum = pageNum + direction + const newCurrentCount = currentCount + direction * defaultPageSize + + setPageNum(newPageNum) + setCurrentCount(newCurrentCount) + } + const pagination = ( setPageNum(pageNum - 1)} - disabled={pageNum <= 1} - data-testid="provider-holdings__pagination-button--previous" - style={defaultPaginationStyles} + disabled={pageNum === 1} + onClick={() => handlePageChange(-1)} /> {generatePaginationItems()} setPageNum(pageNum + 1)} + onClick={() => handlePageChange(1)} disabled={pageNum >= lastPageNum} - data-testid="provider-holdings__pagination-button--next" - style={defaultPaginationStyles} /> ) @@ -206,32 +213,55 @@ const Table = ({ content.push() } - console.log(pagedRows) - console.log(`rowCount: ${rowCount}, defaultPageSize: ${defaultPageSize}, hasPages: ${hasPages}`) - return ( -
- -
- - {renderHeaders} - - - - {content} - - -
- {hasPages && pagination} -
- + + + + + Showing + {' '} + {count > 0 && (currentCount - defaultPageSize)} + - + {currentCount} + {' '} + of + {' '} + {count} + {' '} + Results + + + + + + + + + {renderHeaders} + + + + {content} + + + + + + +
+ {hasPages && pagination} +
+ + + ) } Table.defaultProps = { classNames: [], loading: null, - error: 'No Data to Display' + error: 'No Data to Display', + count: null } Table.propTypes = { @@ -242,7 +272,8 @@ Table.propTypes = { key: PropTypes.string, cells: PropTypes.arrayOf(PropTypes.shape({})) })).isRequired, - error: PropTypes.string + error: PropTypes.string, + count: PropTypes.number } export default Table diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js index c20ab43a0..bfaa22c3f 100644 --- a/static/src/js/components/Table/__tests__/Table.test.js +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -1,9 +1,17 @@ import React from 'react' -import { render, screen } from '@testing-library/react' +import { + render, + screen, + fireEvent +} from '@testing-library/react' import userEvent from '@testing-library/user-event' import Table from '../Table' +jest.mock('../../../../../../static.config.json', () => ({ + application: { defaultPageSize: 2 } +})) + const setup = (overrideProps = {}) => { const props = { headers: [ @@ -33,9 +41,64 @@ const setup = (overrideProps = {}) => { value: ('Row 2 Cell 2') } ] + }, + { + key: 'conceptId003', + cells: [ + { + value: ('Row 3 Cell 1') + }, + { + value: ('Row 3 Cell 2') + } + ] + }, + { + key: 'conceptId004', + cells: [ + { + value: ('Row 4 Cell 1') + }, + { + value: ('Row 4 Cell 2') + } + ] + }, + { + key: 'conceptId005', + cells: [ + { + value: ('Row 5 Cell 1') + }, + { + value: ('Row 5 Cell 2') + } + ] + }, + { + key: 'conceptId006', + cells: [ + { + value: ('Row 6 Cell 1') + }, + { + value: ('Row 6 Cell 2') + } + ] + }, + { + key: 'conceptId007', + cells: [ + { + value: ('Row 7 Cell 1') + }, + { + value: ('Row 7 Cell 2') + } + ] } - ], + count: 7, ...overrideProps } @@ -50,13 +113,73 @@ const setup = (overrideProps = {}) => { } describe('Table', () => { - describe('when the table component is passed markup and data', () => { - test('renders filled table', () => { + describe('when the table component is passed markup and data that is more than the defaultPageSize', () => { + test('renders filled table with Pagination', () => { setup() + // Checking all pages exist upon loading. Previous and 1 are not clickable buttons. + expect(screen.queryByRole('button', { name: 'Previous' })).not.toBeInTheDocument() + expect(screen.getByText('Previous')).toBeInTheDocument() + expect(screen.queryByRole('button', { name: '1' })).not.toBeInTheDocument() + expect(screen.getByText('1')).toBeInTheDocument() expect(screen.getByText('column 1')).toBeInTheDocument() expect(screen.getByRole('table')).toBeInTheDocument() expect(screen.getByText('Row 2 Cell 2')).toBeInTheDocument() + expect(screen.queryByRole('button', { name: '2' })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Next' })).toBeInTheDocument() + expect(screen.getByText('More')).toBeInTheDocument() + expect(screen.getByText('Showing 0-2 of 7 Results')).toBeInTheDocument() + + // Check the next button works + fireEvent.click(screen.getByRole('button', { name: 'Next' })) + expect(screen.queryByRole('button', { name: 'Previous' })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: '1' })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: '2' })).not.toBeInTheDocument() + expect(screen.getByText('2')).toBeInTheDocument() + expect(screen.getByText('column 1')).toBeInTheDocument() + expect(screen.getByRole('table')).toBeInTheDocument() + expect(screen.getByText('Row 3 Cell 1')).toBeInTheDocument() + expect(screen.getByText('Showing 2-4 of 7 Results')).toBeInTheDocument() + + // Check individual buttons work + fireEvent.click(screen.getByRole('button', { name: '3' })) + expect(screen.queryByRole('button', { name: '2' })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: '3' })).not.toBeInTheDocument() + expect(screen.getByText('3')).toBeInTheDocument() + expect(screen.getByText('Showing 4-6 of 7 Results')).toBeInTheDocument() + + // Check Previous Button Works + fireEvent.click(screen.getByRole('button', { name: 'Previous' })) + expect(screen.queryByRole('button', { name: 'Previous' })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: '1' })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: '2' })).not.toBeInTheDocument() + expect(screen.getByText('2')).toBeInTheDocument() + expect(screen.getByText('Showing 2-4 of 7 Results')).toBeInTheDocument() + }) + }) + + describe('when the table compoent is passed markup and data that is less than the defaultPageSize', () => { + test('renders filled table without Pagination', () => { + setup({ + data: [ + { + key: 'conceptId001', + cells: [ + { + value: ('Row 1 Cell 1') + }, + { + value: ('Row 1 Cell 2') + } + ] + } + ] + }) + + expect(screen.getByText('column 1')).toBeInTheDocument() + expect(screen.getByRole('table')).toBeInTheDocument() + expect(screen.getByText('Row 1 Cell 2')).toBeInTheDocument() + expect(screen.queryByRole('button')).not.toBeInTheDocument() }) }) From 3a1d179dc8af58bfac036483de942821a5fb64cb Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Fri, 26 Jan 2024 09:18:29 -0700 Subject: [PATCH 10/21] MMT-3561: Adding tests --- .../components/Table/__tests__/Table.test.js | 117 ++++++++++++++---- 1 file changed, 92 insertions(+), 25 deletions(-) diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js index bfaa22c3f..c933a5c15 100644 --- a/static/src/js/components/Table/__tests__/Table.test.js +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -96,9 +96,86 @@ const setup = (overrideProps = {}) => { value: ('Row 7 Cell 2') } ] + }, + { + key: 'conceptId001b', + cells: [ + { + value: ('Row 1 Cell 1') + }, + { + value: ('Row 1 Cell 2') + } + ] + }, + { + key: 'conceptId002b', + cells: [ + { + value: ('Row 2 Cell 1') + }, + { + value: ('Row 2 Cell 2') + } + ] + }, + { + key: 'conceptId003b', + cells: [ + { + value: ('Row 3 Cell 1') + }, + { + value: ('Row 3 Cell 2') + } + ] + }, + { + key: 'conceptId004b', + cells: [ + { + value: ('Row 4 Cell 1') + }, + { + value: ('Row 4 Cell 2') + } + ] + }, + { + key: 'conceptId005b', + cells: [ + { + value: ('Row 5 Cell 1') + }, + { + value: ('Row 5 Cell 2') + } + ] + }, + { + key: 'conceptId006b', + cells: [ + { + value: ('Row 6 Cell 1') + }, + { + value: ('Row 6 Cell 2') + } + ] + }, + { + key: 'conceptId007b', + cells: [ + { + value: ('Row 7 Cell 1') + }, + { + value: ('Row 7 Cell 2') + } + ] } ], - count: 7, + count: 14, ...overrideProps } @@ -128,33 +205,26 @@ describe('Table', () => { expect(screen.queryByRole('button', { name: '2' })).toBeInTheDocument() expect(screen.queryByRole('button', { name: 'Next' })).toBeInTheDocument() expect(screen.getByText('More')).toBeInTheDocument() - expect(screen.getByText('Showing 0-2 of 7 Results')).toBeInTheDocument() + expect(screen.getByText('Showing 0-2 of 14 Results')).toBeInTheDocument() + + // Check individual buttons work + fireEvent.click(screen.getByRole('button', { name: '2' })) // Check the next button works fireEvent.click(screen.getByRole('button', { name: 'Next' })) - expect(screen.queryByRole('button', { name: 'Previous' })).toBeInTheDocument() - expect(screen.queryByRole('button', { name: '1' })).toBeInTheDocument() - expect(screen.queryByRole('button', { name: '2' })).not.toBeInTheDocument() - expect(screen.getByText('2')).toBeInTheDocument() - expect(screen.getByText('column 1')).toBeInTheDocument() - expect(screen.getByRole('table')).toBeInTheDocument() - expect(screen.getByText('Row 3 Cell 1')).toBeInTheDocument() - expect(screen.getByText('Showing 2-4 of 7 Results')).toBeInTheDocument() - // Check individual buttons work - fireEvent.click(screen.getByRole('button', { name: '3' })) - expect(screen.queryByRole('button', { name: '2' })).toBeInTheDocument() - expect(screen.queryByRole('button', { name: '3' })).not.toBeInTheDocument() - expect(screen.getByText('3')).toBeInTheDocument() - expect(screen.getByText('Showing 4-6 of 7 Results')).toBeInTheDocument() + // Check pages[0] always stays at 1 and two ellipsis render + fireEvent.click(screen.getByRole('button', { name: 'Next' })) + expect(screen.queryAllByText('More')).toHaveLength(2) + + // Make sure onclick for pages[0] function above works + fireEvent.click(screen.getByRole('button', { name: '1' })) - // Check Previous Button Works + // Click on last page + fireEvent.click(screen.getByRole('button', { name: '7' })) + + // Click on Previous Page fireEvent.click(screen.getByRole('button', { name: 'Previous' })) - expect(screen.queryByRole('button', { name: 'Previous' })).toBeInTheDocument() - expect(screen.queryByRole('button', { name: '1' })).toBeInTheDocument() - expect(screen.queryByRole('button', { name: '2' })).not.toBeInTheDocument() - expect(screen.getByText('2')).toBeInTheDocument() - expect(screen.getByText('Showing 2-4 of 7 Results')).toBeInTheDocument() }) }) @@ -177,8 +247,6 @@ describe('Table', () => { }) expect(screen.getByText('column 1')).toBeInTheDocument() - expect(screen.getByRole('table')).toBeInTheDocument() - expect(screen.getByText('Row 1 Cell 2')).toBeInTheDocument() expect(screen.queryByRole('button')).not.toBeInTheDocument() }) }) @@ -201,7 +269,6 @@ describe('Table', () => { }) expect(screen.getByRole('table', { className: 'table table-striped' })).toBeInTheDocument() - expect(screen.queryAllByRole('cell', { className: 'col-md-4' })).toHaveLength(4) }) }) }) From d8de076c8f7ebfbb5ca27109e3272efaac605eb8 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Fri, 26 Jan 2024 09:49:19 -0700 Subject: [PATCH 11/21] MMT-3492: Fixing shortName and title --- static/src/js/components/DraftList/DraftList.jsx | 11 ++++++++--- static/src/js/pages/ManagePage/ManagePage.jsx | 8 +++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 5a4595848..8b35e1580 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -55,7 +55,12 @@ const DraftList = ({ draftType }) => { // Building a Table using Data in items const data = (items.map((item) => { - const { conceptId, revisionDate, previewMetadata: { name, longName } } = item + const { + conceptId, revisionDate, previewMetadata: { + name, longName, shortName, title + } + } = item + const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` return ( @@ -67,14 +72,14 @@ const DraftList = ({ draftType }) => { value: ( - {name || ''} + {name || shortName || ''} ) }, { value: ( - longName || '' + longName || title || '' ) }, { diff --git a/static/src/js/pages/ManagePage/ManagePage.jsx b/static/src/js/pages/ManagePage/ManagePage.jsx index d01943719..69a49be8d 100644 --- a/static/src/js/pages/ManagePage/ManagePage.jsx +++ b/static/src/js/pages/ManagePage/ManagePage.jsx @@ -122,7 +122,9 @@ const ManagePage = () => { conceptId, previewMetadata: { longName, - name + name, + shortName, + title }, revisionDate }) => ( @@ -134,10 +136,10 @@ const ManagePage = () => {
{new Date(revisionDate).toLocaleString('en-US', { hour12: false })} | - {name || ''} + {name || shortName || ''}
- {longName || ''} + {longName || title || ''}
From 368a9201ce24d10089900e8fc7387dc5b55d8fc1 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Fri, 26 Jan 2024 09:56:41 -0700 Subject: [PATCH 12/21] MMT-3561: Fixing conflict --- static.config.json | 1 + 1 file changed, 1 insertion(+) diff --git a/static.config.json b/static.config.json index cb69078e2..25d6dac7f 100644 --- a/static.config.json +++ b/static.config.json @@ -4,6 +4,7 @@ "graphQlHost": "http://localhost:3013/dev/api", "cmrHost": "http://localhost:4000", "version": "development", + "jwtSecret": "jwt_secret", "defaultPageSize": 20 }, "ummVersions": { From c059d731a61eaa665766705fba9fb8dc48d198c6 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Fri, 26 Jan 2024 14:18:06 -0700 Subject: [PATCH 13/21] MMT-3561: Saving progress on limit and offset --- static.config.json | 3 +- .../src/js/components/DraftList/DraftList.jsx | 14 +++++- .../js/components/Pagination/Pagination.jsx | 0 static/src/js/components/Table/Table.jsx | 44 +++++++------------ static/src/js/hooks/useDraftsQuery.js | 3 +- 5 files changed, 32 insertions(+), 32 deletions(-) create mode 100644 static/src/js/components/Pagination/Pagination.jsx diff --git a/static.config.json b/static.config.json index 803434f64..d609d790b 100644 --- a/static.config.json +++ b/static.config.json @@ -5,8 +5,7 @@ "graphQlHost": "http://localhost:3013/dev/api", "cmrHost": "http://localhost:4000", "version": "development", - "jwtSecret": "jwt_secret", - "defaultPageSize": 20 + "jwtSecret": "jwt_secret" }, "ummVersions": { diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 9e3e6fa82..613a9f225 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, { useState } from 'react' import PropTypes from 'prop-types' import { Link, useParams } from 'react-router-dom' import { useLazyQuery } from '@apollo/client' @@ -25,7 +25,14 @@ const DraftList = ({ draftType }) => { const { providerId } = user const { draftType: paramDraftType } = useParams() - const { drafts, error, loading } = useDraftsQuery({ draftType }) + const [offset, setOffset] = useState(0) + const limit = 10 + + const { drafts, error, loading } = useDraftsQuery({ + draftType, + offset, + limit + }) const { count, items = [] } = drafts const noDraftsError = `No ${draftType} drafts exist for the provider ${providerId}` @@ -154,6 +161,9 @@ const DraftList = ({ draftType }) => { data={data} error={noDraftsError} count={count} + setOffset={setOffset} + limit={limit} + offset={offset} /> ) diff --git a/static/src/js/components/Pagination/Pagination.jsx b/static/src/js/components/Pagination/Pagination.jsx new file mode 100644 index 000000000..e69de29bb diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index df58898c4..6a6711ff7 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react' +import React, { useState } from 'react' import PropTypes from 'prop-types' import BootstrapTable from 'react-bootstrap/Table' import Pagination from 'react-bootstrap/Pagination' @@ -6,12 +6,8 @@ import Row from 'react-bootstrap/Row' import Col from 'react-bootstrap/Col' import Container from 'react-bootstrap/Container' import Placeholder from 'react-bootstrap/Placeholder' -import config from '../../../../../static.config.json' import For from '../For/For' - -const { application } = config -const { defaultPageSize } = application /** * Table * @typedef {Object} Table @@ -22,42 +18,32 @@ const { defaultPageSize } = application */ const Table = ({ - headers, classNames, loading, data, error, count + headers, classNames, loading, data, error, count, setOffset, limit, offset }) => { - const [rowCount, setRowCount] = useState(0) const [pageNum, setPageNum] = useState(1) - const [currentCount, setCurrentCount] = useState(defaultPageSize) - - // Start and end index of the current page of rows - const startIndex = (pageNum - 1) * defaultPageSize - const endIndex = Math.min(startIndex + defaultPageSize, rowCount) + const [currentCount, setCurrentCount] = useState(limit) // Current page of rows - const pagedRows = data ? data.slice(startIndex, endIndex) : [] + const pagedRows = data - const lastPageNum = parseInt(Math.ceil(rowCount / defaultPageSize), 10) + const lastPageNum = parseInt(Math.ceil(count / limit), 10) // // Does this provider have enough Rows to page? We hide the pagination buttons if not - const hasPages = rowCount > defaultPageSize + const hasPages = count > limit const defaultPaginationStyles = { minWidth: '2.5rem', textAlign: 'center' } - useEffect(() => { - if (!loading) { - setRowCount(data.length) - } - }, [loading, data]) - const renderHeaders = headers.map((header) => ( )) const handleItemClick = (currentPage) => { setPageNum(currentPage) - setCurrentCount(currentPage * defaultPageSize) + setCurrentCount(currentPage * limit) + setOffset((currentPage - 1) * limit) } const generatePaginationItems = () => { @@ -133,10 +119,11 @@ const Table = ({ const handlePageChange = (direction) => { const newPageNum = pageNum + direction - const newCurrentCount = currentCount + direction * defaultPageSize + const newCurrentCount = currentCount + direction * limit setPageNum(newPageNum) setCurrentCount(newCurrentCount) + setOffset(newPageNum * limit) } const pagination = ( @@ -185,7 +172,7 @@ const Table = ({ } ) - } else if (rowCount > 0) { + } else if (count > 0) { const rowData = pagedRows.map((row) => { const { cells, key } = row const rowKey = key @@ -220,9 +207,9 @@ const Table = ({ Showing {' '} - {count > 0 && (currentCount - defaultPageSize)} + {count > 0 && offset} - - {currentCount} + {data.length < currentCount ? offset + data.length : currentCount} {' '} of {' '} @@ -273,7 +260,10 @@ Table.propTypes = { cells: PropTypes.arrayOf(PropTypes.shape({})) })).isRequired, error: PropTypes.string, - count: PropTypes.number + count: PropTypes.number, + offset: PropTypes.number.isRequired, + setOffset: PropTypes.func.isRequired, + limit: PropTypes.number.isRequired } export default Table diff --git a/static/src/js/hooks/useDraftsQuery.js b/static/src/js/hooks/useDraftsQuery.js index c1674c0bc..f262bb1ba 100644 --- a/static/src/js/hooks/useDraftsQuery.js +++ b/static/src/js/hooks/useDraftsQuery.js @@ -6,7 +6,7 @@ import useAppContext from './useAppContext' import conceptTypeDraftsQueries from '../constants/conceptTypeDraftsQueries' -const useDraftsQuery = ({ draftType, limit }) => { +const useDraftsQuery = ({ draftType, limit, offset }) => { const { setDraft, setOriginalDraft, @@ -25,6 +25,7 @@ const useDraftsQuery = ({ draftType, limit }) => { variables: { params: { limit, + offset, conceptType: draftType, provider: providerId, sortKey: ['-revision_date'] From 9fb74c907b6e0012cbf899449467ac1fd3618dee Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Mon, 29 Jan 2024 13:55:23 -0700 Subject: [PATCH 14/21] MMT-3561: Adding offset and limit --- .../src/js/components/DraftList/DraftList.jsx | 2 +- .../js/components/Pagination/Pagination.jsx | 144 ++++++++++++++++++ static/src/js/components/Table/Table.jsx | 140 +++-------------- 3 files changed, 162 insertions(+), 124 deletions(-) diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 613a9f225..681bfa687 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -26,7 +26,7 @@ const DraftList = ({ draftType }) => { const { draftType: paramDraftType } = useParams() const [offset, setOffset] = useState(0) - const limit = 10 + const limit = 20 const { drafts, error, loading } = useDraftsQuery({ draftType, diff --git a/static/src/js/components/Pagination/Pagination.jsx b/static/src/js/components/Pagination/Pagination.jsx index e69de29bb..1ce276dd1 100644 --- a/static/src/js/components/Pagination/Pagination.jsx +++ b/static/src/js/components/Pagination/Pagination.jsx @@ -0,0 +1,144 @@ +import React, { useState } from 'react' +import PropTypes from 'prop-types' +import Pagination from 'react-bootstrap/Pagination' +import Row from 'react-bootstrap/Row' +import Col from 'react-bootstrap/Col' + +/** + * Table + * @typedef {Object} PaginationComponent + * @property {Number} limit A number that is set in parent element of table + * @property {Number} count A number that indicates how many results are in the total query + * @property {function} setOffset A function that resets the offset of results to come back + */ + +const PaginationComponent = ({ + limit, count, setoffset +}) => { + const [pageNum, setPageNum] = useState(1) + + const lastPageNum = parseInt(Math.ceil(count / limit), 10) + + const defaultPaginationStyles = { + minWidth: '2.5rem', + textAlign: 'center' + } + + const handleItemClick = (currentPage) => { + setPageNum(currentPage) + setoffset((currentPage - 1) * limit) + } + + const generatePaginationItems = () => { + // Only show 3 pages, the current page and one before or after (within the valid range of pages) + const pages = [pageNum - 1, pageNum, pageNum + 1] + .filter((page) => page >= 1 && page <= lastPageNum) + + const returnItems = [] + + // If the first page is not 1, add the pagination item for page 1 + if (pages[0] !== 1) { + returnItems.push( + handleItemClick(1)} + active={pageNum === 1} + style={defaultPaginationStyles} + > + {1} + + ) + + // And if the first page is not 2, add an ellipsis + if (pages[0] !== 2) { + returnItems.push( + + ) + } + } + + pages.forEach((page) => { + returnItems.push( + handleItemClick(page)} + active={page === pageNum} + style={defaultPaginationStyles} + > + {page} + + ) + }) + + // If the last page is not lastPageNum, add the pagination item for the lastPageNum + if (pages[pages.length - 1] !== lastPageNum) { + // And if the last page is not lastPageNum - 1, add an ellipsis + if (pages[pages.length - 1] !== lastPageNum - 1) { + returnItems.push( + + ) + } + + returnItems.push( + handleItemClick(lastPageNum)} + active={pageNum === lastPageNum} + style={defaultPaginationStyles} + > + {lastPageNum} + + ) + } + + return returnItems + } + + const handlePageChange = (direction) => { + const newCurrentPage = pageNum + direction + + setPageNum(newCurrentPage) + setoffset((newCurrentPage - 1) * limit) + } + + return ( + + + {/*

Pagination

*/} +
+ + handlePageChange(-1)} + /> + + {generatePaginationItems()} + + handlePageChange(1)} + disabled={pageNum >= lastPageNum} + /> + +
+ + + ) +} + +PaginationComponent.defaultProps = { + count: null +} + +PaginationComponent.propTypes = { + setoffset: PropTypes.func.isRequired, + limit: PropTypes.number.isRequired, + count: PropTypes.number +} + +export default PaginationComponent diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index 6a6711ff7..e5890939a 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -1,11 +1,11 @@ -import React, { useState } from 'react' +import React from 'react' import PropTypes from 'prop-types' import BootstrapTable from 'react-bootstrap/Table' -import Pagination from 'react-bootstrap/Pagination' import Row from 'react-bootstrap/Row' import Col from 'react-bootstrap/Col' import Container from 'react-bootstrap/Container' import Placeholder from 'react-bootstrap/Placeholder' +import PaginationComponent from '../Pagination/Pagination' import For from '../For/For' /** @@ -20,128 +20,16 @@ import For from '../For/For' const Table = ({ headers, classNames, loading, data, error, count, setOffset, limit, offset }) => { - const [pageNum, setPageNum] = useState(1) - const [currentCount, setCurrentCount] = useState(limit) - // Current page of rows const pagedRows = data - const lastPageNum = parseInt(Math.ceil(count / limit), 10) - - // // Does this provider have enough Rows to page? We hide the pagination buttons if not + // Does this provider have enough Rows to page? We hide the pagination buttons if not const hasPages = count > limit - const defaultPaginationStyles = { - minWidth: '2.5rem', - textAlign: 'center' - } - const renderHeaders = headers.map((header) => ( )) - const handleItemClick = (currentPage) => { - setPageNum(currentPage) - setCurrentCount(currentPage * limit) - setOffset((currentPage - 1) * limit) - } - - const generatePaginationItems = () => { - // Only show 3 pages, the current page and one before or after (within the valid range of pages) - const pages = [pageNum - 1, pageNum, pageNum + 1] - .filter((page) => page >= 1 && page <= lastPageNum) - - const returnItems = [] - - // If the first page is not 1, add the pagination item for page 1 - if (pages[0] !== 1) { - returnItems.push( - handleItemClick(1)} - active={pageNum === 1} - style={defaultPaginationStyles} - > - {1} - - ) - - // And if the first page is not 2, add an ellipsis - if (pages[0] !== 2) { - returnItems.push( - - ) - } - } - - pages.forEach((page) => { - returnItems.push( - handleItemClick(page)} - active={page === pageNum} - style={defaultPaginationStyles} - > - {page} - - ) - }) - - // If the last page is not lastPageNum, add the pagination item for the lastPageNum - if (pages[pages.length - 1] !== lastPageNum) { - // And if the last page is not lastPageNum - 1, add an ellipsis - if (pages[pages.length - 1] !== lastPageNum - 1) { - returnItems.push( - - ) - } - - returnItems.push( - handleItemClick(lastPageNum)} - active={pageNum === lastPageNum} - style={defaultPaginationStyles} - > - {lastPageNum} - - ) - } - - return returnItems - } - - const handlePageChange = (direction) => { - const newPageNum = pageNum + direction - const newCurrentCount = currentCount + direction * limit - - setPageNum(newPageNum) - setCurrentCount(newCurrentCount) - setOffset(newPageNum * limit) - } - - const pagination = ( - - handlePageChange(-1)} - /> - - {generatePaginationItems()} - - handlePageChange(1)} - disabled={pageNum >= lastPageNum} - /> - - ) - const content = [] if (loading) { @@ -209,7 +97,7 @@ const Table = ({ {' '} {count > 0 && offset} - - {data.length < currentCount ? offset + data.length : currentCount} + {data.length === limit ? offset + limit : offset + data.length} {' '} of {' '} @@ -218,6 +106,19 @@ const Table = ({ Results + +
+ { + hasPages && ( + + ) + } +
+ @@ -233,13 +134,6 @@ const Table = ({ - - -
- {hasPages && pagination} -
- - ) } From 607d34b313ae498483ee57acfa73ac40fac6da6f Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Wed, 31 Jan 2024 10:35:57 -0700 Subject: [PATCH 15/21] MMT-3492: Editing table and ManagePage --- .../src/js/components/DraftList/DraftList.jsx | 31 +++++++--- .../operations/queries/getCollectionDrafts.js | 7 +-- static/src/js/pages/ManagePage/ManagePage.jsx | 59 +++++++++++-------- 3 files changed, 61 insertions(+), 36 deletions(-) diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 8b35e1580..491679e72 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -55,11 +55,28 @@ const DraftList = ({ draftType }) => { // Building a Table using Data in items const data = (items.map((item) => { - const { - conceptId, revisionDate, previewMetadata: { - name, longName, shortName, title - } - } = item + let conceptId + let revisionDate + let title + let subTitle + + if (item.ummMetadata) { + const { conceptId: cId, revisionDate: rDate, ummMetadata } = item + const { ShortName, EntryTitle } = ummMetadata + + conceptId = cId + revisionDate = rDate + title = ShortName + subTitle = EntryTitle + } else { + const { conceptId: cId, revisionDate: rDate, previewMetadata } = item + const { name, longName } = previewMetadata + + conceptId = cId + revisionDate = rDate + title = name + subTitle = longName + } const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` @@ -72,14 +89,14 @@ const DraftList = ({ draftType }) => { value: ( - {name || shortName || ''} + {title || ''} ) }, { value: ( - longName || title || '' + subTitle || '' ) }, { diff --git a/static/src/js/operations/queries/getCollectionDrafts.js b/static/src/js/operations/queries/getCollectionDrafts.js index e69f1aa10..5e9020b6b 100644 --- a/static/src/js/operations/queries/getCollectionDrafts.js +++ b/static/src/js/operations/queries/getCollectionDrafts.js @@ -8,12 +8,7 @@ export const GET_COLLECTION_DRAFTS = gql` items { conceptId revisionDate - previewMetadata { - ... on Collection { - shortName - title - } - } + ummMetadata } } } diff --git a/static/src/js/pages/ManagePage/ManagePage.jsx b/static/src/js/pages/ManagePage/ManagePage.jsx index 69a49be8d..46cc28950 100644 --- a/static/src/js/pages/ManagePage/ManagePage.jsx +++ b/static/src/js/pages/ManagePage/ManagePage.jsx @@ -120,30 +120,43 @@ const ManagePage = () => { { ({ conceptId, - previewMetadata: { - longName, - name, - shortName, - title - }, + previewMetadata, + ummMetadata, revisionDate - }) => ( - - -
- {new Date(revisionDate).toLocaleString('en-US', { hour12: false })} - | - {name || shortName || ''} -
-
- {longName || title || ''} -
- - - ) + }) => { + let title + let subTitle + + if (ummMetadata) { + const { ShortName, EntryTitle } = ummMetadata + + title = ShortName + subTitle = EntryTitle + } else { + const { name, longName } = previewMetadata + + title = name + subTitle = longName + } + + return ( + + +
+ {new Date(revisionDate).toLocaleString('en-US', { hour12: false })} + | + {title || ''} +
+
+ {subTitle || ''} +
+ + + ) + } } ) From 8cad2e657a5a943e8719e71c88b76f51c46e7e56 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Wed, 31 Jan 2024 11:44:05 -0700 Subject: [PATCH 16/21] MMT-3492: Trying to get tests to run on github --- static/src/js/pages/ManagePage/ManagePage.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/static/src/js/pages/ManagePage/ManagePage.jsx b/static/src/js/pages/ManagePage/ManagePage.jsx index 46cc28950..f09f1d360 100644 --- a/static/src/js/pages/ManagePage/ManagePage.jsx +++ b/static/src/js/pages/ManagePage/ManagePage.jsx @@ -20,7 +20,6 @@ import './ManagePage.scss' /** * Renders a `ManagePage` component - * * @component * @example * return ( From 5190f2f855a1f568d306cccadf064e029917e44f Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Thu, 1 Feb 2024 09:10:43 -0700 Subject: [PATCH 17/21] MMT-3492: Addressing comments --- static/src/js/components/DraftList/DraftList.jsx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 491679e72..488ba09fa 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -55,25 +55,21 @@ const DraftList = ({ draftType }) => { // Building a Table using Data in items const data = (items.map((item) => { - let conceptId - let revisionDate + const { + conceptId, revisionDate, ummMetadata, previewMetadata + } = item + let title let subTitle - if (item.ummMetadata) { - const { conceptId: cId, revisionDate: rDate, ummMetadata } = item + if (ummMetadata) { const { ShortName, EntryTitle } = ummMetadata - conceptId = cId - revisionDate = rDate title = ShortName subTitle = EntryTitle } else { - const { conceptId: cId, revisionDate: rDate, previewMetadata } = item const { name, longName } = previewMetadata - conceptId = cId - revisionDate = rDate title = name subTitle = longName } From 17c78c5befed3ae18360ed5b87d13d8272df4755 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Thu, 1 Feb 2024 09:47:37 -0700 Subject: [PATCH 18/21] MMT-3492: Changing all conceptTypes to ummMetadata --- .../src/js/components/DraftList/DraftList.jsx | 24 +++++-------------- .../DraftList/__tests__/DraftList.test.js | 13 ++++++---- .../js/operations/queries/getServiceDrafts.js | 1 + .../js/operations/queries/getToolDrafts.js | 1 + .../operations/queries/getVariableDrafts.js | 1 + static/src/js/pages/ManagePage/ManagePage.jsx | 22 ++++------------- .../ManagePage/__tests__/ManagePage.test.js | 22 ++++++++++------- 7 files changed, 37 insertions(+), 47 deletions(-) diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 488ba09fa..8d848f2f6 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -56,23 +56,11 @@ const DraftList = ({ draftType }) => { // Building a Table using Data in items const data = (items.map((item) => { const { - conceptId, revisionDate, ummMetadata, previewMetadata + conceptId, revisionDate, ummMetadata } = item - - let title - let subTitle - - if (ummMetadata) { - const { ShortName, EntryTitle } = ummMetadata - - title = ShortName - subTitle = EntryTitle - } else { - const { name, longName } = previewMetadata - - title = name - subTitle = longName - } + const { + ShortName, EntryTitle, Name, LongName + } = ummMetadata || {} const draftLink = `/drafts/${`${paramDraftType}`}/${conceptId}` @@ -85,14 +73,14 @@ const DraftList = ({ draftType }) => { value: ( - {title || ''} + {Name || ShortName || ''} ) }, { value: ( - subTitle || '' + LongName || EntryTitle || '' ) }, { diff --git a/static/src/js/components/DraftList/__tests__/DraftList.test.js b/static/src/js/components/DraftList/__tests__/DraftList.test.js index 86b22c445..9e99732ae 100644 --- a/static/src/js/components/DraftList/__tests__/DraftList.test.js +++ b/static/src/js/components/DraftList/__tests__/DraftList.test.js @@ -97,9 +97,11 @@ describe('DraftList', () => { { conceptId: 'TD1200000092-MMT_2', revisionDate: '2023-12-08T17:56:09.385Z', + ummMetadata: { + Name: 'Tool TD1200000092 short name', + LongName: 'Tool TD1200000092 long name' + }, previewMetadata: { - name: 'Tool TD1200000092 short name', - longName: 'Tool TD1200000092 long name', __typename: 'Tool' }, __typename: 'Draft' @@ -107,6 +109,7 @@ describe('DraftList', () => { { conceptId: 'TD1200000093-MMT_2', revisionDate: '2023-11-08T17:56:09.385Z', + ummMetadata: {}, previewMetadata: { __typename: 'Tool' }, @@ -115,9 +118,11 @@ describe('DraftList', () => { { conceptId: 'TD1200000094-MMT_2', revisionDate: '2023-10-08T17:56:09.385Z', + ummMetadata: { + Name: 'Tool TD1200000094 short name', + LongName: 'Tool TD1200000094 long name' + }, previewMetadata: { - name: 'Tool TD1200000094 short name', - longName: 'Tool TD1200000094 long name', __typename: 'Tool' }, __typename: 'Draft' diff --git a/static/src/js/operations/queries/getServiceDrafts.js b/static/src/js/operations/queries/getServiceDrafts.js index 38cbbdd30..b4637c9d0 100644 --- a/static/src/js/operations/queries/getServiceDrafts.js +++ b/static/src/js/operations/queries/getServiceDrafts.js @@ -8,6 +8,7 @@ export const GET_SERVICE_DRAFTS = gql` items { conceptId revisionDate + ummMetadata previewMetadata { ... on Service { name diff --git a/static/src/js/operations/queries/getToolDrafts.js b/static/src/js/operations/queries/getToolDrafts.js index 8305cdd78..427f10e73 100644 --- a/static/src/js/operations/queries/getToolDrafts.js +++ b/static/src/js/operations/queries/getToolDrafts.js @@ -8,6 +8,7 @@ export const GET_TOOL_DRAFTS = gql` items { conceptId revisionDate + ummMetadata previewMetadata { ... on Tool { name diff --git a/static/src/js/operations/queries/getVariableDrafts.js b/static/src/js/operations/queries/getVariableDrafts.js index 2dceead35..f1fff380b 100644 --- a/static/src/js/operations/queries/getVariableDrafts.js +++ b/static/src/js/operations/queries/getVariableDrafts.js @@ -8,6 +8,7 @@ export const GET_VARIABLE_DRAFTS = gql` items { conceptId revisionDate + ummMetadata previewMetadata { ... on Variable { name diff --git a/static/src/js/pages/ManagePage/ManagePage.jsx b/static/src/js/pages/ManagePage/ManagePage.jsx index f09f1d360..a7a0802a6 100644 --- a/static/src/js/pages/ManagePage/ManagePage.jsx +++ b/static/src/js/pages/ManagePage/ManagePage.jsx @@ -119,24 +119,12 @@ const ManagePage = () => { { ({ conceptId, - previewMetadata, ummMetadata, revisionDate }) => { - let title - let subTitle - - if (ummMetadata) { - const { ShortName, EntryTitle } = ummMetadata - - title = ShortName - subTitle = EntryTitle - } else { - const { name, longName } = previewMetadata - - title = name - subTitle = longName - } + const { + ShortName, EntryTitle, Name, LongName + } = ummMetadata || {} return ( @@ -147,10 +135,10 @@ const ManagePage = () => {
{new Date(revisionDate).toLocaleString('en-US', { hour12: false })} | - {title || ''} + {Name || ShortName || ''}
- {subTitle || ''} + {LongName || EntryTitle || ''}
diff --git a/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js b/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js index aaf699d00..5d5432076 100644 --- a/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js +++ b/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js @@ -40,19 +40,23 @@ const setup = (overrideMocks) => { { conceptId: 'TD1000000000-TESTPROV', revisionDate: '2023-11-30 00:00:00', + ummMetadata: { + Name: 'Tool Draft 1 Name', + LongName: 'Tool Draft 1 Long Name' + }, previewMetadata: { __typename: 'Tool', - name: 'Tool Draft 1 Name', - longName: 'Tool Draft 1 Long Name' } }, { conceptId: 'TD1000000001-TESTPROV', revisionDate: '2023-11-30 00:00:00', + ummMetadata: { + Name: 'Tool Draft 2 Name', + LongName: 'Tool Draft 2 Long Name' + }, previewMetadata: { - __typename: 'Tool', - name: 'Tool Draft 2 Name', - longName: 'Tool Draft 2 Long Name' + __typename: 'Tool' } } ] @@ -198,10 +202,12 @@ describe('ManagePage component', () => { { conceptId: 'TD1000000000-TESTPROV', revisionDate: '2023-11-30 00:00:00', + ummMetadata: { + Name: null, + LongName: null + }, previewMetadata: { - __typename: 'Tool', - name: null, - longName: null + __typename: 'Tool' } } ] From a57b8648e1f10340b33c8924e83f4639173ba7d1 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Thu, 1 Feb 2024 09:49:30 -0700 Subject: [PATCH 19/21] MMT-3492: lint --- static/src/js/pages/ManagePage/__tests__/ManagePage.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js b/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js index 5d5432076..b64dcf168 100644 --- a/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js +++ b/static/src/js/pages/ManagePage/__tests__/ManagePage.test.js @@ -45,7 +45,7 @@ const setup = (overrideMocks) => { LongName: 'Tool Draft 1 Long Name' }, previewMetadata: { - __typename: 'Tool', + __typename: 'Tool' } }, { From a6e54e2d9f0bba9ca32d8381df6fb969c6e92f83 Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Thu, 1 Feb 2024 12:22:53 -0700 Subject: [PATCH 20/21] MMT-3561: Writing tests for pagination --- .../src/js/components/DraftList/DraftList.jsx | 1 + .../js/components/Pagination/Pagination.jsx | 4 +- .../Pagination/__tests__/Pagination.test.js | 61 ++++++ static/src/js/components/Table/Table.jsx | 4 +- .../components/Table/__tests__/Table.test.js | 198 +----------------- 5 files changed, 72 insertions(+), 196 deletions(-) create mode 100644 static/src/js/components/Pagination/__tests__/Pagination.test.js diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index 1461f833a..eee45e899 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -34,6 +34,7 @@ const DraftList = ({ draftType }) => { limit }) const { count, items = [] } = drafts + console.log(count) const noDraftsError = `No ${draftType} drafts exist for the provider ${providerId}` diff --git a/static/src/js/components/Pagination/Pagination.jsx b/static/src/js/components/Pagination/Pagination.jsx index 1ce276dd1..c6a7541be 100644 --- a/static/src/js/components/Pagination/Pagination.jsx +++ b/static/src/js/components/Pagination/Pagination.jsx @@ -13,7 +13,9 @@ import Col from 'react-bootstrap/Col' */ const PaginationComponent = ({ - limit, count, setoffset + limit, + count, + setoffset }) => { const [pageNum, setPageNum] = useState(1) diff --git a/static/src/js/components/Pagination/__tests__/Pagination.test.js b/static/src/js/components/Pagination/__tests__/Pagination.test.js new file mode 100644 index 000000000..e537e6f1d --- /dev/null +++ b/static/src/js/components/Pagination/__tests__/Pagination.test.js @@ -0,0 +1,61 @@ +import React from 'react' +import { + render, + screen, + fireEvent +} from '@testing-library/react' +import userEvent from '@testing-library/user-event' + +import PaginationComponent from '../Pagination' + +const setoffset = jest.fn() + +const setup = () => { + const props = { + limit: 2, + count: 14, + setoffset + } + render( + + ) + + return { + props, + user: userEvent.setup() + } +} + +describe('Pagination', () => { + describe('when pagination component is passed count < limit', () => { + test('renders pagination bar', () => { + setup() + + expect(screen.queryAllByRole('button')).toHaveLength(3) + + // Check individual buttons work + fireEvent.click(screen.getByRole('button', { name: '2' })) + + // Check the next button works + fireEvent.click(screen.getByRole('button', { name: 'Next' })) + + // // Click on Previous Page + fireEvent.click(screen.getByRole('button', { name: 'Previous' })) + + // // Check pages[0] always stays at 1 and two ellipsis render + fireEvent.click(screen.getByRole('button', { name: 'Next' })) + fireEvent.click(screen.getByRole('button', { name: '4' })) + expect(screen.getByRole('button', { name: '1' })) + expect(screen.queryAllByText('More')).toHaveLength(2) + + // Make sure onclick for pages[0] function above works + fireEvent.click(screen.getByRole('button', { name: '1' })) + + // Can click on last page + fireEvent.click(screen.getByRole('button', { name: '7' })) + fireEvent.click(screen.getByRole('button', { name: 'Previous' })) + fireEvent.click(screen.getByRole('button', { name: 'Previous' })) + expect(screen.queryAllByText('More')).toHaveLength(1) + }) + }) +}) diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index e5890939a..f474287ca 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -60,7 +60,7 @@ const Table = ({ } ) - } else if (count > 0) { + } else if (pagedRows.length > 0) { const rowData = pagedRows.map((row) => { const { cells, key } = row const rowKey = key @@ -95,7 +95,7 @@ const Table = ({ Showing {' '} - {count > 0 && offset} + {count > 0 ? offset : 0} - {data.length === limit ? offset + limit : offset + data.length} {' '} diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js index c933a5c15..49e1509ae 100644 --- a/static/src/js/components/Table/__tests__/Table.test.js +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -1,17 +1,9 @@ import React from 'react' -import { - render, - screen, - fireEvent -} from '@testing-library/react' +import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import Table from '../Table' -jest.mock('../../../../../../static.config.json', () => ({ - application: { defaultPageSize: 2 } -})) - const setup = (overrideProps = {}) => { const props = { headers: [ @@ -41,141 +33,11 @@ const setup = (overrideProps = {}) => { value: ('Row 2 Cell 2') } ] - }, - { - key: 'conceptId003', - cells: [ - { - value: ('Row 3 Cell 1') - }, - { - value: ('Row 3 Cell 2') - } - ] - }, - { - key: 'conceptId004', - cells: [ - { - value: ('Row 4 Cell 1') - }, - { - value: ('Row 4 Cell 2') - } - ] - }, - { - key: 'conceptId005', - cells: [ - { - value: ('Row 5 Cell 1') - }, - { - value: ('Row 5 Cell 2') - } - ] - }, - { - key: 'conceptId006', - cells: [ - { - value: ('Row 6 Cell 1') - }, - { - value: ('Row 6 Cell 2') - } - ] - }, - { - key: 'conceptId007', - cells: [ - { - value: ('Row 7 Cell 1') - }, - { - value: ('Row 7 Cell 2') - } - ] - }, - { - key: 'conceptId001b', - cells: [ - { - value: ('Row 1 Cell 1') - }, - { - value: ('Row 1 Cell 2') - } - ] - }, - { - key: 'conceptId002b', - cells: [ - { - value: ('Row 2 Cell 1') - }, - { - value: ('Row 2 Cell 2') - } - ] - }, - { - key: 'conceptId003b', - cells: [ - { - value: ('Row 3 Cell 1') - }, - { - value: ('Row 3 Cell 2') - } - ] - }, - { - key: 'conceptId004b', - cells: [ - { - value: ('Row 4 Cell 1') - }, - { - value: ('Row 4 Cell 2') - } - ] - }, - { - key: 'conceptId005b', - cells: [ - { - value: ('Row 5 Cell 1') - }, - { - value: ('Row 5 Cell 2') - } - ] - }, - { - key: 'conceptId006b', - cells: [ - { - value: ('Row 6 Cell 1') - }, - { - value: ('Row 6 Cell 2') - } - ] - }, - { - key: 'conceptId007b', - cells: [ - { - value: ('Row 7 Cell 1') - }, - { - value: ('Row 7 Cell 2') - } - ] } ], count: 14, + limit: 2, + offset: 0, ...overrideProps } @@ -191,63 +53,12 @@ const setup = (overrideProps = {}) => { describe('Table', () => { describe('when the table component is passed markup and data that is more than the defaultPageSize', () => { - test('renders filled table with Pagination', () => { + test('renders filled table without correct number of rows', () => { setup() - // Checking all pages exist upon loading. Previous and 1 are not clickable buttons. - expect(screen.queryByRole('button', { name: 'Previous' })).not.toBeInTheDocument() - expect(screen.getByText('Previous')).toBeInTheDocument() - expect(screen.queryByRole('button', { name: '1' })).not.toBeInTheDocument() - expect(screen.getByText('1')).toBeInTheDocument() expect(screen.getByText('column 1')).toBeInTheDocument() expect(screen.getByRole('table')).toBeInTheDocument() - expect(screen.getByText('Row 2 Cell 2')).toBeInTheDocument() - expect(screen.queryByRole('button', { name: '2' })).toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Next' })).toBeInTheDocument() - expect(screen.getByText('More')).toBeInTheDocument() expect(screen.getByText('Showing 0-2 of 14 Results')).toBeInTheDocument() - - // Check individual buttons work - fireEvent.click(screen.getByRole('button', { name: '2' })) - - // Check the next button works - fireEvent.click(screen.getByRole('button', { name: 'Next' })) - - // Check pages[0] always stays at 1 and two ellipsis render - fireEvent.click(screen.getByRole('button', { name: 'Next' })) - expect(screen.queryAllByText('More')).toHaveLength(2) - - // Make sure onclick for pages[0] function above works - fireEvent.click(screen.getByRole('button', { name: '1' })) - - // Click on last page - fireEvent.click(screen.getByRole('button', { name: '7' })) - - // Click on Previous Page - fireEvent.click(screen.getByRole('button', { name: 'Previous' })) - }) - }) - - describe('when the table compoent is passed markup and data that is less than the defaultPageSize', () => { - test('renders filled table without Pagination', () => { - setup({ - data: [ - { - key: 'conceptId001', - cells: [ - { - value: ('Row 1 Cell 1') - }, - { - value: ('Row 1 Cell 2') - } - ] - } - ] - }) - - expect(screen.getByText('column 1')).toBeInTheDocument() - expect(screen.queryByRole('button')).not.toBeInTheDocument() }) }) @@ -255,6 +66,7 @@ describe('Table', () => { test('renders custom error message', () => { setup({ data: [], + count: 0, error: 'Custom Error Message' }) From 868cbae550927e53bf1fe1edf3fca663d43c951c Mon Sep 17 00:00:00 2001 From: Mandy Parson Date: Fri, 2 Feb 2024 10:08:10 -0700 Subject: [PATCH 21/21] MMT-3561: Addressing Comments --- package.json | 1 - static.config.json | 1 - .../src/js/components/DraftList/DraftList.jsx | 6 +-- .../js/components/Pagination/Pagination.jsx | 11 ++-- .../Pagination/__tests__/Pagination.test.js | 4 +- static/src/js/components/Table/Table.jsx | 50 +++++++++++-------- .../components/Table/__tests__/Table.test.js | 14 +++++- 7 files changed, 51 insertions(+), 36 deletions(-) diff --git a/package.json b/package.json index 9f1dac724..4622cb826 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,6 @@ "bootstrap": "^5.3.2", "classnames": "^2.3.2", "commafy": "^0.0.6", - "commafy-anything": "^3.0.4", "compact-object-deep": "^1.0.0", "cookie": "^0.6.0", "esbuild": "^0.19.5", diff --git a/static.config.json b/static.config.json index d609d790b..34fa3e597 100644 --- a/static.config.json +++ b/static.config.json @@ -6,7 +6,6 @@ "cmrHost": "http://localhost:4000", "version": "development", "jwtSecret": "jwt_secret" - }, "ummVersions": { "ummC": "1.17.3", diff --git a/static/src/js/components/DraftList/DraftList.jsx b/static/src/js/components/DraftList/DraftList.jsx index eee45e899..cf053a208 100644 --- a/static/src/js/components/DraftList/DraftList.jsx +++ b/static/src/js/components/DraftList/DraftList.jsx @@ -34,9 +34,8 @@ const DraftList = ({ draftType }) => { limit }) const { count, items = [] } = drafts - console.log(count) - const noDraftsError = `No ${draftType} drafts exist for the provider ${providerId}` + const noDataError = `No ${draftType} drafts exist for the provider ${providerId}` const [downloadDraft] = useLazyQuery(DOWNLOAD_DRAFT, { onCompleted: (getDraftData) => { @@ -161,7 +160,8 @@ const DraftList = ({ draftType }) => { classNames={['col-md-4', 'col-md-4', 'col-auto', 'col-auto']} loading={loading} data={data} - error={noDraftsError} + error={error} + noDataError={noDataError} count={count} setOffset={setOffset} limit={limit} diff --git a/static/src/js/components/Pagination/Pagination.jsx b/static/src/js/components/Pagination/Pagination.jsx index c6a7541be..81af2c308 100644 --- a/static/src/js/components/Pagination/Pagination.jsx +++ b/static/src/js/components/Pagination/Pagination.jsx @@ -15,7 +15,7 @@ import Col from 'react-bootstrap/Col' const PaginationComponent = ({ limit, count, - setoffset + setOffset }) => { const [pageNum, setPageNum] = useState(1) @@ -28,7 +28,7 @@ const PaginationComponent = ({ const handleItemClick = (currentPage) => { setPageNum(currentPage) - setoffset((currentPage - 1) * limit) + setOffset((currentPage - 1) * limit) } const generatePaginationItems = () => { @@ -106,22 +106,19 @@ const PaginationComponent = ({ const newCurrentPage = pageNum + direction setPageNum(newCurrentPage) - setoffset((newCurrentPage - 1) * limit) + setOffset((newCurrentPage - 1) * limit) } return ( - {/*

Pagination

*/}
handlePageChange(-1)} /> - {generatePaginationItems()} - handlePageChange(1)} disabled={pageNum >= lastPageNum} @@ -138,7 +135,7 @@ PaginationComponent.defaultProps = { } PaginationComponent.propTypes = { - setoffset: PropTypes.func.isRequired, + setOffset: PropTypes.func.isRequired, limit: PropTypes.number.isRequired, count: PropTypes.number } diff --git a/static/src/js/components/Pagination/__tests__/Pagination.test.js b/static/src/js/components/Pagination/__tests__/Pagination.test.js index e537e6f1d..281738245 100644 --- a/static/src/js/components/Pagination/__tests__/Pagination.test.js +++ b/static/src/js/components/Pagination/__tests__/Pagination.test.js @@ -8,13 +8,13 @@ import userEvent from '@testing-library/user-event' import PaginationComponent from '../Pagination' -const setoffset = jest.fn() +const setOffset = jest.fn() const setup = () => { const props = { limit: 2, count: 14, - setoffset + setOffset } render( diff --git a/static/src/js/components/Table/Table.jsx b/static/src/js/components/Table/Table.jsx index f474287ca..a90dd1e24 100644 --- a/static/src/js/components/Table/Table.jsx +++ b/static/src/js/components/Table/Table.jsx @@ -11,18 +11,30 @@ import For from '../For/For' /** * Table * @typedef {Object} Table + * @property {Array} headers A list of custom headers + * @property {Array} classNames A list of classNames for each header * @property {Boolean} loading A value provided by whichever useQuery is being used in parent component - * @property {Object[]} headers A list of header titles for a table * @property {Array} data An array of formatted rows with data already populated - * @property {String} error A string with a specific error for table + * @property {String} error A string that comes from { loading, data, error } = useQuery + * @property {String} noDataError An option string for custom error if data.length === 0 + * @property {Number} count A number of the total amount of data without limit + * @property {Function} setOffset A useState function that loads the appropriate data set based on user input + * @property {limit} limit A number that limits the data set that comes in. **Note this number should always be 20 + * @property {limit} offset A number that dictates where the dataset should start from */ const Table = ({ - headers, classNames, loading, data, error, count, setOffset, limit, offset + headers, + classNames, + loading, + data, + error, + noDataError, + count, + setOffset, + limit, + offset }) => { - // Current page of rows - const pagedRows = data - // Does this provider have enough Rows to page? We hide the pagination buttons if not const hasPages = count > limit @@ -30,6 +42,8 @@ const Table = ({
)) + const dataLengthExists = (data && data.length) && data.length + const content = [] if (loading) { @@ -60,8 +74,8 @@ const Table = ({ } ) - } else if (pagedRows.length > 0) { - const rowData = pagedRows.map((row) => { + } else if (dataLengthExists > 0) { + const rowData = data.map((row) => { const { cells, key } = row const rowKey = key @@ -84,6 +98,8 @@ const Table = ({ }) content.push(rowData) + } else if (dataLengthExists === 0) { + content.push() } else { content.push() } @@ -93,17 +109,7 @@ const Table = ({ - Showing - {' '} - {count > 0 ? offset : 0} - - - {data.length === limit ? offset + limit : offset + data.length} - {' '} - of - {' '} - {count} - {' '} - Results + {`Showing ${count > 0 ? offset : 0}-${dataLengthExists === limit ? offset + limit : offset + dataLengthExists} of ${count} Results`} @@ -111,7 +117,7 @@ const Table = ({ { hasPages && ( @@ -141,7 +147,8 @@ const Table = ({ Table.defaultProps = { classNames: [], loading: null, - error: 'No Data to Display', + error: 'Error', + noDataError: 'No Data to Display', count: null } @@ -154,6 +161,7 @@ Table.propTypes = { cells: PropTypes.arrayOf(PropTypes.shape({})) })).isRequired, error: PropTypes.string, + noDataError: PropTypes.string, count: PropTypes.number, offset: PropTypes.number.isRequired, setOffset: PropTypes.func.isRequired, diff --git a/static/src/js/components/Table/__tests__/Table.test.js b/static/src/js/components/Table/__tests__/Table.test.js index 49e1509ae..db075268c 100644 --- a/static/src/js/components/Table/__tests__/Table.test.js +++ b/static/src/js/components/Table/__tests__/Table.test.js @@ -59,6 +59,7 @@ describe('Table', () => { expect(screen.getByText('column 1')).toBeInTheDocument() expect(screen.getByRole('table')).toBeInTheDocument() expect(screen.getByText('Showing 0-2 of 14 Results')).toBeInTheDocument() + expect(screen.getByText('Row 2 Cell 2')).toBeInTheDocument() }) }) @@ -67,13 +68,24 @@ describe('Table', () => { setup({ data: [], count: 0, - error: 'Custom Error Message' + noDataError: 'Custom Error Message' }) expect(screen.getByText('Custom Error Message')).toBeInTheDocument() }) }) + describe('when the table component has an error from useQuery', () => { + test('renders error message', () => { + setup({ + data: null, + error: 'Error Message' + }) + + expect(screen.getByText('Error Message')).toBeInTheDocument() + }) + }) + describe('when the table component is passed loading:true', () => { test('renders loading screen', () => { setup({
{header}
{error}
{header}
{header}
 Renders a `ManagePage` component
{header}
{noDataError}
{error}