From 7915866fe5cbfd1af8201fc257b5af541d894b79 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Mon, 9 Sep 2024 16:40:42 +0100 Subject: [PATCH 1/4] Record who sent the email to the provider The figma designs require us to display the admin who resent the provider verification email. To capture this information we use the existing note mechanism on claims. This is a bit fragile as we assume the only "provider_verification" note is for emails being sent. It seems like the label on notes maps to a task name, so we went with that for the label. If in future we can take other provider verification notes we may want to consider renaming this email specific label. --- ...provider_verification_emails_controller.rb | 6 ++++++ ...in_provider_verification_task_presenter.rb | 4 ++++ .../admin_tasks_presenter.rb | 2 +- .../tasks/provider_verification.html.erb | 20 +++++++++++++------ ...n_claim_further_education_payments_spec.rb | 11 ++++++++++ 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/app/controllers/admin/further_education_payments/provider_verification_emails_controller.rb b/app/controllers/admin/further_education_payments/provider_verification_emails_controller.rb index d847cdf80f..d82194ff20 100644 --- a/app/controllers/admin/further_education_payments/provider_verification_emails_controller.rb +++ b/app/controllers/admin/further_education_payments/provider_verification_emails_controller.rb @@ -6,6 +6,12 @@ class ProviderVerificationEmailsController < Admin::BaseAdminController def create claim = Claim.find(params[:claim_id]) + claim.notes.create!( + created_by: admin_user, + label: "provider_verification", + body: "Verification email sent to #{claim.school.name}" + ) + ClaimMailer.further_education_payment_provider_verification_email(claim).deliver_later flash[:notice] = "Verification email sent to #{claim.school.name}" diff --git a/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb b/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb index b30052f3ed..6483a94a52 100644 --- a/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb +++ b/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb @@ -25,6 +25,10 @@ def rows end end + def latest_email + claim.notes.by_label("provider_verification").order(created_at: :desc).first + end + private def verification diff --git a/app/models/policies/further_education_payments/admin_tasks_presenter.rb b/app/models/policies/further_education_payments/admin_tasks_presenter.rb index da9c1dfb17..a8c78e631b 100644 --- a/app/models/policies/further_education_payments/admin_tasks_presenter.rb +++ b/app/models/policies/further_education_payments/admin_tasks_presenter.rb @@ -10,7 +10,7 @@ def initialize(claim) end def provider_verification - AdminProviderVerificationTaskPresenter.new(claim).rows + AdminProviderVerificationTaskPresenter.new(claim) end def provider_name diff --git a/app/views/admin/tasks/provider_verification.html.erb b/app/views/admin/tasks/provider_verification.html.erb index 4f3eff60ae..ce8e07facc 100644 --- a/app/views/admin/tasks/provider_verification.html.erb +++ b/app/views/admin/tasks/provider_verification.html.erb @@ -33,7 +33,7 @@ - <% @tasks_presenter.provider_verification.each do |row| %> + <% @tasks_presenter.provider_verification.rows.each do |row| %> <%= row.label %> @@ -54,11 +54,19 @@

-

- You need to check the matching details and confirm if this is a - duplicate claim. If it isn't a duplicate claim, send the verification - request to the provider. -

+ <% if @tasks_presenter.provider_verification.latest_email.present? %> + <% verification_email = @tasks_presenter.provider_verification.latest_email %> +

+ The verification request was sent to the provider by + <%= user_details(verification_email.created_by) %> on <%= l(verification_email.created_at) %> +

+ <% else %> +

+ You need to check the matching details and confirm if this is a + duplicate claim. If it isn't a duplicate claim, send the verification + request to the provider. +

+ <% end %> <%= govuk_button_to( "Send provider verification request", admin_claim_further_education_payments_provider_verification_emails_path(@claim), diff --git a/spec/features/admin/admin_claim_further_education_payments_spec.rb b/spec/features/admin/admin_claim_further_education_payments_spec.rb index 4ad7846782..3796367334 100644 --- a/spec/features/admin/admin_claim_further_education_payments_spec.rb +++ b/spec/features/admin/admin_claim_further_education_payments_spec.rb @@ -1,6 +1,12 @@ require "rails_helper" RSpec.feature "Admin claim further education payments" do + around do |example| + travel_to DateTime.new(AcademicYear.current.start_year, 9, 9, 10, 0, 0) do + example.run + end + end + before do create(:journey_configuration, :further_education_payments_provider) sign_in_as_service_operator @@ -49,6 +55,11 @@ click_on "Send provider verification request" end + expect(page).to have_content( + "The verification request was sent to the provider by " \ + "Aaron Admin on 9 September 2024 11:00am" + ) + provider_email_address = claim.school.eligible_fe_provider.primary_key_contact_email_address expect(provider_email_address).to( From f1b8f7362a5b28d9a3a662fbd59aade680b24f7c Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 12 Sep 2024 13:34:33 +0100 Subject: [PATCH 2/4] Update button copy Button copy has been changed --- app/views/admin/tasks/provider_verification.html.erb | 2 +- .../admin/admin_claim_further_education_payments_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/admin/tasks/provider_verification.html.erb b/app/views/admin/tasks/provider_verification.html.erb index ce8e07facc..adf2e60fd3 100644 --- a/app/views/admin/tasks/provider_verification.html.erb +++ b/app/views/admin/tasks/provider_verification.html.erb @@ -68,7 +68,7 @@

<% end %> <%= govuk_button_to( - "Send provider verification request", + "Resend provider verification’", admin_claim_further_education_payments_provider_verification_emails_path(@claim), class: "govuk-!-margin-bottom-0" ) %> diff --git a/spec/features/admin/admin_claim_further_education_payments_spec.rb b/spec/features/admin/admin_claim_further_education_payments_spec.rb index 3796367334..a5fadaba78 100644 --- a/spec/features/admin/admin_claim_further_education_payments_spec.rb +++ b/spec/features/admin/admin_claim_further_education_payments_spec.rb @@ -52,7 +52,7 @@ ) perform_enqueued_jobs do - click_on "Send provider verification request" + click_on "Resend provider verification’" end expect(page).to have_content( From 8c0b3c5f0e11af845d1e0c1154dba8a0d2947796 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 12 Sep 2024 16:34:20 +0100 Subject: [PATCH 3/4] Fix resend verification email logic There's a couple of states this partial can be in * Verification email not sent to the provider - When the claim was flagged as a duplicate so we didn't send the email on claimant journey completion AND and admin is yet to manually send the verification email by clicking the button * Initial verification email sent to the provider - The claim was not flagged as a duplicate so the initial email was sent when the claimant journey was completed * Email sent by the admin - The claim was flagged as a duplicate but an admin has now clicked the send email button To make the view a bit less confussing we've pulled out a partial for the provider verification completed and uncompleted states. --- ...in_provider_verification_task_presenter.rb | 10 +- .../_provider_verification_submitted.html.erb | 38 +++ ...provider_verification_unsubmitted.html.erb | 34 +++ .../tasks/provider_verification.html.erb | 64 +---- ...n_claim_further_education_payments_spec.rb | 257 ++++++++++++++---- 5 files changed, 284 insertions(+), 119 deletions(-) create mode 100644 app/views/admin/tasks/_provider_verification_submitted.html.erb create mode 100644 app/views/admin/tasks/_provider_verification_unsubmitted.html.erb diff --git a/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb b/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb index 6483a94a52..84329057d0 100644 --- a/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb +++ b/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb @@ -25,10 +25,18 @@ def rows end end - def latest_email + def latest_admin_sent_email claim.notes.by_label("provider_verification").order(created_at: :desc).first end + def verification_email_sent? + !claim.eligibility.flagged_as_duplicate? || verification_email_sent_by_admin_team? + end + + def verification_email_sent_by_admin_team? + latest_admin_sent_email.present? + end + private def verification diff --git a/app/views/admin/tasks/_provider_verification_submitted.html.erb b/app/views/admin/tasks/_provider_verification_submitted.html.erb new file mode 100644 index 0000000000..c4143f5852 --- /dev/null +++ b/app/views/admin/tasks/_provider_verification_submitted.html.erb @@ -0,0 +1,38 @@ +

+ This task was verified by the provider + (<%= @tasks_presenter.provider_name %>). +

+ + + + + + + + + + + + <% @tasks_presenter.provider_verification.rows.each do |row| %> + + + + + + <% end %> + +
+ <%= @current_task_name.humanize %> +
+ Eligibility check + + Claimant submitted + + Provider response +
+ <%= row.label %> + + <%= Array.wrap(row.claimant_answer).join("

").html_safe %> + 		+ <%= row.provider_answer %> +
diff --git a/app/views/admin/tasks/_provider_verification_unsubmitted.html.erb b/app/views/admin/tasks/_provider_verification_unsubmitted.html.erb new file mode 100644 index 0000000000..42861d50d4 --- /dev/null +++ b/app/views/admin/tasks/_provider_verification_unsubmitted.html.erb @@ -0,0 +1,34 @@ +<% if @tasks_presenter.provider_verification.verification_email_sent? %> + <% if @tasks_presenter.provider_verification.verification_email_sent_by_admin_team? %> + <% verification_email = @tasks_presenter.provider_verification.latest_admin_sent_email %> +
+

+ The verification request was sent to the provider by + <%= user_details(verification_email.created_by) %> on <%= l(verification_email.created_at) %> +

+
+ <% end %> + + <%= govuk_button_to( + "Resend provider verification request", + admin_claim_further_education_payments_provider_verification_emails_path(@claim), + class: "govuk-!-margin-bottom-0" + ) %> +<% else %> +

+ This task has not been sent to the provider yet. +

+ +
+

+ You need to check the matching details and confirm if this is a + duplicate claim. If it isn't a duplicate claim, send the verification + request to the provider. +

+ <%= govuk_button_to( + "Send provider verification request", + admin_claim_further_education_payments_provider_verification_emails_path(@claim), + class: "govuk-!-margin-bottom-0" + ) %> +
+<% end %> diff --git a/app/views/admin/tasks/provider_verification.html.erb b/app/views/admin/tasks/provider_verification.html.erb index adf2e60fd3..7916ec0939 100644 --- a/app/views/admin/tasks/provider_verification.html.erb +++ b/app/views/admin/tasks/provider_verification.html.erb @@ -10,69 +10,9 @@

<%= @current_task_name.humanize %>

<% if @tasks_presenter.provider_verification_submitted? %> -

- This task was verified by the provider - (<%= @tasks_presenter.provider_name %>). -

- - - - - - - - - - - - <% @tasks_presenter.provider_verification.rows.each do |row| %> - - - - - - <% end %> - -
- <%= @current_task_name.humanize %> -
- Eligibility check - - Claimant submitted - - Provider response -
- <%= row.label %> - - <%= Array.wrap(row.claimant_answer).join("

").html_safe %> - 		- <%= row.provider_answer %> -
+ <%= render "provider_verification_submitted" %> <% else %> -

- This task has not been sent to the provider yet. -

- -
- <% if @tasks_presenter.provider_verification.latest_email.present? %> - <% verification_email = @tasks_presenter.provider_verification.latest_email %> -

- The verification request was sent to the provider by - <%= user_details(verification_email.created_by) %> on <%= l(verification_email.created_at) %> -

- <% else %> -

- You need to check the matching details and confirm if this is a - duplicate claim. If it isn't a duplicate claim, send the verification - request to the provider. -

- <% end %> - <%= govuk_button_to( - "Resend provider verification’", - admin_claim_further_education_payments_provider_verification_emails_path(@claim), - class: "govuk-!-margin-bottom-0" - ) %> -
+ <%= render "provider_verification_unsubmitted" %> <% end %>
diff --git a/spec/features/admin/admin_claim_further_education_payments_spec.rb b/spec/features/admin/admin_claim_further_education_payments_spec.rb index a5fadaba78..30f31809ba 100644 --- a/spec/features/admin/admin_claim_further_education_payments_spec.rb +++ b/spec/features/admin/admin_claim_further_education_payments_spec.rb @@ -15,64 +15,209 @@ describe "Tasks" do describe "provider verification task" do context "when the provider is yet to verify the claim" do - it "shows the task as pending" do - fe_provider = create( - :school, - :further_education, - :fe_eligible, - name: "Springfield A and M" - ) - - claim = create( - :claim, - first_name: "Edna", - surname: "Krabappel", - date_of_birth: Date.new(1945, 7, 3), - reference: "AB123456", - created_at: DateTime.new(2024, 8, 1, 9, 0, 0), - submitted_at: DateTime.new(2024, 8, 1, 9, 0, 0) - ) - - create( - :further_education_payments_eligibility, - contract_type: "fixed_term", - claim: claim, - school: fe_provider, - award_amount: 1500 - ) - - visit admin_claim_path(claim) - - click_on "View tasks" - - click_on "Confirm the provider verification" - - expect(page).to have_content( - "This task has not been sent to the provider yet." - ) - - perform_enqueued_jobs do - click_on "Resend provider verification’" - end + context "when a verification email has not been sent" do + it "allows the admins to sent the email" do + fe_provider = create( + :school, + :further_education, + :fe_eligible, + name: "Springfield A and M" + ) + + claim = create( + :claim, + first_name: "Edna", + surname: "Krabappel", + date_of_birth: Date.new(1945, 7, 3), + reference: "AB123456", + created_at: DateTime.new(2024, 8, 1, 9, 0, 0), + submitted_at: DateTime.new(2024, 8, 1, 9, 0, 0) + ) + + create( + :further_education_payments_eligibility, + contract_type: "fixed_term", + claim: claim, + school: fe_provider, + award_amount: 1500, + flagged_as_duplicate: true + ) - expect(page).to have_content( - "The verification request was sent to the provider by " \ - "Aaron Admin on 9 September 2024 11:00am" - ) - - provider_email_address = claim.school.eligible_fe_provider.primary_key_contact_email_address - - expect(provider_email_address).to( - have_received_email( - "9a25fe46-2ee4-4a5c-8d47-0f04f058a87d", - recipient_name: "Springfield A and M", - claimant_name: "Edna Krabappel", - claim_reference: "AB123456", - claim_submission_date: "1 August 2024", - verification_due_date: "15 August 2024", - verification_url: Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + visit admin_claim_path(claim) + + click_on "View tasks" + + click_on "Confirm the provider verification" + + expect(page).to have_content( + "This task has not been sent to the provider yet." ) - ) + + perform_enqueued_jobs do + click_on "Send provider verification request" + end + + expect(page).to have_content( + "The verification request was sent to the provider by " \ + "Aaron Admin on 9 September 2024 11:00am" + ) + + provider_email_address = claim.school.eligible_fe_provider.primary_key_contact_email_address + + expect(provider_email_address).to( + have_received_email( + "9a25fe46-2ee4-4a5c-8d47-0f04f058a87d", + recipient_name: "Springfield A and M", + claimant_name: "Edna Krabappel", + claim_reference: "AB123456", + claim_submission_date: "1 August 2024", + verification_due_date: "15 August 2024", + verification_url: Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + ) + ) + end + end + + context "when a verification email has been sent" do + context "when the verification email was resent by the admin team" do + it "shows who last sent the email" do + fe_provider = create( + :school, + :further_education, + :fe_eligible, + name: "Springfield A and M" + ) + + claim = create( + :claim, + first_name: "Edna", + surname: "Krabappel", + date_of_birth: Date.new(1945, 7, 3), + reference: "AB123456", + created_at: DateTime.new(2024, 8, 1, 9, 0, 0), + submitted_at: DateTime.new(2024, 8, 1, 9, 0, 0) + ) + + create( + :further_education_payments_eligibility, + contract_type: "fixed_term", + claim: claim, + school: fe_provider, + award_amount: 1500 + ) + + create( + :note, + claim: claim, + label: "provider_verification", + created_by: create( + :dfe_signin_user, + given_name: "Some", + family_name: "Admin" + ) + ) + + visit admin_claim_path(claim) + + click_on "View tasks" + + click_on "Confirm the provider verification" + + expect(page).not_to have_content( + "This task has not been sent to the provider yet." + ) + + expect(page).to have_content( + "The verification request was sent to the provider by " \ + "Some Admin on 9 September 2024 11:00am" + ) + + perform_enqueued_jobs do + click_on "Resend provider verification request" + end + + provider_email_address = claim.school.eligible_fe_provider.primary_key_contact_email_address + + expect(provider_email_address).to( + have_received_email( + "9a25fe46-2ee4-4a5c-8d47-0f04f058a87d", + recipient_name: "Springfield A and M", + claimant_name: "Edna Krabappel", + claim_reference: "AB123456", + claim_submission_date: "1 August 2024", + verification_due_date: "15 August 2024", + verification_url: Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + ) + ) + end + end + + context "when the verification email was sent when the claim was submitted" do + it "allows the admin to resend the email" do + fe_provider = create( + :school, + :further_education, + :fe_eligible, + name: "Springfield A and M" + ) + + claim = create( + :claim, + first_name: "Edna", + surname: "Krabappel", + date_of_birth: Date.new(1945, 7, 3), + reference: "AB123456", + created_at: DateTime.new(2024, 8, 1, 9, 0, 0), + submitted_at: DateTime.new(2024, 8, 1, 9, 0, 0) + ) + + create( + :further_education_payments_eligibility, + contract_type: "fixed_term", + claim: claim, + school: fe_provider, + award_amount: 1500 + ) + + visit admin_claim_path(claim) + + click_on "View tasks" + + click_on "Confirm the provider verification" + + expect(page).not_to have_content( + "This task has not been sent to the provider yet." + ) + + expect(page).not_to have_content( + "The verification request was sent to the provider by " + ) + + perform_enqueued_jobs do + click_on "Resend provider verification request" + end + + # This is the user we're logged in as + expect(page).to have_content( + "The verification request was sent to the provider by " \ + "Aaron Admin on 9 September 2024 11:00am" + ) + + provider_email_address = claim.school.eligible_fe_provider.primary_key_contact_email_address + + expect(provider_email_address).to( + have_received_email( + "9a25fe46-2ee4-4a5c-8d47-0f04f058a87d", + recipient_name: "Springfield A and M", + claimant_name: "Edna Krabappel", + claim_reference: "AB123456", + claim_submission_date: "1 August 2024", + verification_due_date: "15 August 2024", + verification_url: Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + ) + ) + end + end end end From 691e245e7dd31ba0695aebce3b8fd15d0dbfb27d Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 12 Sep 2024 17:03:40 +0100 Subject: [PATCH 4/4] Show all email sending events Feedback on PR we want to show all admin email sending events on the task, not just the latest one --- .../admin_provider_verification_task_presenter.rb | 6 +++--- .../tasks/_provider_verification_unsubmitted.html.erb | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb b/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb index 84329057d0..890ca01694 100644 --- a/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb +++ b/app/models/policies/further_education_payments/admin_provider_verification_task_presenter.rb @@ -25,8 +25,8 @@ def rows end end - def latest_admin_sent_email - claim.notes.by_label("provider_verification").order(created_at: :desc).first + def admin_sent_emails + @admin_sent_emails ||= claim.notes.by_label("provider_verification").order(created_at: :desc) end def verification_email_sent? @@ -34,7 +34,7 @@ def verification_email_sent? end def verification_email_sent_by_admin_team? - latest_admin_sent_email.present? + admin_sent_emails.any? end private diff --git a/app/views/admin/tasks/_provider_verification_unsubmitted.html.erb b/app/views/admin/tasks/_provider_verification_unsubmitted.html.erb index 42861d50d4..adb49e9a92 100644 --- a/app/views/admin/tasks/_provider_verification_unsubmitted.html.erb +++ b/app/views/admin/tasks/_provider_verification_unsubmitted.html.erb @@ -1,11 +1,12 @@ <% if @tasks_presenter.provider_verification.verification_email_sent? %> <% if @tasks_presenter.provider_verification.verification_email_sent_by_admin_team? %> - <% verification_email = @tasks_presenter.provider_verification.latest_admin_sent_email %>
-

- The verification request was sent to the provider by - <%= user_details(verification_email.created_by) %> on <%= l(verification_email.created_at) %> -

+ <% @tasks_presenter.provider_verification.admin_sent_emails.each do |verification_email| %> +

+ The verification request was sent to the provider by + <%= user_details(verification_email.created_by) %> on <%= l(verification_email.created_at) %> +

+ <% end %>
<% end %>