Support validating forms without uploading to server

[jkuester]

Currently we have a few validation steps that are run during the upload-*-forms actions to validate that we are uploading good ODK xml for app/contact/collect forms.

As a part of the incoming Enketo changes to cht-core, we are looking to add additional validations to this process. To make it easier for app-developers to validate forms without necessarily uploading them to the server, I am considering a couple of options for refactoring cht-conf's interface/actions.

A. Add new validation actions

We can take the existing validations (and any new ones) that currently run at the start of the upload-*-forms and add dedicated validate-*-forms actions that could be run on their own (without triggering the upload-*-forms actions. (For the sake of passivity, we could continue to automatically run the validate-*-forms actions when the upload-*-forms actions are called too.)

One big advantage of this approach is that all the validation actions can be run before anything gets uploaded to the server. (This could be left up to the order in which the user specifies the actions, or cht-conf could have a deterministic order that it runs the actions regardless of what order they are specified...)

In this case, the command for converting and validating app and contact forms would look like this:

cht --local convert-app-forms validate-app-forms convert-contact-forms validate-contact-forms

B. Add new --dry_run option

Another thing we could do is just leave the validations to run as a part of the upload-*-forms actions, but add a --dry_run argument that would prevent anything from actually getting uploaded as part of the run. In this case, the upload-*-forms actions would run all their validations, but would stop short of actually uploading the forms, so nothing would be changed on the server.

The downside of this is that when you actually want to do the upload, there is no way to ensure all the forms successfully pass validation before beginning to upload things to the server. (E.g. if you run upload-app-forms upload-contact-forms and one of the contact forms fails validation, we will not upload any contact forms to the server, but the app forms will already have been uploaded and that cannot be undone. This is also true of the current cht-conf behavior.)

cht --local --dry_run convert-app-forms convert-contact-forms upload-app-forms upload-contact-forms

C. Totally refactor available actions to simplify interaction

We could also eliminate the convert-*-forms, validate-*-forms, and upload-*-forms actions altogether in favor of a single *-forms action that can perform all three steps (convert, validate, upload). This could be combined with the --dry_run option to allow for validation of the forms without actually changing anything on the server.

This would simplify the number of actions available to the user and ensure the validation of the forms happens at the correct point in the process (after convert and before upload).

One issue with this approach is that there is no way to avoid re-running the convert on the forms when validating/uploading which can cause unwanted changes to the form xml files (since the pyxform convert does not enforce a deterministic ordering of attributes). (For the record this concern would be mostly resolved by: medic/pyxform#8). We could add another argument to the cht command to skip conversion, trading actions for arguments has diminishing returns (though it does have the benefit of simplifying the happy-path case while allowing for more complex interactions...).

cht --local --dry_run app-forms contact-forms

[garethbowen]

I think eventually we want to get to (C). I think it's the most intuitive for users because it has a smaller set of actions, and the dry-run option means actions behave more consistently. To get there we may need to support backwards compatibility, ie: map the current functions to the new format. It may be ok to go for major that breaks compatibility as it's probably not that much work to migrate CI and scripts to the new format, and you can lock your version for the meantime.

The main question for me is how far do we want/need to go for the enketo-core shift. Is there any need to do this right now, or can we add additional validations to the current structure and take more time to design the ideal CLI?

[binokaryg]

For now, A seems to be the safest option but C with backward compatibility should be better for the future.

add additional validations to this process

How strict will these validations be? If a form is valid, but cht-conf thinks it is invalid, will the users be stuck or we can provide a way to skip the validations?

[jkuester]

@garethbowen the main reason why it would be nice to have some kind of change here as part of the additional Enekto validations is to make it easier to iterate between form changes and running the validation without actually uploading anything. Currently it is not possible to validate forms without at least trying to upload them to a server. Option A should be sufficient to achieve this. That work would also not be wasted when we eventually implement C, since we will want to keep it around for backwards compatibility....

@binokaryg the goal is going to be that the validations are only as strict as necessary. It should error on things that we know will cause actual problems in the forms (e.g. there are form calculations that reference fields that do not exist). For other things, such as using deprecated functionality, it will warn the user, but will still allow the form to be uploaded. That being said, I think we should include support for a --skip_validate flag that can be used when uploading forms.

[garethbowen]

Currently it is not possible to validate forms without at least trying to upload them to a server.

I think that's fine for 4.0.0. We can add the extra validations and app developers can upload them to a test server or localhost to run the validations, just like they do currently.

I think the pragmatic approach is to add the validations to the upload forms actions and release that to clear the path for 4.0.0 upgrades, and then start on C. If we ship A now then we'll be adding to the list of actions that will require backwards compatibility or migration.

[garethbowen]

Having written out the above I checked the next email and realised you've already started on A, so let's go with that!

[jkuester]

@garethbowen sorry, was not trying to jump the gun here! Honestly the most important part of #484 for the Enekto stuff is the refactoring I did to make it cleaner to add new validations. Actually wiring up the new validate actions was pretty simple and we could definitely pull that out of the PR and just let it keep running the validations as a part of the upload actions. (This would not require much effort at all since the actions are just a really thin layer over the lib code. The lib code would be essentially unchanged by removing the actions.)

I would much rather trim down this PR than end up with additional functionality we don't really want, but have to support forever. Just let me know what you think!

[garethbowen]

Let's call it "taking the initiative" not "jumping the gun" :)

Stick with your solution for now. I'm not sure when we'll get around to C so this is a nice compromise.

[medic-ci]

🎉 This issue has been resolved in version 3.12.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[ngaruko]

1. New actions added to supported actions

2. validate app-forms

3. validate-collect-forms

4. validate-contact-forms

.