-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fyst 1516 add email validation step to prior year access flow #5300
Fyst 1516 add email validation step to prior year access flow #5300
Conversation
Heroku app: https://gyr-review-app-5300-d758a122588b.herokuapp.com/ |
Co-authored-by: Drew Proebstel <dproebstel@codeforamerica.org>
… into FYST-1516-add-email-validation-step-to-prior-year-access-flow
app/controllers/state_file/archived_intakes/verification_code_controller.rb
Show resolved
Hide resolved
* Add last year verification email Co-authored-by: Em Barnard-Shao <ebarnard@codeforamerica.org> Co-authored-by: Hugo Melo <hmelo@codeforamerica.org> * Rename previous year to archived intake Co-authored-by: Hugo Melo <hmelo@codeforamerica.org> * normalize Co-authored-by: Hugo Melo <hmelo@codeforamerica.org> --------- Co-authored-by: Em Barnard-Shao <ebarnard@codeforamerica.org>
…il-validation-step-to-prior-year-access-flow
db/migrate/20250113222716_remove_state_file_archived_intake_requests_fields.rb
Outdated
Show resolved
Hide resolved
end | ||
|
||
def perform(email_address:, locale:) | ||
ArchivedIntakeEmailVerificationCodeService.request_code(email_address: email_address, locale: locale) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[pebble] also i wanted to point out here, if you're renaming this file in your PR, you'd want to delete the old one too. i don't see that happening in the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed this!
expect(result).to be_a(StateFileArchivedIntakeAccessLog) | ||
expect(result.event_type).to eq event_type | ||
expect(result.state_file_archived_intake_request).to eq request_instance | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[pebble] can we add an expectation testing what would happen if current_request
is nil
since that's a potential outcome? I'm guessing it creates an access log the same way with nil
as the state_file_archived_intake_request_id
on that record, since there's no NOT NULL constraint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! just a small nitpicks/pebble
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great job!
…s-flow' of github.com:codeforamerica/vita-min into FYST-1516-add-email-validation-step-to-prior-year-access-flow
Link to pivotal/JIRA issue
-https://codeforamerica.atlassian.net/browse/FYST-1516
Is PM acceptance required? (delete one)
Reminder: merge main into this branch and get green tests before merging to main
What was done?
State_File_Archive_Intake
State_File_Archive_Access_Log
andState_File_Archive_Request
modelsHow to test?
Screenshots (for visual changes)
TODO:
Make something more beautiful than this in
app/jobs/archived_intake_email_verification_code_job.rb