Execute validations first to prevent partial upload of settings

[mrsarm]

What feature do you want to improve? As discussed in this PR https://github.com/medic/medic-conf/pull/374#issuecomment-781456814 , the process of upload configurations to the CHT using medic-conf is not transactional: we upload different configurations using different "actions", and if one of the actions fails, the script stops the exection but already executed actions are not rolled back. Would be nice to be able to either rollback all the changes or execute all the validations first to minimize partial uploads as much as possible.

Describe the improvement you'd like ACID transactions are not possible with CouchDB across different documents, but we can ensure that each action execute validations before making any changes, and only allow the action to upload configurations if no error was found in the validations of all the actions, not just its own validations. So, all the actions validations should be executed first, and then start the upload process from each action if no single validation fails.

@garethbowen proposes the following changes in the actions interface in the same PR:

In the upload-app-forms action, just beside the execute function, also expose a validate function which does this work. Modify the main.js around line 185 to add a new loop to go through all actions and if they have a validate function execute that first. This way the validation is run first, if and only if one of the actions chosen by the user defines a validate function.

If the proposal is OK, we don't need to refactor the existing actions because it would require a big refactor moving existing validations from the execute methods within actions to the new validate method, but we can start in the PR mentioned the necessary changes in the medic-conf loop that triggers the actions, and implement for now the validation method only in the upload-app-forms action. Then in subsequent PRs we can move out the rest of the validations one by one, and not necessarily all of them in the same medic-conf release.

UI changes

We log the execution of an action as follow:

INFO Starting action: compile-app-settings… 
... LOGS FROM ACTION ...
INFO compile-app-settings complete.

Now the execution of the validations at the beginning would be as follow:

INFO Starting action validations: compile-app-settings… 
... LOGS FROM ACTION VALIDATIONS ...
INFO compile-app-settings validations complete.

Final notes

If there are validations that cause the whole process to abort and the user needs to upload certain configurations, it will have the option of calling medic-conf with just a list of actions as it is possible now, so the proposal won't prevent medic-conf users to upload settings that are needed if errors are found in one action and cannot be fixed in that moment.

CC @craig-landry and @kennsippell

[garethbowen]

We need to be a little careful about the distinction between errors that should abort execution vs warnings that should just be a prompt. The reason this gets tricky is there are two main users - manual execution and automated scripts (eg: CD). When we know for certain that continuing will break the deployment (eg: compilation errors) and we're running in headless mode then we should abort execution with an error code, which will fail the CD build and be really obvious to the app builder. Non-critical errors or when running manually, we should just warn and not crash.

In other words...

	Automated execution	Manual execution
Critical error	Exit with error code	Prompt user
All other errors	Log a warning and continue	Prompt user

[mrsarm]

Got it! I added the ticket to the "3.11.0" and "App Builders Backlog" projects with the same priority than #331 due that this ticket will be addressed in the same PR.

[mrsarm]

The PR with the change is ready for peer review. About the current implementation and the new implementation:

master version: if an action rise an error the app is stopped logging the error from the action. There are certain errors where the user is prompted, but that are "handled" cases within the "action", if the action does not catch an error, the process is aborted regardless of the execution environment, and most of the errors are raised from the action to the dispatcher (main.js) on purpose because is the only way to stop the execution (severe errors).

new version: same than above, but if an action has defined the validate method, the validations are executed before any other action execution, and if there are more than one action with the validations implemented, all the validations are executed in the same order than the actions execute methods, but no one interrupt the execution of the rest of the validations. After all the validations are executed, if at least one of them failed, the process is also aborted like before, but before run any action "execute" method, preventing partial uploads.

[mrsarm]

Ready for AT.

Check comments here: https://github.com/medic/medic-conf/issues/331#issuecomment-789126267

[mrsarm]

The two tickets that were the origin of this one: https://github.com/medic/medic-conf/issues/331 and https://github.com/medic/cht-core/issues/6557 are going to take a different approach because the design proposed here is not enough to handle properly actions dependencies, so I'll post a new proposal here later, and remove this ticket from the 3.11 project to be re-scheduled later.

[garethbowen]

Good plan, thanks @mrsarm

I think the right approach now is for every action to have three steps: build, validate, upload. That way we can do all the building first, and then validate everything, and only then make changes on the server.

[mrsarm]

I have remove the ticket from the 3.7.0 milestone and lower the priority due that is not a requirement asked by Apps Services. so having partial uploads some times doesn't seem to be a big deal, but refactor the code to avoid it may take considerable effort.