Conversation
raul-gracia
commented
Oct 8, 2025
- Add Year 2 provider verification auto-pass logic
- Fix test failures for Year 2 provider verification
4880b9e to
6ea1201
Compare
6ea1201 to
ee18207
Compare
| module Tasks | ||
| class ProviderVerificationJob < ApplicationJob |
There was a problem hiding this comment.
to keep everything aligned I think we plan to name it FeProviderVerificationV2Job
see:
- https://github.com/DFE-Digital/claim-additional-payments-for-teaching/pull/4107/files#diff-e4eb3c43eb4f027ef2695156f7112467295c87db46877849edd11fe15bc5a39eR1-R2
- https://github.com/DFE-Digital/claim-additional-payments-for-teaching/pull/4138/files#diff-e4eb3c43eb4f027ef2695156f7112467295c87db46877849edd11fe15bc5a39eR1-R2
There was a problem hiding this comment.
I've renamed this to FeProviderVerificationV2Job
spec/features/admin/tasks/further_education_payments/fe_provider_verification_v2_spec.rb
Outdated
Show resolved
Hide resolved
spec/features/admin/tasks/further_education_payments/fe_provider_verification_v2_spec.rb
Outdated
Show resolved
Hide resolved
| module AutomatedChecks | ||
| module ClaimVerifiers | ||
| class ProviderVerification |
There was a problem hiding this comment.
the check was also renamed. otherwise i think calling the class one name and the task name another will lead to confusion. see:
- https://github.com/DFE-Digital/claim-additional-payments-for-teaching/pull/4107/files#diff-ea102a66f389acaff1d3ec606dcfef7e797af8b5dd11ecf90c4bc3429c6c21beR1-R3
- https://github.com/DFE-Digital/claim-additional-payments-for-teaching/pull/4138/files#diff-ea102a66f389acaff1d3ec606dcfef7e797af8b5dd11ecf90c4bc3429c6c21beR1-R3
There was a problem hiding this comment.
Agreed.
I've already updated this in my latest changes:
- Class:
AutomatedChecks::ClaimVerifiers::ProviderVerificationV2 - Task name:
fe_provider_verification_v2 - Job:
Tasks::FeProviderVerificationV2Job
| # this has been completed. | ||
| if claim.eligibility.provider_verification_claimant_employment_check_declaration | ||
| Policies::FurtherEducationPayments.alternative_idv_completed!(claim) | ||
| elsif claim.academic_year != AcademicYear.new("2024/2025") |
There was a problem hiding this comment.
i think this guard is not needed? as it looks like you've already got a guard on #perform on the automated check. i think if we delegate the responsibility down each job/check can decide if it wants to process the claim or not, pushing things down the stack
There was a problem hiding this comment.
Also no 2024/2025 claims will go through this wizard
There was a problem hiding this comment.
You're right, I'll remove the conditional and always call provider_verification_completed!
| verification: { | ||
| "verifier" => { |
There was a problem hiding this comment.
verifier also applies to Y1 and has been superseded by provider_verification_verified_by_id for Y2 I believe
There was a problem hiding this comment.
I've updated the spec
| verification: { | ||
| assertions: [ | ||
| {name: "teaching_responsibilities", outcome: true}, | ||
| {name: "teaching_start_year_matches_claim", outcome: true}, | ||
| {name: "half_teaching_hours", outcome: true}, | ||
| {name: "subjects_taught", outcome: true}, | ||
| {name: "taught_at_least_one_term", outcome: true} | ||
| ], | ||
| verifier: { | ||
| dfe_sign_in_uid: "123", | ||
| first_name: "Seymour", | ||
| last_name: "Skinner", | ||
| email: "seymore.skinner@springfield-elementary.edu", | ||
| dfe_sign_in_organisation_name: "Springfield Elementary", | ||
| dfe_sign_in_role_codes: ["teacher_payments_claim_verifier"] | ||
| }, | ||
| created_at: Time.zone.now | ||
| } |
There was a problem hiding this comment.
For year 2 claims we wont be writing anything to the further_education_payments_eligibilities.verification column. The year 1 journey that did write to this column has been removed. The column is only kept around for displaying last year's claims
| when "more_than_12" | ||
| "20_or_more_hours_per_week" | ||
| when "between_2_5_and_12" | ||
| "2_and_a_half_to_12_hours_per_week" | ||
| when "less_than_2_5" | ||
| "fewer_than_2_and_a_half_hours_per_week" | ||
| else | ||
| provider_hours | ||
| end |
There was a problem hiding this comment.
It looks like if the claimant choses more_than_20 then this task will always pass, even if the provider entered a different value (eg fewer_than_2_and_a_half_hours_per_week)?
| end | ||
|
|
||
| def find_or_create_verifier_user | ||
| verifier_data = verification.fetch("verifier") |
There was a problem hiding this comment.
This will always throw when this task runs as verification will be {}
There was a problem hiding this comment.
| claimant_value = case eligibility.teaching_qualification | ||
| when "yes" | ||
| "Yes" | ||
| when "not_yet" | ||
| "Not yet, I am currently enrolled on one and working towards completing it" | ||
| when "no_but_planned" | ||
| "No, but I plan to enrol on one in the next 12 months" | ||
| when "no_not_planned" | ||
| "No, and I do not plan to enrol on one in the next 12 months" | ||
| else | ||
| "Not provided" | ||
| end |
There was a problem hiding this comment.
Should probably use the locales file for this
I18n.t(eligibility.teaching_qualification, scope: 'further_education_payments.forms.teaching_qualification.options')
| "Timetabled teaching hours", | ||
| claimant_mapped = case eligibility.teaching_hours_per_week | ||
| when "more_than_12" | ||
| "20 hours or more each week" |
There was a problem hiding this comment.
Should this be "12 hours or more each week"?
|
|
||
| VERIFIERS = [ | ||
| AutomatedChecks::ClaimVerifiers::OneLoginIdentity, | ||
| AutomatedChecks::ClaimVerifiers::ProviderVerification, # Year 2+ only |
There was a problem hiding this comment.
I don't think we want this verifier in the list that runs on initial claim submission, I think we only want it to run after the provider has submitted their part of the claim (which we're doing a few lines below with the provider_verification_completed! hook)
| within(".fe_provider_verification_v2") do | ||
| first("a").click | ||
| end |
There was a problem hiding this comment.
Is there a reason we've changed this? Seems quite a bit less readable?
| claim.tasks.create!( | ||
| name: "fe_provider_verification_v2", | ||
| passed: false, | ||
| manual: true | ||
| ) |
There was a problem hiding this comment.
How come we need to create a task here?
If the provider hasn't completed verification then there won't be a task (unless an admin manually submits the task form, which we don't want to allow until the verification has been completed.)
|
|
||
| def within_table_row(label, &block) | ||
| within(first("tr", text: label)) do | ||
| within(first("th", exact_text: label).find(:xpath, "./..")) do |
There was a problem hiding this comment.
It's fine to change this but I'm just wondering why it needed to change if we've not changed any view files?
| # this has been completed. | ||
| if claim.eligibility.provider_verification_claimant_employment_check_declaration | ||
| Policies::FurtherEducationPayments.alternative_idv_completed!(claim) | ||
| elsif claim.academic_year != AcademicYear.new("2024/2025") |
There was a problem hiding this comment.
We still want the provider_verification_completed! hook to run even if the provider had to do ALT IDV before verifying the claim's eligibility.
552bee0 to
3de2159
Compare
f50f551 to
2f29cec
Compare
Implements Year 2 (2025/2026+) enhancements for FE provider verification: - Add new Year 2 specific fields to AdminTasksPresenter: * Teaching qualification with 4 options * Performance measures (renamed label) * Disciplinary action (renamed label) * Teaching hours with hour ranges * Conditional fields (hours next term, full academic year, taught one term) - Create ProviderVerification auto-check verifier: * Auto-pass when all conditions met * Auto-fail when any condition fails * Skip Year 1 (2024/2025) claims * Uses teaching_start_year_matches_claim field * Maps teaching hours correctly (more_than_12 -> 20_or_more_hours_per_week) - Add ProviderVerificationJob to trigger auto-check - Add provider_verification_completed! hook for Year 2+ claims - Add comprehensive test coverage for auto-pass/fail logic Adapted to master architecture: - Uses row-based AdminTasksPresenter approach (not separate presenter) - Field: teaching_start_year_matches_claim (not in_first_five_years) - Contract: no_direct_contract (not employed_by_another_organisation) - Year-based routing already exists via admin_tasks_presenter method
- Update further_education_payments_spec to include ProviderVerification in VERIFIERS - Update fe_provider_verification_v2_spec for Year 2 ordering and field displays - Change task name from 'provider_verification' to 'fe_provider_verification_v2' for Year 2 - Update all provider verification specs to use correct task name - Add academic_year specification to Year 2 tests - Update field label expectations (performance/disciplinary renamed with "Subject to") - Update hours display expectation (more_than_12 -> "20 hours or more each week") - Add teaching qualification and contract covers full year to test expectations
Address PR review feedback: - Rename ProviderVerificationJob to FeProviderVerificationV2Job for consistency - Remove unnecessary FactoryBot.reload from spec - Update verifier to check Year 2 eligibility columns instead of Year 1 assertions - Fix specs to include required Year 2 provider verification columns Changes: - Job: ProviderVerificationJob → FeProviderVerificationV2Job - Verifier: Check provider_verification_* columns directly instead of parsing assertions hash - Specs: Add missing provider_verification_teaching_responsibilities, provider_verification_teaching_start_year_matches_claim, and provider_verification_half_teaching_hours to test data - Remove Year 1 assertions array from Year 2 test data where not needed
- Fix FeProviderVerificationV2Job spec to create FE claims - Add valid reason check for teaching qualification in ProviderVerificationV2 - Fix I18n scope for teaching hours per week - Update AdminTasksPresenter spec to use flexible row matching - Update performance/disciplinary label expectations
2f29cec to
75686d9
Compare
|
No longer required, this is now a stale branch and requirements have been implemented elsewhere |