From 777fc4a848d13012b456395249ce19f416188a13 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Mon, 25 Mar 2024 08:53:10 +0000 Subject: [PATCH 1/3] Send emails after requested This refactors the professional standing request to send emails automatically after it's marked as requested. --- .../assessment_sections_controller.rb | 14 -------------- app/models/professional_standing_request.rb | 14 ++++++++++++-- app/services/submit_application_form.rb | 9 --------- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/app/controllers/assessor_interface/assessment_sections_controller.rb b/app/controllers/assessor_interface/assessment_sections_controller.rb index 6302cb9d1..747c6e43d 100644 --- a/app/controllers/assessor_interface/assessment_sections_controller.rb +++ b/app/controllers/assessor_interface/assessment_sections_controller.rb @@ -31,7 +31,6 @@ def update if @form.save if assessment_section.preliminary? if assessment.all_preliminary_sections_passed? - notify_teacher unassign_assessor else redirect_to [ @@ -92,19 +91,6 @@ def form_params params.require(:assessor_interface_assessment_section_form), ) end - - def notify_teacher - unless application_form.teaching_authority_provides_written_statement - return - end - - DeliverEmail.call( - application_form:, - mailer: TeacherMailer, - action: :initial_checks_passed, - ) - end - def unassign_assessor AssignApplicationFormAssessor.call( application_form:, diff --git a/app/models/professional_standing_request.rb b/app/models/professional_standing_request.rb index fc1f5a208..df721b38e 100644 --- a/app/models/professional_standing_request.rb +++ b/app/models/professional_standing_request.rb @@ -38,8 +38,18 @@ def expires_after end end + def after_requested(*) + if should_send_emails? + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :initial_checks_passed, + ) + end + end + def after_received(*) - if should_send_received_email? + if should_send_emails? DeliverEmail.call( application_form:, mailer: TeacherMailer, @@ -59,7 +69,7 @@ def after_expired(user:) private - def should_send_received_email? + def should_send_emails? application_form.declined_at.nil? && application_form.teaching_authority_provides_written_statement end diff --git a/app/services/submit_application_form.rb b/app/services/submit_application_form.rb index 704c5f035..839652b1d 100644 --- a/app/services/submit_application_form.rb +++ b/app/services/submit_application_form.rb @@ -38,15 +38,6 @@ def call action: :application_received, ) - if !application_form.requires_preliminary_check && - application_form.teaching_authority_provides_written_statement - DeliverEmail.call( - application_form:, - mailer: TeacherMailer, - action: :initial_checks_passed, - ) - end - if region.teaching_authority_requires_submission_email DeliverEmail.call( application_form:, From 96c79997b7c4aad79e2104deaef4be8e66d974c3 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Mon, 25 Mar 2024 08:53:31 +0000 Subject: [PATCH 2/3] Refactor SubmitApplicationForm This refactors the service to make the code easier to read and clearer. --- app/services/submit_application_form.rb | 28 +++++++++++++++++-------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/app/services/submit_application_form.rb b/app/services/submit_application_form.rb index 839652b1d..b7af48a3d 100644 --- a/app/services/submit_application_form.rb +++ b/app/services/submit_application_form.rb @@ -13,7 +13,7 @@ def call ActiveRecord::Base.transaction do application_form.update!( - requires_preliminary_check: region.requires_preliminary_check, + requires_preliminary_check:, subjects: application_form.subjects.compact_blank, submitted_at: Time.zone.now, working_days_since_submission: 0, @@ -21,7 +21,7 @@ def call assessment = AssessmentFactory.call(application_form:) - if application_form.reduced_evidence_accepted + if reduced_evidence_accepted UpdateAssessmentInductionRequired.call(assessment:) end @@ -38,7 +38,7 @@ def call action: :application_received, ) - if region.teaching_authority_requires_submission_email + if teaching_authority_requires_submission_email DeliverEmail.call( application_form:, mailer: TeachingAuthorityMailer, @@ -46,23 +46,33 @@ def call ) end - FindApplicantInDQTJob.perform_later(application_form) - - # Sometimes DQT doesn't find a result the first time - FindApplicantInDQTJob.set(wait: 5.minutes).perform_later(application_form) + perform_duplicate_jobs end private attr_reader :application_form, :user - delegate :region, to: :application_form + delegate :reduced_evidence_accepted, + :region, + :requires_preliminary_check, + :teaching_authority_provides_written_statement, + to: :application_form + + delegate :teaching_authority_requires_submission_email, to: :region def create_professional_standing_request(assessment) - return unless application_form.teaching_authority_provides_written_statement + return unless teaching_authority_provides_written_statement ProfessionalStandingRequest .create!(assessment:) .tap { |requestable| RequestRequestable.call(requestable:, user:) } end + + def perform_duplicate_jobs + FindApplicantInDQTJob.perform_later(application_form) + + # Sometimes DQT doesn't find a result the first time + FindApplicantInDQTJob.set(wait: 5.minutes).perform_later(application_form) + end end From 23f7075ce5cb7da1d58767f58c757660dc9e8d1a Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Mon, 25 Mar 2024 08:53:36 +0000 Subject: [PATCH 3/3] Request professional standing after preliminary checks Currently we mark the professional standing as requested at the point of submission, even if there are preliminary checks due to be checked first. --- .../assessment_sections_controller.rb | 11 ++++ app/services/submit_application_form.rb | 8 ++- .../application_forms_show_view_object.rb | 25 +++++--- spec/factories/assessments.rb | 13 +--- .../pre_assessment_tasks_spec.rb | 62 ++++++++++--------- ...application_forms_show_view_object_spec.rb | 15 ++++- 6 files changed, 81 insertions(+), 53 deletions(-) diff --git a/app/controllers/assessor_interface/assessment_sections_controller.rb b/app/controllers/assessor_interface/assessment_sections_controller.rb index 747c6e43d..8e17e60d4 100644 --- a/app/controllers/assessor_interface/assessment_sections_controller.rb +++ b/app/controllers/assessor_interface/assessment_sections_controller.rb @@ -31,6 +31,7 @@ def update if @form.save if assessment_section.preliminary? if assessment.all_preliminary_sections_passed? + request_professional_standing unassign_assessor else redirect_to [ @@ -91,6 +92,16 @@ def form_params params.require(:assessor_interface_assessment_section_form), ) end + + def request_professional_standing + requestable = assessment.professional_standing_request + return if requestable.nil? || requestable.requested? + + RequestRequestable.call(requestable:, user: current_staff) + + ApplicationFormStatusUpdater.call(application_form:, user: current_staff) + end + def unassign_assessor AssignApplicationFormAssessor.call( application_form:, diff --git a/app/services/submit_application_form.rb b/app/services/submit_application_form.rb index b7af48a3d..f596748a7 100644 --- a/app/services/submit_application_form.rb +++ b/app/services/submit_application_form.rb @@ -64,9 +64,11 @@ def call def create_professional_standing_request(assessment) return unless teaching_authority_provides_written_statement - ProfessionalStandingRequest - .create!(assessment:) - .tap { |requestable| RequestRequestable.call(requestable:, user:) } + requestable = ProfessionalStandingRequest.create!(assessment:) + + unless requires_preliminary_check + RequestRequestable.call(requestable:, user:) + end end def perform_duplicate_jobs diff --git a/app/view_objects/assessor_interface/application_forms_show_view_object.rb b/app/view_objects/assessor_interface/application_forms_show_view_object.rb index fe30190cc..0c09fa21d 100644 --- a/app/view_objects/assessor_interface/application_forms_show_view_object.rb +++ b/app/view_objects/assessor_interface/application_forms_show_view_object.rb @@ -119,15 +119,24 @@ def await_professional_standing_task_list_item I18n.t( "assessor_interface.application_forms.show.assessment_tasks.items.await_professional_standing_request", ), - link: [ - :locate, - :assessor_interface, - application_form, - assessment, - :professional_standing_request, - ], + link: + if professional_standing_request.requested? + [ + :locate, + :assessor_interface, + application_form, + assessment, + :professional_standing_request, + ] + end, status: - professional_standing_request.received? ? :completed : :waiting_on, + if professional_standing_request.received? + :completed + elsif professional_standing_request.requested? + :waiting_on + else + :cannot_start + end, } end diff --git a/spec/factories/assessments.rb b/spec/factories/assessments.rb index 62e9a3ce9..141fc4128 100644 --- a/spec/factories/assessments.rb +++ b/spec/factories/assessments.rb @@ -84,12 +84,7 @@ trait :with_further_information_request do after(:create) do |assessment, _evaluator| - create( - :further_information_request, - :requested, - :with_items, - assessment:, - ) + create(:requested_further_information_request, :with_items, assessment:) end end @@ -99,12 +94,6 @@ end end - trait :with_requested_professional_standing_request do - after(:create) do |assessment, _evaluator| - create(:requested_professional_standing_request, assessment:) - end - end - trait :with_reference_requests do after(:create) do |assessment, _evaluator| assessment.application_form.work_histories.each do |work_history| diff --git a/spec/system/assessor_interface/pre_assessment_tasks_spec.rb b/spec/system/assessor_interface/pre_assessment_tasks_spec.rb index af48087f1..8fd84d9ce 100644 --- a/spec/system/assessor_interface/pre_assessment_tasks_spec.rb +++ b/spec/system/assessor_interface/pre_assessment_tasks_spec.rb @@ -8,10 +8,10 @@ given_there_is_an_application_form_with_professional_standing_request end - it "passes preliminary check" do + it "passes preliminary check and locate professional standing" do when_i_visit_the(:assessor_application_page, reference:) then_i_see_the(:assessor_application_page, reference:) - and_i_see_a_waiting_on_status + and_i_see_a_preliminary_check_status and_i_see_an_unstarted_preliminary_check_task when_i_click_on_the_preliminary_check_task @@ -20,12 +20,26 @@ and_i_see_a_completed_preliminary_check_task and_the_teacher_receives_a_checks_passed_email and_the_assessor_is_unassigned + + when_i_visit_the(:assessor_application_page, reference:) + then_i_see_the(:assessor_application_page, reference:) + and_i_see_a_waiting_on_status + and_i_click_awaiting_professional_standing + then_i_see_the( + :assessor_locate_professional_standing_request_page, + reference:, + ) + + when_i_fill_in_the_locate_form + then_i_see_the(:assessor_application_page, reference:) + and_i_see_a_assessment_not_started_status + and_the_teacher_receives_a_professional_standing_received_email end it "fails preliminary check" do when_i_visit_the(:assessor_application_page, reference:) then_i_see_the(:assessor_application_page, reference:) - and_i_see_a_waiting_on_status + and_i_see_a_preliminary_check_status and_i_see_an_unstarted_preliminary_check_task when_i_click_on_the_preliminary_check_task @@ -41,22 +55,6 @@ and_i_see_the_failure_reasons end - it "locate professional standing" do - when_i_visit_the(:assessor_application_page, reference:) - then_i_see_the(:assessor_application_page, reference:) - and_i_see_a_waiting_on_status - and_i_click_awaiting_professional_standing - then_i_see_the( - :assessor_locate_professional_standing_request_page, - reference:, - ) - - when_i_fill_in_the_locate_form - then_i_see_the(:assessor_application_page, reference:) - and_i_see_a_preliminary_check_status - and_the_teacher_receives_a_professional_standing_received_email - end - private def given_there_is_an_application_form_with_professional_standing_request @@ -64,11 +62,23 @@ def given_there_is_an_application_form_with_professional_standing_request end def and_i_see_a_waiting_on_status + expect(assessor_application_page.status_summary.value).to have_text( + "WAITING ON LOPS", + ) + end + + def and_i_see_a_preliminary_check_status expect(assessor_application_page.status_summary.value).to have_text( "PRELIMINARY CHECK", ) end + def and_i_see_a_assessment_not_started_status + expect(assessor_application_page.status_summary.value).to have_text( + "ASSESSMENT NOT STARTED", + ) + end + def and_i_see_an_unstarted_preliminary_check_task expect(assessor_application_page.task_list).to have_content( "Preliminary check (qualifications)", @@ -78,7 +88,7 @@ def and_i_see_an_unstarted_preliminary_check_task ) expect( assessor_application_page.awaiting_professional_standing_task, - ).to have_content("WAITING ON") + ).to have_content("CANNOT START") end def when_i_click_on_the_preliminary_check_task @@ -155,15 +165,9 @@ def when_i_fill_in_the_locate_form form.submit_button.click end - def and_i_see_a_preliminary_check_status - expect(assessor_application_page.status_summary.value).to have_text( - "PRELIMINARY CHECK", - ) - end - def and_the_teacher_receives_a_professional_standing_received_email - expect(TeacherMailer.deliveries.count).to eq(1) - expect(TeacherMailer.deliveries.first.subject).to eq( + expect(TeacherMailer.deliveries.count).to eq(2) + expect(TeacherMailer.deliveries.second.subject).to eq( I18n.t( "mailer.teacher.professional_standing_received.subject", certificate: "Letter of Professional Standing", @@ -185,7 +189,7 @@ def application_form create( :assessment, :with_preliminary_qualifications_section, - :with_requested_professional_standing_request, + :with_professional_standing_request, application_form:, ) application_form.region.update!( diff --git a/spec/view_objects/assessor_interface/application_forms_show_view_object_spec.rb b/spec/view_objects/assessor_interface/application_forms_show_view_object_spec.rb index 7f0d8c7f5..3341001d5 100644 --- a/spec/view_objects/assessor_interface/application_forms_show_view_object_spec.rb +++ b/spec/view_objects/assessor_interface/application_forms_show_view_object_spec.rb @@ -102,6 +102,7 @@ let!(:professional_standing_request) do create(:professional_standing_request, assessment:) end + before do application_form.update!( teaching_authority_provides_written_statement: true, @@ -112,10 +113,22 @@ is_expected.to include_task_list_item( "Pre-assessment tasks", "Awaiting third-party professional standing", - status: :waiting_on, + status: :cannot_start, ) end + context "and professional standing request is requested" do + before { professional_standing_request.requested! } + + it do + is_expected.to include_task_list_item( + "Pre-assessment tasks", + "Awaiting third-party professional standing", + status: :waiting_on, + ) + end + end + context "and professional standing request received" do before do professional_standing_request.update!(