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

Prevent bulk finalization on instances with save points #5766

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Oct 5, 2023

Closes #5739

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! Was very simple given the we are just failing to finalise for instances with save points.

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?

Just checking the feature is implemented as described should be enough here. If there are any ideas around ways to break it then feel free to play around with that as well though!

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 seadowg changed the base branch from master to v2023.3.x October 5, 2023 12:54
@@ -450,6 +455,25 @@ abstract class Page<T : Page<T>> {
return this as T
}

fun <D : Page<D>?> killAndReopenApp(destination: D): D {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is copied over from master.

@@ -62,6 +66,17 @@ object FormEntryUseCases {
)
}

fun getSavePoint(formController: FormController, cacheDir: File): File? {
Copy link
Member Author

Choose a reason for hiding this comment

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

We should be able to use this in other places we grab save point files, but I'll leave out that refactor for the moment to avoid the risk of merge conflicts that it'd introduce.

@seadowg seadowg marked this pull request as ready for review October 6, 2023 10:22
@srujner
Copy link

srujner commented Oct 9, 2023

Is this correct that now all forms in the Drafts tab has status "incomplete"?
No matter if it contains error or not.

@seadowg
Copy link
Member Author

seadowg commented Oct 9, 2023

@srujner yes because #5760 hasn't merged yet so forms don't get validated when they're saved as draft and nothing is marked "complete".

@srujner
Copy link

srujner commented Oct 9, 2023

@seadowg So shouldn't we start testing this PR after merging #5760?

@seadowg
Copy link
Member Author

seadowg commented Oct 9, 2023

@srujner it's definitely possible to test it without that merging, but I do agree that it's going to be far less confusing so let's go with that.

@seadowg seadowg force-pushed the save-point-bulk-finalize branch 2 times, most recently from 8cf7659 to 38e65f9 Compare October 10, 2023 09:10
@dbemke
Copy link

dbemke commented Oct 10, 2023

What is the state of a form with a save point which was opened and the changes were discarded? If I follow the steps from the AC and then open the form (with save point) and discard changes it’s possible to finalize it by "Finalize all forms”. Does the state of the form change if the changes were discarded?

Steps to reproduce:

  1. Fill a form (without any questions with validation) and save it as a draft.
  2. Go to Drafts and open the form.
  3. Go to the settings and change the theme of the app.
  4. Go back to Drafts and open the form.
  5. Tap the device back button and discard changes.
  6. Tap "Finalize all forms”.

@seadowg
Copy link
Member Author

seadowg commented Oct 10, 2023

What is the state of a form with a save point which was opened and the changes were discarded?

The save point will be deleted won't it? So it should finalize successfully.

@dbemke
Copy link

dbemke commented Oct 10, 2023

It is. Ahhh I get it.. so opening and discarding changes the form with a save point "removes the save point" and the introduced changes in a save point

@dbemke
Copy link

dbemke commented Oct 10, 2023

Tested with Success!

Verified on device with Android 10

Verified cases:

  • steps from Acceptance Criteria
  • finalizing via "Finalize all" forms forms with save points with/without questions with validation
  • save points after going to project settings and after killing the app
  • saveIncomplete and saveIcomplete form with a required question
  • discarding changes form a save point and finilzing the form
  • don’t keep activities enabled/disabled

@srujner
Copy link

srujner commented Oct 10, 2023

Tested with Success!

Verified on device with Android 13

@seadowg seadowg merged commit eaac576 into getodk:v2023.3.x Oct 10, 2023
6 checks passed
@seadowg seadowg deleted the save-point-bulk-finalize branch October 10, 2023 15:24
seadowg added a commit to seadowg/collect that referenced this pull request Oct 18, 2023
Prevent bulk finalization on instances with save points
seadowg added a commit to seadowg/collect that referenced this pull request Oct 24, 2023
Prevent bulk finalization on instances with save points
seadowg added a commit to seadowg/collect that referenced this pull request Oct 30, 2023
Prevent bulk finalization on instances with save points
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