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

Revert 'Do not mark forms as failed to send before sending' in v2023.3 #5794

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Oct 24, 2023

Closes #5790

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 in the same way #5688 has been tested.

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:

  • 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

@grzesiek2010 grzesiek2010 marked this pull request as ready for review October 24, 2023 10:48
@seadowg seadowg added needs testing high priority Should be looked at before other PRs/issues labels Oct 24, 2023
@dbemke
Copy link

dbemke commented Oct 25, 2023

It’s possible to edit a form that is being sent in this case.

Steps to reproduce:

  1. Change the server URL so that it’s wrong (e.g. add some letters).
  2. Go to any form.
  3. Go to end and Send the form.
  4. In the snackbar click View.
  5. Repeat steps 2-4 a few times (usually 3 times).

Usually the third form which is being sent/sending failed allows to edit after viewing it.

@dbemke
Copy link

dbemke commented Oct 25, 2023

Similar case: if there are some forms in "Ready to send" waiting to be send (e.g. waiting for wifi connection) when a user fills a new form and sends the forms (when wifi is on), it's possible to edit the form (forms from "Ready to send" are being sent at that moment).

@srujner
Copy link

srujner commented Oct 25, 2023

It's possible to edit form that is being sent from Bulk Finalize.

Steps to reproduce:

  1. Have Auto-send option On
  2. Create several drafts with big forms or big images
  3. Bulk Finalize all draft
  4. While forms are being sent, create one more Form and Send it
  5. The last form is possible to edit from Ready to Send tab.

More forms with Big images in Save as draft before bulk finalize the easier it is to reproduce

@dbemke
Copy link

dbemke commented Oct 25, 2023

Similar case to the previous one:
After bulk finalizing many forms or big forms while they're being sent, sending a new form and quickly going to it (e.g via View in the sending snakcbar) allows editing.

Steps to reproduce:

  1. Save as draft many forms or a few forms with some big files attached.
  2. Go to Drafts and bulk finalize them.
  3. While they are being sent go to "Start new form" and send a form.
  4. In the sending snackbar click View and edit the form.

@dbemke
Copy link

dbemke commented Oct 25, 2023

It seems that only 1 form is blocked from being edited (the first one in the queque) which allows other forms to be edited after tapping "Send" while other forms are being sent e.g. in situations when sending many forms is triggered after bulk finalizing or getting wifi connection.

@grzesiek2010
Copy link
Member Author

It seems that only 1 form is blocked from being edited (the first one in the queue)

Yes it always worked like that (I was explaining that during one of our meetings) I don't think we should do anything more here than bringing back what we had in previous versions (blocking the form that is being sent)

@dbemke
Copy link

dbemke commented Oct 25, 2023

Yes we know but the thing that worries us is that after adding another option which sends many forms (bulk finalize) the gap in preventing editing forms that are not supposed to be edited gets bigger.

@grzesiek2010
Copy link
Member Author

I agree that now it's easier to discover this possibility of editing forms that should be sent but still, I'm not convinced that's something we have to fix. Let's see what @seadowg and @lognaturel think.

@seadowg
Copy link
Member

seadowg commented Oct 25, 2023

I agree that now it's easier to discover this possibility of editing forms that should be sent but still, I'm not convinced that's something we have to fix.

I'm in two minds here. The "real" fix for this will come in v2023.4 when we properly remove the ability to edit finalized forms, but the v2023.3 situation where bulk finalize and editing finalized forms are both available is pretty open to problems. I think I'd lean towards keeping the behaviour the way it is - it feels like time smoothing out the experience is wasted given that we're removing it.

For the moment, we can merge this if that's the only problem and continue the discussion around if we need to make changes before releasing v2023.3.

@srujner
Copy link

srujner commented Oct 25, 2023

@grzesiek2010 @seadowg and what about our other comments about editing a form that is being sent?

@grzesiek2010
Copy link
Member Author

@grzesiek2010 @seadowg and what about our other comments about editing a form that is being sent?

In all cases, you save and send a few forms so I believe the form you can edit is not the one that is being sent at the moment. Am I right?

@srujner
Copy link

srujner commented Oct 25, 2023

@grzesiek2010 @seadowg and what about our other comments about editing a form that is being sent?

In all cases, you save and send a few forms so I believe the form you can edit is not the one that is being sent at the moment. Am I right?

Correct

@grzesiek2010
Copy link
Member Author

Correct

So it seems like all those problems come down to the same root cause - only the form that is being sent is blocked, not all the others that are in the queue.

@srujner
Copy link

srujner commented Oct 25, 2023

Tested with Success!

Verified on device with Android 13

Verified cases:

  • Issue Issue 5687 "Possible to edit a form while it is being sent" reintroduced  #5790 is no longer reproducing;
  • snackbar after sending a form
  • sending/finilizing wrong URL
  • sending forms with big-sized media
  • all auto send options
  • a few forms being sent at the same time
  • Sending finalized Forms, Editing saved Forms, Viewing sent Forms, Deleting saved Forms;
  • Regression checks and Exploratory testing on Project settings and user settings;
  • Adding project manually or through QR code;
  • Switching between projects, adding duplicated projects;
  • Update settings from older version;

@dbemke
Copy link

dbemke commented Oct 25, 2023

Tested with Success!

Verified on device with Android 10

@seadowg seadowg merged commit da182e4 into getodk:v2023.3.x Oct 25, 2023
6 checks passed
seadowg added a commit to seadowg/collect that referenced this pull request Oct 26, 2023
Revert 'Do not mark forms as failed to send before sending' in v2023.3
seadowg added a commit to seadowg/collect that referenced this pull request Oct 30, 2023
Revert 'Do not mark forms as failed to send before sending' in v2023.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior verified high priority Should be looked at before other PRs/issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue 5687 "Possible to edit a form while it is being sent" reintroduced
4 participants