From 53dee08c51953ef1cb4773d6ab804f3d4eafd810 Mon Sep 17 00:00:00 2001 From: Thomas Zemp Date: Fri, 6 Sep 2024 10:36:17 +0200 Subject: [PATCH] fix: remove request NONE id schemes [DHIS2-18013] --- .../__tests__/getExchangeValues.test.js | 64 +++++++++++++++++++ .../edit/exchange-update/getExchangeValues.js | 37 +++++++++-- src/pages/addItem.test.js | 62 ++++++++++++++++-- 3 files changed, 152 insertions(+), 11 deletions(-) create mode 100644 src/components/edit/exchange-update/__tests__/getExchangeValues.test.js diff --git a/src/components/edit/exchange-update/__tests__/getExchangeValues.test.js b/src/components/edit/exchange-update/__tests__/getExchangeValues.test.js new file mode 100644 index 0000000..b1d13cc --- /dev/null +++ b/src/components/edit/exchange-update/__tests__/getExchangeValues.test.js @@ -0,0 +1,64 @@ +import { cleanUpRequests } from '../getExchangeValues.js' + +describe('cleanUpRequests', () => { + it('removes irrelevant properties', () => { + const originalRequests = [ + { + name: 'a name', + visualization: 'a visualization', + dx: [], + pe: [], + ou: [], + filters: [], + inputIdScheme: 'UID', + outputDataElementIdScheme: 'UID', + outputOrgUnitIdScheme: 'UID', + outputIdScheme: 'UID', + outputDataItemIdScheme: 'UID', + dhis2: [], + team: 'platform', + }, + ] + const expectedRequests = [ + { + name: 'a name', + visualization: 'a visualization', + dx: [], + pe: [], + ou: [], + filters: [], + inputIdScheme: 'UID', + outputDataElementIdScheme: 'UID', + outputOrgUnitIdScheme: 'UID', + outputIdScheme: 'UID', + outputDataItemIdScheme: 'UID', + }, + ] + expect(cleanUpRequests({ requests: originalRequests })).toEqual( + expectedRequests + ) + }) + + it('only returns properties that are not null or undefined on the original', () => { + const originalRequests = [{ dx: [], pe: undefined }] + const expectedRequests = [{ dx: [] }] + expect(cleanUpRequests({ requests: originalRequests })).toEqual( + expectedRequests + ) + }) + + it('removes ID schemes of value NONE (outputDataElementIdScheme, outputOrgUnitIdScheme, outputDataItemIdScheme)', () => { + const originalRequests = [ + { + dx: [], + outputDataElementIdScheme: 'NONE', + outputOrgUnitIdScheme: 'NONE', + outputDataItemIdScheme: 'NONE', + }, + ] + const expectedRequests = [{ dx: [] }] + expect(cleanUpRequests({ requests: originalRequests })).toEqual( + expectedRequests + ) + }) +}) diff --git a/src/components/edit/exchange-update/getExchangeValues.js b/src/components/edit/exchange-update/getExchangeValues.js index eb66934..224de23 100644 --- a/src/components/edit/exchange-update/getExchangeValues.js +++ b/src/components/edit/exchange-update/getExchangeValues.js @@ -4,15 +4,38 @@ import { AUTHENTICATION_TYPES, } from '../shared/index.js' -const removeRequestIDSchemeValuesOfNONE = ({ requests }) => { +// this filters out additional properties that are used by the app UI, but should not be sent to backend +const validProperties = [ + 'name', + 'visualization', + 'dx', + 'pe', + 'ou', + 'filters', + 'inputIdScheme', + 'outputDataElementIdScheme', + 'outputOrgUnitIdScheme', + 'outputIdScheme', + 'outputDataItemIdScheme', +] +const copyRequestWithValidProperties = (request) => + validProperties.reduce((requestCopy, prop) => { + if (request[prop]) { + requestCopy[prop] = request[prop] + } + return requestCopy + }, {}) + +export const cleanUpRequests = ({ requests }) => { const idSchemeProps = [ - 'idScheme', - 'dataElementIdScheme', - 'orgUnitIdScheme', - 'categoryOptionComboIdScheme', + 'outputDataElementIdScheme', + 'outputDataItemIdScheme', + 'outputOrgUnitIdScheme', ] return requests.map((request) => { - const requestCopy = { ...request } + const requestCopy = copyRequestWithValidProperties(request) + + // delete any ID Scheme prop with value of NONE for (const prop of idSchemeProps) { if (requestCopy[prop] === SCHEME_TYPES.none) { delete requestCopy[prop] @@ -78,7 +101,7 @@ const getTargetDetails = ({ values }) => { export const getExchangeValuesFromForm = ({ values, requests }) => ({ name: values.name, target: getTargetDetails({ values }), - source: { requests: removeRequestIDSchemeValuesOfNONE({ requests }) }, + source: { requests: cleanUpRequests({ requests }) }, }) const getIdSchemeValues = ({ exchangeInfo }) => { diff --git a/src/pages/addItem.test.js b/src/pages/addItem.test.js index ca35b9a..bf5974c 100644 --- a/src/pages/addItem.test.js +++ b/src/pages/addItem.test.js @@ -32,20 +32,24 @@ jest.mock('@dhis2/analytics', () => ({ DataDimension: ({ onSelect }) => ( onSelect({ items: [e.target.value] })} + onInput={(e) => + onSelect({ items: [{ id: 'anIdDe', name: e.target.value }] }) + } /> ), PeriodDimension: ({ onSelect }) => ( onSelect({ items: [e.target.value] })} + onInput={(e) => + onSelect({ items: [{ id: 'anIdPe', name: e.target.value }] }) + } /> ), OrgUnitDimension: ({ onSelect }) => ( - onSelect({ items: [{ id: 'anId', name: e.target.value }] }) + onSelect({ items: [{ id: 'anIdOu', name: e.target.value }] }) } /> ), @@ -66,6 +70,8 @@ jest.mock('@dhis2/app-runtime', () => ({ }), })) +const createExchangeMock = jest.fn() + const setUp = ( ui, { @@ -75,8 +81,9 @@ const setUp = ( ) => { const customerProviderData = { attributes: { attributes }, - aggregateDataExchanges: (type) => { + aggregateDataExchanges: (type, query) => { if (type === 'create') { + createExchangeMock(query?.data) return {} } return undefined @@ -213,6 +220,32 @@ describe('', () => { screen.getByTestId('saving-exchange-loader') ).toBeInTheDocument() ) + + const expectedPayload = { + name: 'an exchange name', + source: { + requests: [ + { + dx: ['anIdDe'], + filters: [], + inputIdScheme: 'UID', + name: 'a request name', + ou: ['anIdOu'], + outputIdScheme: 'UID', + pe: ['anIdPe'], + }, + ], + }, + target: { + request: { + idScheme: 'UID', + }, + type: 'INTERNAL', + }, + } + + expect(createExchangeMock).toHaveBeenCalled() + expect(createExchangeMock).toHaveBeenCalledWith(expectedPayload) }) it('does not create an internal exchange if the name is missing', async () => { @@ -855,4 +888,25 @@ describe('', () => { expect(warningModal).not.toBeInTheDocument() }) }) + + it('warns about unsaved changes if user clicks cancel after making changes in the request form', async () => { + const { screen } = setUp(, { + userContext: testUserContext({ canAddExchange: true }), + }) + + expect( + await screen.findByTestId('add-exchange-title') + ).toHaveTextContent('Add exchange') + + screen.getByText('Add request').click() + const requestNameInput = await screen.findByLabelText('Request name') + fireEvent.input(requestNameInput, { target: { value: 'a request' } }) + + const footer = screen.getByTestId('edit-request-footer') + within(footer).getByText('Cancel').click() + + const warningModal = await screen.findByTestId('request-discard-modal') + expect(warningModal).toBeVisible() + expect(warningModal).toHaveTextContent('Discard unsaved changes') + }) })