Skip to content

Separate validationResult and constraintValidationResult#7038

Merged
grzesiek2010 merged 2 commits intogetodk:masterfrom
grzesiek2010:COLLECT-7033
Jan 27, 2026
Merged

Separate validationResult and constraintValidationResult#7038
grzesiek2010 merged 2 commits intogetodk:masterfrom
grzesiek2010:COLLECT-7033

Conversation

@grzesiek2010
Copy link
Copy Markdown
Member

@grzesiek2010 grzesiek2010 commented Jan 20, 2026

Closes #7033

Why is this the best possible solution? Were any other approaches considered?

We had a single validationResult LiveData observed by FormFillingActivity and the map fragment, used for two different cases: validating saved answers in the form view and validating polygons on the map. Initially, it seemed reasonable to reuse the same LiveData for both, but over time the two cases started to diverge.
We stopped saving answers when validating polygons, and later decided that only constraint errors were needed in that flow. As a result, sharing the same LiveData caused more problems than benefits.
We also don’t want to update the form view (e.g. display errors) when validation occurs in the map flow, so separating these cases turned out to be the best solution.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

There should now be no relationship between the errors displayed on the map and those shown in the form view, meaning that these two cases should not affect each ot. We already know how tricky these cases can be, so we need to be careful and test all related scenarios.

Do we need any specific form for testing your changes? If so, please attach one.

The form is linked in the issue.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

  • added or modified tests for any new or changed behavior
  • run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
  • added a comment above any new strings describing it for translators
  • added any new strings with date formatting to DateFormatsTest
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@grzesiek2010 grzesiek2010 requested a review from seadowg January 21, 2026 00:21
@grzesiek2010 grzesiek2010 marked this pull request as ready for review January 21, 2026 00:21
Copy link
Copy Markdown
Member

@seadowg seadowg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused as to how this case was even occurring: when the user hits clear shouldn't validateResult be set to null and remove the validation message? I'm not getting a sense of the root cause here so having a hard time understanding the solution.

@grzesiek2010
Copy link
Copy Markdown
Member Author

I'm confused as to how this case was even occurring: when the user hits clear shouldn't validateResult be set to null and remove the validation message? I'm not getting a sense of the root cause here so having a hard time understanding the solution.

The observer in FormFillingActivity ignores null values:

    private void handleValidationResult(ValidationResult validationResult) {
        if (validationResult instanceof FailedValidationResult failedValidationResult) {
            String errorMessage = failedValidationResult.getCustomErrorMessage();
            if (errorMessage == null) {
                errorMessage = getString(failedValidationResult.getDefaultErrorMessage());
            }
            getCurrentViewIfODKView().setErrorForQuestionWithIndex(failedValidationResult.getIndex(), errorMessage);
            swipeHandler.setBeenSwiped(false);
        } else if (validationResult instanceof SuccessValidationResult) {
            SnackbarUtils.showSnackbar(
                    findViewById(R.id.llParent),
                    getString(org.odk.collect.strings.R.string.success_form_validation),
                    SnackbarUtils.DURATION_LONG,
                    findViewById(R.id.buttonholder)
            );
        }
    }

This means that when an error occurs during validation in Maps, the observer in FormFillingActivity is also triggered, showing an error that is then not cleared. We don’t need to update the form view in such cases, there’s no point in doing so, and it was only causing other similar issues we were trying to handle.

Copy link
Copy Markdown
Member

@seadowg seadowg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The observer in FormFillingActivity ignores null values:

Ah and we don't handle success cases in validateAnswerConstraint. That will actually have to change for #6970, but I agree with you that it's weird for the validation state to be shared between the "page" and the individual question at this point.

What do you think about introducing a QuestionViewModel that exists for the lifecycle of the WidgetAnswerDialogFragment and talks directly to the FormController instead of holding this question "detail" state in FormEntryViewModel? I'm thinking something like:

class QuestionViewModel(index: FormIndex, formSessionRepository: FormSessionRepository, sessionId: String) : ViewModel {

    fun getValidationResult(): LiveData<ValidationResult> {
        ...
    }

    fun getPrompt(): FormEntryPrompt {
        ...
    }


    fun validate(answer: IAnswerData?) {
        ...
    }
}

That could also be a follow-up I guess, but it seems like the state that shouldn't be shared with the full form session should live in its own place. WidgetAnswerDialogFragmetn would still need to talk to FormEntryViewModel to answer the question, but that's state we do want to share.

@grzesiek2010
Copy link
Copy Markdown
Member Author

What do you think about introducing a QuestionViewModel that exists for the lifecycle of the WidgetAnswerDialogFragment and talks directly to the FormController instead of holding this question "detail" state in FormEntryViewModel?

I was also thinking about that. Done.

@grzesiek2010 grzesiek2010 merged commit 1dd7a9d into getodk:master Jan 27, 2026
7 checks passed
@dbemke
Copy link
Copy Markdown

dbemke commented Jan 29, 2026

Tested with Success!

Verified on devices with Android 10

Verified cases:

@WKobus
Copy link
Copy Markdown

WKobus commented Jan 29, 2026

Tested with succes

Verified on Android 16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Geoshape/trace with incremental= true - error message after removing shape/trace and tapping back button

4 participants