diff --git a/i18n/en.pot b/i18n/en.pot index 7592ac8d9..7de727ae6 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2024-10-18T12:58:59.146Z\n" -"PO-Revision-Date: 2024-10-18T12:58:59.146Z\n" +"POT-Creation-Date: 2024-11-29T16:17:15.582Z\n" +"PO-Revision-Date: 2024-11-29T16:17:15.582Z\n" msgid "Not authorized" msgstr "Not authorized" @@ -81,6 +81,9 @@ msgstr "Couldn't validate the form. This does not effect form completion!" msgid "The form can't be completed while invalid" msgstr "The form can't be completed while invalid" +msgid "Compulsory fields must be filled out before completing the form" +msgstr "Compulsory fields must be filled out before completing the form" + msgid "1 comment required" msgstr "1 comment required" @@ -213,6 +216,9 @@ msgstr "Warning, saved" msgid "Locked, not editable" msgstr "Locked, not editable" +msgid "Compulsory field" +msgstr "Compulsory field" + msgid "Help" msgstr "Help" diff --git a/src/bottom-bar/complete-button.test.js b/src/bottom-bar/complete-button.test.js new file mode 100644 index 000000000..12be2aea5 --- /dev/null +++ b/src/bottom-bar/complete-button.test.js @@ -0,0 +1,220 @@ +import { waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import React from 'react' +import { useMetadata } from '../shared/metadata/use-metadata.js' +import { useDataSetId } from '../shared/use-context-selection/use-context-selection.js' +import { useDataValueSet } from '../shared/use-data-value-set/use-data-value-set.js' +import { useImperativeValidate } from '../shared/validation/use-imperative-validate.js' +import { render } from '../test-utils/render.js' +import CompleteButton from './complete-button.js' + +const mockShow = jest.fn() +const mockSetFormCompletion = jest + .fn() + .mockImplementation(() => Promise.resolve()) +const mockValidate = jest.fn().mockImplementation(() => + Promise.resolve({ + commentRequiredViolations: [], + validationRuleViolations: [], + }) +) +const mockSetCompleteAttempted = jest.fn() +const mockIsComplete = jest.fn() + +jest.mock('@dhis2/app-runtime', () => ({ + ...jest.requireActual('@dhis2/app-runtime'), + useAlert: jest.fn(() => ({ + show: mockShow, + })), +})) + +jest.mock('../shared/use-context-selection/use-context-selection.js', () => ({ + ...jest.requireActual( + '../shared/use-context-selection/use-context-selection.js' + ), + useDataSetId: jest.fn(), +})) + +jest.mock('../shared/metadata/use-metadata.js', () => ({ + useMetadata: jest.fn(), +})) + +jest.mock('../shared/validation/use-imperative-validate.js', () => ({ + useImperativeValidate: jest.fn(), +})) + +jest.mock('../shared/completion/use-set-form-completion-mutation.js', () => ({ + ...jest.requireActual( + '../shared/completion/use-set-form-completion-mutation.js' + ), + useSetFormCompletionMutation: jest.fn(() => ({ + mutateAsync: mockSetFormCompletion, + })), +})) + +jest.mock('../shared/stores/entry-form-store.js', () => ({ + ...jest.requireActual('../shared/stores/entry-form-store.js'), + useEntryFormStore: jest.fn().mockImplementation((func) => { + const state = { + setCompleteAttempted: mockSetCompleteAttempted, + } + return func(state) + }), +})) + +jest.mock('../shared/stores/data-value-store.js', () => ({ + ...jest.requireActual('../shared/stores/data-value-store.js'), + useValueStore: jest.fn().mockImplementation((func) => { + const state = { + isComplete: mockIsComplete, + } + return func(state) + }), +})) + +jest.mock('../shared/use-data-value-set/use-data-value-set.js', () => ({ + useDataValueSet: jest.fn(), +})) + +const MOCK_METADATA = { + dataSets: { + data_set_id_1: { + id: 'data_set_id_1', + }, + data_set_id_compulsory_validation_without_cdeo: { + id: 'data_set_id_compulsory_validation_without_cdeo', + compulsoryDataElementOperands: [], + compulsoryFieldsCompleteOnly: true, + }, + data_set_id_compulsory_validation_with_cdeo: { + id: 'data_set_id_compulsory_validation_without_cdeo', + compulsoryDataElementOperands: [ + { + dataElement: { + id: 'de-id-1', + }, + categoryOptionCombo: { + id: 'coc-id-1', + }, + }, + { + dataElement: { + id: 'de-id-2', + }, + categoryOptionCombo: { + id: 'coc-id-2', + }, + }, + ], + compulsoryFieldsCompleteOnly: true, + }, + }, +} + +const MOCK_DATA = { + dataValues: { + 'de-id-1': { + 'coc-id-1': { + value: '5', + }, + }, + 'de-id-2': { + 'coc-id-2': { + value: '10', + }, + }, + }, +} + +const MOCK_DATA_INCOMPLETE = { + dataValues: { + 'de-id-1': { + 'coc-id-1': { + value: '5', + }, + }, + }, +} + +describe('CompleteButton', () => { + beforeEach(() => { + jest.clearAllMocks() + useImperativeValidate.mockReturnValue(mockValidate) + }) + + it('validates form and completes when clicked', () => { + mockIsComplete.mockReturnValue(false) + useDataSetId.mockReturnValue(['data_set_id_1']) + useDataValueSet.mockReturnValue({ data: MOCK_DATA }) + useMetadata.mockReturnValue({ data: MOCK_METADATA }) + const { getByText } = render() + + userEvent.click(getByText('Mark complete')) + expect(mockValidate).toHaveBeenCalledOnce() + expect(mockSetFormCompletion).toHaveBeenCalledWith({ completed: true }) + }) + + it('completes if the compulsoryFieldsCompleteOnly:true but there are no compulsory data element operands', () => { + mockIsComplete.mockReturnValue(false) + useDataSetId.mockReturnValue([ + 'data_set_id_compulsory_validation_without_cdeo', + ]) + useDataValueSet.mockReturnValue({ data: MOCK_DATA }) + useMetadata.mockReturnValue({ data: MOCK_METADATA }) + const { getByText } = render() + + userEvent.click(getByText('Mark complete')) + expect(mockValidate).toHaveBeenCalledOnce() + expect(mockSetFormCompletion).toHaveBeenCalledWith({ completed: true }) + // completeAttempted only set if the complete is rejected due to compulsory data element operands + expect(mockSetCompleteAttempted).not.toHaveBeenCalled() + }) + + it('does not complete and shows error if the compulsoryFieldsCompleteOnly:true and there are compulsory data element operands without values', async () => { + mockIsComplete.mockReturnValue(false) + useDataSetId.mockReturnValue([ + 'data_set_id_compulsory_validation_with_cdeo', + ]) + useDataValueSet.mockReturnValue({ data: MOCK_DATA_INCOMPLETE }) + useMetadata.mockReturnValue({ data: MOCK_METADATA }) + const { getByText } = render() + + userEvent.click(getByText('Mark complete')) + expect(mockValidate).not.toHaveBeenCalled() + expect(mockSetFormCompletion).not.toHaveBeenCalled() + expect(mockSetCompleteAttempted).toHaveBeenCalledWith(true) + await waitFor(() => + expect(mockShow).toHaveBeenCalledWith( + 'Compulsory fields must be filled out before completing the form' + ) + ) + }) + + it('completes if the compulsoryFieldsCompleteOnly:true and there are compulsory data element operands but all have values', () => { + mockIsComplete.mockReturnValue(false) + useDataSetId.mockReturnValue([ + 'data_set_id_compulsory_validation_with_cdeo', + ]) + useDataValueSet.mockReturnValue({ data: MOCK_DATA }) + useMetadata.mockReturnValue({ data: MOCK_METADATA }) + const { getByText } = render() + + userEvent.click(getByText('Mark complete')) + expect(mockValidate).toHaveBeenCalledOnce() + expect(mockSetFormCompletion).toHaveBeenCalledWith({ completed: true }) + // completeAttempted only set if the complete is rejected due to compulsory data element operands + expect(mockSetCompleteAttempted).not.toHaveBeenCalled() + }) + + it('marks form as incomplete if form is completed', () => { + mockIsComplete.mockReturnValue(true) + useDataSetId.mockReturnValue(['data_set_id_1']) + useDataValueSet.mockReturnValue({ data: MOCK_DATA }) + useMetadata.mockReturnValue({ data: MOCK_METADATA }) + const { getByText } = render() + + userEvent.click(getByText('Mark incomplete')) + expect(mockValidate).not.toHaveBeenCalledOnce() + expect(mockSetFormCompletion).toHaveBeenCalledWith({ completed: false }) + }) +}) diff --git a/src/bottom-bar/use-on-complete-callback.js b/src/bottom-bar/use-on-complete-callback.js index f6813c07a..0cb4d8c29 100644 --- a/src/bottom-bar/use-on-complete-callback.js +++ b/src/bottom-bar/use-on-complete-callback.js @@ -13,6 +13,7 @@ import { useSetFormCompletionMutationKey, validationResultsSidebarId, useHasCompulsoryDataElementOperandsToFillOut, + useEntryFormStore, } from '../shared/index.js' const validationFailedMessage = i18n.t( @@ -122,7 +123,9 @@ export default function useOnCompleteCallback() { const { offline } = useConnectionStatus() const { data: metadata } = useMetadata() const [dataSetId] = useDataSetId() - const dataSet = selectors.getDataSetById(metadata, dataSetId) + const dataSet = dataSetId + ? selectors.getDataSetById(metadata, dataSetId) + : {} const { validCompleteOnly } = dataSet const { show: showErrorAlert } = useAlert((message) => message, { critical: true, @@ -138,19 +141,24 @@ export default function useOnCompleteCallback() { useOnCompleteWithoutValidationClick() const hasCompulsoryDataElementOperandsToFillOut = useHasCompulsoryDataElementOperandsToFillOut() + const setCompleteAttempted = useEntryFormStore( + (state) => state.setCompleteAttempted + ) return () => { let promise - // showErrorAlert('Complete compulsory data element operands') - // return Promise.resolve() + if (hasCompulsoryDataElementOperandsToFillOut) { + setCompleteAttempted(true) cancelCompletionMutation() - showErrorAlert( - 'Compulsory fields must be filled out before completing form' + promise = Promise.reject( + new Error( + i18n.t( + 'Compulsory fields must be filled out before completing the form' + ) + ) ) - return Promise.resolve() - } - if (isLoading && offline) { + } else if (isLoading && offline) { cancelCompletionMutation() // No need to complete when the completion request // hasn't been sent yet due to being offline. diff --git a/src/context-selection/contextual-help-sidebar/cell.js b/src/context-selection/contextual-help-sidebar/cell.js index b6b2335fa..547ceed86 100644 --- a/src/context-selection/contextual-help-sidebar/cell.js +++ b/src/context-selection/contextual-help-sidebar/cell.js @@ -25,6 +25,9 @@ const Cell = ({ value, state }) => ( {state === 'SYNCED' && (
)} + {state === 'COMPULSORY' && ( +
*
+ )}
{state === 'HAS_COMMENT' && ( diff --git a/src/context-selection/contextual-help-sidebar/cell.module.css b/src/context-selection/contextual-help-sidebar/cell.module.css index 0354cce3e..833448878 100644 --- a/src/context-selection/contextual-help-sidebar/cell.module.css +++ b/src/context-selection/contextual-help-sidebar/cell.module.css @@ -10,6 +10,7 @@ .input { composes: densePadding from '../../data-workspace/inputs/inputs.module.css'; composes: alignToEnd from '../../data-workspace/inputs/inputs.module.css'; + padding-inline-end: 16px; outline: 1px solid var(--colors-grey400); } @@ -45,3 +46,7 @@ .bottomRightTriangle { composes: bottomRightTriangle from '../../data-workspace/data-entry-cell/data-entry-cell.module.css'; } + +.topRightAsterisk { + composes: topRightAsterisk from '../../data-workspace/data-entry-cell/data-entry-cell.module.css'; +} diff --git a/src/context-selection/contextual-help-sidebar/cells-legend.js b/src/context-selection/contextual-help-sidebar/cells-legend.js index ef945953a..10f038ca1 100644 --- a/src/context-selection/contextual-help-sidebar/cells-legend.js +++ b/src/context-selection/contextual-help-sidebar/cells-legend.js @@ -67,6 +67,12 @@ export default function CellsLegend() { name={i18n.t('Locked, not editable')} state="LOCKED" /> + + + ) } diff --git a/src/data-workspace/category-combo-table-body/category-combo-table-body.test.js b/src/data-workspace/category-combo-table-body/category-combo-table-body.test.js index 4070ee92c..fea40ac11 100644 --- a/src/data-workspace/category-combo-table-body/category-combo-table-body.test.js +++ b/src/data-workspace/category-combo-table-body/category-combo-table-body.test.js @@ -2,10 +2,21 @@ import { Table } from '@dhis2/ui' import { getAllByTestId, getByTestId, getByText } from '@testing-library/react' import React from 'react' import { useMetadata } from '../../shared/metadata/use-metadata.js' +import { useDataSetId } from '../../shared/use-context-selection/use-context-selection.js' import { render } from '../../test-utils/index.js' import { FinalFormWrapper } from '../final-form-wrapper.js' import { CategoryComboTableBody } from './category-combo-table-body.js' +jest.mock( + '../../shared/use-context-selection/use-context-selection.js', + () => ({ + ...jest.requireActual( + '../../shared/use-context-selection/use-context-selection.js' + ), + useDataSetId: jest.fn(), + }) +) + jest.mock('../../shared/metadata/use-metadata.js', () => ({ useMetadata: jest.fn(), })) @@ -453,6 +464,7 @@ const metadata = { describe('', () => { useMetadata.mockReturnValue({ data: metadata }) + useDataSetId.mockReturnValue(['dataSet1']) it('should render rows and columns based on the data elements and categorycombo', () => { const tableDataElements = [ diff --git a/src/data-workspace/data-entry-cell/inner-wrapper.js b/src/data-workspace/data-entry-cell/inner-wrapper.js index 2ede7b2ad..60651230b 100644 --- a/src/data-workspace/data-entry-cell/inner-wrapper.js +++ b/src/data-workspace/data-entry-cell/inner-wrapper.js @@ -73,6 +73,9 @@ export function InnerWrapper({ dataElementId: deId, categoryOptionComboId: cocId, }) + const completeAttempted = useEntryFormStore((state) => + state.getCompleteAttempted() + ) const { input: { value }, @@ -104,6 +107,7 @@ export function InnerWrapper({ (state) => state.clearErrorByDataValueParams ) const warning = useEntryFormStore((state) => state.getWarning(fieldname)) + const fieldErrorMessage = error ?? warning const errorMessage = @@ -117,16 +121,18 @@ export function InnerWrapper({ ) const valueSynced = data.lastSyncedValue === value - const showSynced = dirty && valueSynced + const showSynced = dirty && valueSynced && (!isRequired || !!value) // todo: maybe use mutation state to improve this style handling // see https://dhis2.atlassian.net/browse/TECH-1316 - const cellStateClassName = invalid - ? styles.invalid - : warning - ? styles.warning - : activeMutations === 0 && showSynced - ? styles.synced - : null + + const cellStateClassName = + invalid || (isRequired && !value && completeAttempted) + ? styles.invalid + : warning + ? styles.warning + : activeMutations === 0 && showSynced + ? styles.synced + : null // initalize lastSyncedValue useEffect( diff --git a/src/data-workspace/data-workspace.js b/src/data-workspace/data-workspace.js index 8c1a550ce..aff50d1e1 100644 --- a/src/data-workspace/data-workspace.js +++ b/src/data-workspace/data-workspace.js @@ -15,6 +15,7 @@ import { useIsValidSelection, useValueStore, dataValueSetQueryKey, + useEntryFormStore, } from '../shared/index.js' import styles from './data-workspace.module.css' import { EntryForm } from './entry-form.js' @@ -34,6 +35,10 @@ export const DataWorkspace = ({ selectionHasNoFormMessage }) => { updateStore(initialDataValuesFetch.data) }, [updateStore, initialDataValuesFetch.data]) + const setCompleteAttempted = useEntryFormStore( + (state) => state.setCompleteAttempted + ) + const isValidSelection = useIsValidSelection() const [dataSetId] = useDataSetId() // used to reset form-state when context-selection is changed @@ -55,8 +60,11 @@ export const DataWorkspace = ({ selectionHasNoFormMessage }) => { cancelRefetch: false, } ) + + // reset the completionAttempted store for new form + setCompleteAttempted(false) } - }, [validFormKey, queryClient]) + }, [validFormKey, queryClient, setCompleteAttempted]) if (selectionHasNoFormMessage) { const title = i18n.t('The current selection does not have a form') diff --git a/src/shared/metadata/selectors.js b/src/shared/metadata/selectors.js index 5c7fa191c..c6df21ace 100644 --- a/src/shared/metadata/selectors.js +++ b/src/shared/metadata/selectors.js @@ -124,7 +124,7 @@ export const getCategoryOptionsByCategoryOptionComboId = createCachedSelector( export const getCompulsoryDataElementOperandsSet = createCachedSelector( getDataSetById, (dataSet) => { - if (!dataSet) { + if (!dataSet || !dataSet.compulsoryDataElementOperands) { return new Set() } return new Set( diff --git a/src/shared/stores/entry-form-store.js b/src/shared/stores/entry-form-store.js index 1a51e480d..e3ac81f2f 100644 --- a/src/shared/stores/entry-form-store.js +++ b/src/shared/stores/entry-form-store.js @@ -4,6 +4,7 @@ import create from 'zustand' const inititalState = { errors: {}, warnings: {}, + completeAttempted: false, } export const useEntryFormStore = create((set, get) => ({ @@ -19,6 +20,8 @@ export const useEntryFormStore = create((set, get) => ({ const newWarnings = setIn(warnings, fieldname, warning) ?? {} set({ warnings: newWarnings }) }, + getCompleteAttempted: () => get().completeAttempted, + setCompleteAttempted: (bool) => set({ completeAttempted: bool }), // could add getNumberOfWarnings if needed })) diff --git a/src/shared/use-is-compulsory-data-element-operand.js b/src/shared/use-is-compulsory-data-element-operand.js index be0994431..164de9165 100644 --- a/src/shared/use-is-compulsory-data-element-operand.js +++ b/src/shared/use-is-compulsory-data-element-operand.js @@ -9,6 +9,9 @@ export const useIsCompulsoryDataElementOperand = ({ }) => { const { data: metadata } = useMetadata() const [dataSetId] = useDataSetId() + if (!dataSetId) { + return false + } const compulsoryDataElementOperandsSet = selectors.getCompulsoryDataElementOperandsSet(metadata, dataSetId) return compulsoryDataElementOperandsSet.has( @@ -22,7 +25,10 @@ export const useHasCompulsoryDataElementOperandsToFillOut = () => { const { data: metadata } = useMetadata() const [dataSetId] = useDataSetId() const hasCompulsoryDataElementOperandsToFillOut = useMemo(() => { - const dataSet = selectors.getDataSetById(dataSetId) + if (!dataSetId) { + return false + } + const dataSet = selectors.getDataSetById(metadata, dataSetId) const { compulsoryFieldsCompleteOnly } = dataSet || {} diff --git a/src/shared/validation/index.js b/src/shared/validation/index.js index 5bb9ed80d..c43dc1897 100644 --- a/src/shared/validation/index.js +++ b/src/shared/validation/index.js @@ -1,5 +1,5 @@ export { default as buildValidationResult } from './build-validation-result.js' export { isInteger } from './is-integer.js' export * from './query-key-factory.js' -export { default as useImperativeValidate } from './use-imperative-validate.js' +export { useImperativeValidate } from './use-imperative-validate.js' export { default as useValidationResult } from './use-validation-result.js' diff --git a/src/shared/validation/use-imperative-validate.js b/src/shared/validation/use-imperative-validate.js index 518833eaf..f4cd92562 100644 --- a/src/shared/validation/use-imperative-validate.js +++ b/src/shared/validation/use-imperative-validate.js @@ -12,7 +12,7 @@ import { * @params {Object} options * @params {Boolean} options.enabled - Defaults to `true` **/ -export default function useImperativeValidate() { +export const useImperativeValidate = () => { const client = useQueryClient() const [{ dataSetId, orgUnitId, periodId }] = useContextSelection() const { diff --git a/src/shared/validation/use-imperative-validate.test.js b/src/shared/validation/use-imperative-validate.test.js index 54bcc3876..bb2af9317 100644 --- a/src/shared/validation/use-imperative-validate.test.js +++ b/src/shared/validation/use-imperative-validate.test.js @@ -2,7 +2,7 @@ import { useQueryClient } from '@tanstack/react-query' import { renderHook } from '@testing-library/react-hooks' import React from 'react' import { Wrapper } from '../../test-utils/index.js' -import useImperativeValidate from './use-imperative-validate.js' +import { useImperativeValidate } from './use-imperative-validate.js' jest.mock('@tanstack/react-query', () => { const originalModule = jest.requireActual('@tanstack/react-query')