Skip to content

Commit

Permalink
Remove claim as a form argument
Browse files Browse the repository at this point in the history
Now the forms no longer reference the current claim we can remove it as
an argument.
Of note we've had to hack the reminders form a bit. The reminders
controller passes in the reminder to the form as the claim argument.
We've removed the "current_data_object" method as it's too much
abstraction and added some methods to the reminder forms to still let
them work with the claim, which is really a reminder. The reminders
forms probably shouldn't share a base class with the usual forms but
unpicking that is a future exercise. These hacks will hopefully be
addressed in https://dfedigital.atlassian.net.mcas.ms/browse/CAPT-1719
  • Loading branch information
rjlynch committed Jun 14, 2024
1 parent a7e1429 commit b52d86c
Show file tree
Hide file tree
Showing 64 changed files with 74 additions and 259 deletions.
4 changes: 0 additions & 4 deletions app/controllers/claims_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ class ClaimsController < BasePublicController
include FormSubmittable
include ClaimsFormCallbacks

def current_data_object
current_claim
end

def timeout
end

Expand Down
8 changes: 1 addition & 7 deletions app/controllers/concerns/form_submittable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ module FormSubmittable
before_action :load_form_if_exists, only: [:show, :update, :create]
around_action :handle_form_submission, only: [:update, :create]

def current_data_object
# This is the instance of the main resource handled by the form object,
# and should always be defined in the controller.
nil
end

def new
redirect_to_first_slug
end
Expand Down Expand Up @@ -171,7 +165,7 @@ def log_event(callback_name)
end

def load_form_if_exists
@form ||= journey.form(claim: current_data_object, journey_session:, params:)
@form ||= journey.form(journey_session:, params:)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ class RemindersController < BasePublicController

private

# Wrapping `current_reminder` with an abstract method that is fed to the form object.
def current_data_object
current_reminder
# FIXME RL: Remove this once reminders are writing to the session
def load_form_if_exists
@form ||= AdditionalPaymentsForTeaching::FORMS.dig(
"reminders", params[:slug]
)&.new(claim: current_reminder, journey: Journeys::AdditionalPaymentsForTeaching, journey_session:, params:)
end

def claim_from_session
Expand Down
2 changes: 1 addition & 1 deletion app/forms/current_school_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class CurrentSchoolForm < Form
validates :current_school_id, presence: {message: i18n_error_message(:select_the_school_you_teach_at)}
validate :current_school_must_be_open, if: -> { current_school_id.present? }

def initialize(claim:, journey_session:, journey:, params:)
def initialize(journey_session:, journey:, params:)
super

load_schools
Expand Down
7 changes: 1 addition & 6 deletions app/forms/form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ class Form
include ActiveModel::Serialization
include ActiveModel::Validations::Callbacks

attr_accessor :claim
attr_accessor :journey
attr_accessor :journey_session
attr_accessor :params
Expand All @@ -20,16 +19,12 @@ def self.i18n_error_message(path)
end

# TODO RL: remove journey param and pull it from the journey_session
def initialize(claim:, journey_session:, journey:, params:)
def initialize(journey_session:, journey:, params:)
super

assign_attributes(attributes_with_current_value)
end

def update!(attrs)
claim.update!(attrs)
end

def view_path
journey::VIEW_PATH
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def current_school_id
private

def current_school
@current_school ||= journey_session.recent_tps_school || claim.school
@current_school ||= journey_session.recent_tps_school || answers.current_school
end

def change_school?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ def set_qualification!
QualificationForm.new(
journey: journey,
journey_session: journey_session,
claim: claim,
params: ActionController::Parameters.new(
claim: {
qualification: :postgraduate_itt
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ def self.model_name
self.one_time_password = one_time_password.gsub(/\D/, "")
end

# TODO RL: remove this and the initializer once reminders are writing
# to the session
attr_reader :claim

def initialize(claim:, journey_session:, journey:, params:)
@claim = claim
super(journey_session:, journey:, params:)

assign_attributes(attributes_with_current_value)
end

# TODO RL: remove this once reminders are writing to the session
def update!(attrs)
claim.update!(attrs)
end

def save
return false unless valid?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@ def self.model_name
ActiveModel::Name.new(Form)
end

# TODO RL: remove this and the initializer once reminders are writing
# to the session
attr_reader :claim

def initialize(claim:, journey_session:, journey:, params:)
@claim = claim
super(journey_session:, journey:, params:)

assign_attributes(attributes_with_current_value)
end

# TODO RL: remove this once reminders are writing to the session
def update!(attrs)
claim.update!(attrs)
end

def save
return false unless valid?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ClaimSchoolForm < Form
validates :claim_school_id, presence: {message: i18n_error_message(:select_a_school)}
validate :claim_school_must_exist, if: -> { claim_school_id.present? }

def initialize(claim:, journey_session:, journey:, params:)
def initialize(journey_session:, journey:, params:)
super

load_schools
Expand Down
2 changes: 1 addition & 1 deletion app/forms/personal_details_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def future?
validates :national_insurance_number, presence: {message: "Enter a National Insurance number in the correct format"}
validate :ni_number_is_correct_format

def initialize(claim:, journey_session:, journey:, params:)
def initialize(journey_session:, journey:, params:)
super
assign_date_attributes
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,7 @@ def self.start_page_url
private

def personal_details_form
# FIXME RL: forms expect a claim argument even thought they don't use it
# this will be removed as part of no longer creating a claim at the
# start of the journey work
PersonalDetailsForm.new(
claim: nil,
journey_session: journey_session,
journey: Journeys::AdditionalPaymentsForTeaching,
params: ActionController::Parameters.new
Expand Down
4 changes: 2 additions & 2 deletions app/models/journeys/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ def slug_sequence
self::SlugSequence
end

def form(claim:, journey_session:, params:)
def form(journey_session:, params:)
form = SHARED_FORMS.deep_merge(forms).dig(params[:controller].split("/").last, params[:slug])

form&.new(journey: self, journey_session:, claim:, params:)
form&.new(journey: self, journey_session:, params:)
end

def forms
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ def self.start_page_url

def personal_details_form
PersonalDetailsForm.new(
claim: nil,
journey_session: journey_session,
journey: Journeys::TeacherStudentLoanReimbursement,
params: ActionController::Parameters.new
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<% end %>

<%= form_for @form, url: claim_path(current_journey_routing_name) do |f| %>
<%= form_group_tag f.object.claim do %>
<%= form_group_tag f.object do %>

<%= f.hidden_field :eligible_itt_subject %>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
) %>
<% end %>

<%= form_group_tag f.object.claim do %>
<%= form_group_tag f.object do %>

<%= f.hidden_field :nqt_in_academic_year_after_itt %>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
<fieldset class="govuk-fieldset">
<legend class="govuk-fieldset__legend govuk-fieldset__legend--m"><%= t("questions.details_correct") %></legend>

<%= errors_tag f.object.claim, :qualifications_details_check %>
<%= errors_tag f.object, :qualifications_details_check %>

<div class="govuk-radios govuk-radios--inline" data-module="govuk-radios">
<div class="govuk-radios__item">
Expand Down
2 changes: 1 addition & 1 deletion app/views/claims/gender.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<div class="govuk-grid-column-two-thirds">
<%= render("shared/error_summary", instance: @form, errored_field_id_overrides: { payroll_gender: "claim_payroll_gender_female" }) if @form.errors.any? %>
<%= form_for @form, url: claim_path(current_journey_routing_name) do |f| %>
<%= form_group_tag @form.claim do %>
<%= form_group_tag @form do %>
<fieldset class="govuk-fieldset" aria-describedby="payroll_gender-hint" role="group">

<legend class="govuk-fieldset__legend <%= fieldset_legend_css_class_for_journey(journey) %>">
Expand Down
4 changes: 2 additions & 2 deletions app/views/claims/mobile_number.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

<%= form_for @form, url: claim_path(current_journey_routing_name) do |f| %>
<span class="govuk-caption-xl"><%= t("questions.personal_details") %></span>
<%= form_group_tag f.object.claim do %>
<%= form_group_tag f.object do %>
<h1 class="govuk-label-wrapper">
<%= f.label(
:mobile_number,
Expand All @@ -31,7 +31,7 @@
<%= f.text_field(
:mobile_number,
spellcheck: "false",
class: css_classes_for_input(f.object.claim, :mobile_number),
class: css_classes_for_input(f.object, :mobile_number),
"aria-describedby" => "mobile-number-hint"
) %>

Expand Down
2 changes: 1 addition & 1 deletion app/views/claims/select_mobile.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<%= form_for @form, url: claim_path(current_journey_routing_name) do |f| %>

<span class="govuk-caption-xl"><%= t("questions.personal_details") %></span>
<%= form_group_tag f.object.claim do %>
<%= form_group_tag f.object do %>

<fieldset class="govuk-fieldset" aria-describedby="phone-number-hint">

Expand Down
2 changes: 1 addition & 1 deletion app/views/claims/teacher_reference_number.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
:teacher_reference_number,
spellcheck: "false",
autocomplete: "off",
class: css_classes_for_input(f.object.claim, :teacher_reference_number, 'govuk-input--width-10'),
class: css_classes_for_input(f.object, :teacher_reference_number, 'govuk-input--width-10'),
"aria-describedby" => "teacher_reference_number-hint"
) %>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

<%= form_for @form, url: claim_path(current_journey_routing_name) do |f| %>
<%= f.hidden_field :qualifications_details_check %>
<%= form_group_tag f.object.claim do %>
<%= form_group_tag f.object do %>

<div class="govuk-radios">
<fieldset class="govuk-fieldset">
Expand Down
2 changes: 1 addition & 1 deletion app/views/student_loans/claims/still_teaching.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<div class="govuk-radios">
<%= form.hidden_field :employment_status %>

<%= render partial: "still_teaching_with_#{@form.tps_or_claim_school}", locals: { current_claim: @form.claim, form: form, school: @form.school } %>
<%= render partial: "still_teaching_with_#{@form.tps_or_claim_school}", locals: { form: form, school: @form.school } %>
</div>
</fieldset>
<% end %>
Expand Down
5 changes: 0 additions & 5 deletions spec/forms/address_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
RSpec.describe AddressForm, type: :model do
subject(:form) do
described_class.new(
claim: CurrentClaim.new(claims: [build(:claim)]),
journey: journey,
params: params,
journey_session: journey_session
Expand Down Expand Up @@ -102,10 +101,6 @@
end

describe "#save" do
before do
allow(form).to receive(:update!)
end

context "valid params" do
context "all required address lines provided" do
before { form.save }
Expand Down
1 change: 0 additions & 1 deletion spec/forms/bank_details_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

subject(:form) do
described_class.new(
claim: CurrentClaim.new(claims: [build(:claim)]),
journey_session: journey_session,
journey: journey,
params: ActionController::Parameters.new(slug:, claim: params)
Expand Down
1 change: 0 additions & 1 deletion spec/forms/bank_or_building_society_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

subject(:form) do
described_class.new(
claim: CurrentClaim.new(claims: [build(:claim)]),
journey_session: journey_session,
journey: journey,
params: ActionController::Parameters.new({slug:, claim: claim_params})
Expand Down
6 changes: 0 additions & 6 deletions spec/forms/current_school_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,12 @@
create(:journey_configuration, :additional_payments)
}

let(:current_claim) do
claims = journey::POLICIES.map { |policy| create(:claim, policy: policy) }
CurrentClaim.new(claims: claims)
end

let(:journey_session) { create(:"#{journey::I18N_NAMESPACE}_session") }

let(:slug) { "current-school" }

subject(:form) do
described_class.new(
claim: CurrentClaim.new(claims: [build(:claim)]),
journey_session: journey_session,
journey: journey,
params: params
Expand Down
5 changes: 0 additions & 5 deletions spec/forms/email_address_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@

RSpec.describe EmailAddressForm do
shared_examples "email_address_form" do |journey|
let(:claims) do
journey::POLICIES.map { |policy| create(:claim, policy: policy) }
end

let(:journey_session) do
create(
:"#{journey::I18N_NAMESPACE}_session",
Expand All @@ -28,7 +24,6 @@
described_class.new(
journey: journey,
journey_session: journey_session,
claim: current_claim,
params: params
)
end
Expand Down
7 changes: 0 additions & 7 deletions spec/forms/email_verification_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@

RSpec.describe EmailVerificationForm do
shared_examples "email_verification" do |journey|
let(:claims) do
journey::POLICIES.map { |policy| create(:claim, policy: policy) }
end

let(:current_claim) { CurrentClaim.new(claims: claims) }

let(:params) do
ActionController::Parameters.new(
claim: {
Expand All @@ -29,7 +23,6 @@
described_class.new(
journey: journey,
journey_session: journey_session,
claim: current_claim,
params: params
)
end
Expand Down
6 changes: 0 additions & 6 deletions spec/forms/employed_directly_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,10 @@

let(:journey_session) { create(:additional_payments_session) }

let(:current_claim) do
claims = journey::POLICIES.map { |policy| create(:claim, policy:) }
CurrentClaim.new(claims:)
end

let(:slug) { "employed-directly" }

subject(:form) do
described_class.new(
claim: current_claim,
journey_session:,
journey:,
params:
Expand Down
Loading

0 comments on commit b52d86c

Please sign in to comment.