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

Allow forms with dynamic preload data to be bulk finalized #5775

Merged
merged 12 commits into from
Oct 13, 2023

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Oct 10, 2023

Work towards #5740

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

The simplest thing to do here was to rework how pulldata is setup so it happens for both normal and bulk finalization. pulldata didn't have any existing tests, so I added a new one so the the refactor would be covered.

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?

As well as checking that forms with pulldata/search bulk finalize correctly, it's important to check forms that use pulldata in constraints and check that they fail to bulk finalize in the correct contexts. I don't think it's possible to have search involves in constraints, but happy to be corrected there!

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

@seadowg seadowg marked this pull request as draft October 10, 2023 13:13
override fun create(formDef: FormDef): FormEntryController {
override fun create(formDef: FormDef, formMediaDir: File): FormEntryController {
val externalDataManager = ExternalDataManagerImpl(formMediaDir).also {
Collect.getInstance().externalDataManager = it
Copy link
Member Author

@seadowg seadowg Oct 10, 2023

Choose a reason for hiding this comment

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

I really want to get rid of this static state, but I'm worried about the consequences of doing that while #5665 is in flight. I think that kind of improvement should be done as part of that work instead.

@seadowg seadowg marked this pull request as ready for review October 10, 2023 13:21
@seadowg seadowg mentioned this pull request Oct 11, 2023
5 tasks
@dbemke
Copy link

dbemke commented Oct 12, 2023

Should the validation of a required question in a follow up form for entities work via "Finalize all form"? I can leave the required question empty and save the form, then in Drafts the form is Incomplete but when I tap "Finalize all forms" and the form is finalized.

Steps to reproduce:

  1. Fill in some submissions in a form creating entities.
    child_only_registration.xlsx.txt
    Children follow-up.xlsx.txt
  2. Go to a follow up form and don't fill the required answer (go to the hierarchy view, go to end, save as draft).
  3. Go to Drafts and tap "Finalize all forms".

The form is ready in our test server in project 436 user pulll data.

@seadowg
Copy link
Member Author

seadowg commented Oct 12, 2023

Should the validation of a required question in a follow up form for entities work via "Finalize all form"?

I can't think of anything that would cause entities not to work. There's nothing different about them from a validation point of view. Could you try a form with a required select_one and another one with a required select_one_from_file as well? I'm wondering if there's a problem with how we validate selects.

Oh, and does the form finalize if you hit "Finalize" in form entry with that question unanswered?

@dbemke
Copy link

dbemke commented Oct 12, 2023

Oh, and does the form finalize if you hit "Finalize" in form entry with that question unanswered?

If I try to swipe to next question or finalize the form with required question unanswered there is a info that it's required so I can't finalize it. I can't finalize it also when I leave the answer empty while I edit in Drafts.

@dbemke
Copy link

dbemke commented Oct 12, 2023

Updated steps to reproduce the issue mentioned above.
"Finalize all forms” doesn’t check the updated form version (in this case required question).

Steps to reproduce:

  1. Go to test.getodk.cloud→ project "d testing Central” and find Children follow-up form.
  2. From the form overview go to Versions tab and download version 1 and required.
  3. In a different project upload the form version 1 and add access to the form to use it in Collect.
  4. Fill the form in Collect (probably optional step).
  5. Upload the required version of the form to Central.
  6. Refresh the list of forms in Collect.
  7. Go to the form – don’t fill anything, go to the hierarchy view, go to end and save as draft.
  8. Go to drafts and tap "Finalize all forms”.

@seadowg
Copy link
Member Author

seadowg commented Oct 12, 2023

@dbemke that's fixed! @grzesiek2010 it's probably worth taking another look as I had to move things around a bit.

@srujner
Copy link

srujner commented Oct 13, 2023

Tested with Success! alongside with @dbemke

Verified on devices with Android: 8.1, 10, 12, 13

Verified cases:

  • Regression check on Big File Forms;
  • General check on Forms with Pull Data/Search;
  • General check on Forms with Constraints and Required fields;
  • Issues from the comments are no longer reproducing;
  • Bulk Finalize checked;
  • Screen rotation, minimize app, using backward button;
  • Light and Dark theme.

@seadowg seadowg merged commit 07d4688 into getodk:v2023.3.x Oct 13, 2023
6 checks passed
@seadowg seadowg deleted the bulk-other-form-types branch October 13, 2023 15:59
seadowg added a commit to seadowg/collect that referenced this pull request Oct 18, 2023
Allow forms with dynamic preload data to be bulk finalized
seadowg added a commit to seadowg/collect that referenced this pull request Oct 24, 2023
Allow forms with dynamic preload data to be bulk finalized
seadowg added a commit to seadowg/collect that referenced this pull request Oct 30, 2023
Allow forms with dynamic preload data to be bulk finalized
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.

4 participants