From fe8afe2ef08540c2ad202b84734f66610c2bb5c7 Mon Sep 17 00:00:00 2001 From: Xavier Molloy Date: Wed, 18 Dec 2024 08:07:06 +0100 Subject: [PATCH] fix: [ANDROAPP-6667] Add field to warning list for warning on complete program rule (#3917) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: [ANDROAPP-6667] Add field to warning list for warning on complete rule * fix: [ANDROAPP-6667] display warning/error on complete Signed-off-by: Manu Muñoz * fix: [ANDROAPP-6667] add tests Signed-off-by: Manu Muñoz * fix: [ANDROAPP-6667] fix test Signed-off-by: Manu Muñoz * fix test Signed-off-by: Manu Muñoz --------- Signed-off-by: Manu Muñoz Co-authored-by: Manu Muñoz --- .../programevent/ProgramEventTest.kt | 1 - .../org/dhis2/form/data/FormRepositoryImpl.kt | 52 +++---------- .../dhis2/form/data/RulesUtilsProviderImpl.kt | 3 + .../ui/provider/FormResultDialogProvider.kt | 76 +++++++++++-------- .../dhis2/form/data/FormRepositoryImplTest.kt | 7 +- .../form/data/RulesUtilsProviderImplTest.kt | 1 + .../provider/FormResultDialogProviderTest.kt | 49 ++++++++++++ .../bottomsheet/BottomSheetDialogContent.kt | 14 ++-- 8 files changed, 122 insertions(+), 81 deletions(-) diff --git a/app/src/androidTest/java/org/dhis2/usescases/programevent/ProgramEventTest.kt b/app/src/androidTest/java/org/dhis2/usescases/programevent/ProgramEventTest.kt index cbcbd2d6ff..53f39779b3 100644 --- a/app/src/androidTest/java/org/dhis2/usescases/programevent/ProgramEventTest.kt +++ b/app/src/androidTest/java/org/dhis2/usescases/programevent/ProgramEventTest.kt @@ -62,7 +62,6 @@ class ProgramEventTest : BaseTest() { } eventRobot(composeTestRule) { - openEventDetailsSection() checkEventDetails(eventDate, eventOrgUnit) } } diff --git a/form/src/main/java/org/dhis2/form/data/FormRepositoryImpl.kt b/form/src/main/java/org/dhis2/form/data/FormRepositoryImpl.kt index 0895fd5083..4a383ca5a7 100644 --- a/form/src/main/java/org/dhis2/form/data/FormRepositoryImpl.kt +++ b/form/src/main/java/org/dhis2/form/data/FormRepositoryImpl.kt @@ -234,7 +234,7 @@ class FormRepositoryImpl( return when { (itemsWithErrors.isEmpty() && itemsWithWarning.isEmpty() && mandatoryItemsWithoutValue.isEmpty()) -> { - getSuccessfulResult(eventStatus) + getSuccessfulResult() } (itemsWithErrors.isNotEmpty()) -> { @@ -285,7 +285,7 @@ class FormRepositoryImpl( EventStatus.COMPLETED -> { FieldsWithWarningResult( fieldUidWarningList = itemsWithWarning, - canComplete = false, + canComplete = ruleEffectsResult?.canComplete ?: false, onCompleteMessage = ruleEffectsResult?.messageOnComplete, eventResultDetails = EventResultDetails( formValueStore.eventState(), @@ -427,44 +427,16 @@ class FormRepositoryImpl( } } - private fun getSuccessfulResult(eventStatus: EventStatus?): SuccessfulResult { - return when (eventStatus) { - EventStatus.ACTIVE -> { - SuccessfulResult( - canComplete = ruleEffectsResult?.canComplete ?: true, - onCompleteMessage = ruleEffectsResult?.messageOnComplete, - eventResultDetails = EventResultDetails( - formValueStore.eventState(), - dataEntryRepository.eventMode(), - dataEntryRepository.validationStrategy(), - ), - ) - } - - EventStatus.COMPLETED -> { - SuccessfulResult( - canComplete = false, - onCompleteMessage = ruleEffectsResult?.messageOnComplete, - eventResultDetails = EventResultDetails( - formValueStore.eventState(), - dataEntryRepository.eventMode(), - dataEntryRepository.validationStrategy(), - ), - ) - } - - else -> { - SuccessfulResult( - canComplete = ruleEffectsResult?.canComplete ?: false, - onCompleteMessage = ruleEffectsResult?.messageOnComplete, - eventResultDetails = EventResultDetails( - formValueStore.eventState(), - dataEntryRepository.eventMode(), - validationStrategy = dataEntryRepository.validationStrategy(), - ), - ) - } - } + private fun getSuccessfulResult(): SuccessfulResult { + return SuccessfulResult( + canComplete = ruleEffectsResult?.canComplete ?: true, + onCompleteMessage = ruleEffectsResult?.messageOnComplete, + eventResultDetails = EventResultDetails( + formValueStore.eventState(), + dataEntryRepository.eventMode(), + dataEntryRepository.validationStrategy(), + ), + ) } override fun completedFieldsPercentage(value: List): Float { diff --git a/form/src/main/java/org/dhis2/form/data/RulesUtilsProviderImpl.kt b/form/src/main/java/org/dhis2/form/data/RulesUtilsProviderImpl.kt index 238325287c..7711fe459a 100644 --- a/form/src/main/java/org/dhis2/form/data/RulesUtilsProviderImpl.kt +++ b/form/src/main/java/org/dhis2/form/data/RulesUtilsProviderImpl.kt @@ -385,6 +385,9 @@ class RulesUtilsProviderImpl( val message = warningOnCompletion.content() + " " + data if (model != null) { fieldViewModels[fieldUid] = model.setWarning(message) + fieldsWithWarnings.add( + FieldWithError(fieldUid, message), + ) } messageOnComplete = message diff --git a/form/src/main/java/org/dhis2/form/ui/provider/FormResultDialogProvider.kt b/form/src/main/java/org/dhis2/form/ui/provider/FormResultDialogProvider.kt index 0d3b58e96c..727eab537a 100644 --- a/form/src/main/java/org/dhis2/form/ui/provider/FormResultDialogProvider.kt +++ b/form/src/main/java/org/dhis2/form/ui/provider/FormResultDialogProvider.kt @@ -33,11 +33,15 @@ class FormResultDialogProvider( eventState: EventStatus?, result: DataIntegrityCheckResult, ): Pair> { + val onCompleteMessages = getOnCompleteMessage( + canComplete, + onCompleteMessage, + ) val dialogType = getDialogType( errorFields, emptyMandatoryFields, warningFields, - !canComplete && onCompleteMessage != null, + onCompleteMessages, ) val showSkipButton = when { dialogType == DialogType.WARNING || dialogType == DialogType.SUCCESSFUL -> true @@ -74,12 +78,14 @@ class FormResultDialogProvider( result.fieldUidErrorList, result.mandatoryFields.keys.toList(), result.warningFields, + onCompleteMessages, ) Pair(model, fieldsWithIssues) } is FieldsWithWarningResult -> { val fieldsWithIssues = getFieldsWithIssues( warningFields = result.fieldUidWarningList, + onCompleteFields = onCompleteMessages, ) return Pair(model, fieldsWithIssues) } @@ -101,7 +107,7 @@ class FormResultDialogProvider( Pair(notSavedModel, emptyList()) } is SuccessfulResult -> { - Pair(model, emptyList()) + Pair(model, onCompleteMessages) } } } @@ -166,44 +172,54 @@ class FormResultDialogProvider( errorFields: List = emptyList(), mandatoryFields: List = emptyList(), warningFields: List = emptyList(), + onCompleteFields: List = emptyList(), ): List { - return errorFields.plus( - mandatoryFields.map { - FieldWithIssue( - "uid", - it, - IssueType.MANDATORY, - provider.provideMandatoryField(), - ) - }, - ).plus(warningFields) + return onCompleteFields + .plus(errorFields) + .plus( + mandatoryFields.map { + FieldWithIssue( + "uid", + it, + IssueType.MANDATORY, + provider.provideMandatoryField(), + ) + }, + ).plus(warningFields) + } + + private fun getOnCompleteMessage( + canComplete: Boolean, + onCompleteMessage: String?, + ): List { + val issueOnComplete = onCompleteMessage?.let { + FieldWithIssue( + fieldUid = "", + fieldName = it, + issueType = when (canComplete) { + false -> IssueType.ERROR_ON_COMPLETE + else -> IssueType.WARNING_ON_COMPLETE + }, + message = "", + ) + } + return issueOnComplete?.let { listOf(it) } ?: emptyList() } private fun getDialogType( errorFields: List, mandatoryFields: Map, warningFields: List, - errorOnComplete: Boolean, + onCompleteFields: List, ) = when { - errorOnComplete -> { + onCompleteFields.any { it.issueType == IssueType.ERROR_ON_COMPLETE } -> DialogType.COMPLETE_ERROR - } - - errorFields.isNotEmpty() -> { - DialogType.ERROR - } - - mandatoryFields.isNotEmpty() -> { - DialogType.MANDATORY - } - - warningFields.isNotEmpty() -> { + errorFields.isNotEmpty() -> DialogType.ERROR + mandatoryFields.isNotEmpty() -> DialogType.MANDATORY + warningFields.isNotEmpty() || + onCompleteFields.any { it.issueType == IssueType.WARNING_ON_COMPLETE } -> DialogType.WARNING - } - - else -> { - DialogType.SUCCESSFUL - } + else -> DialogType.SUCCESSFUL } enum class DialogType { ERROR, MANDATORY, WARNING, SUCCESSFUL, COMPLETE_ERROR } } diff --git a/form/src/test/java/org/dhis2/form/data/FormRepositoryImplTest.kt b/form/src/test/java/org/dhis2/form/data/FormRepositoryImplTest.kt index 09fec26410..6582cf13a1 100644 --- a/form/src/test/java/org/dhis2/form/data/FormRepositoryImplTest.kt +++ b/form/src/test/java/org/dhis2/form/data/FormRepositoryImplTest.kt @@ -37,7 +37,6 @@ import org.mockito.kotlin.atLeast import org.mockito.kotlin.doReturn import org.mockito.kotlin.doReturnConsecutively import org.mockito.kotlin.mock -import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -290,16 +289,16 @@ class FormRepositoryImplTest { fun `Should allow to complete only uncompleted events`() { whenever( dataEntryRepository.list(), - ) doReturn Flowable.just(provideMandatoryItemList().filter { !it.mandatory }) + ) doReturn Flowable.just(provideMandatoryItemList()) whenever(dataEntryRepository.isEvent()) doReturn true whenever(formValueStore.eventState()) doReturn EventStatus.ACTIVE repository.fetchFormItems() - assertTrue(repository.runDataIntegrityCheck(false) is SuccessfulResult) + assertTrue(repository.runDataIntegrityCheck(false) is MissingMandatoryResult) assertTrue(repository.runDataIntegrityCheck(false).canComplete) whenever(formValueStore.eventState()) doReturn EventStatus.COMPLETED repository.fetchFormItems() - assertTrue(repository.runDataIntegrityCheck(false) is SuccessfulResult) + assertTrue(repository.runDataIntegrityCheck(false) is MissingMandatoryResult) assertFalse(repository.runDataIntegrityCheck(false).canComplete) } diff --git a/form/src/test/java/org/dhis2/form/data/RulesUtilsProviderImplTest.kt b/form/src/test/java/org/dhis2/form/data/RulesUtilsProviderImplTest.kt index 1a8064ebeb..e87fa45c87 100644 --- a/form/src/test/java/org/dhis2/form/data/RulesUtilsProviderImplTest.kt +++ b/form/src/test/java/org/dhis2/form/data/RulesUtilsProviderImplTest.kt @@ -593,6 +593,7 @@ class RulesUtilsProviderImplTest { assertEquals(testFieldViewModels[testingUid]!!.warning, "content data") assertTrue(result.messageOnComplete == "content data") + assertTrue(result.fieldsWithWarnings.isNotEmpty()) assertTrue(result.canComplete) } diff --git a/form/src/test/java/org/dhis2/form/ui/provider/FormResultDialogProviderTest.kt b/form/src/test/java/org/dhis2/form/ui/provider/FormResultDialogProviderTest.kt index 19ab6cb028..d40e34ffce 100644 --- a/form/src/test/java/org/dhis2/form/ui/provider/FormResultDialogProviderTest.kt +++ b/form/src/test/java/org/dhis2/form/ui/provider/FormResultDialogProviderTest.kt @@ -7,6 +7,7 @@ import org.dhis2.form.data.MissingMandatoryResult import org.dhis2.form.data.SuccessfulResult import org.dhis2.form.model.EventMode import org.dhis2.ui.dialogs.bottomsheet.DialogButtonStyle +import org.dhis2.ui.dialogs.bottomsheet.IssueType import org.hisp.dhis.android.core.common.ValidationStrategy import org.hisp.dhis.android.core.event.EventStatus import org.junit.Assert.assertTrue @@ -98,6 +99,54 @@ class FormResultDialogProviderTest { assertTrue(noErrorsInFormModel.first.mainButton == DialogButtonStyle.CompleteButton) } + @Test + fun `Should configure to show warning on complete message`() { + val completedEventWithNoErrors = SuccessfulResult( + canComplete = true, + onCompleteMessage = "Warning on complete", + eventResultDetails = EventResultDetails( + eventStatus = EventStatus.ACTIVE, + eventMode = EventMode.CHECK, + validationStrategy = ValidationStrategy.ON_COMPLETE, + ), + ) + val noErrorsInFormModel = formResultDialogProvider.invoke( + canComplete = completedEventWithNoErrors.canComplete, + onCompleteMessage = completedEventWithNoErrors.onCompleteMessage, + errorFields = emptyList(), + emptyMandatoryFields = emptyMap(), + warningFields = emptyList(), + eventMode = completedEventWithNoErrors.eventResultDetails.eventMode ?: EventMode.NEW, + eventState = completedEventWithNoErrors.eventResultDetails.eventStatus ?: EventStatus.ACTIVE, + result = completedEventWithNoErrors, + ) + assertTrue(noErrorsInFormModel.second.first().issueType == IssueType.WARNING_ON_COMPLETE) + } + + @Test + fun `Should configure to show error on complete message`() { + val completedEventWithNoErrors = SuccessfulResult( + canComplete = false, + onCompleteMessage = "error on complete", + eventResultDetails = EventResultDetails( + eventStatus = EventStatus.ACTIVE, + eventMode = EventMode.NEW, + validationStrategy = ValidationStrategy.ON_COMPLETE, + ), + ) + val noErrorsInFormModel = formResultDialogProvider.invoke( + canComplete = completedEventWithNoErrors.canComplete, + onCompleteMessage = completedEventWithNoErrors.onCompleteMessage, + errorFields = emptyList(), + emptyMandatoryFields = emptyMap(), + warningFields = emptyList(), + eventMode = completedEventWithNoErrors.eventResultDetails.eventMode ?: EventMode.NEW, + eventState = completedEventWithNoErrors.eventResultDetails.eventStatus ?: EventStatus.ACTIVE, + result = completedEventWithNoErrors, + ) + assertTrue(noErrorsInFormModel.second.first().issueType == IssueType.ERROR_ON_COMPLETE) + } + @Test fun `Should follow validation strategy when trying to save the form with errors`() { val completedEventWithNoErrors = SuccessfulResult( diff --git a/ui-components/src/main/java/org/dhis2/ui/dialogs/bottomsheet/BottomSheetDialogContent.kt b/ui-components/src/main/java/org/dhis2/ui/dialogs/bottomsheet/BottomSheetDialogContent.kt index cd81b20c7b..c5e8720ab0 100644 --- a/ui-components/src/main/java/org/dhis2/ui/dialogs/bottomsheet/BottomSheetDialogContent.kt +++ b/ui-components/src/main/java/org/dhis2/ui/dialogs/bottomsheet/BottomSheetDialogContent.kt @@ -243,11 +243,13 @@ fun IssueItem(fieldWithIssue: FieldWithIssue, onClick: () -> Unit) { color = textPrimary, fontSize = 14.sp, ) - Text( - text = fieldWithIssue.message, - color = textSecondary, - fontSize = 14.sp, - ) + if (fieldWithIssue.message.isNotEmpty()) { + Text( + text = fieldWithIssue.message, + color = textSecondary, + fontSize = 14.sp, + ) + } } } } @@ -308,7 +310,7 @@ fun DialogPreview3() { fun DialogPreview4() { val fieldsWithIssues = listOf( FieldWithIssue("Uid", "Age", IssueType.ERROR, ERROR_MESSAGE), - FieldWithIssue("Uid", DATE_BIRTH, IssueType.ERROR, ERROR_MESSAGE), + FieldWithIssue("Uid", DATE_BIRTH, IssueType.ERROR, ""), FieldWithIssue("Uid", DATE_BIRTH, IssueType.ERROR, ERROR_MESSAGE), FieldWithIssue("Uid", DATE_BIRTH, IssueType.ERROR, ERROR_MESSAGE), FieldWithIssue("Uid", DATE_BIRTH, IssueType.ERROR, ERROR_MESSAGE),