OpenClinica/enketo-express-oc

Closing completed form provides no autoquery option

Closed this issue · 16 comments

Steps:

  1. Open an already complete form that has relevant logic applied to an item
  2. Enter data such that the item is relevant
  3. Enter data for the item
  4. Change data such that the item is no longer relevant
  5. Observe that the item shows a relevant error as expected
  6. Enter Reasons for Change as needed
  7. Click the Close button

Expected:
The user should be prompted to add an autoquery for the item and the form should close after the user chooses to Proceed.

Actual:
The user is not prompted to add autoqueries. A message is shown that the issue must be resolved to complete the form and the form remains open. Note that this also occurs for relevant errors on a group.

Screenshot 2023-02-17 002212

xlsform

empty form: http://localhost:8005/single/fs/i/pMWLRJo1?ecid=a

RFC view with completed record:

curl --user enketorules: -d "server_url=http://localhost:3000&form_id=OC-649&ecid=a&instance_id=b&instance=\
    <data xmlns:enk=\"http://enketo.org/xforms\" xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:orx=\"http://openrosa.org/xforms\" xmlns:oc=\"http://openclinica.org/xforms\" oc:complete=\"true\" id=\"OC-649\"><a>a</a><grp><b>b</b><b_comment/></grp><meta><instanceID>uuid:6f447a47-6e9f-4d22-a7e4-2679f9d84b40</instanceID></meta>\n</data>\
    " http://localhost:8005/oc/api/v1/instance/edit/rfc
  1. clear value for A
    2. change value for B
    3. add RFC (for B, because A doesn't have a DN widget)
  2. click Close

There are 2 reasons for this undesired behavior:

  1. There is a specific section of code in the Complete function that shows an error dialog for any relevant errors without any options. Though this is not desired in this scenario there likely is some other context (incomplete record?, Participate?) where this is desired. What context is this?
  2. There is another piece of code in the Complete that shows a generic error message for any kind of error without any options. Since autoqueries don't actually resolve the relevant errors (unlike constraint and required errors which are resolved), we run into this after the first issue is fixed. What kind of scenario would we just show a simple dialog for ANY error (require, constraint, relevant) when the Complete button is clicked? (incomplete record?, Participate?)
  1. in Participate
  2. basically any record that is incomplete cannot be completed with any remaining relevant errors or strict errors

Any already-completed record can be exited with relevant errors remaining (but logic needs to add autoqueries first)

@MartijnR - Participate should require that relevant errors be resolved by the user. Other modes should offer Autoqueries to allow the user to leave the form.

For an already complete record, we should always let the user add autoqueries and leave the form when they click Close (which is a renamed Complete button). If an item with an error already has an open query, it doesn't need a new autoquery added, but other items will need an autoquery added so that they have at least one open query.

For a non-complete record, the user should be able to Complete and add autoqueries for any non-strict constraint and non-strict required errors and that should allow them to continue with the action of Completing the record. Any non-strict constraint or non-strict required items must have an autoquery added if they do not already have an open query.
However, if there are any relevant errors, strict constraint errors, or strict required errors, then they should prevent the user from continuing until the actual data problem with those items is resolved.

@pbowen-oc, I am suspecting there may be some duplicate logic in the complete and close functions in Enketo that is also coded in the XForms themselves (in XPath), wrt comment-status 'new' and 'updated'.

Is it true that in OC's XForms a required or constraint error will always disappear if that question has comment-status 'new' or 'updated'? (I see all the and comment-status(...) != 'new' and comment-status(...) != 'updated' in example forms).

In that case we can simplify (robustify) Enketo's code to not look at comment-statuses 'new' and 'updated' of discrepancy notes on questions that have constraint or required errors (because they don't exist - they're mutually exclusive).

@MartijnR - Yes, for non-strict constraints and non-strict required, we always put corresponding XPath in to have a query validate the data that would otherwise be invalid. For relevant, strict-constraints, strict required, and Participate views (which do not use queries), we do not include those clauses. Therefore, the only way to resolve an error derived from one of those is to address the form item data itself.

So, if an item has an active error on it, that either means that the error could be resolved by adding a query but there is not a valid query on it OR the error cannot be resolved by a query and there may or may not be queries on the item. In the latter case, we do not want to add an additional query to the item when the user leaves the form if it already has a New or Updated query. The former case cannot have items with those errors and also New or Updated queries since those are mutually exclusive states.

OR the error cannot be resolved by a query and there may or may not be queries on the item

With this you are referring to

For relevant, strict-constraints, strict required, and Participate views

Right?

@MartijnR - Yes, that is correct.

The fix requires careful testing of all behavior after clicking:

  • the Complete button in any form view (including Participate)
  • the Close button when editing an already completed record

@MartijnR - I'm seeing a couple issues with closing an already completed form:

  1. If there is a strict constraint, it is not letting me leave the form. I just see this message:
    Screenshot 2023-03-01 134248

  2. If an item with a relevant error (from the group level) already has an open query, it is still prompting the user to add an autoquery. If the user clicks Proceed, it seems to be simply closing without adding a further autoquery. The expectation here is that it will only prompt the user to add autoqueries if there is a relevant or strict error that does not have an open query and that clicking Proceed will add an autoquery only for those items with errors that do not already have an open query.
    Screenshot 2023-03-01 133934

Just to be sure, the expectation for 1 is to add an autoquery and then let the user exit, right?

record that violates a strict constraint:

curl --user enketorules: -d "server_url=http://localhost:3000&form_id=OC-649&ecid=a&instance_id=b&instance=\
    <data xmlns:enk=\"http://enketo.org/xforms\" xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:orx=\"http://openrosa.org/xforms\" xmlns:oc=\"http://openclinica.org/xforms\" oc:complete=\"true\" id=\"OC-649\"><a>ab</a><grp><b>b</b><b_comment/></grp><meta><instanceID>uuid:6f447a47-6e9f-4d22-a7e4-2679f9d84b40</instanceID></meta>\n</data>\
    " http://localhost:8005/oc/api/v1/instance/edit/rfc

I suspect the second issue may be a longstanding issue.

  • for 1, also check if there is an open query

If #2 is difficult, we can address that separately later since it does not block the user from performing the action.

  • second issue still outstanding (unless accidentally fixed by the fix for the first issue - I haven't started trying to reproduce issue 2 yet)

@MartijnR - I moved the second comment issue to OpenClinica/enketo-oc#43. This behavior already exists in our current production version and is not an urgent priority for us.

Looks good in testing