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

RAS-1298: Add a new survey page #1003

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

SteveScorfield
Copy link
Contributor

What and why?

As part of the user journey improvement work, there is a requirement to update and improve the add survey section. This PR achieves this.

How to test?

  • Run tests locally
  • Deploy this PR to your local env and run acceptance tests
  • check that it matches the requirements laid out in the Jira ticket

Jira

https://jira.ons.gov.uk/browse/RAS-1298

@SteveScorfield
Copy link
Contributor Author

/deploy scorfs

@ras-rm-pr-bot
Copy link
Collaborator

Deploying to dev cluster with following parameters:

  • namespace: scorfs

  • tag: pr-1003

  • configBranch: main

  • paramKey: ``

  • paramValue: ``

@SteveScorfield
Copy link
Contributor Author

/deploy scorfs

@ras-rm-pr-bot
Copy link
Collaborator

Deploying to dev cluster with following parameters:

  • namespace: scorfs

  • tag: pr-1003

  • configBranch: main

  • paramKey: ``

  • paramValue: ``


{% set page_title = "Add a survey" %}
{% block breadcrumbs %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The style inherited from the design system for the breadcrumbs that there is padding of 8px around the link text. Karen is happy for this to replace the "ons-u-mb-s" (18px) margin.

})
%}
<p>If your enrolment code has been lost, does not work or you did not receive one, request a new code.</p>
<p>To do this, call +44 300 1234 931 or <a href='/surveys/help/something-else'>send a message</a></p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have spoken to Karen about where specifically this "send a message" link needs to point to. It will eventually point to the link created as part of the "Send message" journey. I have made a note in both cards and links so that this link is updated once the work has been completed.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you go down this route currently it leads to 500 errors but as the journey is updated in a separate card no issues. Just calling it out for the following card associated with adding a survey.


{% set breadcrumbsData = [
{
"text": "Surveys",
Copy link
Contributor

Choose a reason for hiding this comment

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

Breadcrumbs should display "back" not "surveys"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot. Not sure how I missed this! I've updated it now.

Copy link
Contributor

@matthew-robinson-ons matthew-robinson-ons left a comment

Choose a reason for hiding this comment

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

One small change to the breadcrumbs text, otherwise looks good.

Copy link
Contributor

@matthew-robinson-ons matthew-robinson-ons left a comment

Choose a reason for hiding this comment

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

All looking good and matches the design docs

Copy link
Contributor

@LJBabbage LJBabbage left a comment

Choose a reason for hiding this comment

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

I think we need to take another look at this and refactor /add-survey, it is a bit of mess and this just builds on that. There is all sorts of code and logging duplication, exceptions that can't happen and very odd use of form data which is purely a error dict {"error": {"type": "failed"}} or {"error": {}}. This then makes this bit really odd {% set errorType = data['error']['type'] %} and then the logic % if errorType == "failed" %}.

The defining of vars to use once i.e {% set errorMsg = 'Enter a valid enrolment code' %} is also not something I would expect and should be fed by the validation step in the backend. There is also some mixing of speech marks which I am unsure why we are doing.

@SteveScorfield
Copy link
Contributor Author

/deploy scorfs

@ras-rm-pr-bot
Copy link
Collaborator

Deploying to dev cluster with following parameters:

  • namespace: scorfs

  • tag: pr-1003

  • configBranch: main

  • paramKey: ``

  • paramValue: ``

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.

4 participants