-
Notifications
You must be signed in to change notification settings - Fork 21
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
Prevent invalid employment records from submission #7598
Conversation
02c90cb
to
b184b93
Compare
Review app deployed to https://teaching-vacancies-review-pr-7598.test.teacherservices.cloud on AKS |
061f417
to
2b40a74
Compare
app/models/jobseeker_profile.rb
Outdated
@@ -51,7 +51,7 @@ class JobseekerProfile < ApplicationRecord | |||
|
|||
def self.copy_attributes(record, previous_application) | |||
record.assign_attributes( | |||
employments: previous_application.employments.map(&:duplicate), | |||
employments: previous_application.employments.map(&:clone), |
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.
out of curiosity, why use clone
here? Also the qualifications uses duplicate
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.
that should now be reverted - clone was a mistake. Turns out #dup is the correct way, but dup copies parent associations
|
||
validate :employment_history_gaps_are_explained | ||
validate :employment_history_gaps_are_explained, if: -> { unexplained_employment_gaps_present == "true" && employment_history_section_completed } |
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.
I thought we had migrated field like this unexplained_employment_gaps_present
to boolean?
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.
I've just managed to remove it - it was making failing scenarios pass as well by being nil in quite a few places
spec/system/jobseekers/jobseekers_can_add_employments_to_their_job_application_spec.rb
Show resolved
Hide resolved
9bcbdb6
to
1071cfa
Compare
1071cfa
to
1443c48
Compare
* main: (21 commits) Make publisher ATS API easier to test (#7649) Bump the npm-dependencies group with 2 updates (#7651) Update seeds team members (#7648) API vacancy validations (#7643) [ATS API] Soft delete vacancies (#7644) Little copy to fix support users path (#7647) Add data&schema checklists to PR template (#7645) Prevent invalid employment records from submission (#7598) Remove old unused database columns (#7621) Bump the gem-dependencies group with 4 updates (#7642) Jobseekers emails optout (#7584) CES feedback updates (#7639) Various bug fixes on school page (#7616) Add explicit waits on JS flaky tests (#7637) Change link address (#7636) Bump the npm-dependencies group with 2 updates (#7638) Bump ruby from 3.3.6-alpine3.21 to 3.4.2-alpine3.21 (#7634) Combine pages (#7585) Update binstubs (#7633) Change through filter name (#7620) ...
Trello card URL
https://trello.com/c/avSRIbAT/1655-bug-old-employment-records-can-be-invalid-mainly-due-a-lack-of-leaving-reason-insist-that-they-are-validated-before-application
Changes in this PR:
Create a workflow so that invalid employment records copied from an old job application or profile have to be fixed up before job application can be submitted
After