From d4eaf6fea279855184229d0da4928e064c61ff25 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Fri, 9 Aug 2024 13:05:19 +0100 Subject: [PATCH 01/15] Add initial provider journey happy path This commit adds the happy path for the FE provider journey. This journey starts with the provider visiting a link (that will be emailed to them) to begin approving a claim. The provider is redirected to DfE sign in, and then if authroised shows the claim to approve. This commit is just the happy path, so we haven't added any of the logic around checking DfE sign in permissions. We currently allow any DfE sign in user to approve any claim. We've also not added the questions to the provider needs to answers to the claim form and we don't change any state when the form is submitted. This is very MVP, provider just checks the decleration box and hits submit! We've also added a landing page which we can link to from the manage service page in DfE sign in. Currenlty the journey requires a `claim_id` to work so following the link from DfE sign in just shows the user a message that they can't access the service like that. While the approach of representing approving claims as a "journey" works, it requires us to setup a few models the claims journey expects to be present that our specific use case doesn't require. There's also a requirement, on the distant horizon, to have an index page of claims that require approving by a given approver which might be tricky to fit into the journey based approach. We may want to consider moving to [a different approach](https://github.com/DFE-Digital/claim-additional-payments-for-teaching/pull/3074) --- .../omniauth_callbacks_controller.rb | 52 ++++++++++---- .../provider/claim_submission_form.rb | 17 +++++ .../provider/session_form.rb | 9 +++ .../provider/verify_claim_form.rb | 41 +++++++++++ app/models/journeys.rb | 1 + .../further_education_payments/provider.rb | 22 ++++++ .../provider/eligibility_checker.rb | 12 ++++ .../provider/session.rb | 9 +++ .../provider/session_answers.rb | 14 ++++ .../provider/session_answers_type.rb | 7 ++ .../provider/slug_sequence.rb | 36 ++++++++++ .../provider/claims/complete.html.erb | 44 ++++++++++++ .../provider/claims/sign_in.html.erb | 37 ++++++++++ .../provider/claims/verify_claim.html.erb | 71 +++++++++++++++++++ .../provider/landing_page.html.erb | 17 +++++ config/initializers/omniauth.rb | 22 ++++++ config/locales/en.yml | 14 ++++ config/routes.rb | 4 ++ db/seeds.rb | 1 + spec/factories/journey_configurations.rb | 4 ++ .../provider/session.rb | 8 +++ .../provider_verifying_claims_spec.rb | 66 +++++++++++++++++ .../provider/verify_claim_form_spec.rb | 27 +++++++ spec/models/journeys_spec.rb | 2 + 24 files changed, 522 insertions(+), 15 deletions(-) create mode 100644 app/forms/journeys/further_education_payments/provider/claim_submission_form.rb create mode 100644 app/forms/journeys/further_education_payments/provider/session_form.rb create mode 100644 app/forms/journeys/further_education_payments/provider/verify_claim_form.rb create mode 100644 app/models/journeys/further_education_payments/provider.rb create mode 100644 app/models/journeys/further_education_payments/provider/eligibility_checker.rb create mode 100644 app/models/journeys/further_education_payments/provider/session.rb create mode 100644 app/models/journeys/further_education_payments/provider/session_answers.rb create mode 100644 app/models/journeys/further_education_payments/provider/session_answers_type.rb create mode 100644 app/models/journeys/further_education_payments/provider/slug_sequence.rb create mode 100644 app/views/further_education_payments/provider/claims/complete.html.erb create mode 100644 app/views/further_education_payments/provider/claims/sign_in.html.erb create mode 100644 app/views/further_education_payments/provider/claims/verify_claim.html.erb create mode 100644 app/views/further_education_payments/provider/landing_page.html.erb create mode 100644 spec/factories/journeys/further_education_payments/provider/session.rb create mode 100644 spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb create mode 100644 spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 7d1ad597cd..11cfb00542 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -6,21 +6,14 @@ class OmniauthCallbacksController < ApplicationController def callback auth = request.env["omniauth.auth"] - # Only keep the attributes permitted by the form - teacher_id_user_info_attributes = auth.extra.raw_info.to_h.slice( - *SignInOrContinueForm::TeacherIdUserInfoForm::DFE_IDENTITY_ATTRIBUTES.map(&:to_s) - ) - - redirect_to( - claim_path( - journey: current_journey_routing_name, - slug: "sign-in-or-continue", - claim: { - logged_in_with_tid: true, - teacher_id_user_info_attributes: teacher_id_user_info_attributes - } - ) - ) + case params[:journey] + when "further-education-payments-provider" + further_education_payments_provider_callback(auth) + else + # The callback route for student loans and additional payments isn't + # namespaced under a journey + additional_payments_callback(auth) + end end def failure @@ -128,4 +121,33 @@ def omniauth_hash request.env["omniauth.auth"] end end + + def further_education_payments_provider_callback(auth) + session[:slugs] << "sign-in" + + redirect_to( + claim_path( + journey: current_journey_routing_name, + slug: "verify-claim" + ) + ) + end + + def additional_payments_callback(auth) + # Only keep the attributes permitted by the form + teacher_id_user_info_attributes = auth.extra.raw_info.to_h.slice( + *SignInOrContinueForm::TeacherIdUserInfoForm::DFE_IDENTITY_ATTRIBUTES.map(&:to_s) + ) + + redirect_to( + claim_path( + journey: current_journey_routing_name, + slug: "sign-in-or-continue", + claim: { + logged_in_with_tid: true, + teacher_id_user_info_attributes: teacher_id_user_info_attributes + } + ) + ) + end end diff --git a/app/forms/journeys/further_education_payments/provider/claim_submission_form.rb b/app/forms/journeys/further_education_payments/provider/claim_submission_form.rb new file mode 100644 index 0000000000..42974dc726 --- /dev/null +++ b/app/forms/journeys/further_education_payments/provider/claim_submission_form.rb @@ -0,0 +1,17 @@ +# Required to get page sequence to think this is a "normal" journey +module Journeys + module FurtherEducationPayments + module Provider + class ClaimSubmissionForm + def initialize(journey_session) + @journey_session = journey_session + end + + # We don't want page sequence to redirect us + def valid? + false + end + end + end + end +end diff --git a/app/forms/journeys/further_education_payments/provider/session_form.rb b/app/forms/journeys/further_education_payments/provider/session_form.rb new file mode 100644 index 0000000000..6396c86954 --- /dev/null +++ b/app/forms/journeys/further_education_payments/provider/session_form.rb @@ -0,0 +1,9 @@ +module Journeys + module FurtherEducationPayments + module Provider + class SessionForm < Journeys::SessionForm + attribute :claim_id, :string + end + end + end +end diff --git a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb new file mode 100644 index 0000000000..a37ac25749 --- /dev/null +++ b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb @@ -0,0 +1,41 @@ +module Journeys + module FurtherEducationPayments + module Provider + class VerifyClaimForm < Form + attribute :declaration, :boolean + + validates :declaration, acceptance: true + + # validate claim not already verified + + delegate :claim, to: :answers + + def claim_reference + claim.reference + end + + def claimant_name + claim.full_name + end + + def claimant_date_of_birth + claim.date_of_birth.strftime("%-d %B %Y") + end + + def claimant_trn + claim.eligibility.teacher_reference_number + end + + def claim_date + claim.created_at.to_date.strftime("%-d %B %Y") + end + + def save + return false unless valid? + + true + end + end + end + end +end diff --git a/app/models/journeys.rb b/app/models/journeys.rb index 1e8159125d..14f48c25ae 100644 --- a/app/models/journeys.rb +++ b/app/models/journeys.rb @@ -10,6 +10,7 @@ def self.table_name_prefix TeacherStudentLoanReimbursement, GetATeacherRelocationPayment, FurtherEducationPayments, + FurtherEducationPayments::Provider, EarlyYearsPayment::Provider::Start, EarlyYearsPayment::Provider::Authenticated ].freeze diff --git a/app/models/journeys/further_education_payments/provider.rb b/app/models/journeys/further_education_payments/provider.rb new file mode 100644 index 0000000000..d1e09f0e86 --- /dev/null +++ b/app/models/journeys/further_education_payments/provider.rb @@ -0,0 +1,22 @@ +module Journeys + module FurtherEducationPayments + module Provider + extend Base + extend self + + ROUTING_NAME = "further-education-payments-provider" + VIEW_PATH = "further_education_payments/provider" + I18N_NAMESPACE = "further_education_payments_provider" + + POLICIES = [] + + FORMS = { + "claims" => { + "verify-claim" => VerifyClaimForm + } + } + + CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE = "teacher_payments_claim_verifier" + end + end +end diff --git a/app/models/journeys/further_education_payments/provider/eligibility_checker.rb b/app/models/journeys/further_education_payments/provider/eligibility_checker.rb new file mode 100644 index 0000000000..9b0ceb9dd1 --- /dev/null +++ b/app/models/journeys/further_education_payments/provider/eligibility_checker.rb @@ -0,0 +1,12 @@ +# Required to get page sequence to think this is a "normal" journey +module Journeys + module FurtherEducationPayments + module Provider + class EligibilityChecker < Journeys::EligibilityChecker + def ineligible? + false + end + end + end + end +end diff --git a/app/models/journeys/further_education_payments/provider/session.rb b/app/models/journeys/further_education_payments/provider/session.rb new file mode 100644 index 0000000000..88ed584e20 --- /dev/null +++ b/app/models/journeys/further_education_payments/provider/session.rb @@ -0,0 +1,9 @@ +module Journeys + module FurtherEducationPayments + module Provider + class Session < Journeys::Session + attribute :answers, SessionAnswersType.new + end + end + end +end diff --git a/app/models/journeys/further_education_payments/provider/session_answers.rb b/app/models/journeys/further_education_payments/provider/session_answers.rb new file mode 100644 index 0000000000..1aaf3a5e60 --- /dev/null +++ b/app/models/journeys/further_education_payments/provider/session_answers.rb @@ -0,0 +1,14 @@ +module Journeys + module FurtherEducationPayments + module Provider + class SessionAnswers < Journeys::SessionAnswers + attribute :claim_id, :string + attribute :declaration, :boolean + + def claim + @claim ||= Claim.find(claim_id) + end + end + end + end +end diff --git a/app/models/journeys/further_education_payments/provider/session_answers_type.rb b/app/models/journeys/further_education_payments/provider/session_answers_type.rb new file mode 100644 index 0000000000..6fe76c2889 --- /dev/null +++ b/app/models/journeys/further_education_payments/provider/session_answers_type.rb @@ -0,0 +1,7 @@ +module Journeys + module FurtherEducationPayments + module Provider + class SessionAnswersType < ::Journeys::SessionAnswersType; end + end + end +end diff --git a/app/models/journeys/further_education_payments/provider/slug_sequence.rb b/app/models/journeys/further_education_payments/provider/slug_sequence.rb new file mode 100644 index 0000000000..c1c72b9fea --- /dev/null +++ b/app/models/journeys/further_education_payments/provider/slug_sequence.rb @@ -0,0 +1,36 @@ +module Journeys + module FurtherEducationPayments + module Provider + class SlugSequence + SLUGS = [ + "sign-in", + "verify-claim", + "complete" + ] + + def self.verify_claim_url(claim) + Rails.application.routes.url_helpers.new_claim_path( + module_parent::ROUTING_NAME, + answers: { + claim_id: claim.id + } + ) + end + + def self.start_page_url + Rails.application.routes.url_helpers.landing_page_path( + "further-education-payments-provider" + ) + end + + def initialize(journey_session) + @journey_session = journey_session + end + + def slugs + SLUGS + end + end + end + end +end diff --git a/app/views/further_education_payments/provider/claims/complete.html.erb b/app/views/further_education_payments/provider/claims/complete.html.erb new file mode 100644 index 0000000000..8bef8ed775 --- /dev/null +++ b/app/views/further_education_payments/provider/claims/complete.html.erb @@ -0,0 +1,44 @@ +<% content_for( + :page_title, + page_title( + "Verification complete", + journey: current_journey_routing_name + ) +) %> + +
+
+ +
+

+ Verification complete +

+ +
+ Claim reference number
+ <%= journey_session.answers.claim.reference %> +
+
+ +

+ We have sent you a confirmation email. +

+ +

+ You should keep the confirmation email on file for [recommended time] for + future reference. +

+ +

What happens next

+ +

+ We’ve sent your response to the Further education levelling up premium + payments team. +

+ +

+ They may contact you again to ask for further information. +

+
+
+ diff --git a/app/views/further_education_payments/provider/claims/sign_in.html.erb b/app/views/further_education_payments/provider/claims/sign_in.html.erb new file mode 100644 index 0000000000..0c53a9e0a2 --- /dev/null +++ b/app/views/further_education_payments/provider/claims/sign_in.html.erb @@ -0,0 +1,37 @@ +<% content_for( + :page_title, + page_title( + "Verify a further education retention payment", + journey: current_journey_routing_name + ) +) %> +
+
+

+ Verify a further education retention payment +

+ +

+ Use this service to verify a retention payment for further education + teachers. +

+ +

+ You will need to log in using DfE Sign-in. If you don't have a DfE Sign-in + account yet, we will help you create one. +

+ + <%= button_to( + "/further-education-payments-provider/auth/dfe_fe_provider", + class: "govuk-button govuk-button--start", + data: { + module: "govuk-button" + } + ) do %> + Start now + + <% end %> +
+
diff --git a/app/views/further_education_payments/provider/claims/verify_claim.html.erb b/app/views/further_education_payments/provider/claims/verify_claim.html.erb new file mode 100644 index 0000000000..3b7cf303ff --- /dev/null +++ b/app/views/further_education_payments/provider/claims/verify_claim.html.erb @@ -0,0 +1,71 @@ +<% content_for( + :page_title, + page_title( + @form.t(:title), + journey: current_journey_routing_name, + show_error: @form.errors.any? + ) +) %> + +
+
+

<%= @form.t(:title) %>

+ + <%= govuk_summary_list do |summary_list| %> + <%= summary_list.with_row do |row| %> + <%= row.with_key(text: @form.t(:claim_reference)) %> + <%= row.with_value(text: @form.claim_reference) %> + <% end %> + + <%= summary_list.with_row do |row| %> + <%= row.with_key(text: @form.t(:claimant_name)) %> + <%= row.with_value(text: @form.claimant_name) %> + <% end %> + + <%= summary_list.with_row do |row| %> + <%= row.with_key(text: @form.t(:claimant_date_of_birth)) %> + <%= row.with_value(text: @form.claimant_date_of_birth) %> + <% end %> + + <%= summary_list.with_row do |row| %> + <%= row.with_key(text: @form.t(:claimant_trn)) %> + <%= row.with_value(text: @form.claimant_trn) %> + <% end %> + + <%= summary_list.with_row do |row| %> + <%= row.with_key(text: @form.t(:claim_date)) %> + <%= row.with_value(text: @form.claim_date) %> + <% end %> + <% end %> + + <%= form_with( + model: @form, + url: claim_path(current_journey_routing_name), + builder: GOVUKDesignSystemFormBuilder::FormBuilder, + html: { novalidate: false } + ) do |f| %> + + <%= f.govuk_check_boxes_fieldset( + :declaration, + multiple: false, + legend: { + text: @form.t(:declaration_title), + size: "m" + } + ) do %> + <%= f.govuk_check_box( + :declaration, + "1", + "0", + multiple: false, + link_errors: true, + label: { + text: @form.t(:declaration) + } + ) %> + <% end %> + + <%= f.govuk_submit "Submit" %> + <% end %> +
+
diff --git a/app/views/further_education_payments/provider/landing_page.html.erb b/app/views/further_education_payments/provider/landing_page.html.erb new file mode 100644 index 0000000000..0a08c78564 --- /dev/null +++ b/app/views/further_education_payments/provider/landing_page.html.erb @@ -0,0 +1,17 @@ +
+
+

+ You cannot access this service from DfE Sign-in +

+ +

+ When a claimant applies for a retention payment, an access link will be + sent to your organisation's nominated email address. +

+ +

+ If you have any questions or need support, email: + <%= govuk_mail_to("FE-Levellingup.PremiumPayments@education.gov.uk") %> +

+
+
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 53388abd79..e15de40f9e 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -11,8 +11,11 @@ dfe_sign_in_issuer_uri = ENV["DFE_SIGN_IN_ISSUER"].present? ? URI(ENV["DFE_SIGN_IN_ISSUER"]) : nil +dfe_sign_in_fe_provider_callback_path = "/further-education-payments-provider/auth/callback" + if ENV["DFE_SIGN_IN_REDIRECT_BASE_URL"].present? dfe_sign_in_redirect_uri = URI.join(ENV["DFE_SIGN_IN_REDIRECT_BASE_URL"], "/admin/auth/callback") + dfe_sign_in_fe_provider_redirect_uri = URI.join(ENV["DFE_SIGN_IN_REDIRECT_BASE_URL"], dfe_sign_in_fe_provider_callback_path) end tid_sign_in_endpoint_uri = ENV["TID_SIGN_IN_API_ENDPOINT"].present? ? URI(ENV["TID_SIGN_IN_API_ENDPOINT"]) : nil @@ -70,6 +73,25 @@ def self.bypass? } end + provider :openid_connect, { + name: :dfe_fe_provider, + discovery: true, + response_type: :code, + scope: %i[openid email organisation first_name last_name], + callback_path: dfe_sign_in_fe_provider_callback_path, + path_prefix: "/further-education-payments-provider/auth", + client_options: { + port: dfe_sign_in_issuer_uri&.port, + scheme: dfe_sign_in_issuer_uri&.scheme, + host: dfe_sign_in_issuer_uri&.host, + identifier: ENV["DFE_SIGN_IN_IDENTIFIER"], + secret: ENV["DFE_SIGN_IN_SECRET"], + redirect_uri: dfe_sign_in_fe_provider_redirect_uri&.to_s + }, + issuer: + ("#{dfe_sign_in_issuer_uri}:#{dfe_sign_in_issuer_uri.port}" if dfe_sign_in_issuer_uri.present?) + } + provider :openid_connect, { name: :tid, callback_path: "/claim/auth/tid/callback", diff --git a/config/locales/en.yml b/config/locales/en.yml index 8990ade4c0..3ef21316d7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1094,6 +1094,20 @@ en: By submitting this you are confirming that, to the best of your knowledge, the details you are providing are correct. btn_text: Accept and send + further_education_payments_provider: + journey_name: Claim incentive payments for further education teachers - provider + feedback_email: "FE-Levellingup.PremiumPayments@education.gov.uk" + support_email_address: "FE-Levellingup.PremiumPayments@education.gov.uk" + forms: + verify_claim: + title: "Review a financial incentive payment claim" + declaration_title: "Declaration" + declaration: "To the best of my knowledge, I confirm that the information provided in this form is correct." + claim_reference: "Claim reference" + claimant_name: "Claimant name" + claimant_date_of_birth: "Claimant date of birth" + claimant_trn: "Claimant teacher reference number (TRN)" + claim_date: "Claim date" early_years_payment: claim_description: for an early years financial incentive payment early_years_payment_provider_start: diff --git a/config/routes.rb b/config/routes.rb index 946391b42a..fbbef8bdbd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -61,6 +61,10 @@ def matches?(request) resources :reminders, only: [:show, :update], param: :slug, constraints: {slug: %r{#{Journeys::AdditionalPaymentsForTeaching::SlugSequence::REMINDER_SLUGS.join("|")}}}, controller: "journeys/additional_payments_for_teaching/reminders" end + scope constraints: {journey: "further-education-payments-provider"} do + get "auth/callback", to: "omniauth_callbacks#callback" + end + scope path: "/", constraints: {journey: Regexp.new(Journeys.all_routing_names.join("|"))} do get "landing-page", to: "static_pages#landing_page", as: :landing_page end diff --git a/db/seeds.rb b/db/seeds.rb index 5e2d658d9a..96d65f3a7f 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -11,6 +11,7 @@ Journeys::Configuration.create!(routing_name: Journeys::AdditionalPaymentsForTeaching::ROUTING_NAME, current_academic_year: AcademicYear.current) Journeys::Configuration.create!(routing_name: Journeys::GetATeacherRelocationPayment::ROUTING_NAME, current_academic_year: AcademicYear.current) Journeys::Configuration.create!(routing_name: Journeys::FurtherEducationPayments::ROUTING_NAME, current_academic_year: AcademicYear.current) + Journeys::Configuration.create!(routing_name: Journeys::FurtherEducationPayments::Provider::ROUTING_NAME, current_academic_year: AcademicYear.current) Journeys::Configuration.create!(routing_name: Journeys::EarlyYearsPayment::Provider::Start::ROUTING_NAME, current_academic_year: AcademicYear.current) Journeys::Configuration.create!(routing_name: Journeys::EarlyYearsPayment::Provider::Authenticated::ROUTING_NAME, current_academic_year: AcademicYear.current) diff --git a/spec/factories/journey_configurations.rb b/spec/factories/journey_configurations.rb index 57907de655..ca2db29ffb 100644 --- a/spec/factories/journey_configurations.rb +++ b/spec/factories/journey_configurations.rb @@ -30,6 +30,10 @@ routing_name { Journeys::FurtherEducationPayments::ROUTING_NAME } end + trait :further_education_payments_provider do + routing_name { Journeys::FurtherEducationPayments::Provider::ROUTING_NAME } + end + trait :early_years_payment_provider_start do routing_name { Journeys::EarlyYearsPayment::Provider::Start::ROUTING_NAME } end diff --git a/spec/factories/journeys/further_education_payments/provider/session.rb b/spec/factories/journeys/further_education_payments/provider/session.rb new file mode 100644 index 0000000000..cf18f311f9 --- /dev/null +++ b/spec/factories/journeys/further_education_payments/provider/session.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory( + :further_education_payments_provider_session, + class: "Journeys::FurtherEducationPayments::Provider::Session" + ) do + journey { Journeys::FurtherEducationPayments::Provider::ROUTING_NAME } + end +end diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb new file mode 100644 index 0000000000..9b1426fc4c --- /dev/null +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -0,0 +1,66 @@ +require "rails_helper" + +RSpec.feature "Provider verifying claims" do + before do + create(:journey_configuration, :further_education_payments_provider) + end + + scenario "provider approves the claim" do + fe_provider = create(:school, :further_education, name: "Springfield A&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) + ) + + mock_dfe_sign_in_auth_session( + provider: :dfe_fe_provider, + auth_hash: { + uid: "11111", + extra: { + raw_info: { + organisation: { + id: "22222", + ukprn: fe_provider.ukprn + } + } + } + } + ) + + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + + visit claim_link + + click_on "Start now" + + expect(page).to have_text "Review a financial incentive payment claim" + # The text generated by the dl tag doesn not include a space between the + # label and value (displays as expected in browser). + expect(page).to have_text "Claim referenceAB123456" + expect(page).to have_text "Claimant nameEdna Krabappel" + expect(page).to have_text "Claimant date of birth3 July 1945" + # FIXME RL enable this test once we've added the TRN to the eligibility + # expect(page).to have_text "Claimant teacher reference number (TRN)1234567" + expect(page).to have_text "Claim date1 August 2024" + + check "To the best of my knowledge, I confirm that the information provided in this form is correct." + + click_on "Submit" + + expect(page).to have_content "Verification complete" + expect(page).to have_text "Claim reference number AB123456" + end + + scenario "provider visits the landing page" do + visit( + Journeys::FurtherEducationPayments::Provider::SlugSequence.start_page_url + ) + + expect(page).to have_text "You cannot access this service from DfE Sign-in" + end +end diff --git a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb new file mode 100644 index 0000000000..b5150dfc85 --- /dev/null +++ b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb @@ -0,0 +1,27 @@ +require "rails_helper" + +RSpec.describe Journeys::FurtherEducationPayments::Provider::VerifyClaimForm, type: :model do + let(:journey) { Journeys::FurtherEducationPayments::Provider } + + let(:journey_session) do + create(:further_education_payments_provider_session) + end + + let(:params) { ActionController::Parameters.new } + + let(:form) do + described_class.new( + journey: journey, + journey_session: journey_session, + params: params + ) + end + + describe "validations" do + subject { form } + + it do + is_expected.to validate_acceptance_of(:declaration) + end + end +end diff --git a/spec/models/journeys_spec.rb b/spec/models/journeys_spec.rb index 7530daada1..a7ac57d110 100644 --- a/spec/models/journeys_spec.rb +++ b/spec/models/journeys_spec.rb @@ -10,6 +10,7 @@ Journeys::TeacherStudentLoanReimbursement, Journeys::GetATeacherRelocationPayment, Journeys::FurtherEducationPayments, + Journeys::FurtherEducationPayments::Provider, Journeys::EarlyYearsPayment::Provider::Start, Journeys::EarlyYearsPayment::Provider::Authenticated ]) @@ -23,6 +24,7 @@ Journeys::TeacherStudentLoanReimbursement::ROUTING_NAME, Journeys::GetATeacherRelocationPayment::ROUTING_NAME, Journeys::FurtherEducationPayments::ROUTING_NAME, + Journeys::FurtherEducationPayments::Provider::ROUTING_NAME, Journeys::EarlyYearsPayment::Provider::Start::ROUTING_NAME, Journeys::EarlyYearsPayment::Provider::Authenticated::ROUTING_NAME ]) From 91dc5a26c38196d3e61964f41f4c6494542939b3 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Fri, 9 Aug 2024 14:56:58 +0100 Subject: [PATCH 02/15] Add school to FE eligibility As part of the FE provider journey we need to check the ukprn on the claim's school. Claims get their school from their eligibility's `current_school` method, so we need to make sure that is set for FE eligibility. We've named the field `school_id` rather than `current_school_id` as the FE journey stores the school in the answers `school_id` column. We alias `school` to `current_school` so the `Claim#school` method still works. --- .../policies/further_education_payments/eligibilities.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/factories/policies/further_education_payments/eligibilities.rb b/spec/factories/policies/further_education_payments/eligibilities.rb index 5867f6554f..7a6dfee311 100644 --- a/spec/factories/policies/further_education_payments/eligibilities.rb +++ b/spec/factories/policies/further_education_payments/eligibilities.rb @@ -1,5 +1,7 @@ FactoryBot.define do factory :further_education_payments_eligibility, class: "Policies::FurtherEducationPayments::Eligibility" do + school { create(:school, :further_education) } + trait :eligible do end end From 0f0cfb89b4772c3d2f03b33212f1f8ae13084bfa Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Fri, 9 Aug 2024 16:59:53 +0100 Subject: [PATCH 03/15] Require a matching school to verify claims Only users who are signed in to DfE sign in with an organisation that matches the school the claim is for can verify claims. This commit introduces the concept of "AuthorisedSlugs" to guard certain slugs in a journey. To restrict access to certain slugs a journey needs to define * `SlugSequence#requires_authorisation?` which accepts a `slug` and returns a boolean representing whether or not that slug requires authorisation * `SlugSequence#unauthorised_path` which accepts a `slug` and a `failure_reason` and returns a url to send users who fail authorisation * `Authorisation` a class that will be initialized with the journey session `SessionAnswers` and the `current_slug` and needs to implement the method `Authorisation#failure_reason` returning a string or symbol representing why the journey session failed authorisation As `Authorisation` is only checked if the `current_slug` requires authorisation journeys that don't restrict access to slugs don't need to implement and authorisation class. Additionally the `PageSequence` checks if the `requires_authorisation?` method exists on the `SlugSequence` before trying to call it, so journeys without restricted slugs don't need to define anything extra on their `SlugSequence`. We've added `unauthorised` to the `PageSequence::DEAD_END_SLUGS` list so we don't generate back links on the unauthorised page and we avoid getting redirected, in an infinite loop, back to the slug we're unauthorised for (we don't updated the visited slugs list). A future improvement would be to move `DEAD_END_SLUGS` into the `SlugSequence` but I'm trying to avoid changing too much! On the FE provider journey specific changes: We've added the OmniauthCallbackForm to handle mapping the omniauth payload to the journey session's fields. I'd considered splitting out the actually using the AuthorisedSlugs feature for the FE provider journey into a separate commit but decided it's better to keep the context of how it's used in the commit that introduces it. --- app/controllers/claims_controller.rb | 1 + app/controllers/concerns/authorised_slugs.rb | 29 +++++++++ .../omniauth_callbacks_controller.rb | 5 ++ .../provider/omniauth_callback_form.rb | 33 ++++++++++ .../provider/authorisation.rb | 26 ++++++++ .../provider/session_answers.rb | 4 +- .../provider/slug_sequence.rb | 18 ++++++ app/models/journeys/page_sequence.rb | 10 ++- .../provider/claims/unauthorised.html.erb | 33 ++++++++++ .../provider/answers.rb | 7 +++ .../eligibilities.rb | 1 + .../provider_verifying_claims_spec.rb | 63 +++++++++++++++++++ .../provider/omniauth_callback_form_spec.rb | 40 ++++++++++++ .../provider/authorisation_spec.rb | 34 ++++++++++ 14 files changed, 302 insertions(+), 2 deletions(-) create mode 100644 app/controllers/concerns/authorised_slugs.rb create mode 100644 app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb create mode 100644 app/models/journeys/further_education_payments/provider/authorisation.rb create mode 100644 app/views/further_education_payments/provider/claims/unauthorised.html.erb create mode 100644 spec/factories/journeys/further_education_payments/provider/answers.rb create mode 100644 spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb create mode 100644 spec/models/journeys/further_education_payments/provider/authorisation_spec.rb diff --git a/app/controllers/claims_controller.rb b/app/controllers/claims_controller.rb index 48c578d128..ddfc521242 100644 --- a/app/controllers/claims_controller.rb +++ b/app/controllers/claims_controller.rb @@ -14,6 +14,7 @@ class ClaimsController < BasePublicController include FormSubmittable include ClaimsFormCallbacks + include AuthorisedSlugs def existing_session @existing_session = journey_sessions.first diff --git a/app/controllers/concerns/authorised_slugs.rb b/app/controllers/concerns/authorised_slugs.rb new file mode 100644 index 0000000000..540ba9d4d5 --- /dev/null +++ b/app/controllers/concerns/authorised_slugs.rb @@ -0,0 +1,29 @@ +module AuthorisedSlugs + extend ActiveSupport::Concern + + included do + before_action :authorise_slug! + end + + def authorise_slug! + if page_sequence.requires_authorisation?(current_slug) && !authorised? + redirect_to( + page_sequence.unauthorised_path( + current_slug, + authorisation.failure_reason + ) + ) + end + end + + def authorised? + authorisation.failure_reason.nil? + end + + def authorisation + journey::Authorisation.new( + answers: journey_session.answers, + slug: current_slug + ) + end +end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 11cfb00542..35145faabb 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -123,6 +123,11 @@ def omniauth_hash end def further_education_payments_provider_callback(auth) + Journeys::FurtherEducationPayments::Provider::OmniauthCallbackForm.new( + journey_session: journey_session, + auth: auth + ).save! + session[:slugs] << "sign-in" redirect_to( diff --git a/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb b/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb new file mode 100644 index 0000000000..654ae02370 --- /dev/null +++ b/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb @@ -0,0 +1,33 @@ +module Journeys + module FurtherEducationPayments + module Provider + class OmniauthCallbackForm + def initialize(journey_session:, auth:) + @journey_session = journey_session + @auth = auth + end + + def save! + journey_session.answers.assign_attributes( + dfe_sign_in_uid: dfe_sign_in_uid, + dfe_sign_in_organisation_ukprn: dfe_sign_in_organisation_ukprn + ) + + journey_session.save! + end + + private + + attr_reader :journey_session, :auth + + def dfe_sign_in_uid + auth["uid"] + end + + def dfe_sign_in_organisation_ukprn + auth.dig("extra", "raw_info", "organisation", "ukprn") + end + end + end + end +end diff --git a/app/models/journeys/further_education_payments/provider/authorisation.rb b/app/models/journeys/further_education_payments/provider/authorisation.rb new file mode 100644 index 0000000000..cdf102ef63 --- /dev/null +++ b/app/models/journeys/further_education_payments/provider/authorisation.rb @@ -0,0 +1,26 @@ +module Journeys + module FurtherEducationPayments + module Provider + class Authorisation + def initialize(answers:, slug:) + @answers = answers + @slug = slug + end + + def failure_reason + return :organisation_mismatch if organisation_mismatch? + + nil + end + + private + + attr_reader :answers, :slug + + def organisation_mismatch? + answers.claim.school.ukprn != answers.dfe_sign_in_organisation_ukprn + end + end + end + end +end diff --git a/app/models/journeys/further_education_payments/provider/session_answers.rb b/app/models/journeys/further_education_payments/provider/session_answers.rb index 1aaf3a5e60..221088bdaf 100644 --- a/app/models/journeys/further_education_payments/provider/session_answers.rb +++ b/app/models/journeys/further_education_payments/provider/session_answers.rb @@ -4,9 +4,11 @@ module Provider class SessionAnswers < Journeys::SessionAnswers attribute :claim_id, :string attribute :declaration, :boolean + attribute :dfe_sign_in_uid, :string + attribute :dfe_sign_in_organisation_ukprn, :string def claim - @claim ||= Claim.find(claim_id) + @claim ||= Claim.includes(eligibility: :school).find(claim_id) end end end diff --git a/app/models/journeys/further_education_payments/provider/slug_sequence.rb b/app/models/journeys/further_education_payments/provider/slug_sequence.rb index c1c72b9fea..a4c90fa449 100644 --- a/app/models/journeys/further_education_payments/provider/slug_sequence.rb +++ b/app/models/journeys/further_education_payments/provider/slug_sequence.rb @@ -4,6 +4,12 @@ module Provider class SlugSequence SLUGS = [ "sign-in", + "verify-claim", + "complete", + "unauthorised" + ] + + RESTRICTED_SLUGS = [ "verify-claim", "complete" ] @@ -30,6 +36,18 @@ def initialize(journey_session) def slugs SLUGS end + + def requires_authorisation?(slug) + RESTRICTED_SLUGS.include?(slug) + end + + def unauthorised_path(slug, failure_reason) + Rails.application.routes.url_helpers.claim_path( + self.class.module_parent::ROUTING_NAME, + "unauthorised", + failure_reason: failure_reason + ) + end end end end diff --git a/app/models/journeys/page_sequence.rb b/app/models/journeys/page_sequence.rb index e7633f9e5f..66f0f01e65 100644 --- a/app/models/journeys/page_sequence.rb +++ b/app/models/journeys/page_sequence.rb @@ -5,7 +5,7 @@ module Journeys class PageSequence attr_reader :current_slug - DEAD_END_SLUGS = %w[complete existing-session eligible-later future-eligibility ineligible check-your-email] + DEAD_END_SLUGS = %w[complete existing-session eligible-later future-eligibility ineligible check-your-email unauthorised] OPTIONAL_SLUGS = %w[postcode-search select-home-address reset-claim] def initialize(slug_sequence, completed_slugs, current_slug, journey_session) @@ -61,6 +61,14 @@ def next_required_slug (slugs - completed_slugs - OPTIONAL_SLUGS).first end + def requires_authorisation?(slug) + @slug_sequence.try(:requires_authorisation?, slug) || false + end + + def unauthorised_path(slug, failure_reason) + @slug_sequence.unauthorised_path(slug, failure_reason) + end + private delegate :answers, to: :@journey_session diff --git a/app/views/further_education_payments/provider/claims/unauthorised.html.erb b/app/views/further_education_payments/provider/claims/unauthorised.html.erb new file mode 100644 index 0000000000..2d213ba10e --- /dev/null +++ b/app/views/further_education_payments/provider/claims/unauthorised.html.erb @@ -0,0 +1,33 @@ +<% content_for( + :page_title, + page_title( + "Unauthorised", + journey: current_journey_routing_name + ) +) %> + +
+
+ <% case params[:failure_reason] %> + <% when "organisation_mismatch" %> +

+ You cannot verify this claim +

+ +

+ The organisation you have used to log in to DfE Sign-in does not match + the organisation in the claim. +

+ +

+ If your DfE Sign-in account is linked to multiple organisations, check + that you have logged in using the correct one. +

+ +

+ Email <%= govuk_mail_to("FE-Levellingup.PremiumPayments@education.gov.uk") %> + if you have logged in with the correct organisation and need support. +

+ <% end %> +
+
diff --git a/spec/factories/journeys/further_education_payments/provider/answers.rb b/spec/factories/journeys/further_education_payments/provider/answers.rb new file mode 100644 index 0000000000..fb74f11da3 --- /dev/null +++ b/spec/factories/journeys/further_education_payments/provider/answers.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory( + :further_education_payments_provider_answers, + class: "Journeys::FurtherEducationPayments::Provider::SessionAnswers" + ) do + end +end diff --git a/spec/factories/policies/further_education_payments/eligibilities.rb b/spec/factories/policies/further_education_payments/eligibilities.rb index 7a6dfee311..c522f5d82e 100644 --- a/spec/factories/policies/further_education_payments/eligibilities.rb +++ b/spec/factories/policies/further_education_payments/eligibilities.rb @@ -1,5 +1,6 @@ FactoryBot.define do factory :further_education_payments_eligibility, class: "Policies::FurtherEducationPayments::Eligibility" do + claim school { create(:school, :further_education) } trait :eligible do diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index 9b1426fc4c..6723aaf1e8 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -5,6 +5,63 @@ create(:journey_configuration, :further_education_payments_provider) end + scenario "provider visits a claim for the wrong organisation" do + fe_provider = create(:school, :further_education, name: "Springfield A&M") + + other_provider = create( + :school, + :further_education, + name: "Springfield Elementary" + ) + + 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) + ) + + create( + :further_education_payments_eligibility, + claim: claim, + school: fe_provider + ) + + mock_dfe_sign_in_auth_session( + provider: :dfe_fe_provider, + auth_hash: { + uid: "11111", + extra: { + raw_info: { + organisation: { + id: "22222", + ukprn: other_provider.ukprn + } + } + } + } + ) + + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + + visit claim_link + + click_on "Start now" + + expect(page).to have_text( + "The organisation you have used to log in to DfE Sign-in does not match the organisation in the claim." + ) + + # Try to visit the restricted slug directly + visit "/further-education-payments-provider/verify-claim" + + expect(page).to have_text( + "The organisation you have used to log in to DfE Sign-in does not match the organisation in the claim." + ) + end + scenario "provider approves the claim" do fe_provider = create(:school, :further_education, name: "Springfield A&M") @@ -17,6 +74,12 @@ created_at: DateTime.new(2024, 8, 1, 9, 0, 0) ) + create( + :further_education_payments_eligibility, + claim: claim, + school: fe_provider + ) + mock_dfe_sign_in_auth_session( provider: :dfe_fe_provider, auth_hash: { diff --git a/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb new file mode 100644 index 0000000000..2f8e93dcaf --- /dev/null +++ b/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb @@ -0,0 +1,40 @@ +require "rails_helper" + +RSpec.describe Journeys::FurtherEducationPayments::Provider::OmniauthCallbackForm do + let(:journey_session) do + create(:further_education_payments_provider_session) + end + + let(:form) do + described_class.new( + journey_session: journey_session, + auth: auth + ) + end + + describe "#save!" do + let(:auth) do + OmniAuth::AuthHash.new( + "uid" => "11111", + "extra" => { + "raw_info" => { + "organisation" => { + "id" => "22222", + "ukprn" => "12345678" + } + } + } + ) + end + + it "updates the session with the auth details from dfe signin" do + expect { form.save! }.to( + change(journey_session.answers, :dfe_sign_in_uid).from(nil).to("11111").and( + change(journey_session.answers, :dfe_sign_in_organisation_ukprn) + .from(nil) + .to("12345678") + ) + ) + end + end +end diff --git a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb new file mode 100644 index 0000000000..a45d41701f --- /dev/null +++ b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb @@ -0,0 +1,34 @@ +require "rails_helper" + +RSpec.describe Journeys::FurtherEducationPayments::Provider::Authorisation do + let(:eligibility) { create(:further_education_payments_eligibility) } + + let(:organisation) { eligibility.school } + + let(:claim) { eligibility.claim } + + let(:journey_session) do + create( + :further_education_payments_provider_session, + answers: answers.merge(claim_id: claim.id) + ) + end + + let(:authorisation) do + described_class.new(answers: journey_session.answers, slug: "verify-claim") + end + + describe "#failure_reason" do + subject { authorisation.failure_reason } + + context "when the ukprns don't match" do + let(:answers) do + { + dfe_sign_in_organisation_ukprn: "mismatch" + } + end + + it { is_expected.to eq(:organisation_mismatch) } + end + end +end From 8a5d671b10c77ed1e9da17805bba5a5829429481 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Mon, 12 Aug 2024 14:36:53 +0100 Subject: [PATCH 04/15] Allow api user to access response If a user doesn't have access to the service the response code will be a 404. `https://github.com/DFE-Digital/login.dfe.public-api?tab=readme-ov-file#get-user-access-to-service` As part of the FE provider work we want to be able to tell if a user has service access, so we need to be able to check the response status rather than throwing an error. Accessing the `role_codes` method without service access will still throw an error to preserve the existing behaviour. --- lib/dfe_sign_in/api/user.rb | 20 ++++++++++--- lib/dfe_sign_in/utils.rb | 14 +++++---- spec/lib/dfe_sign_in/api/user_spec.rb | 42 ++++++++++++++++++++++++++- spec/support/dfe_sign_in_helpers.rb | 6 ++-- 4 files changed, 69 insertions(+), 13 deletions(-) diff --git a/lib/dfe_sign_in/api/user.rb b/lib/dfe_sign_in/api/user.rb index f7464ef05e..9a11ba8043 100644 --- a/lib/dfe_sign_in/api/user.rb +++ b/lib/dfe_sign_in/api/user.rb @@ -58,20 +58,32 @@ def has_role?(role_code) end def role_codes + unless service_access? + raise ExternalServerError, "#{response.code}: #{response.body}" unless response.code.eql?("200") + end + body["roles"].map { |r| r["code"] } end + def service_access? + response.code == "200" + end + + def service_error? + response.code == "500" + end + private def body - @body ||= get(uri) + @body ||= JSON.parse(response.body) end - def uri - @uri ||= begin + def response + @response ||= begin uri = URI(DfeSignIn.configuration.base_url) uri.path = "/services/#{DfeSignIn.configuration.client_id}/organisations/#{organisation_id}/users/#{user_id}" - uri + dfe_sign_in_request(uri) end end end diff --git a/lib/dfe_sign_in/utils.rb b/lib/dfe_sign_in/utils.rb index b2a123a8a0..879e45d391 100644 --- a/lib/dfe_sign_in/utils.rb +++ b/lib/dfe_sign_in/utils.rb @@ -10,17 +10,21 @@ def generate_jwt_token end def get(uri) + response = dfe_sign_in_request(uri) + + raise ExternalServerError, "#{response.code}: #{response.body}" unless response.code.eql?("200") + + JSON.parse(response.body) + end + + def dfe_sign_in_request(uri) request = Net::HTTP::Get.new(uri) request["Authorization"] = "bearer #{generate_jwt_token}" request["Content-Type"] = "application/json" - response = Net::HTTP.start(uri.hostname, uri.port, use_ssl: true) { |http| + Net::HTTP.start(uri.hostname, uri.port, use_ssl: true) { |http| http.request(request) } - - raise ExternalServerError, "#{response.code}: #{response.body}" unless response.code.eql?("200") - - JSON.parse(response.body) end end end diff --git a/spec/lib/dfe_sign_in/api/user_spec.rb b/spec/lib/dfe_sign_in/api/user_spec.rb index 4930089955..3a427bd183 100644 --- a/spec/lib/dfe_sign_in/api/user_spec.rb +++ b/spec/lib/dfe_sign_in/api/user_spec.rb @@ -63,7 +63,7 @@ end end - context "with an invalid response" do + context "with a invalid response" do before do stub_failed_dfe_sign_in_user_info_request(999, 456) end @@ -76,4 +76,44 @@ ) end end + + describe "#service_access?" do + subject { user.service_access? } + + context "when the response is successful" do + before do + stub_dfe_sign_in_user_info_request(999, 456, "my_role") + end + + it { is_expected.to be true } + end + + context "when the response is not successful" do + before do + stub_failed_dfe_sign_in_user_info_request(999, 456, status: 404) + end + + it { is_expected.to be false } + end + end + + describe "#service_error?" do + subject { user.service_error? } + + context "when the response is not a server error" do + before do + stub_dfe_sign_in_user_info_request(999, 456, "my_role") + end + + it { is_expected.to be false } + end + + context "when the response is an error" do + before do + stub_failed_dfe_sign_in_user_info_request(999, 456, status: 500) + end + + it { is_expected.to be true } + end + end end diff --git a/spec/support/dfe_sign_in_helpers.rb b/spec/support/dfe_sign_in_helpers.rb index 205d5a25ca..fda29d71a1 100644 --- a/spec/support/dfe_sign_in_helpers.rb +++ b/spec/support/dfe_sign_in_helpers.rb @@ -73,7 +73,7 @@ def dfe_sign_in_auth_hash(attributes) }.deep_merge(attributes.deep_stringify_keys) end - def stub_dfe_sign_in_user_info_request(user_id, organisation_id, role_code) + def stub_dfe_sign_in_user_info_request(user_id, organisation_id, role_code, service_id: "XXXXXXX") url = dfe_sign_in_user_info_url(user_id, organisation_id) api_response = { "userId" => user_id, @@ -102,14 +102,14 @@ def stub_dfe_sign_in_user_info_request(user_id, organisation_id, role_code) .to_return(status: 200, body: api_response) end - def stub_failed_dfe_sign_in_user_info_request(user_id, organisation_id) + def stub_failed_dfe_sign_in_user_info_request(user_id, organisation_id, status: 500) url = dfe_sign_in_user_info_url(user_id, organisation_id) api_response = { error: "An error occurred" }.to_json stub_request(:get, url) - .to_return(status: 500, body: api_response) + .to_return(status: status, body: api_response) end def stub_dfe_sign_in_user_list_request(number_of_pages: 1, page_number: nil) From d8a502d24bcddddefb3ecc19e8573ada055b4052 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Mon, 12 Aug 2024 16:56:49 +0100 Subject: [PATCH 05/15] Check service access We require the provider to have access to the service in DfE sign in before they can verify claims. If they don't have access to the service we want to show them instructions on how to request service access. We can determine service access by whether or not the request to get their organisation roles for the services is successful or not, https://github.com/DFE-Digital/login.dfe.public-api?tab=readme-ov-file#get-user-access-to-service We've updated the omniauth_callback_form to request their service roles and store whether the request was successful or not (we'll store the role information in a future commit). --- .../provider/omniauth_callback_form.rb | 19 ++++- .../further_education_payments/provider.rb | 8 ++ .../provider/authorisation.rb | 1 + .../provider/session_answers.rb | 6 ++ .../provider/claims/unauthorised.html.erb | 36 +++++++++ .../provider_verifying_claims_spec.rb | 74 +++++++++++++++++++ .../provider/omniauth_callback_form_spec.rb | 54 ++++++++++++-- .../provider/authorisation_spec.rb | 12 +++ 8 files changed, 202 insertions(+), 8 deletions(-) diff --git a/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb b/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb index 654ae02370..860b913249 100644 --- a/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb +++ b/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb @@ -10,7 +10,9 @@ def initialize(journey_session:, auth:) def save! journey_session.answers.assign_attributes( dfe_sign_in_uid: dfe_sign_in_uid, - dfe_sign_in_organisation_ukprn: dfe_sign_in_organisation_ukprn + dfe_sign_in_organisation_id: dfe_sign_in_organisation_id, + dfe_sign_in_organisation_ukprn: dfe_sign_in_organisation_ukprn, + dfe_sign_in_service_access: dfe_sign_in_service_access? ) journey_session.save! @@ -27,6 +29,21 @@ def dfe_sign_in_uid def dfe_sign_in_organisation_ukprn auth.dig("extra", "raw_info", "organisation", "ukprn") end + + def dfe_sign_in_organisation_id + auth.dig("extra", "raw_info", "organisation", "id") + end + + def dfe_sign_in_service_access? + dfe_sign_in_user.service_access? + end + + def dfe_sign_in_user + @dfe_sign_in_user ||= DfeSignIn::Api::User.new( + organisation_id: dfe_sign_in_organisation_id, + user_id: dfe_sign_in_uid + ) + end end end end diff --git a/app/models/journeys/further_education_payments/provider.rb b/app/models/journeys/further_education_payments/provider.rb index d1e09f0e86..da82eec0bc 100644 --- a/app/models/journeys/further_education_payments/provider.rb +++ b/app/models/journeys/further_education_payments/provider.rb @@ -17,6 +17,14 @@ module Provider } CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE = "teacher_payments_claim_verifier" + + def self.request_service_access_url(session) + [ + "https://services.signin.education.gov.uk", + "request-service", DfeSignIn.configuration.client_id, + "users", session.answers.dfe_sign_in_uid + ].join("/") + end end end end diff --git a/app/models/journeys/further_education_payments/provider/authorisation.rb b/app/models/journeys/further_education_payments/provider/authorisation.rb index cdf102ef63..bf992669c6 100644 --- a/app/models/journeys/further_education_payments/provider/authorisation.rb +++ b/app/models/journeys/further_education_payments/provider/authorisation.rb @@ -9,6 +9,7 @@ def initialize(answers:, slug:) def failure_reason return :organisation_mismatch if organisation_mismatch? + return :no_service_access unless answers.dfe_sign_in_service_access? nil end diff --git a/app/models/journeys/further_education_payments/provider/session_answers.rb b/app/models/journeys/further_education_payments/provider/session_answers.rb index 221088bdaf..5a6ae016ad 100644 --- a/app/models/journeys/further_education_payments/provider/session_answers.rb +++ b/app/models/journeys/further_education_payments/provider/session_answers.rb @@ -5,11 +5,17 @@ class SessionAnswers < Journeys::SessionAnswers attribute :claim_id, :string attribute :declaration, :boolean attribute :dfe_sign_in_uid, :string + attribute :dfe_sign_in_organisation_id, :string attribute :dfe_sign_in_organisation_ukprn, :string + attribute :dfe_sign_in_service_access, :boolean, default: false def claim @claim ||= Claim.includes(eligibility: :school).find(claim_id) end + + def dfe_sign_in_service_access? + !!dfe_sign_in_service_access + end end end end diff --git a/app/views/further_education_payments/provider/claims/unauthorised.html.erb b/app/views/further_education_payments/provider/claims/unauthorised.html.erb index 2d213ba10e..41da10cca6 100644 --- a/app/views/further_education_payments/provider/claims/unauthorised.html.erb +++ b/app/views/further_education_payments/provider/claims/unauthorised.html.erb @@ -28,6 +28,42 @@ Email <%= govuk_mail_to("FE-Levellingup.PremiumPayments@education.gov.uk") %> if you have logged in with the correct organisation and need support.

+ <% when "no_service_access" %> +

+ You do not have access to this service +

+ +

+ When a claimant applies for a retention payment, an access link will be + sent to your organisation's nominated email address. +

+ +

+ You can <%= govuk_link_to( + "request access", + Journeys::FurtherEducationPayments::Provider.request_service_access_url(journey_session) + ) %> to verify retention payments for further + education teachers using DfE Sign-in. +

+ +

+ Your request will be sent to all approvers at your organisation. Most + requests will be approved or rejected within 5 days of being raised. +

+ +

+ If you have waited 5 days or longer and you still haven't had a response + to your request: +

+ + <%= govuk_list( + [ + "Log in to DfE Sign-in and go to the 'Organisations' tab.", + "Find the email address of an approver at your organisation.", + "Send a chaser email." + ], + type: :bullet + ) %> <% end %> diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index 6723aaf1e8..562db65f1d 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -5,6 +5,68 @@ create(:journey_configuration, :further_education_payments_provider) end + scenario "provider visits a claim without service access" do + fe_provider = create(:school, :further_education, name: "Springfield A&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) + ) + + create( + :further_education_payments_eligibility, + claim: claim, + school: fe_provider + ) + + mock_dfe_sign_in_auth_session( + provider: :dfe_fe_provider, + auth_hash: { + uid: "11111", + extra: { + raw_info: { + organisation: { + id: "22222", + ukprn: fe_provider.ukprn + } + } + } + } + ) + + # https://github.com/DFE-Digital/login.dfe.public-api?tab=readme-ov-file#get-user-access-to-service + stub_failed_dfe_sign_in_user_info_request( + "11111", + "22222", + status: 404 + ) + + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + + visit claim_link + + click_on "Start now" + + expect(page).to have_text("You do not have access to this service") + + expect(page).to have_text( + "You can request access to verify retention payments for further education teachers using DfE Sign-in." + ) + + # Try to visit the restricted slug directly + visit "/further-education-payments-provider/verify-claim" + + expect(page).to have_text("You do not have access to this service") + + expect(page).to have_text( + "You can request access to verify retention payments for further education teachers using DfE Sign-in." + ) + end + scenario "provider visits a claim for the wrong organisation" do fe_provider = create(:school, :further_education, name: "Springfield A&M") @@ -44,6 +106,12 @@ } ) + stub_dfe_sign_in_user_info_request( + "11111", + "22222", + "some-role" + ) + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) visit claim_link @@ -95,6 +163,12 @@ } ) + stub_dfe_sign_in_user_info_request( + "11111", + "22222", + Journeys::FurtherEducationPayments::Provider::CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE + ) + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) visit claim_link diff --git a/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb index 2f8e93dcaf..e5979809d2 100644 --- a/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb @@ -13,6 +13,14 @@ end describe "#save!" do + before do + allow(DfeSignIn::Api::User).to receive(:new).and_return(dfe_sign_in_user) + end + + let(:dfe_sign_in_user) do + instance_double(DfeSignIn::Api::User, service_access?: service_access) + end + let(:auth) do OmniAuth::AuthHash.new( "uid" => "11111", @@ -27,14 +35,46 @@ ) end - it "updates the session with the auth details from dfe signin" do - expect { form.save! }.to( - change(journey_session.answers, :dfe_sign_in_uid).from(nil).to("11111").and( - change(journey_session.answers, :dfe_sign_in_organisation_ukprn) - .from(nil) - .to("12345678") + context "with access to the service" do + let(:service_access) { true } + + it "updates the session with the auth details from dfe signin" do + expect { form.save! }.to( + change(journey_session.answers, :dfe_sign_in_uid).from(nil).to("11111").and( + change(journey_session.answers, :dfe_sign_in_organisation_ukprn) + .from(nil) + .to("12345678") + ).and( + change(journey_session.answers, :dfe_sign_in_organisation_id) + .from(nil) + .to("22222") + ).and( + change(journey_session.answers, :dfe_sign_in_service_access?) + .from(false) + .to(true) + ) + ) + end + end + + context "without access to the service" do + let(:service_access) { false } + + it "sets the service access flag to false" do + expect { form.save! }.to( + change(journey_session.answers, :dfe_sign_in_uid).from(nil).to("11111").and( + change(journey_session.answers, :dfe_sign_in_organisation_ukprn) + .from(nil) + .to("12345678") + ).and( + change(journey_session.answers, :dfe_sign_in_organisation_id) + .from(nil) + .to("22222") + ).and( + not_change(journey_session.answers, :dfe_sign_in_service_access?) + ) ) - ) + end end end end diff --git a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb index a45d41701f..10e99ef952 100644 --- a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb +++ b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb @@ -24,11 +24,23 @@ context "when the ukprns don't match" do let(:answers) do { + dfe_sign_in_service_access: true, dfe_sign_in_organisation_ukprn: "mismatch" } end it { is_expected.to eq(:organisation_mismatch) } end + + context "when the user does not have access to the service" do + let(:answers) do + { + dfe_sign_in_service_access: false, + dfe_sign_in_organisation_ukprn: organisation.ukprn + } + end + + it { is_expected.to eq(:no_service_access) } + end end end From 18d96e97783c5c5894511ea006fd1d27d9368a8c Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Tue, 13 Aug 2024 14:13:14 +0100 Subject: [PATCH 06/15] Verify correct role to approve claims We only allow provider users with the role `teacher_payments_claim_verifier` to verify claims. The unauthorised view is getting a bit large so will split that into partials in a future commit. --- .../provider/omniauth_callback_form.rb | 9 ++- .../provider/authorisation.rb | 7 +++ .../provider/session_answers.rb | 1 + .../provider/claims/unauthorised.html.erb | 17 ++++++ .../provider_verifying_claims_spec.rb | 61 +++++++++++++++++++ .../provider/omniauth_callback_form_spec.rb | 12 +++- .../provider/authorisation_spec.rb | 12 ++++ 7 files changed, 117 insertions(+), 2 deletions(-) diff --git a/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb b/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb index 860b913249..27034c6699 100644 --- a/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb +++ b/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb @@ -12,7 +12,8 @@ def save! dfe_sign_in_uid: dfe_sign_in_uid, dfe_sign_in_organisation_id: dfe_sign_in_organisation_id, dfe_sign_in_organisation_ukprn: dfe_sign_in_organisation_ukprn, - dfe_sign_in_service_access: dfe_sign_in_service_access? + dfe_sign_in_service_access: dfe_sign_in_service_access?, + dfe_sign_in_role_codes: dfe_sign_in_role_codes ) journey_session.save! @@ -44,6 +45,12 @@ def dfe_sign_in_user user_id: dfe_sign_in_uid ) end + + def dfe_sign_in_role_codes + return [] unless dfe_sign_in_service_access? + + dfe_sign_in_user.role_codes + end end end end diff --git a/app/models/journeys/further_education_payments/provider/authorisation.rb b/app/models/journeys/further_education_payments/provider/authorisation.rb index bf992669c6..128646a77f 100644 --- a/app/models/journeys/further_education_payments/provider/authorisation.rb +++ b/app/models/journeys/further_education_payments/provider/authorisation.rb @@ -10,6 +10,7 @@ def initialize(answers:, slug:) def failure_reason return :organisation_mismatch if organisation_mismatch? return :no_service_access unless answers.dfe_sign_in_service_access? + return :incorrect_role unless role_permits_access? nil end @@ -21,6 +22,12 @@ def failure_reason def organisation_mismatch? answers.claim.school.ukprn != answers.dfe_sign_in_organisation_ukprn end + + def role_permits_access? + answers.dfe_sign_in_role_codes.include?( + CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE + ) + end end end end diff --git a/app/models/journeys/further_education_payments/provider/session_answers.rb b/app/models/journeys/further_education_payments/provider/session_answers.rb index 5a6ae016ad..9349d35285 100644 --- a/app/models/journeys/further_education_payments/provider/session_answers.rb +++ b/app/models/journeys/further_education_payments/provider/session_answers.rb @@ -8,6 +8,7 @@ class SessionAnswers < Journeys::SessionAnswers attribute :dfe_sign_in_organisation_id, :string attribute :dfe_sign_in_organisation_ukprn, :string attribute :dfe_sign_in_service_access, :boolean, default: false + attribute :dfe_sign_in_role_codes, default: [] def claim @claim ||= Claim.includes(eligibility: :school).find(claim_id) diff --git a/app/views/further_education_payments/provider/claims/unauthorised.html.erb b/app/views/further_education_payments/provider/claims/unauthorised.html.erb index 41da10cca6..3cce41cc91 100644 --- a/app/views/further_education_payments/provider/claims/unauthorised.html.erb +++ b/app/views/further_education_payments/provider/claims/unauthorised.html.erb @@ -64,6 +64,23 @@ ], type: :bullet ) %> + + <% when "incorrect_role" %> +

+ You do not have access to verify claims for this organisation +

+ +

+ If you think you should have access, you need to: +

+ + <%= govuk_list( + [ + "check you have logged in to DfE Sign-in using the correct organisation", + "contact an approver at your organisation to confirm your access rights" + ], + type: :bullet + ) %> <% end %> diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index 562db65f1d..17d4504d0f 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -130,6 +130,67 @@ ) end + scenario "provider visits claim with the wrong role" do + fe_provider = create(:school, :further_education, name: "Springfield A&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) + ) + + create( + :further_education_payments_eligibility, + claim: claim, + school: fe_provider + ) + + mock_dfe_sign_in_auth_session( + provider: :dfe_fe_provider, + auth_hash: { + uid: "11111", + extra: { + raw_info: { + organisation: { + id: "22222", + ukprn: fe_provider.ukprn + } + } + } + } + ) + + stub_dfe_sign_in_user_info_request( + "11111", + "22222", + "some-other-role" + ) + + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + + visit claim_link + + click_on "Start now" + + expect(page).to have_text( + "You do not have access to verify claims for this organisation" + ) + + expect(page).to have_text( + "contact an approver at your organisation to confirm your access rights" + ) + + # Try to visit the restricted slug directly + visit "/further-education-payments-provider/verify-claim" + + expect(page).to have_text( + "You do not have access to verify claims for this organisation" + ) + end + scenario "provider approves the claim" do fe_provider = create(:school, :further_education, name: "Springfield A&M") diff --git a/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb index e5979809d2..57b3faf623 100644 --- a/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb @@ -18,7 +18,11 @@ end let(:dfe_sign_in_user) do - instance_double(DfeSignIn::Api::User, service_access?: service_access) + instance_double( + DfeSignIn::Api::User, + service_access?: service_access, + role_codes: ["teacher_payments_claim_verifier"] + ) end let(:auth) do @@ -52,6 +56,10 @@ change(journey_session.answers, :dfe_sign_in_service_access?) .from(false) .to(true) + ).and( + change(journey_session.answers, :dfe_sign_in_role_codes) + .from([]) + .to(["teacher_payments_claim_verifier"]) ) ) end @@ -72,6 +80,8 @@ .to("22222") ).and( not_change(journey_session.answers, :dfe_sign_in_service_access?) + ).and( + not_change(journey_session.answers, :dfe_sign_in_role_codes) ) ) end diff --git a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb index 10e99ef952..a0b97e7e60 100644 --- a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb +++ b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb @@ -42,5 +42,17 @@ it { is_expected.to eq(:no_service_access) } end + + context "when the user does not have the required role" do + let(:answers) do + { + dfe_sign_in_service_access: true, + dfe_sign_in_organisation_ukprn: organisation.ukprn, + dfe_sign_in_role_codes: ["incorrect_role"] + } + end + + it { is_expected.to eq(:incorrect_role) } + end end end From 7bfb5e8fb6539b2c2fbc54a24096a837d8d4db9e Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Tue, 13 Aug 2024 14:29:54 +0100 Subject: [PATCH 07/15] Extract partials for auth failure reasons Just splitting out some partials to make the view a bit easier to work with --- .../_unauthorised_incorrect_role.html.erb | 15 ++++ .../_unauthorised_no_service_access.html.erb | 35 +++++++++ ...nauthorised_organisation_mismatch.html.erb | 18 +++++ .../provider/claims/unauthorised.html.erb | 72 +------------------ 4 files changed, 71 insertions(+), 69 deletions(-) create mode 100644 app/views/further_education_payments/provider/claims/_unauthorised_incorrect_role.html.erb create mode 100644 app/views/further_education_payments/provider/claims/_unauthorised_no_service_access.html.erb create mode 100644 app/views/further_education_payments/provider/claims/_unauthorised_organisation_mismatch.html.erb diff --git a/app/views/further_education_payments/provider/claims/_unauthorised_incorrect_role.html.erb b/app/views/further_education_payments/provider/claims/_unauthorised_incorrect_role.html.erb new file mode 100644 index 0000000000..b81de7154f --- /dev/null +++ b/app/views/further_education_payments/provider/claims/_unauthorised_incorrect_role.html.erb @@ -0,0 +1,15 @@ +

+ You do not have access to verify claims for this organisation +

+ +

+ If you think you should have access, you need to: +

+ +<%= govuk_list( + [ + "check you have logged in to DfE Sign-in using the correct organisation", + "contact an approver at your organisation to confirm your access rights" + ], + type: :bullet +) %> diff --git a/app/views/further_education_payments/provider/claims/_unauthorised_no_service_access.html.erb b/app/views/further_education_payments/provider/claims/_unauthorised_no_service_access.html.erb new file mode 100644 index 0000000000..28942b8179 --- /dev/null +++ b/app/views/further_education_payments/provider/claims/_unauthorised_no_service_access.html.erb @@ -0,0 +1,35 @@ +

+ You do not have access to this service +

+ +

+ When a claimant applies for a retention payment, an access link will be + sent to your organisation's nominated email address. +

+ +

+You can <%= govuk_link_to( + "request access", + Journeys::FurtherEducationPayments::Provider.request_service_access_url(journey_session) +) %> to verify retention payments for further + education teachers using DfE Sign-in. +

+ +

+ Your request will be sent to all approvers at your organisation. Most + requests will be approved or rejected within 5 days of being raised. +

+ +

+ If you have waited 5 days or longer and you still haven't had a response + to your request: +

+ +<%= govuk_list( + [ + "Log in to DfE Sign-in and go to the 'Organisations' tab.", + "Find the email address of an approver at your organisation.", + "Send a chaser email." + ], + type: :bullet +) %> diff --git a/app/views/further_education_payments/provider/claims/_unauthorised_organisation_mismatch.html.erb b/app/views/further_education_payments/provider/claims/_unauthorised_organisation_mismatch.html.erb new file mode 100644 index 0000000000..5779c1faa0 --- /dev/null +++ b/app/views/further_education_payments/provider/claims/_unauthorised_organisation_mismatch.html.erb @@ -0,0 +1,18 @@ +

+ You cannot verify this claim +

+ +

+ The organisation you have used to log in to DfE Sign-in does not match + the organisation in the claim. +

+ +

+ If your DfE Sign-in account is linked to multiple organisations, check + that you have logged in using the correct one. +

+ +

+ Email <%= govuk_mail_to("FE-Levellingup.PremiumPayments@education.gov.uk") %> + if you have logged in with the correct organisation and need support. +

diff --git a/app/views/further_education_payments/provider/claims/unauthorised.html.erb b/app/views/further_education_payments/provider/claims/unauthorised.html.erb index 3cce41cc91..0623cf4d5f 100644 --- a/app/views/further_education_payments/provider/claims/unauthorised.html.erb +++ b/app/views/further_education_payments/provider/claims/unauthorised.html.erb @@ -10,77 +10,11 @@
<% case params[:failure_reason] %> <% when "organisation_mismatch" %> -

- You cannot verify this claim -

- -

- The organisation you have used to log in to DfE Sign-in does not match - the organisation in the claim. -

- -

- If your DfE Sign-in account is linked to multiple organisations, check - that you have logged in using the correct one. -

- -

- Email <%= govuk_mail_to("FE-Levellingup.PremiumPayments@education.gov.uk") %> - if you have logged in with the correct organisation and need support. -

+ <%= render "unauthorised_organisation_mismatch" %> <% when "no_service_access" %> -

- You do not have access to this service -

- -

- When a claimant applies for a retention payment, an access link will be - sent to your organisation's nominated email address. -

- -

- You can <%= govuk_link_to( - "request access", - Journeys::FurtherEducationPayments::Provider.request_service_access_url(journey_session) - ) %> to verify retention payments for further - education teachers using DfE Sign-in. -

- -

- Your request will be sent to all approvers at your organisation. Most - requests will be approved or rejected within 5 days of being raised. -

- -

- If you have waited 5 days or longer and you still haven't had a response - to your request: -

- - <%= govuk_list( - [ - "Log in to DfE Sign-in and go to the 'Organisations' tab.", - "Find the email address of an approver at your organisation.", - "Send a chaser email." - ], - type: :bullet - ) %> - + <%= render "unauthorised_no_service_access" %> <% when "incorrect_role" %> -

- You do not have access to verify claims for this organisation -

- -

- If you think you should have access, you need to: -

- - <%= govuk_list( - [ - "check you have logged in to DfE Sign-in using the correct organisation", - "contact an approver at your organisation to confirm your access rights" - ], - type: :bullet - ) %> + <%= render "unauthorised_incorrect_role" %> <% end %>
From 4e7c173e9d056c3ab02eb293cdbfe5ef27a26c67 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Wed, 14 Aug 2024 15:35:37 +0100 Subject: [PATCH 08/15] Restrict admins from verifying claims A business rule is to not allow claim admins to verify claims. We need to check for the admin role codes before checking for the verify claim role code to handle the case where an admin without the verify claim role tries to verify a claim, otherwise they'd see the general "you don't have access" unauthorised page. --- .../provider/authorisation.rb | 15 +++++ .../claims/_unauthorised_claim_admin.html.erb | 22 +++++++ .../provider/claims/unauthorised.html.erb | 2 + .../provider_verifying_claims_spec.rb | 63 +++++++++++++++++++ .../provider/authorisation_spec.rb | 15 +++++ spec/support/dfe_sign_in_helpers.rb | 8 +-- 6 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 app/views/further_education_payments/provider/claims/_unauthorised_claim_admin.html.erb diff --git a/app/models/journeys/further_education_payments/provider/authorisation.rb b/app/models/journeys/further_education_payments/provider/authorisation.rb index 128646a77f..463abb7e39 100644 --- a/app/models/journeys/further_education_payments/provider/authorisation.rb +++ b/app/models/journeys/further_education_payments/provider/authorisation.rb @@ -10,6 +10,7 @@ def initialize(answers:, slug:) def failure_reason return :organisation_mismatch if organisation_mismatch? return :no_service_access unless answers.dfe_sign_in_service_access? + return :claim_admin if claim_admin? return :incorrect_role unless role_permits_access? nil @@ -28,6 +29,20 @@ def role_permits_access? CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE ) end + + def claim_admin? + claim_admin_roles.any? do |role_code| + answers.dfe_sign_in_role_codes.include?(role_code) + end + end + + def claim_admin_roles + [ + DfeSignIn::User::SERVICE_OPERATOR_DFE_SIGN_IN_ROLE_CODE, + DfeSignIn::User::SUPPORT_AGENT_DFE_SIGN_IN_ROLE_CODE, + DfeSignIn::User::PAYROLL_OPERATOR_DFE_SIGN_IN_ROLE_CODE + ] + end end end end diff --git a/app/views/further_education_payments/provider/claims/_unauthorised_claim_admin.html.erb b/app/views/further_education_payments/provider/claims/_unauthorised_claim_admin.html.erb new file mode 100644 index 0000000000..8308dce1c4 --- /dev/null +++ b/app/views/further_education_payments/provider/claims/_unauthorised_claim_admin.html.erb @@ -0,0 +1,22 @@ +

+ You do not have access to verify claims for this organisation +

+ +

+ DfE staff do not have access to verify retention payments for further + education teachers. +

+ +

+ If you need to view a claim: +

+ +<%= govuk_list( + [ + "Log in to DfE Sign-in using the ‘Manage Teacher Payments’ organisation.", + "Go to the ‘Search’ tab.", + "Search for the claim using the claim reference number." + ], + type: :number +) %> + diff --git a/app/views/further_education_payments/provider/claims/unauthorised.html.erb b/app/views/further_education_payments/provider/claims/unauthorised.html.erb index 0623cf4d5f..7b5927bbd2 100644 --- a/app/views/further_education_payments/provider/claims/unauthorised.html.erb +++ b/app/views/further_education_payments/provider/claims/unauthorised.html.erb @@ -15,6 +15,8 @@ <%= render "unauthorised_no_service_access" %> <% when "incorrect_role" %> <%= render "unauthorised_incorrect_role" %> + <% when "claim_admin" %> + <%= render "unauthorised_claim_admin" %> <% end %> diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index 17d4504d0f..1d063220c1 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -191,6 +191,69 @@ ) end + scenario "admin visits the claim" do + fe_provider = create(:school, :further_education, name: "Springfield A&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) + ) + + create( + :further_education_payments_eligibility, + claim: claim, + school: fe_provider + ) + + mock_dfe_sign_in_auth_session( + provider: :dfe_fe_provider, + auth_hash: { + uid: "11111", + extra: { + raw_info: { + organisation: { + id: "22222", + ukprn: fe_provider.ukprn + } + } + } + } + ) + + stub_dfe_sign_in_user_info_request( + "11111", + "22222", + [ + DfeSignIn::User::SERVICE_OPERATOR_DFE_SIGN_IN_ROLE_CODE + ] + ) + + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + + visit claim_link + + click_on "Start now" + + expect(page).to have_text( + "You do not have access to verify claims for this organisation" + ) + + expect(page).to have_text( + "DfE staff do not have access to verify retention payments for further education teachers." + ) + + # Try to visit the restricted slug directly + visit "/further-education-payments-provider/verify-claim" + + expect(page).to have_text( + "You do not have access to verify claims for this organisation" + ) + end + scenario "provider approves the claim" do fe_provider = create(:school, :further_education, name: "Springfield A&M") diff --git a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb index a0b97e7e60..002fb2b98d 100644 --- a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb +++ b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb @@ -54,5 +54,20 @@ it { is_expected.to eq(:incorrect_role) } end + + context "when the user is a claim admin" do + let(:answers) do + { + dfe_sign_in_service_access: true, + dfe_sign_in_organisation_ukprn: organisation.ukprn, + dfe_sign_in_role_codes: [ + DfeSignIn::User::SERVICE_OPERATOR_DFE_SIGN_IN_ROLE_CODE, + Journeys::FurtherEducationPayments::Provider::CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE + ] + } + end + + it { is_expected.to eq(:claim_admin) } + end end end diff --git a/spec/support/dfe_sign_in_helpers.rb b/spec/support/dfe_sign_in_helpers.rb index fda29d71a1..7d61c15b44 100644 --- a/spec/support/dfe_sign_in_helpers.rb +++ b/spec/support/dfe_sign_in_helpers.rb @@ -79,17 +79,17 @@ def stub_dfe_sign_in_user_info_request(user_id, organisation_id, role_code, serv "userId" => user_id, "serviceId" => "XXXXXXX", "organisationId" => organisation_id, - "roles" => [ + "roles" => Array.wrap(role_code).map.each_with_index do |code, i| { "id" => "YYYYYYY", "name" => "Access to Teacher Payments", - "code" => role_code, + "code" => code, "numericId" => "162", "status" => { - "id" => 1 + "id" => i + 1 } } - ], + end, "identifiers" => [ { "key" => "groups", From f430567549ba565e1040268b3ae3d1ccb4bca305 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Thu, 15 Aug 2024 17:34:11 +0100 Subject: [PATCH 09/15] Add fixed contract assertions to verification form When displaying the claim verification form to the verifier we need to ask them yes/no questions about the claimant's eligibility answers. The questions we ask depend on the eligibility answers. We model the questions we need to ask as "AssertionForm" instances and, in a future commit, choose the list of assertions the verifier needs to make based on the answers given by the claimant. This commit doesn't address how we're going to store the verifiers answers, that is a decision for later! When displaying the `subjects_taught` question we need to show the list of courses the claimant has indicated that they teach. This list of courses is stored in a json array on the eligibility model. To make the list of courses easier to work with, we've introduced a `Course` object containing the course and subject strings and handling generating the description of the course, which may include a link. There's also some stubbed out methods on the eligibility, these will be removed once the eligibility model piece has been completed and we rebase this commit. --- .../provider/verify_claim_form.rb | 74 +++++++++++++++++++ .../further_education_payments/eligibility.rb | 31 ++++++++ .../provider/claims/verify_claim.html.erb | 37 +++++++++- config/locales/en.yml | 8 ++ .../provider_verifying_claims_spec.rb | 65 +++++++++++++++- .../provider/verify_claim_form_spec.rb | 18 ++++- 6 files changed, 227 insertions(+), 6 deletions(-) diff --git a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb index a37ac25749..f5d7202101 100644 --- a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb +++ b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb @@ -2,10 +2,25 @@ module Journeys module FurtherEducationPayments module Provider class VerifyClaimForm < Form + include CoursesHelper + + ASSERTIONS = %i[ + contract_type + teaching_responsibilities + further_education_teaching_start_year + teaching_hours_per_week + hours_teaching_eligible_subjects + subjects_taught + ] + + attribute :assertions_attributes + attribute :declaration, :boolean validates :declaration, acceptance: true + validate :all_assertions_answered + # validate claim not already verified delegate :claim, to: :answers @@ -30,11 +45,70 @@ def claim_date claim.created_at.to_date.strftime("%-d %B %Y") end + def course_descriptions + @course_descriptions ||= claim.eligibility.courses_taught.map(&:description) + end + + def assertions + @assertions ||= ASSERTIONS.map do |assertion_name| + AssertionForm.new(name: assertion_name) + end + end + + def assertions_attributes=(params) + (params || {}).each do |_, assertion_params| + assertions + .detect { |a| a.name == assertion_params[:name] } + &.assign_attributes(assertion_params) + end + end + def save return false unless valid? true end + + private + + def permitted_attributes + super + [assertions_attributes: AssertionForm.attribute_names] + end + + # Make sure the errors in the summary link to the correct nested field + def all_assertions_answered + assertions.each(&:validate).each_with_index do |assertion, i| + assertion.errors.each do |error| + errors.add( + "assertions_attributes[#{i}][#{error.attribute}]", + error.full_message + ) + end + end + end + + class AssertionForm + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :name, :string + attribute :outcome, :boolean + + validates :name, presence: true + validates :outcome, inclusion: { + in: [true, false], + message: "Select an option" + } + + def radio_options + [ + RadioOption.new(id: true, name: "Yes"), + RadioOption.new(id: false, name: "No") + ] + end + + class RadioOption < Struct.new(:id, :name, keyword_init: true); end + end end end end diff --git a/app/models/policies/further_education_payments/eligibility.rb b/app/models/policies/further_education_payments/eligibility.rb index d3aa06bf1f..c4de3cc1d8 100644 --- a/app/models/policies/further_education_payments/eligibility.rb +++ b/app/models/policies/further_education_payments/eligibility.rb @@ -3,6 +3,21 @@ module FurtherEducationPayments class Eligibility < ApplicationRecord self.table_name = "further_education_payments_eligibilities" + class Course < Struct.new(:subject, :name, keyword_init: true) + include Journeys::FurtherEducationPayments::CoursesHelper + + def taught? + name != "none" + end + + def description + I18n.t( + "further_education_payments.forms.#{subject}_courses.options.#{name}", + link: link_for_course("#{subject}_courses", name) + ) + end + end + has_one :claim, as: :eligibility, inverse_of: :eligibility belongs_to :possible_school, optional: true, class_name: "School" @@ -18,6 +33,22 @@ def policy def ineligible? false end + + def courses_taught + courses.select(&:taught?) + end + + def courses + subjects_taught.map do |subject| + public_send(:"#{subject}_courses").map do |course| + Course.new(subject: subject, name: course) + end + end.flatten + end + + def fixed_contract? + contract_type != "variable_hours" + end end end end diff --git a/app/views/further_education_payments/provider/claims/verify_claim.html.erb b/app/views/further_education_payments/provider/claims/verify_claim.html.erb index 3b7cf303ff..921fcd40c8 100644 --- a/app/views/further_education_payments/provider/claims/verify_claim.html.erb +++ b/app/views/further_education_payments/provider/claims/verify_claim.html.erb @@ -38,18 +38,51 @@ <% end %> <% end %> +

+ We need you to verify some information +

+ <%= form_with( model: @form, url: claim_path(current_journey_routing_name), builder: GOVUKDesignSystemFormBuilder::FormBuilder, html: { novalidate: false } ) do |f| %> + <%= f.govuk_error_summary %> + + <%= f.fields_for :assertions do |ff| %> + <%= ff.govuk_collection_radio_buttons( + :outcome, + ff.object.radio_options, + :id, + :name, + nil, + legend: { + size: "s", + text: f.object.t( + [:assertions, ff.object.name], + claimant: f.object.claimant_name, + provider: f.object.claim.school.name, + ), + }, + hint: -> do + if ff.object.name.to_s == "subjects_taught" + govuk_list( + f.object.course_descriptions.map(&:html_safe), + type: :bullet + ) + end + end + ) %> + + <%= ff.hidden_field :name %> + <% end %> <%= f.govuk_check_boxes_fieldset( :declaration, multiple: false, legend: { - text: @form.t(:declaration_title), + text: f.object.t(:declaration_title), size: "m" } ) do %> @@ -60,7 +93,7 @@ multiple: false, link_errors: true, label: { - text: @form.t(:declaration) + text: f.object.t(:declaration) } ) %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 3ef21316d7..7ead5b0bca 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1108,6 +1108,14 @@ en: claimant_date_of_birth: "Claimant date of birth" claimant_trn: "Claimant teacher reference number (TRN)" claim_date: "Claim date" + assertions: + contract_type: "Does %{claimant} have a permanent contract of employment at %{provider}?" + teaching_responsibilities: "Is %{claimant} a member of staff with teaching responsibilities?" + further_education_teaching_start_year: "Is %{claimant} in the first 5 years of their further education teaching career in England?" + teaching_hours_per_week: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" + hours_teaching_eligible_subjects: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" + subjects_taught: "For at least half of their timetabled teaching hours, does %{claimant} teach:" + early_years_payment: claim_description: for an early years financial incentive payment early_years_payment_provider_start: diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index 1d063220c1..38a99993fc 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -254,8 +254,8 @@ ) end - scenario "provider approves the claim" do - fe_provider = create(:school, :further_education, name: "Springfield A&M") + scenario "provider approves a fixed contract claim" do + fe_provider = create(:school, :further_education, name: "Springfield A and M") claim = create( :claim, @@ -269,7 +269,13 @@ create( :further_education_payments_eligibility, claim: claim, - school: fe_provider + school: fe_provider, + contract_type: "fixed_term", + subjects_taught: ["engineering_manufacturing"], + engineering_manufacturing_courses: [ + "approved_level_321_transportation", + "level2_3_apprenticeship" + ] ) mock_dfe_sign_in_auth_session( @@ -309,6 +315,59 @@ # expect(page).to have_text "Claimant teacher reference number (TRN)1234567" expect(page).to have_text "Claim date1 August 2024" + within_fieldset( + "Does Edna Krabappel have a permanent contract of employment at " \ + "Springfield A and M?" + ) do + choose "Yes" + end + + within_fieldset( + "Is Edna Krabappel a member of staff with teaching responsibilities?" + ) do + choose "Yes" + end + + within_fieldset( + "Is Edna Krabappel in the first 5 years of their further education " \ + "teaching career in England?" + ) do + choose "Yes" + end + + within_fieldset( + "Is Edna Krabappel timetabled to teach an average of 12 hours per " \ + "week during the current term?" + ) do + choose "Yes" + end + + within_fieldset( + "For at least half of their timetabled teaching hours, does " \ + "Edna Krabappel teach 16- to 19-year-olds, including those up to " \ + "age 25 with an Education, Health and Care Plan (EHCP)?" + ) do + choose "Yes" + end + + expect(page).to have_text( + "Qualifications approved for funding at level 3 and below in the " \ + "transportation operations and maintenance (opens in new tab) sector " \ + "subject area" + ) + + expect(page).to have_text( + "Level 2 or level 3 apprenticeships in the engineering and " \ + "manufacturing occupational route (opens in new tab)" + ) + + within_fieldset( + "For at least half of their timetabled teaching hours, does " \ + "Edna Krabappel teach:" + ) do + choose "Yes" + end + check "To the best of my knowledge, I confirm that the information provided in this form is correct." click_on "Submit" diff --git a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb index b5150dfc85..21060d1a9a 100644 --- a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb @@ -3,8 +3,16 @@ RSpec.describe Journeys::FurtherEducationPayments::Provider::VerifyClaimForm, type: :model do let(:journey) { Journeys::FurtherEducationPayments::Provider } + let(:claim) { create(:further_education_payments_eligibility).claim } + let(:journey_session) do - create(:further_education_payments_provider_session) + create( + :further_education_payments_provider_session, + answers: { + claim_id: claim.id, + dfe_sign_in_uid: "123" + } + ) end let(:params) { ActionController::Parameters.new } @@ -23,5 +31,13 @@ it do is_expected.to validate_acceptance_of(:declaration) end + + it "validates all assertions are answered" do + form.validate + + form.assertions.each do |assertion| + expect(assertion.errors[:outcome]).to eq(["Select an option"]) + end + end end end From 67bc95588d522292e58d5c57c66bce731f17d215 Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Fri, 16 Aug 2024 15:55:20 +0100 Subject: [PATCH 10/15] Branch assertions on contract type Depending on the type of contract the claimant has we need to show different assertions to the verifier. This commit adds the mechanisim for doing so. --- .../provider/verify_claim_form.rb | 28 +++++++++++++------ .../provider/claims/verify_claim.html.erb | 2 +- config/locales/en.yml | 13 +++++---- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb index f5d7202101..5703ee63a9 100644 --- a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb +++ b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb @@ -4,14 +4,16 @@ module Provider class VerifyClaimForm < Form include CoursesHelper - ASSERTIONS = %i[ - contract_type - teaching_responsibilities - further_education_teaching_start_year - teaching_hours_per_week - hours_teaching_eligible_subjects - subjects_taught - ] + ASSERTIONS = { + fixed_contract: %i[ + contract_type + teaching_responsibilities + further_education_teaching_start_year + teaching_hours_per_week + hours_teaching_eligible_subjects + subjects_taught + ] + } attribute :assertions_attributes @@ -50,7 +52,7 @@ def course_descriptions end def assertions - @assertions ||= ASSERTIONS.map do |assertion_name| + @assertions ||= ASSERTIONS.fetch(contract_type).map do |assertion_name| AssertionForm.new(name: assertion_name) end end @@ -69,6 +71,14 @@ def save true end + def contract_type + if claim.eligibility.fixed_contract? + :fixed_contract + else + :variable_contract + end + end + private def permitted_attributes diff --git a/app/views/further_education_payments/provider/claims/verify_claim.html.erb b/app/views/further_education_payments/provider/claims/verify_claim.html.erb index 921fcd40c8..0902792c1d 100644 --- a/app/views/further_education_payments/provider/claims/verify_claim.html.erb +++ b/app/views/further_education_payments/provider/claims/verify_claim.html.erb @@ -60,7 +60,7 @@ legend: { size: "s", text: f.object.t( - [:assertions, ff.object.name], + [:assertions, f.object.contract_type, ff.object.name], claimant: f.object.claimant_name, provider: f.object.claim.school.name, ), diff --git a/config/locales/en.yml b/config/locales/en.yml index 7ead5b0bca..30a9695b5a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1109,12 +1109,13 @@ en: claimant_trn: "Claimant teacher reference number (TRN)" claim_date: "Claim date" assertions: - contract_type: "Does %{claimant} have a permanent contract of employment at %{provider}?" - teaching_responsibilities: "Is %{claimant} a member of staff with teaching responsibilities?" - further_education_teaching_start_year: "Is %{claimant} in the first 5 years of their further education teaching career in England?" - teaching_hours_per_week: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" - hours_teaching_eligible_subjects: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" - subjects_taught: "For at least half of their timetabled teaching hours, does %{claimant} teach:" + fixed_contract: + contract_type: "Does %{claimant} have a permanent contract of employment at %{provider}?" + teaching_responsibilities: "Is %{claimant} a member of staff with teaching responsibilities?" + further_education_teaching_start_year: "Is %{claimant} in the first 5 years of their further education teaching career in England?" + teaching_hours_per_week: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" + hours_teaching_eligible_subjects: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" + subjects_taught: "For at least half of their timetabled teaching hours, does %{claimant} teach:" early_years_payment: claim_description: for an early years financial incentive payment From 33579c40ce50ec96e04f6a06bb88aa71de7e0b2d Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Sat, 17 Aug 2024 15:47:47 +0100 Subject: [PATCH 11/15] Add variable contract assertions When the claimant has a variable contract we need to show a different set of assertions to the verifier --- .../provider/verify_claim_form.rb | 10 ++ config/locales/en.yml | 9 ++ .../provider_verifying_claims_spec.rb | 139 ++++++++++++++++++ .../provider/verify_claim_form_spec.rb | 54 ++++++- 4 files changed, 211 insertions(+), 1 deletion(-) diff --git a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb index 5703ee63a9..6f7e84614f 100644 --- a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb +++ b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb @@ -12,6 +12,16 @@ class VerifyClaimForm < Form teaching_hours_per_week hours_teaching_eligible_subjects subjects_taught + ], + variable_contract: %i[ + contract_type + teaching_responsibilities + further_education_teaching_start_year + taught_at_least_one_term + teaching_hours_per_week + hours_teaching_eligible_subjects + subjects_taught + teaching_hours_per_week_next_term ] } diff --git a/config/locales/en.yml b/config/locales/en.yml index 30a9695b5a..46dd8210a0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1116,6 +1116,15 @@ en: teaching_hours_per_week: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" hours_teaching_eligible_subjects: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" subjects_taught: "For at least half of their timetabled teaching hours, does %{claimant} teach:" + variable_contract: + contract_type: "Does %{claimant} have a variable hour contract of employment at %{provider}?" + teaching_responsibilities: "Is %{claimant} a member of staff with teaching responsibilities?" + further_education_teaching_start_year: "Is %{claimant} in the first 5 years of their further education teaching career in England?" + taught_at_least_one_term: "Has %{claimant} taught for at least one academic term at %{provider}?" + teaching_hours_per_week: "Is %{claimant} timetabled to teach an average of 12 hours per week during the current term?" + hours_teaching_eligible_subjects: "For at least half of their timetabled teaching hours, does %{claimant} teach 16- to 19-year-olds, including those up to age 25 with an Education, Health and Care Plan (EHCP)?" + subjects_taught: "For at least half of their timetabled teaching hours, does %{claimant} teach:" + teaching_hours_per_week_next_term: "Will %{claimant} be timetabled to teach at least 2.5 hours per week next term?" early_years_payment: claim_description: for an early years financial incentive payment diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index 38a99993fc..fbaf20f3dd 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -376,6 +376,145 @@ expect(page).to have_text "Claim reference number AB123456" end + scenario "provider approves a variable contract claim" do + fe_provider = create(:school, :further_education, 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) + ) + + create( + :further_education_payments_eligibility, + claim: claim, + school: fe_provider, + contract_type: "variable_hours", + subjects_taught: ["engineering_manufacturing"], + engineering_manufacturing_courses: [ + "approved_level_321_transportation", + "level2_3_apprenticeship" + ] + ) + + mock_dfe_sign_in_auth_session( + provider: :dfe_fe_provider, + auth_hash: { + uid: "11111", + extra: { + raw_info: { + organisation: { + id: "22222", + ukprn: fe_provider.ukprn + } + } + } + } + ) + + stub_dfe_sign_in_user_info_request( + "11111", + "22222", + Journeys::FurtherEducationPayments::Provider::CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE + ) + + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + + visit claim_link + + click_on "Start now" + + expect(page).to have_text "Review a financial incentive payment claim" + # The text generated by the dl tag doesn not include a space between the + # label and value (displays as expected in browser). + expect(page).to have_text "Claim referenceAB123456" + expect(page).to have_text "Claimant nameEdna Krabappel" + expect(page).to have_text "Claimant date of birth3 July 1945" + # FIXME RL enable this test once we've added the TRN to the eligibility + # expect(page).to have_text "Claimant teacher reference number (TRN)1234567" + expect(page).to have_text "Claim date1 August 2024" + + within_fieldset( + "Does Edna Krabappel have a variable hour contract of employment at " \ + "Springfield A and M?" + ) do + choose "Yes" + end + + within_fieldset( + "Is Edna Krabappel a member of staff with teaching responsibilities?" + ) do + choose "Yes" + end + + within_fieldset( + "Is Edna Krabappel in the first 5 years of their further education " \ + "teaching career in England?" + ) do + choose "Yes" + end + + within_fieldset( + "Has Edna Krabappel taught for at least one academic term at " \ + "Springfield A and M?" + ) do + choose "Yes" + end + + within_fieldset( + "Is Edna Krabappel timetabled to teach an average of 12 hours per " \ + "week during the current term?" + ) do + choose "Yes" + end + + within_fieldset( + "For at least half of their timetabled teaching hours, does " \ + "Edna Krabappel teach 16- to 19-year-olds, including those up to " \ + "age 25 with an Education, Health and Care Plan (EHCP)?" + ) do + choose "Yes" + end + + expect(page).to have_text( + "Qualifications approved for funding at level 3 and below in the " \ + "transportation operations and maintenance (opens in new tab) sector " \ + "subject area" + ) + + expect(page).to have_text( + "Level 2 or level 3 apprenticeships in the engineering and " \ + "manufacturing occupational route (opens in new tab)" + ) + + within_fieldset( + "For at least half of their timetabled teaching hours, does " \ + "Edna Krabappel teach:" + ) do + choose "Yes" + end + + within_fieldset( + "Will Edna Krabappel be timetabled to teach at least 2.5 hours per " \ + "week next term?" + ) do + choose "Yes" + end + + check( + "To the best of my knowledge, I confirm that the information " \ + "provided in this form is correct." + ) + + click_on "Submit" + + expect(page).to have_content "Verification complete" + expect(page).to have_text "Claim reference number AB123456" + end + scenario "provider visits the landing page" do visit( Journeys::FurtherEducationPayments::Provider::SlugSequence.start_page_url diff --git a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb index 21060d1a9a..ca36498dc5 100644 --- a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb @@ -3,7 +3,9 @@ RSpec.describe Journeys::FurtherEducationPayments::Provider::VerifyClaimForm, type: :model do let(:journey) { Journeys::FurtherEducationPayments::Provider } - let(:claim) { create(:further_education_payments_eligibility).claim } + let(:eligibility) { create(:further_education_payments_eligibility) } + + let(:claim) { eligibility.claim } let(:journey_session) do create( @@ -40,4 +42,54 @@ end end end + + describe "#assertions" do + subject { form.assertions.map(&:name) } + + context "with a fixed term contract" do + let(:eligibility) do + create( + :further_education_payments_eligibility, + contract_type: "permanent" + ) + end + + it do + is_expected.to eq( + %w[ + contract_type + teaching_responsibilities + further_education_teaching_start_year + teaching_hours_per_week + hours_teaching_eligible_subjects + subjects_taught + ] + ) + end + end + + context "with a variable hours contract" do + let(:eligibility) do + create( + :further_education_payments_eligibility, + contract_type: "variable_hours" + ) + end + + it do + is_expected.to eq( + %w[ + contract_type + teaching_responsibilities + further_education_teaching_start_year + taught_at_least_one_term + teaching_hours_per_week + hours_teaching_eligible_subjects + subjects_taught + teaching_hours_per_week_next_term + ] + ) + end + end + end end From 1651eff23ce66d835c73dabc70c878c1890b391b Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Mon, 19 Aug 2024 15:11:18 +0100 Subject: [PATCH 12/15] Capture name and email from DSI We'll need to present who verified the claim in the admin view so we'll want to capture that info from DSI. --- .../provider/omniauth_callback_form.rb | 17 ++++++++++- .../provider/session_answers.rb | 3 ++ .../provider/omniauth_callback_form_spec.rb | 29 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb b/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb index 27034c6699..d433e341df 100644 --- a/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb +++ b/app/forms/journeys/further_education_payments/provider/omniauth_callback_form.rb @@ -13,7 +13,10 @@ def save! dfe_sign_in_organisation_id: dfe_sign_in_organisation_id, dfe_sign_in_organisation_ukprn: dfe_sign_in_organisation_ukprn, dfe_sign_in_service_access: dfe_sign_in_service_access?, - dfe_sign_in_role_codes: dfe_sign_in_role_codes + dfe_sign_in_role_codes: dfe_sign_in_role_codes, + dfe_sign_in_first_name: dfe_sign_in_first_name, + dfe_sign_in_last_name: dfe_sign_in_last_name, + dfe_sign_in_email: dfe_sign_in_email ) journey_session.save! @@ -51,6 +54,18 @@ def dfe_sign_in_role_codes dfe_sign_in_user.role_codes end + + def dfe_sign_in_first_name + auth.dig("info", "first_name") + end + + def dfe_sign_in_last_name + auth.dig("info", "last_name") + end + + def dfe_sign_in_email + auth.dig("info", "email") + end end end end diff --git a/app/models/journeys/further_education_payments/provider/session_answers.rb b/app/models/journeys/further_education_payments/provider/session_answers.rb index 9349d35285..cdde91c7c1 100644 --- a/app/models/journeys/further_education_payments/provider/session_answers.rb +++ b/app/models/journeys/further_education_payments/provider/session_answers.rb @@ -9,6 +9,9 @@ class SessionAnswers < Journeys::SessionAnswers attribute :dfe_sign_in_organisation_ukprn, :string attribute :dfe_sign_in_service_access, :boolean, default: false attribute :dfe_sign_in_role_codes, default: [] + attribute :dfe_sign_in_first_name, :string + attribute :dfe_sign_in_last_name, :string + attribute :dfe_sign_in_email, :string def claim @claim ||= Claim.includes(eligibility: :school).find(claim_id) diff --git a/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb index 57b3faf623..3c5b263959 100644 --- a/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/omniauth_callback_form_spec.rb @@ -28,6 +28,11 @@ let(:auth) do OmniAuth::AuthHash.new( "uid" => "11111", + "info" => { + "email" => "seymore.skinner@springfield-elementary.edu", + "first_name" => "Seymoure", + "last_name" => "Skinner" + }, "extra" => { "raw_info" => { "organisation" => { @@ -60,6 +65,18 @@ change(journey_session.answers, :dfe_sign_in_role_codes) .from([]) .to(["teacher_payments_claim_verifier"]) + ).and( + change(journey_session.answers, :dfe_sign_in_first_name) + .from(nil) + .to("Seymoure") + ).and( + change(journey_session.answers, :dfe_sign_in_last_name) + .from(nil) + .to("Skinner") + ).and( + change(journey_session.answers, :dfe_sign_in_email) + .from(nil) + .to("seymore.skinner@springfield-elementary.edu") ) ) end @@ -82,6 +99,18 @@ not_change(journey_session.answers, :dfe_sign_in_service_access?) ).and( not_change(journey_session.answers, :dfe_sign_in_role_codes) + ).and( + change(journey_session.answers, :dfe_sign_in_first_name) + .from(nil) + .to("Seymoure") + ).and( + change(journey_session.answers, :dfe_sign_in_last_name) + .from(nil) + .to("Skinner") + ).and( + change(journey_session.answers, :dfe_sign_in_email) + .from(nil) + .to("seymore.skinner@springfield-elementary.edu") ) ) end From d402446d8ac2ff30298d737c34ca8ebb921b888c Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Mon, 19 Aug 2024 15:34:35 +0100 Subject: [PATCH 13/15] Store verification details We need to capture information about who verified the claim and what checks were shown to present in the admin section off the app. Usually we'd want to model this by introducing a couple of models something like: ```mermaid erDiagram Claim ||--|| Verification : "has one" Verifier ||--o{ Verification : "has many" Verification ||--o{ Assertion : "has many" ``` However, given we're eventually planning on removing the eligibility models and making them JSON fields on the claim it seemed more in keeping with the plans for the app to store verification as a JSON field. Additionally we're unlikely to be querying verification information and we're unlikely to be updating it, so the downsides of a blob of JSON shouldn't be too bad. We can always migrate to a relational approach if needs be. We've stored the verification on the eligibility rather than on the claim as, currently, this verification step is policy specific. It may be needed on other journeys, at which point it shouldn't be too tricky to move this field to the claim. Serializing / deserializing this JSON structure into objects is left for a future commit when we have more detail on what we need (may not require a serializer. --- .../provider/verify_claim_form.rb | 15 ++++ config/analytics_blocklist.yml | 1 + ...urther_education_payments_eligibilities.rb | 5 ++ db/schema.rb | 1 + .../provider/verify_claim_form_spec.rb | 69 ++++++++++++++++++- 5 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20240819141523_add_verification_to_further_education_payments_eligibilities.rb diff --git a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb index 6f7e84614f..ce163c3e37 100644 --- a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb +++ b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb @@ -78,6 +78,21 @@ def assertions_attributes=(params) def save return false unless valid? + claim.eligibility.update!( + verification: { + assertions: assertions.map(&:attributes), + verifier: { + dfe_sign_in_uid: answers.dfe_sign_in_uid, + first_name: answers.dfe_sign_in_first_name, + last_name: answers.dfe_sign_in_last_name, + email: answers.dfe_sign_in_email + }, + created_at: DateTime.now + } + ) + + claim.save! + true end diff --git a/config/analytics_blocklist.yml b/config/analytics_blocklist.yml index 2711fdefac..ff3998d33f 100644 --- a/config/analytics_blocklist.yml +++ b/config/analytics_blocklist.yml @@ -121,5 +121,6 @@ - teacher_reference_number :further_education_payments_eligibilities: - teacher_reference_number + - verification :journeys_sessions: - answers diff --git a/db/migrate/20240819141523_add_verification_to_further_education_payments_eligibilities.rb b/db/migrate/20240819141523_add_verification_to_further_education_payments_eligibilities.rb new file mode 100644 index 0000000000..b961231495 --- /dev/null +++ b/db/migrate/20240819141523_add_verification_to_further_education_payments_eligibilities.rb @@ -0,0 +1,5 @@ +class AddVerificationToFurtherEducationPaymentsEligibilities < ActiveRecord::Migration[7.0] + def change + add_column :further_education_payments_eligibilities, :verification, :jsonb, default: {} + end +end diff --git a/db/schema.rb b/db/schema.rb index 0e77b03a1f..58012cdeb7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -246,6 +246,7 @@ t.boolean "subject_to_formal_performance_action" t.boolean "subject_to_disciplinary_action" t.boolean "half_teaching_hours" + t.jsonb "verification", default: {} t.index ["possible_school_id"], name: "index_fe_payments_eligibilities_on_possible_school_id" t.index ["school_id"], name: "index_fe_payments_eligibilities_on_school_id" end diff --git a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb index ca36498dc5..177d11f011 100644 --- a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb @@ -12,7 +12,10 @@ :further_education_payments_provider_session, answers: { claim_id: claim.id, - dfe_sign_in_uid: "123" + dfe_sign_in_uid: "123", + dfe_sign_in_first_name: "Seymoure", + dfe_sign_in_last_name: "Skinner", + dfe_sign_in_email: "seymore.skinner@springfield-elementary.edu" } ) end @@ -92,4 +95,68 @@ end end end + + describe "#save" do + let(:params) do + ActionController::Parameters.new( + { + claim: { + declaration: "1", + assertions_attributes: { + "0": {name: "contract_type", outcome: "1"}, + "1": {name: "teaching_responsibilities", outcome: "1"}, + "2": {name: "further_education_teaching_start_year", outcome: "1"}, + "3": {name: "teaching_hours_per_week", outcome: "1"}, + "4": {name: "hours_teaching_eligible_subjects", outcome: "0"}, + "5": {name: "subjects_taught", outcome: "0"} + } + } + } + ) + end + + it "verifies the claim" do + travel_to DateTime.new(2024, 1, 1, 12, 0, 0) do + form.save + end + + expect(claim.reload.eligibility.verification).to match( + { + "assertions" => [ + { + "name" => "contract_type", + "outcome" => true + }, + { + "name" => "teaching_responsibilities", + "outcome" => true + }, + { + "name" => "further_education_teaching_start_year", + "outcome" => true + }, + { + "name" => "teaching_hours_per_week", + "outcome" => true + }, + { + "name" => "hours_teaching_eligible_subjects", + "outcome" => false + }, + { + "name" => "subjects_taught", + "outcome" => false + } + ], + "verifier" => { + "dfe_sign_in_uid" => "123", + "first_name" => "Seymoure", + "last_name" => "Skinner", + "email" => "seymore.skinner@springfield-elementary.edu" + }, + "created_at" => "2024-01-01T12:00:00.000+00:00" + } + ) + end + end end From 15c58dfc53332f0b788d313ae16db1ec09f19e0a Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Mon, 19 Aug 2024 16:33:36 +0100 Subject: [PATCH 14/15] Handle multiple inflight session If the provider visits a second magic link while the first is still in progress make sure they're on the journey for the latest link they clicked. It's tricky to change the existing session in progress logic (we need to write a url param to the journey session) and it doesn't make sense for this journey, as there's only a single screen so there isn't any information in progress to lose. We piggy back off the existing `start_with_magic_link?` logic that skips the session in progress page, and as we don't set `code` or `email` params the `handle_magic_link` method is also skipped. --- .../further_education_payments/provider.rb | 2 + .../provider_verifying_claims_spec.rb | 71 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/app/models/journeys/further_education_payments/provider.rb b/app/models/journeys/further_education_payments/provider.rb index da82eec0bc..d6d79b4c10 100644 --- a/app/models/journeys/further_education_payments/provider.rb +++ b/app/models/journeys/further_education_payments/provider.rb @@ -18,6 +18,8 @@ module Provider CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE = "teacher_payments_claim_verifier" + START_WITH_MAGIC_LINK = true + def self.request_service_access_url(session) [ "https://services.signin.education.gov.uk", diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index fbaf20f3dd..de74caaf19 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -254,6 +254,77 @@ ) end + scenario "provider visits a claim with an inprogress session" do + fe_provider = create(:school, :further_education, name: "Springfield A and M") + + claim_1 = create( + :claim, + first_name: "Edna", + surname: "Krabappel", + date_of_birth: Date.new(1945, 7, 3), + reference: "CLAIM1", + created_at: DateTime.new(2024, 8, 1, 9, 0, 0) + ) + + create( + :further_education_payments_eligibility, + claim: claim_1, + school: fe_provider + ) + + claim_2 = create( + :claim, + first_name: "Edna", + surname: "Krabappel", + date_of_birth: Date.new(1945, 7, 3), + reference: "CLAIM2", + created_at: DateTime.new(2024, 8, 1, 9, 0, 0) + ) + + create( + :further_education_payments_eligibility, + claim: claim_2, + school: fe_provider + ) + + mock_dfe_sign_in_auth_session( + provider: :dfe_fe_provider, + auth_hash: { + uid: "11111", + extra: { + raw_info: { + organisation: { + id: "22222", + ukprn: fe_provider.ukprn + } + } + } + } + ) + + stub_dfe_sign_in_user_info_request( + "11111", + "22222", + Journeys::FurtherEducationPayments::Provider::CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE + ) + + claim_1_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim_1) + + claim_2_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim_2) + + visit claim_1_link + + click_on "Start now" + + visit claim_2_link + + click_on "Start now" + + expect(page).to have_text "Review a financial incentive payment claim" + + expect(page).to have_text "Claim referenceCLAIM2" + end + scenario "provider approves a fixed contract claim" do fe_provider = create(:school, :further_education, name: "Springfield A and M") From c2819f802fd5d38ce4dac839384fdc8e4061e9be Mon Sep 17 00:00:00 2001 From: Richard Lynch Date: Tue, 20 Aug 2024 10:20:11 +0100 Subject: [PATCH 15/15] Handle already verified claims We want to prevent providers from submitting mulitple verifications. We've had to move the `Authorisation` concern so it runs before the form callback stuff such that the answers haven't yet been updated when we check if the claim has been verified on POST. --- app/controllers/claims_controller.rb | 2 +- .../provider/verify_claim_form.rb | 8 ++- .../provider/authorisation.rb | 5 ++ .../further_education_payments/eligibility.rb | 4 ++ .../_unauthorised_already_verified.html.erb | 21 ++++++++ .../provider/claims/unauthorised.html.erb | 2 + .../eligibilities.rb | 40 ++++++++++++++ .../provider_verifying_claims_spec.rb | 54 +++++++++++++++++++ .../provider/verify_claim_form_spec.rb | 14 +++++ .../provider/authorisation_spec.rb | 18 +++++++ 10 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 app/views/further_education_payments/provider/claims/_unauthorised_already_verified.html.erb diff --git a/app/controllers/claims_controller.rb b/app/controllers/claims_controller.rb index ddfc521242..2813617f68 100644 --- a/app/controllers/claims_controller.rb +++ b/app/controllers/claims_controller.rb @@ -12,9 +12,9 @@ class ClaimsController < BasePublicController before_action :persist_claim, only: [:new, :create] before_action :handle_magic_link, only: [:new], if: -> { journey.start_with_magic_link? } + include AuthorisedSlugs include FormSubmittable include ClaimsFormCallbacks - include AuthorisedSlugs def existing_session @existing_session = journey_sessions.first diff --git a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb index ce163c3e37..8e9d21a8ae 100644 --- a/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb +++ b/app/forms/journeys/further_education_payments/provider/verify_claim_form.rb @@ -33,7 +33,7 @@ class VerifyClaimForm < Form validate :all_assertions_answered - # validate claim not already verified + validate :claim_not_already_verified delegate :claim, to: :answers @@ -122,6 +122,12 @@ def all_assertions_answered end end + def claim_not_already_verified + if claim.eligibility.verified? + errors.add(:base, "Claim has already been verified") + end + end + class AssertionForm include ActiveModel::Model include ActiveModel::Attributes diff --git a/app/models/journeys/further_education_payments/provider/authorisation.rb b/app/models/journeys/further_education_payments/provider/authorisation.rb index 463abb7e39..6d53442c8f 100644 --- a/app/models/journeys/further_education_payments/provider/authorisation.rb +++ b/app/models/journeys/further_education_payments/provider/authorisation.rb @@ -12,6 +12,7 @@ def failure_reason return :no_service_access unless answers.dfe_sign_in_service_access? return :claim_admin if claim_admin? return :incorrect_role unless role_permits_access? + return :already_verified if already_verified? && slug != "complete" nil end @@ -43,6 +44,10 @@ def claim_admin_roles DfeSignIn::User::PAYROLL_OPERATOR_DFE_SIGN_IN_ROLE_CODE ] end + + def already_verified? + answers.claim.eligibility.verified? + end end end end diff --git a/app/models/policies/further_education_payments/eligibility.rb b/app/models/policies/further_education_payments/eligibility.rb index c4de3cc1d8..233d9ce9c9 100644 --- a/app/models/policies/further_education_payments/eligibility.rb +++ b/app/models/policies/further_education_payments/eligibility.rb @@ -49,6 +49,10 @@ def courses def fixed_contract? contract_type != "variable_hours" end + + def verified? + verification.present? + end end end end diff --git a/app/views/further_education_payments/provider/claims/_unauthorised_already_verified.html.erb b/app/views/further_education_payments/provider/claims/_unauthorised_already_verified.html.erb new file mode 100644 index 0000000000..f0daa2eec6 --- /dev/null +++ b/app/views/further_education_payments/provider/claims/_unauthorised_already_verified.html.erb @@ -0,0 +1,21 @@ +

+ This claim has already been verified +

+ +

+ Email <%= govuk_mail_to("FE-Levellingup.PremiumPayments@education.gov.uk") %> + if you need to: +

+ +<%= govuk_list( + [ + "view the verification form", + "make changes to the details provided in the verification form" + ], + type: :bullet +) %> + +

+ Make sure you include the claim reference number in your email so we can find + the claim and process any changes promptly. +

diff --git a/app/views/further_education_payments/provider/claims/unauthorised.html.erb b/app/views/further_education_payments/provider/claims/unauthorised.html.erb index 7b5927bbd2..83aba72ed3 100644 --- a/app/views/further_education_payments/provider/claims/unauthorised.html.erb +++ b/app/views/further_education_payments/provider/claims/unauthorised.html.erb @@ -17,6 +17,8 @@ <%= render "unauthorised_incorrect_role" %> <% when "claim_admin" %> <%= render "unauthorised_claim_admin" %> + <% when "already_verified" %> + <%= render "unauthorised_already_verified" %> <% end %> diff --git a/spec/factories/policies/further_education_payments/eligibilities.rb b/spec/factories/policies/further_education_payments/eligibilities.rb index c522f5d82e..bb1447651d 100644 --- a/spec/factories/policies/further_education_payments/eligibilities.rb +++ b/spec/factories/policies/further_education_payments/eligibilities.rb @@ -5,5 +5,45 @@ trait :eligible do end + + trait :verified do + verification do + { + "assertions" => [ + { + "name" => "contract_type", + "outcome" => true + }, + { + "name" => "teaching_responsibilities", + "outcome" => true + }, + { + "name" => "further_education_teaching_start_year", + "outcome" => true + }, + { + "name" => "teaching_hours_per_week", + "outcome" => true + }, + { + "name" => "hours_teaching_eligible_subjects", + "outcome" => false + }, + { + "name" => "subjects_taught", + "outcome" => false + } + ], + "verifier" => { + "dfe_sign_in_uid" => "123", + "first_name" => "Seymoure", + "last_name" => "Skinner", + "email" => "seymore.skinner@springfield-elementary.edu" + }, + "created_at" => "2024-01-01T12:00:00.000+00:00" + } + end + end end end diff --git a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb index de74caaf19..baef59cae5 100644 --- a/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb +++ b/spec/features/further_education_payments/provider/provider_verifying_claims_spec.rb @@ -325,6 +325,60 @@ expect(page).to have_text "Claim referenceCLAIM2" end + scenario "provider visits a claim that has already been verified" do + fe_provider = create(:school, :further_education, 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) + ) + + create( + :further_education_payments_eligibility, + :verified, + claim: claim, + school: fe_provider + ) + + mock_dfe_sign_in_auth_session( + provider: :dfe_fe_provider, + auth_hash: { + uid: "11111", + extra: { + raw_info: { + organisation: { + id: "22222", + ukprn: fe_provider.ukprn + } + } + } + } + ) + + stub_dfe_sign_in_user_info_request( + "11111", + "22222", + Journeys::FurtherEducationPayments::Provider::CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE + ) + + claim_link = Journeys::FurtherEducationPayments::Provider::SlugSequence.verify_claim_url(claim) + + visit claim_link + + click_on "Start now" + + expect(page).to have_text "This claim has already been verified" + + # Try to visit the restricted slug directly + visit "/further-education-payments-provider/verify-claim" + + expect(page).to have_text "This claim has already been verified" + end + scenario "provider approves a fixed contract claim" do fe_provider = create(:school, :further_education, name: "Springfield A and M") diff --git a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb index 177d11f011..835c20c6c1 100644 --- a/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb +++ b/spec/forms/journeys/further_education_payments/provider/verify_claim_form_spec.rb @@ -44,6 +44,20 @@ expect(assertion.errors[:outcome]).to eq(["Select an option"]) end end + + it "validates the claim hasn't already been verified" do + eligibility.update!( + verification: { + assertions: [ + {name: "contract_type", outcome: true} + ] + } + ) + + form.validate + + expect(form.errors[:base]).to eq(["Claim has already been verified"]) + end end describe "#assertions" do diff --git a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb index 002fb2b98d..39ddc72784 100644 --- a/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb +++ b/spec/models/journeys/further_education_payments/provider/authorisation_spec.rb @@ -69,5 +69,23 @@ it { is_expected.to eq(:claim_admin) } end + + context "when the claim has already been verified" do + let(:answers) do + { + dfe_sign_in_service_access: true, + dfe_sign_in_organisation_ukprn: organisation.ukprn, + dfe_sign_in_role_codes: [ + Journeys::FurtherEducationPayments::Provider::CLAIM_VERIFIER_DFE_SIGN_IN_ROLE_CODE + ] + } + end + + let(:eligibility) do + create(:further_education_payments_eligibility, :verified) + end + + it { is_expected.to eq(:already_verified) } + end end end