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

CAPT-2081 Lower email address max length to 129 #3519

Merged
merged 4 commits into from
Jan 22, 2025
Merged

CAPT-2081 Lower email address max length to 129 #3519

merged 4 commits into from
Jan 22, 2025

Conversation

kenfodder
Copy link
Contributor

No description provided.

@kenfodder kenfodder added the deploy Deploy a review app for this PR label Jan 15, 2025
@github-actions GitHub Actions
Copy link

Deployments

Journey URL
Additional Payments https://claim-additional-payments-for-teaching-review-3519-web.test.teacherservices.cloud/additional-payments/claim
Student Loans https://claim-additional-payments-for-teaching-review-3519-web.test.teacherservices.cloud/student-loans/claim
Further Education https://claim-additional-payments-for-teaching-review-3519-web.test.teacherservices.cloud/further-education-payments/landing-page
Early Years Payment https://claim-additional-payments-for-teaching-review-3519-web.test.teacherservices.cloud/early-years-payment/landing-page
Relocation Payments https://claim-additional-payments-for-teaching-review-3519-web.test.teacherservices.cloud/get-a-teacher-relocation-payment/landing-page
Admin https://claim-additional-payments-for-teaching-review-3519-web.test.teacherservices.cloud/admin

@kenfodder kenfodder marked this pull request as ready for review January 16, 2025 08:36
rjlynch
rjlynch approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@rjlynch rjlynch left a comment

Choose a reason for hiding this comment

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

LGTM - max on production is 54 right?

@kenfodder kenfodder marked this pull request as draft January 16, 2025 10:58
@kenfodder kenfodder marked this pull request as ready for review January 16, 2025 12:21
@kenfodder
Copy link
Contributor Author

@rjlynch Yeah the longest email currently in the prod db is 54 the last time I checked.

Might need a re-review on the second commit I just added, I took the opportunity to extract the max length out so we use the same max length everywhere we validate email address length.

@kenfodder kenfodder requested a review from rjlynch January 16, 2025 12:24
slorek
slorek approved these changes Jan 22, 2025
View reviewed changes
app/forms/email_address_form.rb Outdated Show resolved Hide resolved
@kenfodder
CAPT-2081 Lower email address max length to 129
f36389e
@kenfodder
Refactor email address length validation
009df70 
* Make the max length a global config
* Add extra test coverage for max length checks
* Make error message use a parameter for the length
@kenfodder
Move comment
c64629a
@kenfodder
Add validation tests
cc754f5
@kenfodder kenfodder force-pushed the CAPT-2081-max-email-address-length branch from d89d267 to cc754f5 Compare January 22, 2025 17:58
@kenfodder kenfodder merged commit 369a9c4 into master Jan 22, 2025
15 checks passed
@kenfodder kenfodder deleted the CAPT-2081-max-email-address-length branch January 22, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slorek slorek slorek approved these changes

@rjlynch rjlynch Awaiting requested review from rjlynch

Assignees
No one assigned
Labels
deploy Deploy a review app for this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kenfodder @slorek @rjlynch