-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Revert "Do not mark forms as failed to send before sending" #5688
Conversation
This reverts commit 142af38.
@getodk/testers in addition to checking this fixes the issue, I think doing some exploratory testing around sending, failed submissions and editing finalized forms would be good. This clearly an area we're having a hard time keeping intact with the recent behaviour changes, so some extra confidence would be nice! |
(I installed the CI build) Something is wrong with the snackbar when I send a form. Sometimes the snackbar appears and sometimes not. |
9b5f89e
to
fbbc9bb
Compare
It’s still possible to edit a form that is being sent.
Usually the third form which is being sent/sending failed allows to edit after viewing it. I don’t know if the failed status of sending or the queue of forms that triggers the possibility to edit forms. |
Another way to reproduce it Steps to reproduce:
|
I think it's because you are fast enough to open the form for editing before it's marked as submission failed (which is our workaround to block editing such forms). I'm pretty sure the same should be possible on v2023.1 but there is no snackbar so you would need to navigate to |
I checked 2023.1.1 (auto send: wifi only and the wifi off) if I Mark form as finalized and click "Save Form and Exit" the form is both in "Edit Saved Form" and "Send Finalized Form" so it's editable, so I guess in 2023.1.1 it was still possible to edit "ready to send" form. |
no, no it has never worked like that. If you have auto-send enabled but the requirements are not met like in this case sending is not triggered and editing forms is not blocked.
It's not a perfect mechanism but it worked like that and we should keep this for now. Thankfully in October, the problem will be gone. |
so comparing v2023.2.x (with this pr) to older versions where editing finalized forms was possible without the grace period (like v2023.1) I believe the behavior is similar. In both cases, if you are fast enough you might be able to edit a form that is queued to be sent. @seadowg do you have anything to add here? |
No! As far as I can see, this just moves us back to the v2023.1 behaviour as you said. One of the big motivations to prevent editing of finalized forms was to stop this edge case (being able to change a form while it sends) from occurring. Once the grace period ends/v2023.3 comes out, this will no longer be a problem (as @grzesiek2010 says). |
Tested with Success! Verified on device with Android 13 Verified cases:
|
Tested with Success! Verified on device with Android 10 |
Closes #5687
What has been done to verify that this works as intended?
I've tested the fix manually.
Why is this the best possible solution? Were any other approaches considered?
In #5609 we removed marking forms as failed before sending them to avoid editing (this was a workaround) because we blocked editing finalized forms. Recently we have introduced the grace period and because of that wee need the workaround again.
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?
Please test the scenario described in the issue.
Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew checkAll
and confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTest
locally.