Add RevertDraftFormService

[thomasiles]

Add a new service to revert the changes in a draft form to the live version

Trello card: https://trello.com/c/tP5Fd96d/2610-add-the-ability-to-delete-a-draft-of-a-live-form-with-a-confirmation-page

Motivation

Form creators with a live form can make changes without altering the live form. We call this form the draft form and the live form is not affected. Form creators can make a series of updates to the form, check it and then make the new version live.

Sometimes a form creator makes changes but then doesn't want to make them live. Instead they want to start the editing again from the current live version. This is thought of as deleting the draft version. Again the live version isn't changed.

How are live and draft forms implemented?

See ADR17 and ADR25 for more context.

We have one model which represents the form. It holds the current edited state of the form. It has an association to pages, which themselves can contain conditions.

A state machine is used to model a Form's current publishing lifecycle status.

Form's start off in the draft state. When they are ready to be made live they transition to :live and a FormDocument is created. The FormDocument contains a snapshot of the forms data to be used by the runner to display the form to form fillers.

When a form creator views a form in the admin interface they are redirected to different views based on the form's state. There is a live view and view for editing drafts (the tasklist).

When a form creator wants to make changes to live form, they follow a link to the tasklist view. If they make any changes to a form in the :live state, it's transitioned into the :live_with_draft state.

Deleting drafts

Because :draft and :live_with_draft are just states on the form there is not technically anything to delete. Instead we must reset everything in the Form to match the snapshot taken when the form was made live.

Because the only information about what the contained is in the live FormDocument, we need to pick through it and find the data we need there.

This PR adds a new service which takes this approach. It contains a whitelist of attributes to copy from the form and then goes through the pages of the form, known as steps in the FormDocument, making sure they match.

I don't think there is a definite check to say that the form matches the live version. In the tests we rely on the idea that if we were to make the draft version live again it should match the live FormDocument.

Things to consider when reviewing

• Is this a maintainable solution for future changes? Is it likely adding new features will catch us out?
• is there a better place to put this code? Should it be tied more closely to the code which makes FormDocuments from Forms?
• Are there any alternatives to this approach? Could we easily change things to make all changes FormDocuments so that we would have something to delete?

[chao-xian]

Would it be easier, and shorter, to maintain a list of exclusions?

[thomasiles]

yeah I think that's a fair question. Thats how we started out yesterday with the except code.

I think a list of things to include is more explicit. The real difference is how we want it in the future when we add attributes to form. Do we want to include them by default or not? I'm not sure either way 🤷

[chao-xian]

Then can we make that list a module that we mix-in and only update once?

[DavidBiddle]

The as_form_document method uses a list of exceptions - I think it could potentially be confusing if the attributes are implicit when creating a form document from the current state, but explicit when doing the reverse

[thomasiles]

I've switched this to use a list of exceptions rather than explicitly stating what should be copied.

I've pulled out the list of exceptions in Form into a constant and use that combined with another list of attributes which shouldn't be reverted.

Overall, I think we'd be better off making it more explicit about what goes in a form document. I think that's a bigger conversation though so this brings it in line with how we are creating the form_document in Form.

[lfdebrux]

I'm not sure about including language and available_languages in this list... I don't understand yet how the Welsh language feature is going to work, but it seems surprising behaviour that deleting the draft for one language could delete the draft for another, which is what this implies to me...

[thomasiles]

Yeah, I'm not sure how this will work for welsh either.

I think if you create a draft form and add welsh to it, then delete the draft, you would expect the welsh version to be deleted as well.

In that case language should be included.

This will need to be updated when we add translations because we'll need to take some measures to use the right set of formDocuments when we update the form. I think.

[DavidBiddle]

We also have an archived_with_draft status, I'm not sure whether restoring from an archived version is something we should cover as part of this card or make a new one for it, but it feels like a valid thing to do

[thomasiles]

I've updated the service to make it more generic so we can add deleting drafts from archived forms in the future.

[chao-xian]

I think I'm not fully understanding the difference between RoutingConditions and the other Conditions...? How come we only copy over

[thomasiles]

Condition is the ActiveRecord model. "routing_conditions" is how they are stored in form_document. Take a look at https://github.com/alphagov/forms/blob/main/ADR/ADR034-api-v2-for-forms.md#form-document-response to see an example.

[chao-xian]

Why do check_conditions and goto_conditions not go into the FormDocument? Also asking in relation to copying a Form, and whether to also ignore them or if they need to be copied as I'm not clear what they're for

[thomasiles]

All conditions for a form go into the document. Conditions in a form document are stored once under each ["step"]["routing_condition"].

Here is a section from an example in the above ADR:

{
     "step": {
          id: 18
      /* ..other steps attributes... */
      "routing_conditions": [
        {
          "id": 3,
          "check_step_id": 18,
          "routing_step_id": 18,
          "goto_step_id": null,
          "answer_value": "Yes",
          "created_at": "2024-08-02T08:54:07.479Z",
          "updated_at": "2024-08-02T08:54:07.479Z",
          "skip_to_end": true,
          "validation_errors": []
        }
      ]
   }
}

The step.id, is the id of the step (which is a Page in active record but stored as step in the form document).

step.routing_conditions contains an array of Conditions linked to the step through routing_step_id. check_step_id and goto_step_id are references to other steps in the form. These are just references, not extra conditions. All Conditions have a routing_step_id which links them to a Step.

In this PR we are extracting values from the json form document, so we go through each "step" and create them, setting the references as we go.

In your PR, you will go through each Condition in Page.routing_conditions setting the values directly from the Condition. This will ensure you have all of Conditions.

[chao-xian]

I wonder if revert_draft_from_form_document(tag) is a little better, as to mean it would read as revert draft from form document (that's live or archived)

[chao-xian]

Please can we reword the commit message from blacklist to blocklist 🙏

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-2258.admin.review.forms.service.gov.uk/

It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready
after 5 minutes, there may be something wrong with the ECS task. You will need to go to the integration AWS account
to debug, or otherwise ask an infrastructure person.

For the sign in details and more information, see the review apps wiki page.