Improvements to bulk finalize #5776

seadowg · 2023-10-11T13:10:38Z

Closes #5740
Closes #5763
Closes #5741
~~Blocked by #5775~~

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

Comments inline.

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?

Other than the behaviour described in the three issues, I think testing QR imports/exports would be good as we've added and removed settings keys.

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 · 2023-10-16T09:04:45Z

settings/src/test/java/org/odk/collect/settings/ODKAppSettingsMigratorTest.java

+    }
+
+    @Test
+    public void migratesFinalizeInFormEntryToNewKey() {


I couldn't think of another way to support upgrades and imports from up to date versions other than creating a new key. Otherwise, we'd end up with QR codes from up to date versions with only "finalise in form entry" disabled also disabling bulk finalise.

This means that if you have the existing finalize key in your QR code, it will forever more turn off both finalization at end of form and bulk finalize when you scan it in, right? That sounds fine and good.

If you start from a clean Collect install and you hide finalization from form entry, that will no affect bulk finalization. If you build a QR code from those settings, bulk finalization will be enabled unless you also explicitly unchecked it.

Unless I made a mistake in my interpretation, I think the approach is great.

seadowg · 2023-10-16T09:05:27Z

androidshared/src/main/java/org/odk/collect/androidshared/ui/SnackbarUtils.kt

+
+    data class Action(val text: String, val listener: () -> Unit)
+
+    abstract class SnackbarPresenterObserver<T : Any?>(private val parentView: View) :


We can probably use this in a few more places, but as with other PRs for v2023.3 I wanted to minimise the merge conflict risk.

seadowg · 2023-10-16T09:07:56Z

collect_app/src/main/java/org/odk/collect/android/activities/InstanceChooserList.java

+            DraftsMenuProvider draftsMenuProvider = new DraftsMenuProvider(this, bulkFinalizationViewModel::finalizeAllDrafts);
+            addMenuProvider(draftsMenuProvider, this);
+            bulkFinalizationViewModel.getDraftsCount().observe(this, draftsCount -> {
+                draftsMenuProvider.setDraftsCount(draftsCount);


We've previously played around with handling observations in MenuProviders i.e. treating them more like a Fragment than a View. I was finding that I needed a lot of test infrastructure to get this working however and that the component seems to prefer this more View like setup.

There's unfortunately no docs around MenuProvider yet, so it's hard to know what the intention is.

collect_app/src/main/java/org/odk/collect/android/formmanagement/InstancesDataService.kt

...p/src/main/java/org/odk/collect/android/instancemanagement/FinalizeAllSnackbarPresenter.java

collect_app/src/main/res/xml/form_entry_access_preferences.xml

srujner · 2023-10-19T11:26:54Z

In the Acceptance there is: And I uncheck "Finalize all drafts" in the "Uncheck to hide from Drafts" section”
But in the app it is called “Finalize all forms” instead of “Finalize all drafts”. Is there a bug in the Acceptance Criteria or in the app?

srujner · 2023-10-19T11:27:57Z

Drafts with save points
In the Acceptance: "These should fail to bulk finalize, but the user should see a message that drafts with save points cannot be bulk finalized"
Actual Message in the app: "Some forms needs to be finalized manually"

dbemke · 2023-10-19T13:57:07Z

What is the expected result for an encrypted form in a non-encrypted project? The same as in encrypted projects?

lognaturel · 2023-10-19T15:05:54Z

We’re trying a new process where the strings will be finalized separately. So as long as there’s some relevant text it’s ok for now.

What is the expected result for an encrypted form in a non-encrypted project? The same as in encrypted projects?

Yes. So I guess in an encrypted project nothing could ever be bulk finalized. A more interesting case is when some forms have encryptions and others don’t.

… in form entry disabled

grzesiek2010 · 2023-10-20T09:11:07Z

I've updated the branch. Probably it didn't contain all recently merged changes.

srujner · 2023-10-20T10:06:34Z

Just to make sure:
During editing the form that already is in Complete status I killed the app (Save Point has been created).
Then I returned to Drafts and the Edited Form is still in Complete status but when I click on the Finalize all Forms button then the error is shown.
This is correct that the Form is in Complete status and not Incomplete?

srujner · 2023-10-20T10:11:29Z

Confirm before bulk finalizing dialog is in old UI

dbemke · 2023-10-20T10:12:08Z

After trying to bulk finalize an encrypted form "the user should see a message that encrypted forms are currently not supported".. should the text in the toast say something about encryption like in mentioned quote from AC?

grzesiek2010 · 2023-10-20T10:33:26Z

Confirm before bulk finalizing dialog is in old UI

All dialogs on this branch are in old UI because #5676 was merged to master and will be a part of v2023.4 not v2023.3

grzesiek2010 · 2023-10-20T10:36:08Z

After trying to bulk finalize an encrypted form "the user should see a message that encrypted forms are currently not supported".. should the text in the toast say something about encryption like in mentioned quote from AC?

That message could be improved but as @lognaturel said:

We’re trying a new process where the strings will be finalized separately. So as long as there’s some relevant text it’s ok for now.

it's ok for now.

grzesiek2010 · 2023-10-20T11:56:06Z

Just to make sure:
During editing the form that already is in Complete status I killed the app (Save Point has been created).
Then I returned to Drafts and the Edited Form is still in Complete status but when I click on the Finalize all Forms button then the error is shown.
This is correct that the Form is in Complete status and not Incomplete?

Maybe it would be better to see the incomplete status in this case but this is not something important or common so we can merge this pr without changing anything there.
The problem is also that we don't have any mechanism for detecting killing the app so we can't save the incomplete status in form filling when it happens. That means to solve this issue we would rather not even update the status in the database but just check if a draft has a savepoint and then update the status displayed in the list of drafts (during building every single list element).

dbemke · 2023-10-20T12:08:57Z

I'm trying to figure out the expected result or differences in settings/outcomes after using a QR code from 2022.3.0. In e.g. 2023.2.4 there are also different settings for finalizing and saving drafts and also in the PR settings changed so I feel I may miss something. I'm posting the results so maybe someone will notice something that shouldn't be there or some unexpected changes.

I created a QR code in v2022.3.0 in which there is "Mark form as finalized" setting (not "Finalize" like newer versions). I unchecked "Mark form as finalized" and scanned the QR code with that setting with PR version. As a result in the new/PR version "Save as draft"(in Form End) is unchecked.

If I create a QR code in 2022.3.0 with "Save Form" unchecked and add the QR code in the PR version than the settings look like this.

grzesiek2010 · 2023-10-20T12:47:11Z

I created a QR code in v2022.3.0 in which there is "Mark form as finalized" setting (not "Finalize" like newer versions). I unchecked "Mark form as finalized" and scanned the QR code with that setting with PR version. As a result in the new/PR version "Save as draft"(in Form End) is unchecked.

In v2023.2 we added this: #5416

Given I've scanned an old QR code
And "Default to finalized" in Form Entry was toggled off when the code was generated
And "Mark form as finalized" was disabled in Access Control (under Form Entry Settings) when the code was generated
When I go to Form Entry Settings in Access Control
Then "Finalize" is disabled

 Given I've scanned an old QR code
And "Default to finalized" in Form Entry was toggled on when the code was generated
And "Mark form as finalized" was disabled in Access Control (under Form Entry Settings) when the code was generated
When I go to Form Entry Settings in Access Control
Then "Save as draft" is disabled

so if you upgrade from v2022.3.0 the upgrade path is like:
v2022.3-> v2022.4 -> v2023.1 -> v2023.2 -> v2023.3 so that's why you end up with that setting unchecked. Does that make sense?

If I create a QR code in 2022.3.0 with "Save Form" unchecked and add the QR code in the PR version than the settings look like this.

The old Save Form was renamed to Save as Draft see #5580 but it's the same setting so if it was checked in a qr code it should be unchecked after scanning it right?

srujner · 2023-10-20T13:59:52Z

@lognaturel Could you take one last look at all of our comments before @grzesiek2010 will merge this pull request?
We listed all the things that somehow confused us. If you're Ok with them, then you can remove the Needs Testing label and we're good to go :)

dbemke · 2023-10-20T14:00:44Z

Tested with Success!

Verified on device with Android 10

Verified cases:

bulk finalize in an encrypted project
bulk finalize in a non-encrypted project with forms that are encrypted
bulk finalize forms with a savepoint
toast after trying to bulk finalize encrypted form and a form with a save point
complete and incomplete pill changed colors
bulk finalize complete and incomplete forms: required, constraints, pull data etc.
confirm dialog before bulk finalizing
all auto send options after bulk finalizing + forms with auto send set to true/false
new Finalize all forms setting
scanning, importing, exporting and reconfiguring QR codes from older version of Collect to the PR version
upgrading from an older version with different settings connected with finalizing and saving drafts
don’t keep activities enabled/disabled
dark, light theme

srujner · 2023-10-20T14:02:42Z

Tested with Success!

Verified on device with Android 13

lognaturel · 2023-10-20T14:07:51Z

Thank you @srujner and @dbemke! This one is tricky!

The strings are fine, we’ll do a pass over all of them in a bit. We’ll talk about how we all liked this process in our retro.

Agreed with @grzesiek2010 that the app killing case is ok.

I think we should file an issue about settings from versions prior to 2023.2. For completeness, if “Mark as finalized” was hidden and its default value was unchecked (so forms were always saved as draft from form end), I think we should hide the bulk finalize option so we don’t open a path to finalizing. Good find! But again, let’s do it as its own issue.

Improvements to bulk finalize

seadowg changed the title ~~Encrypted bulk~~ Show an error when bulk finalizing unsupported drafts Oct 11, 2023

seadowg added the blocked label Oct 11, 2023

seadowg force-pushed the encrypted-bulk branch from 744ab93 to 7da40de Compare October 13, 2023 08:42

seadowg changed the title ~~Show an error when bulk finalizing unsupported drafts~~ Show an error when bulk finalizing unsupported drafts and add auto send for supported drafts Oct 13, 2023

seadowg changed the title ~~Show an error when bulk finalizing unsupported drafts and add auto send for supported drafts~~ Improvements to bulk finalize Oct 13, 2023

seadowg force-pushed the encrypted-bulk branch from c25d4ba to 561c7db Compare October 13, 2023 16:19

seadowg removed the blocked label Oct 13, 2023

seadowg force-pushed the encrypted-bulk branch from 5bb89af to df0d3ef Compare October 14, 2023 12:20

seadowg commented Oct 16, 2023

View reviewed changes

seadowg marked this pull request as ready for review October 16, 2023 09:29

seadowg requested a review from grzesiek2010 October 16, 2023 13:21

lognaturel mentioned this pull request Oct 16, 2023

Update bulk finalization strings #5779

Merged

5 tasks

This was linked to issues Oct 18, 2023

Adapt bulk finalisation to current settings #5741

Closed

Confirm before bulk finalizing #5763

Closed

Make sure all forms can be bulk finalized #5740

Closed

grzesiek2010 requested changes Oct 18, 2023

View reviewed changes

collect_app/src/main/java/org/odk/collect/android/formmanagement/InstancesDataService.kt Show resolved Hide resolved

...p/src/main/java/org/odk/collect/android/instancemanagement/FinalizeAllSnackbarPresenter.java Outdated Show resolved Hide resolved

seadowg requested review from lognaturel and grzesiek2010 and removed request for lognaturel October 18, 2023 12:14

grzesiek2010 requested changes Oct 18, 2023

View reviewed changes

collect_app/src/main/res/xml/form_entry_access_preferences.xml Outdated Show resolved Hide resolved

seadowg requested a review from grzesiek2010 October 18, 2023 14:01

grzesiek2010 approved these changes Oct 18, 2023

View reviewed changes

grzesiek2010 added the needs testing label Oct 18, 2023

grzesiek2010 force-pushed the encrypted-bulk branch from 4c9d8a4 to 02b71ea Compare October 19, 2023 10:17

seadowg added 8 commits October 20, 2023 10:58

Add setting to disable bulk finalize

e809762

Disable bulk finalize for configurations that previously had finalize…

f137a4e

… in form entry disabled

Fix references in tests

b06aaad

Make sure old keys are still importable

ad3fc15

Upate schema

a7cfcf1

Add analytics for unsupported drafts

869fde3

Convert presenter to Kotlin

b9202f2

Add styling to section header

5db872d

grzesiek2010 force-pushed the encrypted-bulk branch from 02b71ea to 5db872d Compare October 20, 2023 09:02

lognaturel added behavior verified and removed needs testing labels Oct 20, 2023

Merge remote-tracking branch 'upstream/v2023.3.x' into encrypted-bulk

f8dff30

lognaturel mentioned this pull request Oct 20, 2023

Include "finalize all drafts" setting in migration of old "mark form as finalized" #5788

Closed

lognaturel merged commit 7b70b37 into getodk:v2023.3.x Oct 20, 2023
6 checks passed

seadowg deleted the encrypted-bulk branch October 24, 2023 08:49

seadowg pushed a commit to seadowg/collect that referenced this pull request Oct 25, 2023

Merge pull request getodk#5776 from seadowg/encrypted-bulk

43cef0e

Improvements to bulk finalize

seadowg pushed a commit to seadowg/collect that referenced this pull request Oct 30, 2023

Merge pull request getodk#5776 from seadowg/encrypted-bulk

7e9bb64

Improvements to bulk finalize

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to bulk finalize #5776

Improvements to bulk finalize #5776

seadowg commented Oct 11, 2023 •

edited

Loading

seadowg Oct 16, 2023

lognaturel Oct 16, 2023

seadowg Oct 16, 2023

seadowg Oct 16, 2023

srujner commented Oct 19, 2023

srujner commented Oct 19, 2023 •

edited

Loading

dbemke commented Oct 19, 2023

lognaturel commented Oct 19, 2023

grzesiek2010 commented Oct 20, 2023

srujner commented Oct 20, 2023

srujner commented Oct 20, 2023

dbemke commented Oct 20, 2023 •

edited

Loading

grzesiek2010 commented Oct 20, 2023

grzesiek2010 commented Oct 20, 2023

grzesiek2010 commented Oct 20, 2023 •

edited

Loading

dbemke commented Oct 20, 2023 •

edited

Loading

grzesiek2010 commented Oct 20, 2023

srujner commented Oct 20, 2023

dbemke commented Oct 20, 2023

srujner commented Oct 20, 2023

lognaturel commented Oct 20, 2023 •

edited

Loading


		data class Action(val text: String, val listener: () -> Unit)

		abstract class SnackbarPresenterObserver<T : Any?>(private val parentView: View) :

Improvements to bulk finalize #5776

Improvements to bulk finalize #5776

Conversation

seadowg commented Oct 11, 2023 • edited Loading

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

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?

Before submitting this PR, please make sure you have:

seadowg Oct 16, 2023

Choose a reason for hiding this comment

lognaturel Oct 16, 2023

Choose a reason for hiding this comment

seadowg Oct 16, 2023

Choose a reason for hiding this comment

seadowg Oct 16, 2023

Choose a reason for hiding this comment

srujner commented Oct 19, 2023

srujner commented Oct 19, 2023 • edited Loading

dbemke commented Oct 19, 2023

lognaturel commented Oct 19, 2023

grzesiek2010 commented Oct 20, 2023

srujner commented Oct 20, 2023

srujner commented Oct 20, 2023

dbemke commented Oct 20, 2023 • edited Loading

grzesiek2010 commented Oct 20, 2023

grzesiek2010 commented Oct 20, 2023

grzesiek2010 commented Oct 20, 2023 • edited Loading

dbemke commented Oct 20, 2023 • edited Loading

grzesiek2010 commented Oct 20, 2023

srujner commented Oct 20, 2023

dbemke commented Oct 20, 2023

srujner commented Oct 20, 2023

lognaturel commented Oct 20, 2023 • edited Loading

seadowg commented Oct 11, 2023 •

edited

Loading

srujner commented Oct 19, 2023 •

edited

Loading

dbemke commented Oct 20, 2023 •

edited

Loading

grzesiek2010 commented Oct 20, 2023 •

edited

Loading

dbemke commented Oct 20, 2023 •

edited

Loading

lognaturel commented Oct 20, 2023 •

edited

Loading