Show banner after editing finalized form #5762

seadowg · 2023-10-03T16:49:22Z

Most of the work to make finalised forms editable had already been done, so this just adds a banner that is displayed after a user edits a finalised form until the next time they launch the app.

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

New tests.

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

Not a lot to discuss here. I ended up storing the state in memory rather than in prefs/on disk so that we didn't have to deal with clearing it later. It also feels natural to me to have this stick around until the next launch and then reappear if there is another finalised form edited.

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 wasn't a lot touched here, so just checking the banner is displayed as expected (and not displayed when it's not expected) should be enough.

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-10-03T17:30:11Z

@alyblenkin do you have any suggestions for the banner text here? I'm linking "Learn more" to https://forum.getodk.org/t/42007.

alyblenkin · 2023-10-03T17:45:11Z

@seadowg - what do you think about these options for the banner message? I'm trying to use similar language to the post.

lognaturel · 2023-10-03T17:48:45Z

I really like the first one! Maybe we should say "in the next release"? We're not totally sure whether it will be 2023.4 or 2024.1. "Starting in the next release"?

seadowg · 2023-10-03T17:49:05Z

@alyblenkin let's go with the second one for now

alyblenkin · 2023-10-03T17:54:38Z

Good point! I'll leave these here for now, but I think it makes sense to go with the second one until we know.

seadowg · 2023-10-03T18:02:42Z

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

@@ -264,8 +264,6 @@ private void logFormEdit(Cursor cursor) {

        if (status.equals(Instance.STATUS_INCOMPLETE)) {
            AnalyticsUtils.logFormEvent(AnalyticsEvents.EDIT_NON_FINALIZED_FORM, formId, formTitle);
-        } else if (status.equals(Instance.STATUS_COMPLETE)) {
-            AnalyticsUtils.logFormEvent(AnalyticsEvents.EDIT_FINALIZED_FORM, formId, formTitle);


You're no longer able to edit finalised forms from Drafts so this would never fire!

lognaturel · 2023-10-03T19:27:18Z

But what does "New update" mean? Maybe "In the next version"? I think it helps people plan to have a sense of when things will come into effect.

seadowg · 2023-10-03T19:33:16Z

But what does "New update" mean? Maybe "In the next version"? I think it helps people plan to have a sense of when things will come into effect.

Good point. How about "In later releases, finalized forms will no longer be editable."?

lognaturel · 2023-10-03T19:41:42Z

Maybe version means more to normal humans than releases, I'm not entirely sure.

alyblenkin · 2023-10-03T20:11:50Z

But what does "New update" mean? Maybe "In the next version"? I think it helps people plan to have a sense of when things will come into effect.

I was trying to avoid mentioning the release or the version... just a new update they should know about, and then they can go to the link for more info. I agree that some people may want to have the version as a reference, so maybe it's better to be more specific.

grzesiek2010 · 2023-10-10T14:24:25Z

collect_app/src/main/java/org/odk/collect/android/mainmenu/MainMenuActivity.kt

+        val editedFinalizedForm =
+            application.getState().get<Boolean>(EDITED_FINALIZED_FORM) ?: false
+
+        if (usingGoogleDrive) {


Don't we ever need to display both banners at the same time?

We decided that we'd just show the Google Drive one if both apply. From the issue:

Google Drive banner should be shown if they're both applicable

grzesiek2010 · 2023-10-10T16:41:34Z

collect_app/src/main/java/org/odk/collect/android/utilities/ApplicationConstants.java

+
+    public abstract static class AppStateKeys {
+
+        public static final String EDITED_FINALIZED_FORM = "editedFinalizedForm";


One thing to consider: maybe this should be placed in AppState class. I'm not sure if keeping constant used in different parts of the app and different functionalities in one class is still a good idea.

Yeah I'm also not sure about ApplicationConstants, but it did seem like the right place for this at the moment. Maybe we should look at splitting it up next time we want to add something?

I don't think that keys for AppState should live in the AppState class as the keys should be owned by the component/module/package using AppState. I guess you can think about it like a Map - it shouldn't need to know anything about the things it stores to do it's job of just storing them.

dbemke · 2023-10-12T13:18:07Z

Is this PR included in the CircleCI apk? #5688

seadowg · 2023-10-12T14:11:35Z

Is this PR included in the CircleCI apk? #5688

I'm not actually sure! I'd need to do some digging. Are you seeing some problems related to that?

dbemke · 2023-10-12T14:41:51Z

I think not connected with the PR (banner) exactly, rather editing a finalized form but if that PR wasn't included that would explain it. Which version should we compare this PR to?

seadowg · 2023-10-12T14:56:20Z

Which version should we compare this PR to?

The last release (v2023.2.4).

dbemke · 2023-10-13T07:50:18Z

Just to double check if it's ok:

the banner isn't shown when a finalized form is opened and the changes get discarded
if in device settings there is "no background processes" I go to a different app and back to Collect the banner disappears

seadowg · 2023-10-13T07:58:04Z

the banner isn't shown when a finalized form is opened and the changes get discarded

Yeah that's fine! We don't want to tell people off for viewing forms before sending.

if in device settings there is "no background processes" I go to a different app and back to Collect the banner disappears

Yeah that's ok.

dbemke · 2023-10-13T10:25:44Z

There is also a part in which we need to test Google Drive project. Do you think it would be ok to test it during the regression testing or it's better to test it in this PR (so this means we would need the released apk build)?

seadowg · 2023-10-13T10:51:58Z

There is also a part in which we need to test Google Drive project. Do you think it would be ok to test it during the regression testing or it's better to test it in this PR (so this means we would need the released apk build)?

Let's leave it for regression.

srujner · 2023-10-13T11:27:42Z

Tested with Success!

Verified on device with Android: 13

Verified cases:

Works toward Allow finalized forms to be edited from "Ready to send" #5715;
Regression check on Editing finalized forms;
Editing Forms in Ready to Send;
Editing Forms using "view" in snackbar;
Banner is shown in the main menu after editing a finalized form;
Don't keep activities enabled/disabled;
No background processing option turned on/off;

dbemke · 2023-10-13T11:29:20Z

Tested with Success!

Verified on device with Android: 10

seadowg added 4 commits October 3, 2023 14:00

Genericize deprectation banner

fc3d35e

Show banner after editing finalized form

6468094

Extract constant for key

da66779

Move analytics event

bcd3630

seadowg changed the base branch from master to v2023.3.x October 3, 2023 17:19

Update test

e8cdc89

seadowg commented Oct 3, 2023

View reviewed changes

seadowg marked this pull request as ready for review October 3, 2023 18:04

seadowg requested a review from grzesiek2010 October 3, 2023 18:04

Update copy

7a0f0bd

Make both warnings match

2665ddc

grzesiek2010 requested changes Oct 10, 2023

View reviewed changes

seadowg requested a review from grzesiek2010 October 10, 2023 14:29

grzesiek2010 approved these changes Oct 10, 2023

View reviewed changes

grzesiek2010 added the needs testing label Oct 10, 2023

grzesiek2010 reviewed Oct 10, 2023

View reviewed changes

dbemke added behavior verified and removed needs testing labels Oct 13, 2023

grzesiek2010 merged commit eaebea5 into getodk:v2023.3.x Oct 13, 2023

seadowg deleted the edit-finalized-forms branch October 13, 2023 12:23

dbemke mentioned this pull request Oct 23, 2023

Editing finalized banner doesn't appear in GD project #5789

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show banner after editing finalized form #5762

Show banner after editing finalized form #5762

seadowg commented Oct 3, 2023 •

edited

Loading

seadowg commented Oct 3, 2023

alyblenkin commented Oct 3, 2023

lognaturel commented Oct 3, 2023

seadowg commented Oct 3, 2023

alyblenkin commented Oct 3, 2023

seadowg Oct 3, 2023

lognaturel commented Oct 3, 2023

seadowg commented Oct 3, 2023

lognaturel commented Oct 3, 2023

alyblenkin commented Oct 3, 2023

grzesiek2010 Oct 10, 2023

seadowg Oct 10, 2023

grzesiek2010 Oct 10, 2023

seadowg Oct 11, 2023

dbemke commented Oct 12, 2023

seadowg commented Oct 12, 2023

dbemke commented Oct 12, 2023 •

edited

Loading

seadowg commented Oct 12, 2023

dbemke commented Oct 13, 2023

seadowg commented Oct 13, 2023

dbemke commented Oct 13, 2023

seadowg commented Oct 13, 2023

srujner commented Oct 13, 2023 •

edited

Loading

dbemke commented Oct 13, 2023


		public abstract static class AppStateKeys {

		public static final String EDITED_FINALIZED_FORM = "editedFinalizedForm";

Show banner after editing finalized form #5762

Show banner after editing finalized form #5762

Conversation

seadowg commented Oct 3, 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 commented Oct 3, 2023

alyblenkin commented Oct 3, 2023

lognaturel commented Oct 3, 2023

seadowg commented Oct 3, 2023

alyblenkin commented Oct 3, 2023

seadowg Oct 3, 2023

Choose a reason for hiding this comment

lognaturel commented Oct 3, 2023

seadowg commented Oct 3, 2023

lognaturel commented Oct 3, 2023

alyblenkin commented Oct 3, 2023

grzesiek2010 Oct 10, 2023

Choose a reason for hiding this comment

seadowg Oct 10, 2023

Choose a reason for hiding this comment

grzesiek2010 Oct 10, 2023

Choose a reason for hiding this comment

seadowg Oct 11, 2023

Choose a reason for hiding this comment

dbemke commented Oct 12, 2023

seadowg commented Oct 12, 2023

dbemke commented Oct 12, 2023 • edited Loading

seadowg commented Oct 12, 2023

dbemke commented Oct 13, 2023

seadowg commented Oct 13, 2023

dbemke commented Oct 13, 2023

seadowg commented Oct 13, 2023

srujner commented Oct 13, 2023 • edited Loading

dbemke commented Oct 13, 2023

seadowg commented Oct 3, 2023 •

edited

Loading

dbemke commented Oct 12, 2023 •

edited

Loading

srujner commented Oct 13, 2023 •

edited

Loading