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

Unable to reopen the bad instance name form after error and going to the hierachy view #5507

Closed
dbemke opened this issue Mar 21, 2023 · 10 comments · Fixed by #5560
Closed

Unable to reopen the bad instance name form after error and going to the hierachy view #5507

dbemke opened this issue Mar 21, 2023 · 10 comments · Fixed by #5560
Assignees
Milestone

Comments

@dbemke
Copy link

dbemke commented Mar 21, 2023

ODK Collect version

2022.4.4, 2023.1 Beta 4

Android version

10, 11

Device used

Redmi 9T, Galaxy M12

Problem description

Going to the hierarchy view after an error is displayed in bad_instance_form, the form is closed and the user is not able to open the form again.
badinstancenameerror

Steps to reproduce the problem

  1. Go to bad_instance_name form.
    bad-instance-name.xml.txt
  2. Click the „+” icon.
  3. Add new group.
  4. In group error message choose "ok”
  5. Go to the hierarchy view.
  6. Go to end.
  7. Tap ok in the dialog (the form closes).
  8. Try to open the form again.

Expected behavior

The user should be able to reopen the form.

@lognaturel
Copy link
Member

lognaturel commented Mar 21, 2023

This looks expected to me. The instance name expression is invalid so the form is not usable. It's unfortunate that it can be opened at all but there's not much we can do about that without dramatically changing how forms get validated. The first time the form is open, the expression appears valid because there is not more than one repeat instance.

Is this a change in behavior?

@seadowg seadowg moved this to not ready in ODK Collect Mar 21, 2023
@dbemke
Copy link
Author

dbemke commented Mar 22, 2023

I checked only 2022.4.4 and it worked the same. We've got a few test to check it, but in the steps there wasn't the part about going to the hierarchy view (I just added it by accident). In expected result there is information when the error should be visible and that the user should be able to finalize the form. Should those test cases be in the regression test cycle?

@lognaturel
Copy link
Member

Very interesting, it does sound like a change in behavior, then. I'll give this some more thought for .2

@lognaturel
Copy link
Member

@grzesiek2010 grzesiek2010 self-assigned this Apr 24, 2023
@grzesiek2010
Copy link
Member

I can confirm that it's regression and saving such forms with mistakes is now impossible.

@lognaturel
Copy link
Member

it's regression

On what version did you get it to work? I find that very surprising -- fundamentally the expression is flawed. I would tend to close this as a form design issue.

@grzesiek2010
Copy link
Member

On what version did you get it to work?

I didn't check, I only investigated the code and it's clear that there is a bug in the code responsible for managing form errors (plus I remember that it wasn't like that in the past) so it's not only related to this particular type of form issue.
This is another issue that is caused by the same bug in the code (maybe not the same but it's related) #5361. I think we should address both.

@grzesiek2010
Copy link
Member

I can check when it was introduced but as I said this is not only about this particular type of form design issue so I think we should fix it.

@lognaturel
Copy link
Member

Interesting! It's not clear to me what we could do that would make more sense given that the expression doesn't make sense. Ideally we would detect the flaw earlier (e.g. in pyxform). This feels pretty low priority but we can talk about it if you think it should be higher.

@grzesiek2010
Copy link
Member

grzesiek2010 commented Apr 25, 2023

Sorry! My mistake it turns out it was always like that but for some reason, I've always thought it's a long-standing regression. Let me explain what is happening here...

We have two types of form errors:

  • non-fatal errors - minor errors that should not block users from filling a form
  • fatal errors - critical errors that should block filling a form

We have a mechanism for restoring the error dialog. That means if a user rotates a screen or minimizes the app the dialog is restored. This makes a lot of sense in case of fatal errors because we want to block users from filling such forms and those forms should be immediately closed after closing the error dialog.
However, it doesn't make sense in the case of non-fatal errors. Here the error is remembered and even when we close the error dialog and want to continue filling a form like described in the issue the error dialog will be recreated when for example:

  • a user rotates the device
  • a user goes to the hierarchy view and back to the form
  • a user minimizes the app and reopens it

In all those cases the error dialog will be recreated and it might be displayed in a completely different place in a form (not in the place when it really took place). This is very confusing. Additionally, such a recreated non-fatal error becomes a fatal one and when a user clicks the ok button to close it the whole form is closed like it was a fatal error. This is exactly what is happening in this issue.

This is not something very important if it has existed for a long time but it's very confusing not only to the users but to us as well. I fink we should fix this by introducing two types of errors as I described above non-fatal and fatal because now we manage it by passing a boolean. It would be something like here: #5560
Since I've already investigated it we can fix the issue or move to v2023.3 whatever you prefer.

@seadowg seadowg moved this from not ready to backlog in ODK Collect May 3, 2023
@seadowg seadowg modified the milestones: v2023.2, v2023.3 Jun 5, 2023
@seadowg seadowg moved this from backlog to not ready in ODK Collect Jun 5, 2023
@seadowg seadowg moved this from not ready to backlog in ODK Collect Jun 5, 2023
@seadowg seadowg moved this from backlog to in progress in ODK Collect Jun 21, 2023
@github-project-automation github-project-automation bot moved this from in progress to done in ODK Collect Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: done
Development

Successfully merging a pull request may close this issue.

4 participants