Stop prefilling applications with profiles and vice versa#8667
Stop prefilling applications with profiles and vice versa#8667KyleMacPherson merged 4 commits intomainfrom
Conversation
|
Review app https://teaching-vacancies-review-pr-8667.test.teacherservices.cloud was successfully deleted |
KyleMacPherson
force-pushed
the
disconnect-profiles
branch
from
d0746e1 to
47869ea
Compare
| # If we don't know the user's status, or they have the right perform the role | ||
| # then we can send them straight to the 'quick apply' screen, otherwise we display the | ||
| # (badly named) about_your_application screen which suggests they might not be qualified for the role. | ||
| if !vacancy.visa_sponsorship_available? && profile.present? && profile.needs_visa_for_uk? | ||
| render "about_your_application" | ||
| else | ||
| @has_previous_application = previous_application? | ||
| end |
There was a problem hiding this comment.
unsure why this has been deleted?
There was a problem hiding this comment.
My thinking around deleting this is that if I didn't it would mean that the application flow retains a dependency on the jobseeker profile and means we still have to consider the application flow when making any further changes to the jobseeker profile. That being said I have added this logic back in and we can revisit it if it becomes an issue.
|
|
||
| def quick_apply? | ||
| previous_application? || profile.present? | ||
| previous_application? |
There was a problem hiding this comment.
unsure about this change?
There was a problem hiding this comment.
I think that as we don't prefill with profile details now this is only going to be true if we have a previous application right?
| context "with a non-visa-seeking profile" do | ||
| before do | ||
| create(:jobseeker_profile, jobseeker: jobseeker, personal_details: build(:personal_details, has_right_to_work_in_uk: true)) | ||
| end | ||
|
|
||
| it "renders 'new'" do | ||
| expect(get(new_jobseekers_job_job_application_path(vacancy.id))) | ||
| .to render_template(:new) | ||
| end | ||
| end |
There was a problem hiding this comment.
see comment above 0- I would have expected this behaviour to stay?
There was a problem hiding this comment.
Addressed this one above!
| def previous_application? | ||
| current_jobseeker.has_submitted_native_job_application? | ||
| end | ||
|
|
There was a problem hiding this comment.
Non blocking suggestion: I think this is obvious enough to inline it in #quick_apply?
| def previous_application? | |
| current_jobseeker.has_submitted_native_job_application? | |
| end |
|
|
||
| def quick_apply? | ||
| previous_application? || profile.present? | ||
| previous_application? |
There was a problem hiding this comment.
Non blocking suggestion:
I personally find that previous_application? adds little to nothing over the current_jobseeker.has_submitted_native_job_application?
So we could just
| previous_application? | |
| current_jobseeker.has_submitted_native_job_application? |
| def job_application | ||
| return new_job_application if @vacancy.uploaded_form? | ||
| return new_job_application unless has_data_available_to_prefill_with? | ||
| return new_job_application unless previously_submitted_application? |
There was a problem hiding this comment.
Same non-blocking suggestion. I think we could just call and be as easy to understand as the method name.
| return new_job_application unless previously_submitted_application? | |
| return new_job_application unless jobseeker.has_submitted_native_job_application? |
| def previously_submitted_application? | ||
| jobseeker.has_submitted_native_job_application? | ||
| end |
There was a problem hiding this comment.
Non blocking:
| def previously_submitted_application? | |
| jobseeker.has_submitted_native_job_application? | |
| end |
KyleMacPherson
force-pushed
the
disconnect-profiles
branch
from
5f27bf3 to
549f55e
Compare
3 participants
Trello card URL
https://trello.com/c/mdntsKhY/2751-stop-auto-filling-profiles
Changes in this PR:
This ticket is to disconnect the logic between jobseeker profiles and job applications as we are looking to make the two flows totally separate/different going forward.
Screenshots of UI changes:
Before
After
Checklists:
Data & Schema Changes
If this PR modifies data structures or validations, check the following:
If any of the above options has changed then the author must check/resolve all of the following...
Integration Impact
Does this change affect any of these integrations?
User Experience & Data Integrity
Could this change impact: