From ae22f17df19e11f32b33451337f20b15a01f6b25 Mon Sep 17 00:00:00 2001 From: Richard Worrall Date: Mon, 16 Feb 2026 17:53:18 +0000 Subject: [PATCH 1/5] Push next_available_output_fee_statement into Declarations::Clawback --- app/services/api/declarations/clawback.rb | 28 ++++------- app/services/declarations/clawback.rb | 27 +++++++++-- .../api/declarations/clawback_spec.rb | 48 +++++++++++++++---- spec/services/declarations/clawback_spec.rb | 28 ++++++++--- 4 files changed, 94 insertions(+), 37 deletions(-) diff --git a/app/services/api/declarations/clawback.rb b/app/services/api/declarations/clawback.rb index 7edb3bcefc..97550ceda7 100644 --- a/app/services/api/declarations/clawback.rb +++ b/app/services/api/declarations/clawback.rb @@ -9,15 +9,18 @@ class Clawback def clawback return false unless valid? - Declarations::Clawback.new( - author:, - declaration:, - next_available_output_fee_statement: - ).clawback + clawback_service.clawback end private + def clawback_service + @clawback_service ||= Declarations::Clawback.new( + author:, + declaration: + ) + end + def paid return if errors[:declaration_api_id].any? return if declaration.payment_status_paid? @@ -34,7 +37,7 @@ def not_already_refunded def output_fee_statement_available return if errors[:declaration_api_id].any? - return if next_available_output_fee_statement.present? + return if clawback_service.next_available_output_fee_statement.present? no_output_fee_statement_error_message = <<~TXT.squish You cannot submit or void declarations for the #{contract_period_year} @@ -44,19 +47,6 @@ def output_fee_statement_available errors.add(:declaration_api_id, no_output_fee_statement_error_message) end - def next_available_output_fee_statement - @next_available_output_fee_statement ||= Statements::Search - .new( - lead_provider_id: declaration.training_period.lead_provider.id, - contract_period_years: contract_period_year, - fee_type: "output", - deadline_date: Date.current.., - order: :deadline_date - ) - .statements - .first - end - def contract_period_year = declaration.training_period.contract_period.year end end diff --git a/app/services/declarations/clawback.rb b/app/services/declarations/clawback.rb index 8645fda622..66622ea59c 100644 --- a/app/services/declarations/clawback.rb +++ b/app/services/declarations/clawback.rb @@ -1,15 +1,20 @@ module Declarations class Clawback - attr_reader :author, :declaration, :voided_by_user_id, :next_available_output_fee_statement + include ActiveModel::Model - def initialize(author:, declaration:, next_available_output_fee_statement:, voided_by_user_id: nil) + attr_reader :author, :declaration, :voided_by_user_id + + validates :next_available_output_fee_statement, presence: true + + def initialize(author:, declaration:, voided_by_user_id: nil) @author = author @declaration = declaration @voided_by_user_id = voided_by_user_id - @next_available_output_fee_statement = next_available_output_fee_statement end def clawback + return false unless valid? + ActiveRecord::Base.transaction do if voided_by_user_id declaration.voided_by_user_at = Time.current @@ -25,8 +30,24 @@ def clawback declaration end + def next_available_output_fee_statement + @next_available_output_fee_statement ||= Statements::Search + .new( + lead_provider_id: lead_provider.id, + contract_period_years: contract_period.year, + fee_type: "output", + deadline_date: Date.current.., + order: :deadline_date + ) + .statements + .first + end + private + delegate :training_period, to: :declaration + delegate :contract_period, :lead_provider, to: :training_period + def complete_mentor! MentorCompletion.new(author:, declaration:).perform end diff --git a/spec/services/api/declarations/clawback_spec.rb b/spec/services/api/declarations/clawback_spec.rb index f1e1eafcb0..3bb2948222 100644 --- a/spec/services/api/declarations/clawback_spec.rb +++ b/spec/services/api/declarations/clawback_spec.rb @@ -4,6 +4,7 @@ end let(:lead_provider_id) { declaration.training_period.lead_provider.id } + let(:contract_period) { declaration.training_period.contract_period } before { ensure_active_lead_providers_are_consistent } @@ -37,6 +38,29 @@ it { is_expected.to have_error(:declaration_api_id, "The declaration will or has been refunded") } end + context "when there are no future output fee statements available" do + let(:declaration) { FactoryBot.create(:declaration, :paid) } + + before do + payment_statement = declaration.payment_statement + payment_statement.update!(deadline_date: Date.yesterday) + Statement.where("deadline_date > ?", payment_statement.deadline_date).destroy_all + end + + it { is_expected.to have_one_error_per_attribute } + + it { is_expected.not_to be_valid } + + it "has the correct error message" do + error_message = <<~TXT.squish + You cannot submit or void declarations for the #{contract_period.year} + contract period. The funding contract for this contract period has + ended. Get in touch if you need to discuss this with us + TXT + expect(instance).to have_error(:declaration_api_id, error_message) + end + end + context "when the declaration's payment statement has no output fee" do let(:declaration) { FactoryBot.create(:declaration, :paid) } @@ -45,9 +69,8 @@ it { is_expected.to have_one_error_per_attribute } it "has the correct error message" do - contract_period_year = declaration.training_period.contract_period.year error_message = <<~TXT.squish - You cannot submit or void declarations for the #{contract_period_year} + You cannot submit or void declarations for the #{contract_period.year} contract period. The funding contract for this contract period has ended. Get in touch if you need to discuss this with us TXT @@ -63,9 +86,8 @@ it { is_expected.to have_one_error_per_attribute } it "has the correct error message" do - contract_period_year = declaration.training_period.contract_period.year error_message = <<~TXT.squish - You cannot submit or void declarations for the #{contract_period_year} + You cannot submit or void declarations for the #{contract_period.year} contract period. The funding contract for this contract period has ended. Get in touch if you need to discuss this with us TXT @@ -76,6 +98,13 @@ context "when declaration has not been refunded and output fee is available" do let(:declaration) { FactoryBot.create(:declaration, :paid) } + before do + FactoryBot.create( + :statement, + active_lead_provider: declaration.training_period.active_lead_provider + ) + end + it { is_expected.to be_valid } end @@ -83,18 +112,21 @@ subject(:clawback) { instance.clawback } let(:declaration) { FactoryBot.create(:declaration, :paid) } + let(:clawback_service) { instance_double(Declarations::Clawback) } - it "claws back via the service" do - clawback_service = instance_double(Declarations::Clawback) + before do allow(Declarations::Clawback) .to receive(:new) .with( author: instance_of(Events::LeadProviderAPIAuthor), - declaration:, - next_available_output_fee_statement: declaration.payment_statement + declaration: ) .and_return(clawback_service) + allow(clawback_service).to receive(:next_available_output_fee_statement).and_return(FactoryBot.create(:statement, :payable, contract_period:)) + end + + it "claws back via the service" do expect(clawback_service).to receive(:clawback).once clawback diff --git a/spec/services/declarations/clawback_spec.rb b/spec/services/declarations/clawback_spec.rb index c2837da599..8156615181 100644 --- a/spec/services/declarations/clawback_spec.rb +++ b/spec/services/declarations/clawback_spec.rb @@ -3,18 +3,18 @@ described_class.new( author:, declaration:, - voided_by_user_id:, - next_available_output_fee_statement: + voided_by_user_id: ) end let(:declaration) { FactoryBot.create(:declaration, :paid) } - let(:author) do - lead_provider = declaration.training_period.lead_provider - Events::LeadProviderAPIAuthor.new(lead_provider:) - end + let(:author) { Events::LeadProviderAPIAuthor.new(lead_provider:) } let(:voided_by_user_id) { FactoryBot.create(:user).id } - let(:next_available_output_fee_statement) { declaration.payment_statement } + + let(:lead_provider) { declaration.training_period.lead_provider } + let(:contract_period) { declaration.training_period.contract_period } + let(:active_lead_provider) { FactoryBot.create(:active_lead_provider, lead_provider:, contract_period:) } + let!(:next_available_output_fee_statement) { FactoryBot.create(:statement, :output_fee, active_lead_provider:) } describe "#clawback" do subject(:clawback) { instance.clawback } @@ -85,5 +85,19 @@ expect { clawback }.not_to(change(declaration, :voided_by_user_at)) end end + + context "when there is no next available output fee statement" do + let(:next_available_output_fee_statement) { nil } + + before { instance.clawback } + + it "is invalid" do + expect(instance).to have_error(:next_available_output_fee_statement) + end + + it "does not clawback the declaration" do + expect(declaration.clawback_status).to eq("no_clawback") + end + end end end From ab05ddd06733684607f5e2b1aab2fb961a7213dc Mon Sep 17 00:00:00 2001 From: Richard Worrall Date: Mon, 16 Feb 2026 17:53:47 +0000 Subject: [PATCH 2/5] Implement admin void declaration flow --- .../admin/finance/voids_controller.rb | 47 ++++++++ .../admin/finance/void_declaration_form.rb | 54 +++++++++ app/helpers/declaration_helper.rb | 12 ++ app/models/declaration.rb | 2 +- app/views/admin/finance/voids/new.html.erb | 64 +++++++++++ .../declarations/_declaration.html.erb | 13 ++- config/routes/admin.rb | 4 + .../void_declaration_spec.rb | 56 +++++++++ .../finance/void_declaration_form_spec.rb | 58 ++++++++++ spec/helpers/declaration_helper_spec.rb | 48 ++++++++ .../void_declarations_spec.rb | 82 +++++++++++++ spec/services/declarations/clawback_spec.rb | 13 ++- .../admin/finance/voids/new.html.erb_spec.rb | 108 ++++++++++++++++++ .../_declaration.html.erb_spec.rb | 52 ++++++++- 14 files changed, 607 insertions(+), 6 deletions(-) create mode 100644 app/controllers/admin/finance/voids_controller.rb create mode 100644 app/forms/admin/finance/void_declaration_form.rb create mode 100644 app/views/admin/finance/voids/new.html.erb create mode 100644 spec/features/admin/finance/void_declarations/void_declaration_spec.rb create mode 100644 spec/forms/admin/finance/void_declaration_form_spec.rb create mode 100644 spec/requests/admin/finance/void_declarations/void_declarations_spec.rb create mode 100644 spec/views/admin/finance/voids/new.html.erb_spec.rb diff --git a/app/controllers/admin/finance/voids_controller.rb b/app/controllers/admin/finance/voids_controller.rb new file mode 100644 index 0000000000..554280d3a1 --- /dev/null +++ b/app/controllers/admin/finance/voids_controller.rb @@ -0,0 +1,47 @@ +module Admin + module Finance + class VoidsController < Admin::Finance::BaseController + layout "full" + + before_action :set_declaration + before_action :set_teacher + before_action :set_void_declaration_form + + def new + end + + def create + if @void_declaration_form.void! + flash[:alert] = "Declaration voided" + redirect_to admin_teacher_declarations_path(@teacher) + else + render :new, status: :unprocessable_content + end + end + + private + + def set_declaration + @declaration = Declaration.find(params[:declaration_id]) + end + + def set_teacher + @teacher = @declaration.teacher + end + + def set_void_declaration_form + @void_declaration_form = VoidDeclarationForm.new( + declaration: @declaration, + author: current_user, + **void_declaration_form_params + ) + end + + def void_declaration_form_params + return {} unless params.key?(:admin_finance_void_declaration_form) + + params.expect(admin_finance_void_declaration_form: [:confirmed]) + end + end + end +end diff --git a/app/forms/admin/finance/void_declaration_form.rb b/app/forms/admin/finance/void_declaration_form.rb new file mode 100644 index 0000000000..2f78ae7744 --- /dev/null +++ b/app/forms/admin/finance/void_declaration_form.rb @@ -0,0 +1,54 @@ +module Admin + module Finance + class VoidDeclarationForm + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :declaration + attribute :author + attribute :confirmed, :boolean + + validates :confirmed, acceptance: { message: "Confirm you want to void this declaration", allow_nil: false } + + validate :clawback_statement_available, if: :paid_declaration? + + def void! + return false unless valid? + + if paid_declaration? + clawback_service.clawback + else + void_service.void + end + end + + private + + def paid_declaration? + declaration&.payment_status_paid? + end + + def clawback_statement_available + return if clawback_service.next_available_output_fee_statement.present? + + errors.add(:base, "This declaration has been paid, and no future statement exists for clawback") + end + + def clawback_service + @clawback_service ||= Declarations::Clawback.new(**service_attributes) + end + + def void_service + @void_service ||= Declarations::Void.new(**service_attributes) + end + + def service_attributes + { + author:, + declaration:, + voided_by_user_id: author&.id + } + end + end + end +end diff --git a/app/helpers/declaration_helper.rb b/app/helpers/declaration_helper.rb index f25a18b352..4627a51e2d 100644 --- a/app/helpers/declaration_helper.rb +++ b/app/helpers/declaration_helper.rb @@ -44,4 +44,16 @@ def declaration_course_identifier(declaration) "ecf-mentor" end end + + def declaration_voided_by_caption(declaration, event) + return unless event.event_type.in?(%w[teacher_declaration_voided teacher_declaration_clawed_back]) + + text = if declaration.voided_by_user.present? + "Voided by #{declaration.voided_by_user.name} (#{declaration.voided_by_user.email})" + else + "Voided by lead provider" + end + + tag.span(text, class: "govuk-caption-m") + end end diff --git a/app/models/declaration.rb b/app/models/declaration.rb index 5ead7099d1..4af4fc7682 100644 --- a/app/models/declaration.rb +++ b/app/models/declaration.rb @@ -79,7 +79,7 @@ class Declaration < ApplicationRecord validates :declaration_type, inclusion: { in: Declaration.declaration_types.keys, message: "Choose a valid declaration type" } validates :evidence_type, inclusion: { in: Declaration.evidence_types.keys, message: "Choose a valid evidence type" }, allow_nil: true validates :mentorship_period, absence: { message: "Mentor teacher can only be assigned to declarations for ECTs" }, if: :for_mentor? - validates :payment_statement, presence: { message: "Payment statement must be associated for declarations with a payment status" }, unless: :payment_status_no_payment? + validates :payment_statement, presence: { message: "Payment statement must be associated for declarations with a payment status" }, unless: -> { payment_status_no_payment? || payment_status_voided? } validates :clawback_statement, presence: { message: "Clawback statement must be associated for declarations with a clawback status" }, unless: :clawback_status_no_clawback? validates :delivery_partner_when_created, presence: { message: "Delivery partner when the declaration was created must be specified" } validate :mentorship_period_belongs_to_teacher diff --git a/app/views/admin/finance/voids/new.html.erb b/app/views/admin/finance/voids/new.html.erb new file mode 100644 index 0000000000..30bcb7d60f --- /dev/null +++ b/app/views/admin/finance/voids/new.html.erb @@ -0,0 +1,64 @@ +<% page_data( + title: "Void declaration for #{teacher_full_name(@teacher)}", + backlink_href: admin_teacher_declarations_path(@teacher) +) %> + +
+
+ <%= + govuk_summary_list(actions: false) do |summary_list| + summary_list.with_row do |row| + row.with_key { "Declaration ID" } + row.with_value { tag.span(@declaration.api_id, class: "app-mono") } + end + + summary_list.with_row do |row| + row.with_key { "Course identifier" } + row.with_value { tag.span(declaration_course_identifier(@declaration), class: "app-mono") } + end + + summary_list.with_row do |row| + row.with_key { "Declaration type" } + row.with_value { tag.span(@declaration.declaration_type, class: "app-mono") } + end + + summary_list.with_row do |row| + row.with_key { "State" } + row.with_value { declaration_state_tag(@declaration) } + end + + summary_list.with_row do |row| + row.with_key { "Declaration date and time" } + row.with_value { @declaration.declaration_date.to_fs(:govuk) } + end + + summary_list.with_row do |row| + row.with_key { "Lead provider" } + row.with_value { @declaration.lead_provider&.name } + end + + summary_list.with_row do |row| + row.with_key { "Delivery partner" } + row.with_value { @declaration.delivery_partner_when_created&.name || "Delivery partner not recorded" } + end + + summary_list.with_row do |row| + row.with_key { "Evidence held" } + row.with_value { tag.span(@declaration.evidence_type, class: "app-mono") } + end + end + %> + + <%= form_with model: @void_declaration_form, url: admin_declaration_voids_path(@declaration), method: :post do |f| %> + <%= content_for(:error_summary) { f.govuk_error_summary } %> + + <%= f.govuk_check_boxes_fieldset :confirmed, multiple: false, legend: nil do %> + <%= f.govuk_check_box(:confirmed, "1", false, link_errors: true, multiple: false, label: { + text: "I confirm I want to void this declaration and I understand that it cannot be undone" + }) %> + <% end %> + + <%= f.govuk_submit "Confirm void declaration", warning: true, class: "govuk-!-margin-top-6" %> + <% end %> +
+
diff --git a/app/views/admin/teachers/declarations/_declaration.html.erb b/app/views/admin/teachers/declarations/_declaration.html.erb index fdce123b4a..ed56493738 100644 --- a/app/views/admin/teachers/declarations/_declaration.html.erb +++ b/app/views/admin/teachers/declarations/_declaration.html.erb @@ -64,11 +64,22 @@ declaration.events.earliest_first.each do |event| body.with_row do |row| row.with_cell(text: declaration_event_state_name(event), header: true) - row.with_cell(text: event.happened_at.to_fs(:govuk)) + row.with_cell do + safe_join([ + event.happened_at.to_fs(:govuk), + declaration_voided_by_caption(declaration, event) + ].compact) + end end end end end %> <% end %> + + <% if declaration.billable_or_changeable? %> +

+ <%= govuk_button_link_to "Void declaration", new_admin_declaration_void_path(declaration), warning: true %> +

+ <% end %> <% end %> diff --git a/config/routes/admin.rb b/config/routes/admin.rb index ffdcd83cf4..0324bc0c95 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -99,6 +99,10 @@ member { get :delete } end end + + resources :declarations, only: [] do + resources :voids, only: %i[new create] + end end end end diff --git a/spec/features/admin/finance/void_declarations/void_declaration_spec.rb b/spec/features/admin/finance/void_declarations/void_declaration_spec.rb new file mode 100644 index 0000000000..be89fffc57 --- /dev/null +++ b/spec/features/admin/finance/void_declarations/void_declaration_spec.rb @@ -0,0 +1,56 @@ +RSpec.describe "Void a declaration" do + before { sign_in_as_dfe_user(role: :finance) } + + scenario "Void an unpaid declaration" do + given_a_declaration_exists + and_a_future_output_fee_statement_exists + + when_i_visit_the_teacher_declarations_page + and_i_click_void_declaration + then_i_see_the_confirmation_page + + when_i_click_confirm_void_declaration + then_i_see_the_success_message + and_the_declaration_is_voided + end + +private + + def given_a_declaration_exists + @declaration = FactoryBot.create(:declaration, :no_payment) + @teacher = @declaration.teacher + end + + def and_a_future_output_fee_statement_exists + active_lead_provider = FactoryBot.create(:active_lead_provider, lead_provider: @declaration.training_period.lead_provider, contract_period: @declaration.training_period.contract_period) + FactoryBot.create(:statement, :output_fee, active_lead_provider:) + end + + def when_i_visit_the_teacher_declarations_page + page.goto(admin_teacher_declarations_path(@teacher)) + end + + def and_i_click_void_declaration + page.locator("summary").get_by_text(@declaration.api_id).click + page.get_by_role("link", name: "Void declaration").click + end + + def then_i_see_the_confirmation_page + expect(page.get_by_text("Void declaration for")).to be_visible + expect(page.get_by_text(@declaration.api_id)).to be_visible + expect(page.get_by_role("button", name: "Confirm void declaration")).to be_visible + end + + def when_i_click_confirm_void_declaration + page.get_by_label("I confirm I want to void this declaration").check + page.get_by_role("button", name: "Confirm void declaration").click + end + + def then_i_see_the_success_message + expect(page.get_by_text("Declaration voided")).to be_visible + end + + def and_the_declaration_is_voided + expect(@declaration.reload.payment_status).to eq("voided") + end +end diff --git a/spec/forms/admin/finance/void_declaration_form_spec.rb b/spec/forms/admin/finance/void_declaration_form_spec.rb new file mode 100644 index 0000000000..29cc8068c8 --- /dev/null +++ b/spec/forms/admin/finance/void_declaration_form_spec.rb @@ -0,0 +1,58 @@ +RSpec.describe Admin::Finance::VoidDeclarationForm, type: :model do + subject(:form) { described_class.new(declaration:, author:, confirmed:) } + + let(:confirmed) { "1" } + let(:declaration) { FactoryBot.create(:declaration, :no_payment) } + let(:author) { FactoryBot.create(:dfe_user, role: "finance") } + + shared_examples "does not call the void service" do + it "does not call the void service" do + expect(Declarations::Void).not_to receive(:new) + subject + end + end + + shared_examples "does not call the clawback service" do + it "does not call the clawback service" do + expect(Declarations::Clawback).not_to receive(:new) + subject + end + end + + describe "#void!" do + subject { form.void! } + + context "when the form is not confirmed" do + let(:confirmed) { "0" } + + it { is_expected.to be false } + + include_examples "does not call the void service" + include_examples "does not call the clawback service" + end + + context "when voiding an unpaid declaration" do + it { is_expected.to be_truthy } + + it "calls the void service" do + expect(Declarations::Void).to receive(:new).with(author:, declaration:, voided_by_user_id: author.id).and_call_original + form.void! + end + + include_examples "does not call the clawback service" + end + + context "when voiding a paid declaration" do + let(:declaration) { FactoryBot.create(:declaration, :paid) } + + it { is_expected.to be_truthy } + + include_examples "does not call the void service" + + it "calls the clawback service" do + expect(Declarations::Clawback).to receive(:new).with(author:, declaration:, voided_by_user_id: author.id).and_call_original + form.void! + end + end + end +end diff --git a/spec/helpers/declaration_helper_spec.rb b/spec/helpers/declaration_helper_spec.rb index 01c900aa2b..ba52b6caa5 100644 --- a/spec/helpers/declaration_helper_spec.rb +++ b/spec/helpers/declaration_helper_spec.rb @@ -112,4 +112,52 @@ it { is_expected.to eq("ecf-mentor") } end end + + describe "#declaration_voided_by_caption" do + subject { helper.declaration_voided_by_caption(declaration, event) } + + let(:event) { instance_double(Event, event_type:) } + + context "when the event type is not a voided or clawed_back event" do + let(:event_type) { "teacher_declaration_created" } + + it { is_expected.to be_nil } + end + + context "when the event type is 'teacher_declaration_voided'" do + let(:event_type) { "teacher_declaration_voided" } + + context "when voided by an admin user" do + let(:admin_user) { instance_double(User, name: "Admin User", email: "admin@example.com") } + + before { allow(declaration).to receive(:voided_by_user).and_return(admin_user) } + + it { is_expected.to include("Voided by Admin User (admin@example.com)") } + end + + context "when voided by a lead provider" do + before { allow(declaration).to receive(:voided_by_user).and_return(nil) } + + it { is_expected.to include("Voided by lead provider") } + end + end + + context "when the event type is 'teacher_declaration_clawed_back'" do + let(:event_type) { "teacher_declaration_clawed_back" } + + context "when clawed back by an admin user" do + let(:admin_user) { instance_double(User, name: "Finance User", email: "finance@example.com") } + + before { allow(declaration).to receive(:voided_by_user).and_return(admin_user) } + + it { is_expected.to include("Voided by Finance User (finance@example.com)") } + end + + context "when clawed back by a lead provider" do + before { allow(declaration).to receive(:voided_by_user).and_return(nil) } + + it { is_expected.to include("Voided by lead provider") } + end + end + end end diff --git a/spec/requests/admin/finance/void_declarations/void_declarations_spec.rb b/spec/requests/admin/finance/void_declarations/void_declarations_spec.rb new file mode 100644 index 0000000000..cc21c1b3b8 --- /dev/null +++ b/spec/requests/admin/finance/void_declarations/void_declarations_spec.rb @@ -0,0 +1,82 @@ +RSpec.describe "Admin finance void declarations", type: :request do + let(:declaration) { FactoryBot.create(:declaration) } + + shared_examples "requires finance access" do + context "when not signed in" do + it { is_expected.to redirect_to(sign_in_path) } + end + + context "with an authenticated non-DfE user" do + include_context "sign in as non-DfE user" + + it { is_expected.to have_http_status(:unauthorized) } + end + + context "when signed in as a non-finance DfE user" do + include_context "sign in as DfE user" + + it { is_expected.to have_http_status(:unauthorized) } + + it "renders the finance access error message" do + expect(subject.body).to include( + "This is to access financial information for Register early career teachers. To gain access, contact the product team." + ) + end + end + end + + describe "GET /admin/finance/declarations/:id/voids/new" do + subject do + get "/admin/finance/declarations/#{declaration.id}/voids/new" + response + end + + include_examples "requires finance access" + + context "when signed in as a finance DfE user" do + include_context "sign in as finance DfE user" + + it { is_expected.to have_http_status(:ok) } + + it "renders the void confirmation page" do + expect(subject.body).to include("Void declaration for") + end + end + end + + describe "POST /admin/finance/void-declarations/:id" do + subject do + post "/admin/finance/declarations/#{declaration.id}/voids", params: { admin_finance_void_declaration_form: { confirmed: "1" } } + response + end + + include_examples "requires finance access" + + context "when signed in as a finance DfE user" do + include_context "sign in as finance DfE user" + + let(:form) { Admin::Finance::VoidDeclarationForm.new(declaration:, author: user, confirmed: "1") } + + before do + allow(Admin::Finance::VoidDeclarationForm).to receive(:new).and_return(form) + end + + context "when the void is successful" do + before { allow(form).to receive(:void!).and_return(true) } + + it { is_expected.to redirect_to(admin_teacher_declarations_path(declaration.teacher)) } + + it "sets a flash message" do + subject + expect(flash[:alert]).to eq("Declaration voided") + end + end + + context "when the void is not successful" do + before { allow(form).to receive(:void!).and_return(false) } + + it { is_expected.to have_http_status(:unprocessable_content) } + end + end + end +end diff --git a/spec/services/declarations/clawback_spec.rb b/spec/services/declarations/clawback_spec.rb index 8156615181..674435918d 100644 --- a/spec/services/declarations/clawback_spec.rb +++ b/spec/services/declarations/clawback_spec.rb @@ -16,6 +16,11 @@ let(:active_lead_provider) { FactoryBot.create(:active_lead_provider, lead_provider:, contract_period:) } let!(:next_available_output_fee_statement) { FactoryBot.create(:statement, :output_fee, active_lead_provider:) } + before do + # make payment statement precede clawback statement + declaration.payment_statement.update!(deadline_date: Date.yesterday) + end + describe "#clawback" do subject(:clawback) { instance.clawback } @@ -87,11 +92,13 @@ end context "when there is no next available output fee statement" do - let(:next_available_output_fee_statement) { nil } - - before { instance.clawback } + before do + next_available_output_fee_statement.destroy! + instance.clawback + end it "is invalid" do + # binding.irb expect(instance).to have_error(:next_available_output_fee_statement) end diff --git a/spec/views/admin/finance/voids/new.html.erb_spec.rb b/spec/views/admin/finance/voids/new.html.erb_spec.rb new file mode 100644 index 0000000000..5ef21b1ed2 --- /dev/null +++ b/spec/views/admin/finance/voids/new.html.erb_spec.rb @@ -0,0 +1,108 @@ +RSpec.describe "admin/finance/voids/new.html.erb", type: :view do + subject { Capybara.string(rendered) } + + let(:teacher) { FactoryBot.build_stubbed(:teacher) } + let(:lead_provider) { FactoryBot.build_stubbed(:lead_provider, name: "Test Lead Provider") } + let(:delivery_partner) { FactoryBot.build_stubbed(:delivery_partner, name: "Test Delivery Partner") } + let(:declaration) do + FactoryBot.build_stubbed( + :declaration, + declaration_type: "started", + declaration_date: Date.new(2024, 1, 15), + api_id: "test-declaration-uuid", + evidence_type: "training-event-attended" + ).tap do |d| + allow(d).to receive_messages( + api_id: "test-declaration-uuid", + lead_provider:, + delivery_partner_when_created: delivery_partner, + for_ect?: true, + for_mentor?: false, + teacher:, + overall_status: "eligible" + ) + end + end + let(:void_declaration_form) { Admin::Finance::VoidDeclarationForm.new(declaration:, author: nil) } + + before do + assign(:declaration, declaration) + assign(:teacher, teacher) + assign(:void_declaration_form, void_declaration_form) + + # rubocop:disable RSpec/AnyInstance - needed for boundary with a helper + allow_any_instance_of(DeclarationHelper).to receive(:declaration_course_identifier).and_return("ecf-induction") + allow_any_instance_of(TeacherHelper).to receive(:teacher_full_name).and_return("Test Teacher") + # rubocop:enable RSpec/AnyInstance + + render + end + + it "sets the page title" do + expect(view.content_for(:page_title)).to include("Void declaration for Test Teacher") + end + + it { is_expected.to have_summary_list_row("Declaration ID", value: "test-declaration-uuid", visible: :all) } + it { is_expected.to have_summary_list_row("Course identifier", value: "ecf-induction", visible: :all) } + it { is_expected.to have_summary_list_row("Declaration type", value: "started", visible: :all) } + it { is_expected.to have_summary_list_row("State", value: "Eligible", visible: :all) } + it { is_expected.to have_summary_list_row("Declaration date and time", value: "15 January 2024, 12:00am", visible: :all) } + it { is_expected.to have_summary_list_row("Lead provider", value: "Test Lead Provider", visible: :all) } + it { is_expected.to have_summary_list_row("Evidence held", value: "training-event-attended", visible: :all) } + + describe "delivery partner" do + context "when the declaration has no delivery partner" do + let(:delivery_partner) { nil } + + it { is_expected.to have_summary_list_row("Delivery partner", value: "Delivery partner not recorded", visible: :all) } + end + + context "when the declaration has a delivery partner" do + it { is_expected.to have_summary_list_row("Delivery partner", value: "Test Delivery Partner", visible: :all) } + end + end + + it "has a confirmation checkbox" do + expect(rendered).to have_field("admin_finance_void_declaration_form[confirmed]", type: "checkbox") + expect(rendered).to have_content("I confirm I want to void this declaration and I understand that it cannot be undone") + end + + it "has a warning-styled confirm button" do + expect(rendered).to have_css("button.govuk-button--warning[type='submit']", text: "Confirm void declaration") + end + + describe "error state" do + context "when there is no error" do + it "does not display an error summary" do + expect(view.content_for(:error_summary)).to be_blank + end + + it "does not have error styling on the form group" do + expect(rendered).not_to have_css(".govuk-form-group--error") + end + end + + context "when there is an error" do + before do + form_with_errors = Admin::Finance::VoidDeclarationForm.new(declaration:, author: nil, confirmed: "0") + form_with_errors.valid? + assign(:void_declaration_form, form_with_errors) + render + end + + it "displays an error summary" do + expect(view.content_for(:error_summary)).to have_css(".govuk-error-summary") + expect(view.content_for(:error_summary)).to have_content("There is a problem") + expect(view.content_for(:error_summary)).to have_link("Confirm you want to void this declaration") + end + + it "has error styling on the form group" do + expect(rendered).to have_css(".govuk-form-group--error") + end + + it "displays the error message by the checkbox" do + expect(rendered).to have_css(".govuk-error-message", text: "Confirm you want to void this declaration") + end + end + end +end diff --git a/spec/views/admin/teachers/declarations/_declaration.html.erb_spec.rb b/spec/views/admin/teachers/declarations/_declaration.html.erb_spec.rb index 8d9d065601..6d7ab78e56 100644 --- a/spec/views/admin/teachers/declarations/_declaration.html.erb_spec.rb +++ b/spec/views/admin/teachers/declarations/_declaration.html.erb_spec.rb @@ -18,7 +18,8 @@ for_ect?: true, for_mentor?: false, overall_status: "no_payment", - events: Event.none + events: Event.none, + billable_or_changeable?: false ) end end @@ -85,6 +86,55 @@ expect(page).to have_css("td", text: "16 January 2024, 12:00pm") end end + + context "when voided by an admin user" do + let(:admin_user) { FactoryBot.build_stubbed(:user, name: "Admin User", email: "admin@example.com") } + + before do + allow(declaration).to receive(:voided_by_user).and_return(admin_user) + render locals: { declaration: } + end + + it "shows who voided the declaration" do + within "table tbody tr:nth-child(2)" do + expect(page).to have_content("Voided by Admin User (admin@example.com)") + end + end + end + + context "when voided by a lead provider" do + before do + allow(declaration).to receive(:voided_by_user).and_return(nil) + render locals: { declaration: } + end + + it "shows it was voided by lead provider" do + within "table tbody tr:nth-child(2)" do + expect(page).to have_content("Voided by lead provider") + end + end + end + end + end + + describe "void declaration button" do + subject { rendered } + + before do + allow(declaration).to receive(:billable_or_changeable?).and_return(billable_or_changeable) + render locals: { declaration: } + end + + context "when the declaration can be voided by admin" do + let(:billable_or_changeable) { true } + + it { is_expected.to have_css("a.govuk-button--warning", text: "Void declaration", visible: :all) } + end + + context "when the declaration cannot be voided by admin" do + let(:billable_or_changeable) { false } + + it { is_expected.not_to have_link("Void declaration", visible: :all) } end end end From bdc2eb9d567eae98e7806c2d1984aca374b946cd Mon Sep 17 00:00:00 2001 From: Richard Worrall Date: Wed, 18 Feb 2026 12:13:56 +0000 Subject: [PATCH 3/5] Explicitly validate declaration status is voidable --- app/forms/admin/finance/void_declaration_form.rb | 7 +++++++ .../forms/admin/finance/void_declaration_form_spec.rb | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/app/forms/admin/finance/void_declaration_form.rb b/app/forms/admin/finance/void_declaration_form.rb index 2f78ae7744..8d16a05d29 100644 --- a/app/forms/admin/finance/void_declaration_form.rb +++ b/app/forms/admin/finance/void_declaration_form.rb @@ -10,6 +10,7 @@ class VoidDeclarationForm validates :confirmed, acceptance: { message: "Confirm you want to void this declaration", allow_nil: false } + validate :declaration_voidable validate :clawback_statement_available, if: :paid_declaration? def void! @@ -24,6 +25,12 @@ def void! private + def declaration_voidable + return if declaration.billable_or_changeable? + + errors.add(:declaration, "This declaration cannot be voided") + end + def paid_declaration? declaration&.payment_status_paid? end diff --git a/spec/forms/admin/finance/void_declaration_form_spec.rb b/spec/forms/admin/finance/void_declaration_form_spec.rb index 29cc8068c8..dc08b35ea7 100644 --- a/spec/forms/admin/finance/void_declaration_form_spec.rb +++ b/spec/forms/admin/finance/void_declaration_form_spec.rb @@ -26,6 +26,17 @@ let(:confirmed) { "0" } it { is_expected.to be false } + it { expect(form).to have_error(:confirmed) } + + include_examples "does not call the void service" + include_examples "does not call the clawback service" + end + + context "when the declaration is not voidable" do + let(:declaration) { FactoryBot.create(:declaration, :voided) } + + it { is_expected.to be false } + it { expect(form).to have_error(:declaration) } include_examples "does not call the void service" include_examples "does not call the clawback service" From 4511520cee11afa9caf1d82ba4b9d05c985e4ff6 Mon Sep 17 00:00:00 2001 From: Richard Worrall Date: Wed, 18 Feb 2026 12:14:12 +0000 Subject: [PATCH 4/5] Remove debug comment --- spec/services/declarations/clawback_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/services/declarations/clawback_spec.rb b/spec/services/declarations/clawback_spec.rb index 674435918d..47f833e6fb 100644 --- a/spec/services/declarations/clawback_spec.rb +++ b/spec/services/declarations/clawback_spec.rb @@ -98,7 +98,6 @@ end it "is invalid" do - # binding.irb expect(instance).to have_error(:next_available_output_fee_statement) end From 4fe407ebdfd0e85845b50ffceb8b8378c91b74c1 Mon Sep 17 00:00:00 2001 From: Richard Worrall Date: Thu, 19 Feb 2026 15:37:04 +0000 Subject: [PATCH 5/5] Change the event recorded when a clawback is initiated from "clawed_back" to "awaiting_clawback" The actual clawback occurs when the clawback statement is paid At this point we've attached a clawback to a clawback statement, but it hasn't happened yet --- app/helpers/declaration_helper.rb | 4 ++-- app/models/event.rb | 2 +- app/services/declarations/clawback.rb | 2 +- app/services/events/record.rb | 6 +++--- db/seeds/teacher_histories.rb | 4 ++-- spec/helpers/declaration_helper_spec.rb | 10 +++++----- spec/services/declarations/clawback_spec.rb | 2 +- spec/services/events/record_spec.rb | 8 ++++---- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/helpers/declaration_helper.rb b/app/helpers/declaration_helper.rb index 4627a51e2d..826caab378 100644 --- a/app/helpers/declaration_helper.rb +++ b/app/helpers/declaration_helper.rb @@ -22,7 +22,7 @@ module DeclarationHelper DECLARATION_EVENT_STATE_NAMES = { "teacher_declaration_created" => "Submitted", "teacher_declaration_voided" => "Voided", - "teacher_declaration_clawed_back" => "Clawed back", + "teacher_declaration_awaiting_clawback" => "Awaiting clawback", }.freeze def declaration_state_tag(declaration) @@ -46,7 +46,7 @@ def declaration_course_identifier(declaration) end def declaration_voided_by_caption(declaration, event) - return unless event.event_type.in?(%w[teacher_declaration_voided teacher_declaration_clawed_back]) + return unless event.event_type.in?(%w[teacher_declaration_voided teacher_declaration_awaiting_clawback]) text = if declaration.voided_by_user.present? "Voided by #{declaration.voided_by_user.name} (#{declaration.voided_by_user.email})" diff --git a/app/models/event.rb b/app/models/event.rb index 7034727df4..a127922178 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -57,7 +57,7 @@ class Event < ApplicationRecord teacher_withdraws_training_period teacher_changes_schedule_training_period teacher_declaration_voided - teacher_declaration_clawed_back + teacher_declaration_awaiting_clawback teacher_declaration_created mentor_completion_status_change training_period_assigned_to_school_partnership diff --git a/app/services/declarations/clawback.rb b/app/services/declarations/clawback.rb index 66622ea59c..ab28b8bfbe 100644 --- a/app/services/declarations/clawback.rb +++ b/app/services/declarations/clawback.rb @@ -57,7 +57,7 @@ def attach_clawback_statement end def record_clawback_event! - Events::Record.record_teacher_declaration_clawed_back!( + Events::Record.record_teacher_declaration_awaiting_clawback!( author:, teacher: declaration.training_period.teacher, training_period: declaration.training_period, diff --git a/app/services/events/record.rb b/app/services/events/record.rb index 030ef7dc4d..ccb39c60eb 100644 --- a/app/services/events/record.rb +++ b/app/services/events/record.rb @@ -560,10 +560,10 @@ def self.record_teacher_declaration_voided!(author:, teacher:, training_period:, new(event_type:, author:, heading:, teacher:, training_period:, declaration:, happened_at:).record_event! end - def self.record_teacher_declaration_clawed_back!(author:, teacher:, training_period:, declaration:, happened_at: Time.current) - event_type = :teacher_declaration_clawed_back + def self.record_teacher_declaration_awaiting_clawback!(author:, teacher:, training_period:, declaration:, happened_at: Time.current) + event_type = :teacher_declaration_awaiting_clawback teacher_name = Teachers::Name.new(teacher).full_name - heading = "#{teacher_name}’s declaration was clawed back" + heading = "#{teacher_name}’s declaration was marked as awaiting clawback" new(event_type:, author:, heading:, teacher:, training_period:, declaration:, happened_at:).record_event! end diff --git a/db/seeds/teacher_histories.rb b/db/seeds/teacher_histories.rb index 7c4c50fc44..c7ecae7b0a 100644 --- a/db/seeds/teacher_histories.rb +++ b/db/seeds/teacher_histories.rb @@ -578,10 +578,10 @@ def find_or_create_school_partnership!(school:, delivery_partner:, lead_provider happened_at: alan_rickman_completed_date.at_midday, **alan_rickman_lp_author) FactoryBot.create(:event, - event_type: "teacher_declaration_clawed_back", + event_type: "teacher_declaration_awaiting_clawback", declaration: decl, teacher: alan_rickman, - heading: "Declaration clawed back", + heading: "Declaration awaiting clawback", happened_at: alan_rickman_completed_date.at_midday + 30.days, **alan_rickman_lp_author) describe_declaration(decl) diff --git a/spec/helpers/declaration_helper_spec.rb b/spec/helpers/declaration_helper_spec.rb index ba52b6caa5..8f664a1856 100644 --- a/spec/helpers/declaration_helper_spec.rb +++ b/spec/helpers/declaration_helper_spec.rb @@ -82,10 +82,10 @@ it { is_expected.to eq("Voided") } end - context "when the event type is 'teacher_declaration_clawed_back'" do - let(:event_type) { "teacher_declaration_clawed_back" } + context "when the event type is 'teacher_declaration_awaiting_clawback'" do + let(:event_type) { "teacher_declaration_awaiting_clawback" } - it { is_expected.to eq("Clawed back") } + it { is_expected.to eq("Awaiting clawback") } end context "when the event type is not recognised" do @@ -142,8 +142,8 @@ end end - context "when the event type is 'teacher_declaration_clawed_back'" do - let(:event_type) { "teacher_declaration_clawed_back" } + context "when the event type is 'teacher_declaration_awaiting_clawback'" do + let(:event_type) { "teacher_declaration_awaiting_clawback" } context "when clawed back by an admin user" do let(:admin_user) { instance_double(User, name: "Finance User", email: "finance@example.com") } diff --git a/spec/services/declarations/clawback_spec.rb b/spec/services/declarations/clawback_spec.rb index 47f833e6fb..8c1fcff64e 100644 --- a/spec/services/declarations/clawback_spec.rb +++ b/spec/services/declarations/clawback_spec.rb @@ -52,7 +52,7 @@ it "records an event" do expect(Events::Record) - .to receive(:record_teacher_declaration_clawed_back!) + .to receive(:record_teacher_declaration_awaiting_clawback!) .with( author:, teacher: declaration.training_period.teacher, diff --git a/spec/services/events/record_spec.rb b/spec/services/events/record_spec.rb index 7ca3858c47..231bc6e5d8 100644 --- a/spec/services/events/record_spec.rb +++ b/spec/services/events/record_spec.rb @@ -2066,7 +2066,7 @@ end end - describe ".record_teacher_declaration_clawed_back" do + describe ".record_teacher_declaration_awaiting_clawback" do let(:ect_at_school_period) do FactoryBot.create(:ect_at_school_period, teacher:) end @@ -2080,7 +2080,7 @@ it "queues a RecordEventJob with the correct values" do freeze_time - Events::Record.record_teacher_declaration_clawed_back!( + Events::Record.record_teacher_declaration_awaiting_clawback!( author:, teacher:, training_period:, @@ -2088,8 +2088,8 @@ ) expect(RecordEventJob).to have_received(:perform_later).with( - event_type: :teacher_declaration_clawed_back, - heading: "Rhys Ifans’s declaration was clawed back", + event_type: :teacher_declaration_awaiting_clawback, + heading: "Rhys Ifans’s declaration was marked as awaiting clawback", teacher:, training_period:, declaration:,