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

Add ability to preview form definitions and edit submissions for forms that are closed or closing #469

Open
matthew-white opened this issue Mar 17, 2022 · 9 comments

Comments

@matthew-white
Copy link
Member

matthew-white commented Mar 17, 2022

There are three form states: open, closing, closed.

How form states operate with ODK Collect and OpenRosa

The three form states map very well to ODK Collect usage:

  • open: data collectors can download the blank form and make submissions.
  • closing: data collectors can't download the blank form (and it will disappear from their list of forms to fill) but if they already have submissions, they can send them in.
  • closed: data collectors can't download the blank form and also can't submit existing submissions.

From an OpenRosa API standpoint:

  • open: form def available in formList, submissions accepted by submission
  • closing: form def NOT available in formList, submissions accepted by submission
  • closed: form def NOT available in formList, submissions NOT accepted by submission

How form states currently operate with Enketo and what should change

The form states don't currently work well with Enketo because we use Enketo for different purposes:

  1. Collecting new data, either using the Enketo links + Central Web User auth or using Enketo links + a special token (Public Links)
  2. Showing previews of published forms
  3. Enabling edits of submissions

Currently form-filling links (1 above) work as desired for the three states1 because that usage is very similar to Collect's. It's important that this behavior stays the same. It's NOT enough to block the buttons for submissions from Central Frontend because people distribute Enketo links directly. Specifically, form-filling links should work as follows:

  • If a data collector opened a form-filling Enketo link in the open state and then the state moves to closing, that data collector should be able to submit
  • If a data collector opened a form-filling Enketo link in the open state and then the state moves to closed, that data collector should NOT be able to submit
  • If a data collector attempts to open a form-filling Enketo link in the closed or closing state, they should see an error message ("Oops, this form doesn't exist (anymore). Most likely the owner of the form has deleted it, archived it, or disabled it. Please contact the form's owner to confirm. Detailed error: Form with ID all-widgets not found in /formList.")

Currently, previews (2 above) and edits (3 above) are also affected by form state changes but they should not be:

  • It should be possible to load previews and edits regardless of the form state. This doesn't work right now because the Enketo backend makes an OpenRosa formList request to Central. Currently Central always returns the same formList contents which is filtered by form state.
  • It should be possible to submit edits regardless of the form state. Enketo makes an OpenRosa submission request to Central to submit an edit. Currently, whenever Central receives a submission request, it uses the form ID in the submission XML to check the form state, then uses the state to determine whether the submission should be accepted. Right now new submissions and edits behave the same in this way.

A note about form drafts

Enketo can be used to preview a form draft and fill a submission to it. However, form drafts aren't related to form state: you can use Enketo links for a form draft regardless of the form's state. Under the hood, form drafts use a different set of OpenRosa endpoints from published forms, prefixed with /test. Those endpoints contain a token, so if you have a link to preview or fill a form draft, no auth is needed. It's not possible to edit a draft submission in Enketo.

This is only intended as background information. Because form drafts aren't related to form state, nothing about those endpoints should have to change for this issue.

enketoId and how Enketo links are structured

The three uses of Enketo above — collecting new data, previews, and edits — result in Enketo links with a similar structure:

  1. Collecting new data uses Enketo's online-only view by default (/-/<enketoId>). Users also know they can add /x/ to the URL to get an offline-capable view (/-/x/<enketoId>).
  2. Previews use Enketo's preview view (/-/preview/<enketoId>).
  3. Edits use Enketo's edit view (/-/edit/<enketoId>).

Each published form has an enketoId property used to construct these links. You can see how these links are constructed in Frontend in the EnketoFill and EnketoPreview components.

Each published form also has an additional enketoOnceId property that is used with single-submission Public Links. It's important to know that each form can be associated with multiple Enketo IDs.

Form drafts have their own separate enketoId. Publishing a form draft does not change the enketoId of a form that is already published.

An Enketo ID is generated when Central sends a request to Enketo specifying a server_url.

Next steps

Allowing edits in the closing state looks like a good first step because it only requires changes related to formList. This would likely make for a targeted first PR to discuss. It should keep in mind the following two requirements:

  • Allowing previews in the closing and closed state may be candidates for a similar approach. It's also only related to formList.
  • Allowing edits in the closed state requires changes to the submission endpoint. Currently submissions are rejected based on closed state before they're identified as new vs. edit.

@matthew-white's original writeup

Show original writeup Right now it is only possible to edit a submission if the form's state is open. We explicitly disallow editing for a closed or closing form: 6c5708d. I think that's because when we allowed it, the QA team encountered Enketo errors. Possibly two different errors seen here:

One idea we've discussed is passing Enketo different endpoints from the regular OpenRosa endpoints. I checked, and from what I can tell, we currently have Enketo use the regular endpoints. We could pass Enketo alternative OpenRosa endpoints that would list/allow closed and closing forms.

Question: we could start passing Enketo different endpoints going forward, but would that do anything for existing forms? Is there a way to change this behavior for existing forms without changing forms' Enketo links?

Footnotes

  1. Almost. There's a problem with the offlineable view. In Collect, submissions are separate from their form defs so the form definition can disappear but form submissions can still be sent. In Enketo, you need to be able to load the offlineable view in order to see the submission queue and submit. That means if you have offline Enketo submissions for a form in the closing state, you need to be able to load the page (or make sure not to refresh the cached version) to submit them. Let's consider that separately.

@lognaturel
Copy link
Member

lognaturel commented Jun 15, 2022

At a high-level, for closed and closing forms, we want submission edits to work but submission creation to continue failing as it does today for closed.

An idea to perhaps look into: I believe Enketo will be requesting the formList from a URL that has /edit or some other predictable piece to it (e.g. an instanceID query parameter). If so, Central might be able to serve a different formList based on that. Enketo would still make the same request to the same endpoint but Central would include or exclude forms based on their status.

@lognaturel lognaturel changed the title Add ability to edit submission to form that is closed or closing Add ability to preview form definitions and edit submissions for forms that are closed or closing Jul 8, 2022
@alxndrsn
Copy link
Contributor

alxndrsn commented Jul 14, 2022

Here's an attempt to capture the text above:

. form state enketo purpose currently permitted? should be permitted? behaving correctly? fixed in
1 open central web auth - start new submission yes yes ✔️
2 closing central web auth - start new submission no no ✔️
3 closed central web auth - start new submission no no ✔️
4 open central web auth - save new submission yes yes ✔️
5 closing central web auth - save new submission yes yes ✔️
6 closed central web auth - save new submission no no ✔️
7 open magic link - start new submission yes yes ✔️
8 closing magic link - start new submission no no ✔️
9 closed magic link - start new submission no no ✔️
10 open magic link - save new submission yes yes ✔️
11 closing magic link - save new submission yes yes ✔️
12 closed magic link - save new submission no no ✔️
13 open preview published form yes yes ✔️
14 closing preview published form no yes
15 closed preview published form no yes
16 open load existing submission yes yes ✔️
17 closing load existing submission no yes
18 closed load existing submission no yes
19 open update existing submission yes yes ✔️
20 closing update existing submission yes yes ✔️
21 closed update existing submission yes for OR
no for REST
yes no yes ✔️❌ #530

@lognaturel does this look accurate to you? (yes)

(edit: keeping the table updated)
(edit: fixed table)

@alxndrsn
Copy link
Contributor

All values in the currently permitted? column confirmed locally ✔️

@alxndrsn
Copy link
Contributor

Is there any way to check if a form is a draft inside the Form class at https://github.com/getodk/central-backend/blob/master/lib/model/frames/form.js#L65?

@lognaturel
Copy link
Member

Is there any way to check if a form is a draft inside the Form class

Given https://github.com/getodk/central-backend/pull/530/files#diff-b48115a435dee55dae6048783669f3de6e30e561e4ed20195d0be48fad879c50R113 I think you've been able to satisfying the underlying need, right? Let us know if not!

@alxndrsn
Copy link
Contributor

@lognaturel which cases are missing from the table in #469 (comment)?

Would they be:

  • OpenRosa API endpoints for:

    • getting forms (in all states)
    • getting submissions (in all form states)
    • updating existing submissions (in all form states)
    • creating new submissions (in all form states)

    ?

@lognaturel
Copy link
Member

lognaturel commented Oct 19, 2022

I'm not visualizing how the cases you outlined would match with the table! I agree that the 4 areas of functionality you outlined are likely candidates for change, both for OpenRosa and REST. At least that would be the case if we try to solve this with minimal Enketo changes.

The table update for #530 looks wrong. The first no should have been updated to a yes. You're right that this would also be incomplete, maybe that can be qualified to say OpenRosa only? Maybe we also make it clear that changes have to be made to OpenRosa and REST endpoints and make sure future PRs do both? Fixed!

That said, I think we should consider reverting #530 because of the inconsistency between OpenRosa and REST and because it makes something possible by API that's not actually possible through Enketo. Enketo won't be able to load a closed form for edit currently.

If I recall correctly, the main challenge in this work is that currently Enketo uses the OpenRosa formList to determine whether a form should be accessible and the formList provided by Central is based only on form state. We would like to take into account what kind of Enketo view is being requested so that edits and previews can have special behavior. However, when Enketo backend makes a formList request, that doesn't include information on the kind of view it's for.

I believe the best option we've discussed is for Enketo to provide that information in something like a custom header or perhaps a Referer (sic) header.

The other thing we've mentioned is that we could explore opportunities for tighter integration with Enketo. For example, could Central manage the transformed form and pass that to Enketo? That way it could be entirely responsible for deciding whether a certain use of the form is allowed. This may not be something we get to for a while.

Since we created this issue, another thing that came to mind is that we should probably also introduce a read-only form state that prevents any edits from happening when we do this work.

@matthew-white
Copy link
Member Author

matthew-white commented Oct 25, 2022

I think we should consider reverting #530

@alxndrsn, what do you think about reverting #530? If you think that's a good idea, would you mind creating a PR for that?

UPDATE: Done in #650.

alxndrsn pushed a commit to alxndrsn/odk-central-backend that referenced this issue Nov 18, 2022
This reverts commit 8377a0f.

This commit may be re-introduced alongside similar changes to the
OpenRosa API.

See: getodk#469
alxndrsn added a commit that referenced this issue Nov 18, 2022
This reverts commit 8377a0f.

This commit may be re-introduced alongside similar changes to the
OpenRosa API.

See: #469
@matthew-white
Copy link
Member Author

Once ODK Web Forms are fully in place, this should no longer be an issue. That's because Frontend will request the form XML and pass it to Web Forms and won't have any difficulty doing so for closed or closing forms. It seems to me that this issue stems from how Central communicates with Enketo and will naturally be resolved once we swap out Enketo for Web Forms. I don't think we're planning to prioritize this issue before we swap out Enketo, so I think we could go ahead and close out this issue as a "won't fix". @lognaturel, how does that sound to you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants