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

Re-submitting a submission which was deleted #794

Open
dbemke opened this issue Nov 22, 2024 · 7 comments
Open

Re-submitting a submission which was deleted #794

dbemke opened this issue Nov 22, 2024 · 7 comments
Assignees
Labels
backend Requires a change to the API server bug nicer error More understandable error messages

Comments

@dbemke
Copy link

dbemke commented Nov 22, 2024

Problem description

Re-submitting a submission which was deleted

URL of the page

https://staging.getodk.cloud/

Steps to reproduce the problem

  1. In Collect send a submission of a form.
  2. In Central delete the submission.
  3. In Collect go to "Ready to send- Change View- Show Sent and UnsentForms”.
  4. Select the sent submission and tap "Send Selected”

Screenshot

sendSentDeletedSub

Expected behavior

From the thread in collect slack- Ln :
"When re-submitting a deleted submission, here are some reasonable outcomes I can think of:

  • Central accepts the submission but doesn’t do anything with it (behavior when a submission isn’t deleted)
  • Central accepts the submission and un-deletes it
  • Central rejects the submission with an error saying the submission has been received and then deleted (similar to the second case but with a better error message)"

Central version shown in version.txt

https://staging.getodk.cloud/
versions:
2e5c4f7 (v2024.2.1-3-g2e5c4f7)
+4a5a9ff81efff48a1500214d9c826b590c9ddaed client (v2024.2.1-16-g4a5a9ff8)
+746ff058d32d968f13cf4e7b0a48b81f88fb21df server (v2024.2.0-79-g746ff058)

Other notes (if any)

If a user tries to re-submit a submission which was edited in Central there is: “You tried to update a submission, but the copy you were editing (undefined) is now out of date. Please get the new version that has been submitted, and make your edits again.”
errorOutOfDate

@matthew-white matthew-white added bug backend Requires a change to the API server labels Nov 22, 2024
@ktuite
Copy link
Member

ktuite commented Nov 22, 2024

I'll look into this... a similar issue was coming up while i was working on #749 / getodk/central-backend#1299

Basically, what is the behavior when you resubmit a submission that was soft-deleted for both regular submissions like in this issue and for draft submissions? I had been assuming it was simpler than the form delete case for some reason, but it's not. I'm not sure which of the three scenarios @dbemke mentioned is best

  • Central accepts the submission but doesn’t do anything with it (behavior when a submission isn’t deleted)
  • Central accepts the submission and un-deletes it
  • Central rejects the submission with an error saying the submission has been received and then deleted (similar to the second case but with a better error message)"

In the invisible soft-delete to be able to recover deleted draft submissions, it should probably be the second scenario where you can resubmit that submission or another submission with the same instance id and not have it complain.

@github-project-automation github-project-automation bot moved this to 🕒 backlog in ODK Central Nov 22, 2024
@ktuite ktuite self-assigned this Nov 22, 2024
@matthew-white
Copy link
Member

Not that this was suggested, but I don't think we should allow there to be multiple submissions with the same instance ID, even if one is deleted. Otherwise, we'd have to expose the numeric submissionId for undeleting submissions, similar to how we have to expose formId for undeleting forms. That's because xmlFormId doesn't necessarily uniquely identify forms when you include deleted forms.

Central rejects the submission with an error saying the submission has been received and then deleted (similar to the second case but with a better error message)

I think this would be the easiest thing to do. I also like that it's explicit about what the situation is. Could we run with this, then see how often this situation comes up?

@lognaturel
Copy link
Member

Could we run with this, then see how often this situation comes up?

That seems reasonable! Do you think Central should be responsible for the better error message or Collect? I can't remember, are we currently sending the error code (e.g. 409.2) in any way?

@lognaturel
Copy link
Member

I think it would be reasonable for Central to have a special message like "This submission was previously received and deleted." If we want to be more precise, we could go with something like "The ID of this submission matches the ID of a deleted submission." but I don't think that's as helpful for users.

Alternatively, we'd want to have a special code that Collect can detect that is just about this deleted match case.

@matthew-white
Copy link
Member

Do you think Central should be responsible for the better error message or Collect?

No strong feelings, but it kind of feels like a server concern to me. Central already returns specific Problems for uniqueness violations that we know can happen in the course of normal Central usage. For example, if you try to publish a form def with a duplicate version string, that's a specific Problem with a more informative error message (a 409.6). I think we should do something similar in this case.

are we currently sending the error code (e.g. 409.2) in any way?

I'm not 100% sure, but I don't think we send it over OpenRosa. From what I can tell, it'd be easy to do so though.

@lognaturel
Copy link
Member

that's a specific Problem with a more informative error message (a 409.6). I think we should do something similar in this case.

Perfect, let's do that.

I don't think we send it over OpenRosa. From what I can tell, it'd be easy to do so though.

I think we should to give clients the option of having their own localized and potentially more detailed error messages. That will probably be a whole separate project, though, and we don't have to do it now.

@matthew-white
Copy link
Member

I think we should to give clients the option of having their own localized and potentially more detailed error messages.

I personally feel like we would benefit from having server-side localization at some point. Both error messages and emails would benefit from being localized. If Backend did more localization, then Frontend would have less of that homework (#489). More to the current point, clients like Collect could tell Backend what locale they prefer for error messages via the Accept-Language header.

That will probably be a whole separate project, though, and we don't have to do it now.

Sounds good. 👍 Seeing localized error messages in the client definitely sound like a nice thing at some point.

@matthew-white matthew-white added the nicer error More understandable error messages label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Requires a change to the API server bug nicer error More understandable error messages
Projects
Status: 🕒 backlog
Development

No branches or pull requests

4 participants