-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: remove request NONE id schemes [DHIS2-18013] #98
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,20 +32,24 @@ jest.mock('@dhis2/analytics', () => ({ | |
DataDimension: ({ onSelect }) => ( | ||
<input | ||
data-test="fake-data-selector" | ||
onInput={(e) => onSelect({ items: [e.target.value] })} | ||
onInput={(e) => | ||
onSelect({ items: [{ id: 'anIdDe', name: e.target.value }] }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've modified these, so that they produce a more realistic payload when saving. |
||
} | ||
/> | ||
), | ||
PeriodDimension: ({ onSelect }) => ( | ||
<input | ||
data-test="fake-period-selector" | ||
onInput={(e) => onSelect({ items: [e.target.value] })} | ||
onInput={(e) => | ||
onSelect({ items: [{ id: 'anIdPe', name: e.target.value }] }) | ||
} | ||
/> | ||
), | ||
OrgUnitDimension: ({ onSelect }) => ( | ||
<input | ||
data-test="fake-orgunit-selector" | ||
onInput={(e) => | ||
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('<AddItem/>', () => { | |
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated this test to check the payload being sent on save (which seems reasonable in general), and also has the benefit of confirming that the "NONE" ID Schemes that are selected by default get filtered out. |
||
}) | ||
|
||
it('does not create an internal exchange if the name is missing', async () => { | ||
|
@@ -855,4 +888,25 @@ describe('<AddItem/>', () => { | |
expect(warningModal).not.toBeInTheDocument() | ||
}) | ||
}) | ||
|
||
it('warns about unsaved changes if user clicks cancel after making changes in the request form', async () => { | ||
const { screen } = setUp(<AddItem />, { | ||
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') | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was the source of the error before. I had the wrong names for the ID schemes relevant for requests.