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.

Sign up for GitHub

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

Jump to bottom

Er 506 registration journey #368

Merged
merged 13 commits into from
Nov 11, 2022
Merged

Er 506 registration journey #368

merged 13 commits into from
Nov 11, 2022

Conversation

ebrett
Copy link
Contributor

@ebrett ebrett commented Nov 10, 2022 •
edited
Loading

Small fixes for registration journey

  • white font for drop down (british spelling)
  • set unused role type and local authority to 'Not applicable'
  • additional specs
  • remove postcode and ofsted number

@ebrett ebrett temporarily deployed to test November 10, 2022 10:25 Inactive
@viezly
Copy link

viezly bot commented Nov 10, 2022 •
edited
Loading

This pull request is split into 4 parts for easier review.
👀 Review pull request on Viezly

Changed files are located in these folders:

  • app
  • config/locales
  • db
  • spec

@github-actions github-actions bot added the frontend Changes to assets detected label Nov 10, 2022
@ebrett ebrett force-pushed the ER-506-registration-journey branch from 35ae05a to f278ad7 Compare November 10, 2022 12:31
@github-actions github-actions bot added the content Course and page content changes label Nov 10, 2022
@ebrett ebrett marked this pull request as ready for review November 10, 2022 15:57
@peterdavidhamilton
Copy link
Contributor

@ebrett you have a failure for being polite ;)

@ebrett
Copy link
Contributor Author

ebrett commented Nov 10, 2022

@ebrett you have a failure for being polite ;)

ugh, too tired this week

@ebrett ebrett temporarily deployed to test November 10, 2022 16:07 Inactive
@peterdavidhamilton peterdavidhamilton added the deployed Review app deployed for testing label Nov 10, 2022
@peterdavidhamilton peterdavidhamilton temporarily deployed to content November 10, 2022 16:08 Inactive
@github-actions
Copy link

https://ey-recovery-pr-368.london.cloudapps.digital

@ebrett ebrett temporarily deployed to test November 10, 2022 16:25 Inactive
@ebrett ebrett temporarily deployed to content November 10, 2022 16:25 Inactive
@peterdavidhamilton
Copy link
Contributor

Only just started testing but if we are now helping our validation along whilst also improving user experience by using Not applicable where applicable 😉 , should we also have Multiple appear when there is more than one local authority?

ey-recovery-pr-368 london cloudapps digital_registration_local-authority_edit

ey-recovery-pr-368 london cloudapps digital_registration_local-authority_edit (1)

@ebrett
Additional session controller specs
28d70ea
@ebrett
Set local authority/role type to Not applicable
f7a8a76 
For certain setting types, marke local authority and role
types as not applicable
@ebrett
More spec, fix spelling
95dff77
@ebrett
Move sessions controller spec
a9b4cf2
@ebrett
Remove unused attributes from user
3cd43bb 
Add private beta seed user
@ebrett
No need for completed
063ab81 
All information must be filled out (even if not applicable)
@ebrett ebrett force-pushed the ER-506-registration-journey branch from 91a78db to 5c77dca Compare November 10, 2022 16:33
@ebrett ebrett temporarily deployed to test November 10, 2022 16:33 Inactive
@ebrett ebrett temporarily deployed to content November 10, 2022 16:33 Inactive
@peterdavidhamilton
Copy link
Contributor

peterdavidhamilton commented Nov 10, 2022 •
edited
Loading

I hit a 500 with:
ey-recovery-pr-368 london cloudapps digital_registration_local-authority_edit (2)

ey-recovery-pr-368 london cloudapps digital_registration_setting-type-other_edit (1)

peterdavidhamilton
peterdavidhamilton approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@peterdavidhamilton peterdavidhamilton left a comment

Choose a reason for hiding this comment

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

If it is possible to prevent an error for users who don't understand autocomplete and treat it like free text that would be cool. Otherwise very happy. I have assigned you the sentry error.

@peterdavidhamilton
Copy link
Contributor

testing 123 @shamila-yaqub

@peterdavidhamilton peterdavidhamilton added this to the Next release milestone Nov 10, 2022
@ebrett ebrett temporarily deployed to test November 10, 2022 17:04 Inactive
@ebrett ebrett temporarily deployed to content November 10, 2022 17:04 Inactive
@github-actions
Copy link

https://ey-recovery-pr-368.london.cloudapps.digital

@ebrett
Copy link
Contributor Author

ebrett commented Nov 10, 2022

If it is possible to prevent an error for users who don't understand autocomplete and treat it like free text that would be cool. Otherwise very happy. I have assigned you the sentry error.

I got it and did what I hope is a fix for it. Will let you know :)

@shamila-yaqub
Copy link

shamila-yaqub commented Nov 10, 2022 via email

@ebrett ebrett temporarily deployed to content November 11, 2022 10:23 Inactive
@ebrett ebrett temporarily deployed to test November 11, 2022 10:23 Inactive
@ebrett ebrett temporarily deployed to test November 11, 2022 12:46 Inactive
@ebrett ebrett temporarily deployed to content November 11, 2022 12:46 Inactive
@ebrett ebrett merged commit 792f609 into main Nov 11, 2022
@ebrett ebrett temporarily deployed to production November 11, 2022 16:12 Inactive
@ebrett ebrett deleted the ER-506-registration-journey branch November 11, 2022 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agoldstone93 agoldstone93 agoldstone93 approved these changes

@peterdavidhamilton peterdavidhamilton peterdavidhamilton approved these changes

@mo-zag mo-zag Awaiting requested review from mo-zag

Assignees
No one assigned
Labels
content Course and page content changes deployed Review app deployed for testing frontend Changes to assets detected
Projects
None yet
Milestone
v0.1.3
Development

Successfully merging this pull request may close these issues.
4 participants
@ebrett @peterdavidhamilton @shamila-yaqub @agoldstone93