Conversation
|
Review app deployed to https://teaching-vacancies-review-pr-8418.test.teacherservices.cloud on AKS |
starswan
force-pushed
the
add-manual-reference
branch
from
edd863d to
54da408
Compare
starswan
force-pushed
the
add-manual-reference
branch
from
54da408 to
f9fda3f
Compare
starswan
force-pushed
the
add-manual-reference
branch
from
f9fda3f to
e72ebdb
Compare
KyleMacPherson
left a comment
KyleMacPherson
left a comment
There was a problem hiding this comment.
A few minor comments, but only one that i think could be blocking is the 30 second sleep in one of the tests
| if referee_form.uploaded_details | ||
| referee = @job_application.referees.create!(name: referee_form.name, | ||
| is_most_recent_employer: false) | ||
| referee.create_reference_request!(reference_form: referee_form.reference_document, |
There was a problem hiding this comment.
[Minor] Take this with a pinch of salt as I am not too familiar with the whole requesting a reference domain/flow, but I find it a bit surprising that we need to create a reference_request after we have already received the reference. It looks like in the code we only ever create a job_reference from a reference_request, is that the reason we need to do this?
Don't think it's blocking or necessarily a big deal, just feels a bit weird, but as I say I may be misunderstanding how everything is meant to fit together here.
There was a problem hiding this comment.
Yes we always have a request. The 'referee' is created by the job-seeker, so the 'request' is the next bit triggered by the HS
| end | ||
|
|
||
| click_on "Save reference" | ||
| expect(Referee.last.attributes.symbolize_keys.except(:created_at, :updated_at, :email_ciphertext, :id, |
There was a problem hiding this comment.
[Minor] Might also be good to assert which page we land on here and check that we use the correct success message, that the status changes on the page etc. If we are checking for Referee should we also be checking that a reference request has been created too?
|
|
||
| page.attach_file("publishers-vacancies-job-applications-referee-form-reference-document-field", Rails.root.join("spec/fixtures/files/blank_job_spec.pdf")) | ||
| click_on "Save reference" | ||
| expect(Referee.last.name).to eq(referee_name) |
There was a problem hiding this comment.
[Minor] Might also be good to assert which page we land on here and check that we use the correct success message, that the status changes on the page etc. If we are checking for Referee should we also be checking that a reference request has been created too?
| before do | ||
| allow(Publishers::DocumentVirusCheck).to receive(:new).and_return(instance_double(Publishers::DocumentVirusCheck, safe?: true)) | ||
| choose "Upload a reference" | ||
| sleep 30 |
There was a problem hiding this comment.
Do we need sleep 30 here? 😴
| expect(publisher_ats_self_disclosure_page.conduct_details.heading.text).to eq("Conduct self-disclosure") | ||
| expect(publisher_ats_self_disclosure_page.confirmation_details.heading.text).to eq("Confirmation self-disclosure") | ||
| end | ||
| scenario "not really changing your mind" do |
There was a problem hiding this comment.
[Minor] This is a bit cryptic! Does this mean opting to use TV for self disclosure but then deciding not to?
There was a problem hiding this comment.
Yes - it is a test refactor, so it was already there.
starswan
force-pushed
the
add-manual-reference
branch
from
902f391 to
d5dda53
Compare
KyleMacPherson
left a comment
KyleMacPherson
left a comment
There was a problem hiding this comment.
Looks good to me!
starswan
force-pushed
the
add-manual-reference
branch
from
4800d85 to
1aa468d
Compare
starswan
force-pushed
the
add-manual-reference
branch
from
1aa468d to
85fe7d1
Compare
starswan
force-pushed
the
add-manual-reference
branch
from
85fe7d1 to
48005a3
Compare
starswan
force-pushed
the
add-manual-reference
branch
from
48005a3 to
ea82014
Compare
2 participants
Trello card URL
https://trello.com/c/Bbexlu9i/2423-allow-hiring-staff-to-manually-add-a-reference
Changes in this PR:
Allow HS to add a referee and/or upload a document containing a reference
Screenshots of UI changes:
Before
After