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

Move completed transaction survey from frontend to feedback #1601

Merged

Conversation

Rosa-Fox
Copy link
Contributor

@Rosa-Fox Rosa-Fox commented Feb 1, 2023

this will need to be deployed with a branch from Frontend and Publisher.

Trello

Aim of this work

Completed transaction ‘done’ pages have a url of /done/transaction-name. They are published from the Publisher application. Here are some examples:

completed-transaction-done-publisher

The aim of this change is to move the rendering of completed_transaction ‘done’ pages from the Frontend application to the Feedback application. Previously the views were rendered in the Frontend application, but the data was posted to a route in the Feedback app which was defined here before this PR.

The problem with this was that there was no form validation, as the errors couldn’t be re-rendered between applications. Given that the views rendered based on data from the content store and the data is posted to the Feedback application, it made sense to move the completed_transaction code out of Frontend and into Feedback.

Types of Completed Transaction feedback forms

Service Feedback

Most of the /done/completed-transaction pages render a Service Feedback form. An example is www.gov.uk/done/vehicle-tax.

Assisted Digital

There are also three assisted digital feedback forms:

https://www.gov.uk/done/register-flood-risk-exemption
https://www.gov.uk/done/waste-carrier-or-broker-registration
https://www.gov.uk/done/register-waste-exemption

In this PR, they are sent to the appropriate controller using a routing constraint.

Transaction finished

https://www.gov.uk/done/transaction-finished
This doesn’t display a form… just content to say the transaction is finished.

Where is the data sent?

The Service Feedback form fields also exist within the Assisted Digital Feedback form. They are foundationally the same, but Assisted Digital Feedback has some extra fields.

The Service Feedback form data from both forms is sent to the Support API. It can be viewed using the Support app within the Feedback Explorer. Therefore this app does not have its own database and does not store data, it just passes it through.

In addition to sending some data to the Support API, the data from the other fields plus some data from hidden fields appended using JS (referrer and javascript_enabled) are written to a Google spreadsheet.

Rendering data from the Content Item

This functionality needed to be added in. It uses the base path to find the content item in the Content Store and then presents the necessary information on the views, which is a common GOV.UK pattern.

Validations

Conveniently the model validations had already been written into the ServiceFeedback and AssistedDigitialFeedback models, they just weren’t actually rendered before. I changed the order of a few of them to make the messaging more user friendly.

The error message data is stored within instance variables generated based on the field names. I took this approach as it moves logic from the views to the controllers making the views cleaner.

Translations

I added some functionality to check the locale in the content item and then render the translation accordingly. Translations are English and Welsh, en by default. I am currently waiting on the Welsh translations for the validation errors and for some of the promotional content. This will be updated in due course.

Testing locally

I couldn’t actually run posting to the Support API locally so I tested that on integration.

For testing the form validations try some service feedback pages such as:

http://feedback.dev.gov.uk/done/vehicle-tax

and assisted digital feedback pages such as:

http://feedback.dev.gov.uk/done/register-flood-risk-exemption

A welsh page:

http://feedback.dev.gov.uk/done/ceisio-am-lun-id-tystysgrif-awdurdod-pleidleiswyr

Checking the styling:

Rendering the step by step nav and header:
http://feedback.dev.gov.uk/done/find-pension-contact-details

Rendering the footer:
http://feedback.dev.gov.uk/done/vehicle-tax

Rendering the side nav:
https://www.gov.uk/done/transaction-finished

For submitting the Assisted Digital Feedback form you will need to get Google API credentials from the integration server and add them to a dotenv file. I can help with this and also finding the spreadsheet.

Testing on integration also involves deploying a branch of Frontend (which removes the routing) and a branch of Publisher (to switch the rendering app) and republishing a document. I will put together a deploy plan, but it will just involve deploying those PRs and republishing the completed transactions. There is a pre-existing rake task in Publisher to republish per format which I plan to use.

Associated PRs:

Frontend
Publisher

Screen grabs from testing on Integration - please not that the styling has been updated a little bit since this.

(Click on the them to see larger image)

Service Feedback Form

Rendered Validation errors Content Item Support App
service-feedback-rendered service-feedback-validation-errors-local service-feedback-content-item service-feedback-support-app
Rendered Validation errors Content Item Support App Spreadsheet
assisted-digital-feedback-rendered assisted-digital-feedback-validations assisted-digital-feedback-content-item assisted-digital-feedback-support-app assisted-digital-feedback-spreadsheet-1

Welsh page

Rendered Validation errors Content Item Support App
welsh-page-rendered welsh-page-validation welsh-page-content-item welsh-page-support-app

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 February 1, 2023 12:45 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from 1794c42 to ab5ede8 Compare February 1, 2023 17:56
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 February 1, 2023 17:56 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from ab5ede8 to 8c61ef5 Compare February 2, 2023 14:21
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 February 2, 2023 14:22 Inactive
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 February 24, 2023 18:32 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from 70a76cf to 9cab730 Compare February 27, 2023 16:48
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 February 27, 2023 16:48 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from 9cab730 to 7bb81fa Compare February 27, 2023 18:27
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 February 27, 2023 18:27 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from 7bb81fa to ed9de28 Compare February 28, 2023 15:00
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 February 28, 2023 15:01 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from ed9de28 to 2222c3c Compare February 28, 2023 15:05
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 February 28, 2023 15:05 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from 2222c3c to 9aff595 Compare March 20, 2023 11:44
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 March 20, 2023 11:44 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from 9aff595 to 85ab48e Compare March 20, 2023 11:51
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 March 20, 2023 11:51 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from 85ab48e to f5ad6f4 Compare March 20, 2023 11:58
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 March 20, 2023 11:58 Inactive
Copy link
Contributor

@gclssvglx gclssvglx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is amazing work @Rosa-Fox! The scale of these changes is really substantial - great stuff working all this out. Super impressive and I'm totally in awe of your tests 🥇

I know you're not quite finished with yet @Rosa-Fox, so, but I thought it'd be useful to see some of my comments as some are easy fixes and some might help remove some duplication. No blockers in here though.

config/routes.rb Show resolved Hide resolved
spec/lib/format_routing_constraint_spec.rb Outdated Show resolved Hide resolved
spec/presenters/content_item_presenter_spec.rb Outdated Show resolved Hide resolved
app/controllers/service_feedback_controller.rb Outdated Show resolved Hide resolved
app/controllers/service_feedback_controller.rb Outdated Show resolved Hide resolved
app/models/assisted_digital_feedback.rb Show resolved Hide resolved
value: @value_assistance_improvement_comments_other
},
maxlength: 1250
} %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be missing an empty last line I think

spec/requests/assisted_digital_spec.rb Outdated Show resolved Hide resolved
tell_family_organ_donation:
thanks_for_visiting:
very_dissatisfied: Anfodlon iawn
very_satisfied: Bodlon iawn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing empty line maybe

@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from f5ad6f4 to 192ccca Compare March 31, 2023 10:49
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 March 31, 2023 10:49 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from 192ccca to f8cb08b Compare April 12, 2023 15:16
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 April 12, 2023 15:16 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from f8cb08b to d1ec78c Compare April 12, 2023 16:34
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 April 12, 2023 16:34 Inactive
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 April 13, 2023 10:57 Inactive
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 May 5, 2023 13:34 Inactive
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from dd5bb30 to fa6c865 Compare May 5, 2023 13:38
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 May 5, 2023 13:38 Inactive
Rosa-Fox and others added 17 commits May 31, 2023 15:57
The completed_transaction done pages are being moved from Frontend to Feedback so that model validations can be rendered, as they are defined within Feedback.

The completed_transaction done pages are either standard surveys (service_feedback) such as www.gov.uk/done/vehicle-tax or assisted digital satisfaction surveys (assisted_digital_feedback) such as https://www.gov.uk/done/register-flood-risk-exemption. There are only three assisted digital satisfaction surveys.

This routing approach has been lifted from the Frontend application by which there is a routing constraint on /done/completed-transaction paths. This looks up the content item, checks that it is a completed_transaction and caches it. If it is a completed_transaction then it is routed to a controller action. This routing/constraint will be removed from the Frontend application.

In the Frontend application, the same routing constraint is used for all of the done pages
and then different partials are rendered depending on whether the form is an Assisted Digital survey. This would be problematic here due to both forms needing to post to different controllers to render the correct model validations. If the same routing constraint was used then the application would struggle to determine which controller action would re-render the form. Therefore, the approach is different and the request is routed to the correct controller from the outset in Routes.rb.

Given there are only three assisted digital routes they have been manually defined. This was also due to it being difficult to think of a regex that would be able to identify these pages from the other completed_transaction done pages.
Removing these controllers as they will be re-created outside of the contact/govuk namespace in future commits
The content on the forms (such as the title and whether to display a promo box) is rendered dynamically from the content item. Therefore, a call needed to be made to Content Store to retrieve the correct content to render.

When the form was in Frontend, it submitted the data to the Feedback app, so the controllers already existed. They all inherit from ContactController, so the #new action was overwritten in the subclasses to render @Publication. @Publication stores the presented content item (fetched from the Content Store if it isn’t already cached). Fetching and formatting the content item is handled by ServiceFeedbackHelper and ContentItemPresenter.
The application kept throwing 429 errors. This was due to rate limiting set in RackAttack. The post request needs to make another request to content store when re-rendering the page (if it isn't cached) in order to re-render the content specific to the service on the view. It has therefore been excluded.
If there are validation errors then the form re-renders. Fortunately the validations were already in place. When the form was in the Frontend app, data was submitted to this Feedback application, but the code that did the validation was purposefully not run within Feedback as it was not possible to render the validations back onto the Frontend app. Moving the form into feedback will mean that the pre-existing model validations can be rendered.

There is no database in this application, so the form values are stored as instance variables. Instance variables are also generated to store the error messages for each field and the error-id so that the message summary links to the correct view component. This approach was taken as opposed to using Flash messages as it enabled making use of the existing validation errors.
Displays Service Feedback form. Page is generated based on content item data.

The following have been added incase of validation errors:
- an error summary
- error above the individual component
- an error-id so that the links in the summary link to the correct components
- remembers the previously submitted value when the form is re-rendered.
If there are validation errors then the form re-renders. Fortunately the validations were already in place. When the form was in the Frontend app, data was submitted to this Feedback application, but the code that did the validation was purposefully not run within Feedback as it was not possible to render the validations back onto the Frontend app. Moving the form into feedback means that the pre-existing model validations can now be rendered.

There is no database in this application, so the form values are stored as instance variables. Instance variables are also generated to store the error messages for each field and the error-id so that the message summary links to the correct view component. This approach was taken as opposed to using Flash messages as it enabled making use of the existing validation errors.

There is some duplication in the error message ivar generating methods between this controller and service_feedback_controller. There was an attempt to move them into a helper but the view didn’t recognise them. They didn’t feel generic enough to move to the parent class so they have been duplicated.

The assisted_digital_feedback form displays extra parts of the form depending on what the user selects. The assistance_satisfaction_rating and assistance_improvement_comments were being used by multiple parts of the form which caused the validation errors to render in the wrong place. Therefore fields called assistance_satisfaction_rating_other and assistance_improvement_comments_other were added to prevent this issue. The order of the validation errors was also changed at points so that the more user friendly error displayed first.
Displays Assisted Digital Feedback form. Page is generated based on content item data.

The following have been added incase of validation errors:
 - an error summary
 - error above the individual component
 - an error-id so that the links in the summary link to the correct components
 - remembers the previously submitted value when the form is re-rendered.
Append Referrer and js_enabled fields to service_feedback/assisted_digital_feedback forms.
- Check content item for locale
- Render content based on locale being en or cy
Previously the error messages for Service Feedback and Assisted Digital Feedback were either set in the model or generated automatically by Rails. They have now been set to read from locale files. This will mean that error messages won't render in English on Welsh pages.

- TODO - currently waiting on Welsh translations from an external agency
Reimplements a change that was added into the Frontend application recently [1]. The change therefore needed to be re-implemented in here. It adds a promo box for bringing photo ID to vote. This change is temporary and these promo boxes will need to be removed on 04/05/2023 according to [2]. The author of the change introduced a nice refactor for rendering the promo box and translations.

[1](https://github.com/alphagov/frontend/pull/3531/files)
[2](alphagov/frontend#3544)
Adding DOTENV to enable use of Google API credentials in local development.
Updated due to a rebase from the main branch which changed how CSS components are loaded.

The new way of loading individual components: https://github.com/alphagov/govuk_publishing_components/blob/main/docs/set-up-individual-component-css-loading.md

PR that introduced the change: #1622
- encapsulate handling form validation errors
This was removed in the Frontend application [1]. It no longer needs to be reimplemented in this app.

[1](alphagov/frontend@2bfe7a9)
@Rosa-Fox Rosa-Fox force-pushed the move-completed-transaction-survey-from-frontend-to-feedback branch from fa6c865 to ea81b37 Compare May 31, 2023 15:05
@govuk-ci govuk-ci temporarily deployed to feedback-pr-1601 May 31, 2023 15:06 Inactive
@Rosa-Fox Rosa-Fox changed the title [DO NOT MERGE!!] Move completed transaction survey from frontend to feedback Move completed transaction survey from frontend to feedback Jun 1, 2023
@Rosa-Fox Rosa-Fox merged commit 2f69947 into main Jun 1, 2023
6 checks passed
@Rosa-Fox Rosa-Fox deleted the move-completed-transaction-survey-from-frontend-to-feedback branch June 1, 2023 13:59
Rosa-Fox added a commit that referenced this pull request Jun 1, 2023
Add documentation about the completed transaction feedback forms to the README. The rendering of these forms was recently moved from Frontend to the Feedback application[1].

[1](#1601).
@Rosa-Fox Rosa-Fox mentioned this pull request Jun 1, 2023
Rosa-Fox added a commit that referenced this pull request Aug 18, 2023
The naming in the service feedback and assisted digital feedback was causing some confusion as there are places where what is actually the 'base_path' is referred to as the 'slug'.

In reality we need the 'base_path' for rendering the form from the content item and we need the 'slug' to be passed through the form as a hidden field and onto the support API. Using 'slug' in the routing constraint was due to following the same pattern as was used in the Frontend application, which is where the '/done/transaction' pages were previously rendered from.

[The PR to switch the rendering of completed_transactions is here](#1601).
Rosa-Fox added a commit that referenced this pull request Aug 18, 2023
It was realised that the naming in the completed_transaction code caused some confusion as there were places where what is actually the 'base_path' is referred to as the 'slug'. In reality we need the 'base_path' for rendering the form from the content item and we need the 'slug' to be passed through the form as a hidden field and onto the Support API..

Using 'slug' in the routing constraint etc was due to following the same pattern as was used in the Frontend application, which is where the '/done/transaction' pages were previously rendered from.

[The PR to switch the rendering of completed_transactions is here](#1601).
Rosa-Fox added a commit that referenced this pull request Dec 8, 2023
It was realised that the naming in the completed_transaction code caused some confusion as there were places where what is actually the 'base_path' is referred to as the 'slug'. In reality we need the 'base_path' for rendering the form from the content item and we need the 'slug' to be passed through the form as a hidden field and onto the Support API..

Using 'slug' in the routing constraint etc was due to following the same pattern as was used in the Frontend application, which is where the '/done/transaction' pages were previously rendered from.

[The PR to switch the rendering of completed_transactions is here](#1601).
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

Successfully merging this pull request may close these issues.

None yet

3 participants