Skip to content
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

Specify when formIndex should be updated when there are errors during validating answers #5848

Merged
merged 4 commits into from
Nov 27, 2023

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Nov 22, 2023

Closes #5847

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

The problem was that every time we validated a form if there were errors the form index was changed. This should not happen if we just save the form using the save icon from the toolbar (we still need to validate answers to know if there are errors in this case). Updating the index is needed when we Check for errors or try to finalize a form to move a user to the invalid question. So the solution was to introduce a new parameter to explicitly tell the method that validates answers if the index should be updated.

Another option would be to move updating index out of the validateAnswers method but then we would need to add this to more than one method and those methods are not better places for such an operation.

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?

We need to test validating answers carefully:

  • finalizing forms with errors
  • using the Check for errors option from the toolbar (with errors)
  • saving forms using the save icon from the toolbar

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

Any form with required questions that could be invalid so required questions without answers or questions with constraints.

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 checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • added a comment above any new strings describing it for translators
  • 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 added the high priority Should be looked at before other PRs/issues label Nov 22, 2023
@seadowg
Copy link
Member

seadowg commented Nov 23, 2023

Another option would be to move updating index out of the validateAnswers method but then we would need to add this to more than one method and those methods are not better places for such an operation.

I do think this is the better long term move, but I agree that it would require a little more of a rethink around the interface than we'll want to do for this bug fix.

@dbemke
Copy link

dbemke commented Nov 23, 2023

Can you check the form mentioned in the issue (two-question-required.xml.txt))? Central can't upload the form because there isn't meta group in the form.

@grzesiek2010
Copy link
Member Author

Can you check the form mentioned in the issue (two-question-required.xml.txt))? Central can't upload the form because there isn't meta group in the form.

I took it from our tests but it could be any form with two questions (the second one should be required). This one should work:
req.xlsx

@srujner
Copy link

srujner commented Nov 27, 2023

Tested with Success!

Verified on device with Android 13

Verified cases:

@dbemke
Copy link

dbemke commented Nov 27, 2023

Tested with Success!

Verified on device with Android 10

@grzesiek2010 grzesiek2010 merged commit 7e40238 into getodk:v2023.3.x Nov 27, 2023
6 checks passed
seadowg pushed a commit to seadowg/collect that referenced this pull request Nov 30, 2023
Specify when formIndex should be updated when there are errors during validating answers
seadowg pushed a commit to seadowg/collect that referenced this pull request Dec 7, 2023
Specify when formIndex should be updated when there are errors during validating answers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior verified high priority Should be looked at before other PRs/issues
Projects
None yet
4 participants