Remove grace period restriction time restriction from editing finalized forms #5717

seadowg · 2023-08-28T11:49:47Z

As discussed. We're removing the restriction entirely from v2023.2 and v2023.3 and disabling editing of finalised forms again in v2023.4.

What has been done to verify that this works as intended?

Modified existing tests.

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

Not a lot to discuss here. I just removed clauses that restricted the grace period to a specific time window from the code.

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?

Most important thing to check here is that finalized forms can be edited from "Ready to send" and that there's no references to that going way after a certain point in time (but that it'll go away in a later release).

Before submitting this PR, please make sure you have:

run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
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-08-28T12:56:59Z

strings/src/main/res/values/strings.xml

@@ -1215,5 +1215,5 @@
    <string name="view_form">View</string>
    <string name="close_snackbar">Close snackbar</string>

-    <string name="edit_finalized_form_warning">After September 30th, you will not be able to edit finalized forms. Save forms as draft to edit them later.\n\nYou can check for errors in a draft form by tapping the three dots (⋮) and then Check for errors.</string>
+    <string name="edit_finalized_form_warning">In later releases, you will not be able to edit finalized forms. Save forms as draft to edit them later.\n\nYou can check for errors in a draft form by tapping the three dots (⋮) and then Check for errors.</string>


I'm not a big fan this message. Any ideas for a better one @lognaturel?

grzesiek2010 · 2023-08-28T19:39:32Z

collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstanceExt.kt

@@ -4,14 +4,12 @@ import org.odk.collect.forms.instances.Instance
 import org.odk.collect.settings.SettingsProvider
 import org.odk.collect.settings.keys.ProtectedProjectKeys

-const val OCTOBER_1st_2023_UTC = 1696118400000
-
 fun Instance.canBeEdited(settingsProvider: SettingsProvider): Boolean {
    return this.status == Instance.STATUS_INCOMPLETE &&
        settingsProvider.getProtectedSettings().getBoolean(ProtectedProjectKeys.KEY_EDIT_SAVED)
 }

 fun Instance.canBeEditedWithGracePeriod(settingsProvider: SettingsProvider): Boolean {


This method should be renamed or even merged with the one above.

dbemke · 2023-08-30T14:52:31Z

Problem description

A finalized form can be saved as a draft again when accessed via Ready to send. Afterwards for a moment it’s possible to send the form or edit it again from Ready to send (while it’s really a draft).

Steps to reproduce the problem

In Collect settings auto send is set to off.
Go to a form and finalize it.
Go to Ready to send and open the form.
In the end page of the form, tap Save as draft.
When moved to Ready to send list, quickly select the form and send it (or try to open it and edit it again).

Other information

The issue also occurs in the v2023.2.3. In the master version 602cac5 editing finalized forms is not allowed so the issue is impossible to reproduce.

dbemke · 2023-08-30T14:57:43Z

When a finalized form is edited via Ready to send it’s possible to tap Save as draft in the exit dialog. The changes are saved but the form stays in Ready to send.

Steps to reproduce the problem

In Collect settings auto send is set to off.
Go to a form and finalize it.
Go to Ready to send and open the form.
Tap the device back button.
In the dialog tap Save as draft.

Other information

The issue also occurs in the v2023.2.3.

dbemke · 2023-08-31T08:32:53Z

The first issue is fixed. The second issue is still present but I don't know if the form should be moved from Ready to send to draft in that case.

dbemke · 2023-08-31T08:35:37Z

After editing a finalized form the subtext in Ready to send is a bit misleading. The subtext shows the date and time when the form was finalized.

Steps to reproduce:

In Collect settings auto send is set to off.
Go to a form and finalize it.
Go to Ready to send and open the form.
Edit some answers.
Tap the save button (floppy disc).
Tap the device and discard changes.

The changes are saved but the subtext isn’t changed (Finalized with the date and time of the first finalization). If you enter the form and tap the device back button, in the exit dialog there is information that the form was saved/edited and the new date and time is present.
If in step 6 in the exit dialog the user taps Save as draft the date and time is updated in the subtext but it’s still Finalized.

seadowg · 2023-08-31T09:30:33Z

When a finalized form is edited via Ready to send it’s possible to tap Save as draft in the exit dialog. The changes are saved but the form stays in Ready to send.

After editing a finalized form the subtext in Ready to send is a bit misleading. The subtext shows the date and time when the form was finalized.

I don't think either of these are critical, but they do make everything confusing. It seems like both are consequence of a hangover of the old world where finalisation was less "final". When you hit the new "Save as draft" button on the end screen, Collect explicitly saves the form as a draft (the "incomplete" status) whereas for the floppy disk and the "Save as draft" button in the quit dialog, it will save the form as finalised if the form is already finalised. This hadn't really come up before, because it doesn't matter if you can't edit finalized forms (and the weirdness must have been missed the first time we added the grace period).

I think that we should remove the extra "intelligence" on the floppy/quit dialog save buttons and just have them always save the form as a draft leaving the only way to save a finalised form and keep it finalised is to hit "Finalize" itself. This introduces a bit of disruption to people that edit finalized forms and then immediately send them as they'll have to actually hit "Finalize", but I think the current situation allows for very mixed messaging as you're able "Save as draft" and then immediately send. I think the only alternative (other than keeping the current behaviour), would be to add an extra "Finalize" button to the quit dialog and I'm not sure how we'd deal with the floppy button. I'll go ahead and make the change and we can revert if it's not something that we're agreed upon (cc @grzesiek2010 and @lognaturel)

The changes are saved but the subtext isn’t changed (Finalized with the date and time of the first finalization). If you enter the form and tap the device back button, in the exit dialog there is information that the form was saved/edited and the new date and time is present.

I don't think we'll want to add a specific piece of text for quitting a form that was finalised as we're still going to be taking the ability to edit a finalised form away later. "Last saved" isn't innaccuracte so I think it's ok for the moment. I think making the changes I've suggest above should make the scenario less confusing.

seadowg · 2023-08-31T10:12:48Z

@dbemke I've made changes as described!

seadowg · 2023-08-31T10:13:28Z

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

@@ -1035,7 +1034,7 @@ public boolean onOptionsItemSelected(MenuItem item) {

            case R.id.menu_save:
                // don't exit
-                saveForm(false, InstancesDaoHelper.isInstanceComplete(getFormController()), null, true);
+                saveForm(false, false, null, true);


This was a super easy change as the tests already describe the desired behaviour. I just had to get rid of the extra untested logic

grzesiek2010 · 2023-08-31T10:18:21Z

I think that we should remove the extra "intelligence" on the floppy/quit dialog save buttons and just have them always save the form as a draft leaving the only way to save a finalised form and keep it finalised is to hit "Finalize" itself. This introduces a bit of disruption to people that edit finalized forms and then immediately send them as they'll have to actually hit "Finalize", but I think the current situation allows for very mixed messaging as you're able "Save as draft" and then immediately send. I think the only alternative (other than keeping the current behaviour), would be to add an extra "Finalize" button to the quit dialog and I'm not sure how we'd deal with the floppy button. I'll go ahead and make the change and we can revert if it's not something that we're agreed upon (cc @grzesiek2010 and @lognaturel)

to me, that solution would be ok. Still confusing a little bit but it is what it is I would rather avoid adding new buttons for finalization.

dbemke · 2023-08-31T11:03:08Z

After the fix forms are moved to Drafts. The only thing is the scenario below (save button + discard changes) in which after those steps the user is moved to Ready to send view and the form is still there (for a moment before going back to the main menu - like in the first issue) and it's possible to send it (although it's a draft).

Steps to reproduce:

In Collect settings auto send is set to off.
Go to a form and finalize it.
Go to Ready to send and open the form.
Edit some answers.
Tap the save button (floppy disc).
Tap the device back button and discard changes.
When moved to the Ready to send list, try to send the form.

seadowg · 2023-08-31T12:16:53Z

The only thing is the scenario below (save button + discard changes) in which after those steps the user is moved to Ready to send view and the form is still there (for a moment before going back to the main menu - like in the first issue) and it's possible to send it (although it's a draft).

Good find! Not sure why we end up on "Ready to send" instead of the Main Menu there, but will fix that.

seadowg · 2023-08-31T13:11:36Z

@dbemke that should be fixed now!

dbemke · 2023-09-01T13:08:55Z

Tested with Success!

Verified on device with Android 10

Verified cases:

editing finalized form after the 30th of October
issues mention in the PR are fixed
changing finalized forms to drafts (forms with validation and audit logs)
moving backwards not allowed
all auto send options
validation upon forward swipe/ defer validation until finalized
killing the app after tapping the save button in a finalized form
editing finalized form accessed via the form map in Start new form
editing finalized form accessed via a snackbar
trying to edit a form with a „sending failed” status
subtexts in Ready to send

srujner · 2023-09-01T13:10:19Z

Tested with Success!

Verified on device with Android 13

seadowg added 2 commits August 28, 2023 11:36

Always allow finalized forms to be edited

4465b48

Edit string for new finalize behaviour

f720024

seadowg changed the base branch from master to v2023.2.x August 28, 2023 11:50

seadowg commented Aug 28, 2023

View reviewed changes

seadowg changed the title ~~Remove grace period restriction from editing finalized forms~~ Remove grace period restriction time restriction from editing finalized forms Aug 28, 2023

seadowg requested a review from grzesiek2010 August 28, 2023 12:59

seadowg marked this pull request as ready for review August 28, 2023 12:59

seadowg mentioned this pull request Aug 28, 2023

Allow finalized forms to be edited from "Ready to send" #5715

Closed

6 tasks

grzesiek2010 requested changes Aug 28, 2023

View reviewed changes

Move InstanceExt methods to their calling classes

f4c7e0a

seadowg requested a review from grzesiek2010 August 29, 2023 14:26

grzesiek2010 approved these changes Aug 29, 2023

View reviewed changes

grzesiek2010 added the needs testing label Aug 29, 2023

seadowg added 2 commits August 30, 2023 16:19

Add failing test for finalized edit flow

2f1b35f

Return to Main Menu after editing finalized form

baa477e

lognaturel mentioned this pull request Aug 30, 2023

Add finalized form edit warning string for translation #5720

Merged

seadowg added 2 commits August 31, 2023 10:45

Only allow Finalize button to save finalzied form

2d0708c

Show snackbar after editing finalized form

6302b8d

seadowg commented Aug 31, 2023

View reviewed changes

Fix discard changes when editing finalized form

46e2266

seadowg added the high priority Should be looked at before other PRs/issues label Aug 31, 2023

srujner added behavior verified and removed needs testing labels Sep 1, 2023

grzesiek2010 mentioned this pull request Sep 1, 2023

Making the "Ready to Send" screen more actionable #5693

Merged

3 tasks

seadowg merged commit 8739006 into getodk:v2023.2.x Sep 1, 2023
6 checks passed

seadowg deleted the remove-grade-period branch September 1, 2023 13:49

seadowg added this to the v2023.2.x milestone Sep 1, 2023

seadowg mentioned this pull request Sep 11, 2023

Remove grace period for v2023.3 #5728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove grace period restriction time restriction from editing finalized forms #5717

Remove grace period restriction time restriction from editing finalized forms #5717

seadowg commented Aug 28, 2023 •

edited

Loading

seadowg Aug 28, 2023

grzesiek2010 Aug 28, 2023

dbemke commented Aug 30, 2023

dbemke commented Aug 30, 2023

dbemke commented Aug 31, 2023

dbemke commented Aug 31, 2023

seadowg commented Aug 31, 2023

seadowg commented Aug 31, 2023

seadowg Aug 31, 2023

grzesiek2010 commented Aug 31, 2023

dbemke commented Aug 31, 2023 •

edited

Loading

seadowg commented Aug 31, 2023

seadowg commented Aug 31, 2023

dbemke commented Sep 1, 2023

srujner commented Sep 1, 2023

Remove grace period restriction time restriction from editing finalized forms #5717

Remove grace period restriction time restriction from editing finalized forms #5717

Conversation

seadowg commented Aug 28, 2023 • edited Loading

What has been done to verify that this works as intended?

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 Aug 28, 2023

Choose a reason for hiding this comment

grzesiek2010 Aug 28, 2023

Choose a reason for hiding this comment

dbemke commented Aug 30, 2023

Problem description

Steps to reproduce the problem

Other information

dbemke commented Aug 30, 2023

Steps to reproduce the problem

Other information

dbemke commented Aug 31, 2023

dbemke commented Aug 31, 2023

seadowg commented Aug 31, 2023

seadowg commented Aug 31, 2023

seadowg Aug 31, 2023

Choose a reason for hiding this comment

grzesiek2010 commented Aug 31, 2023

dbemke commented Aug 31, 2023 • edited Loading

seadowg commented Aug 31, 2023

seadowg commented Aug 31, 2023

dbemke commented Sep 1, 2023

srujner commented Sep 1, 2023

seadowg commented Aug 28, 2023 •

edited

Loading

dbemke commented Aug 31, 2023 •

edited

Loading