Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove claim as a form argument #2854

Merged
merged 2 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,10 @@ class RemindersController < BasePublicController

private

# Wrapping `current_reminder` with an abstract method that is fed to the form object.
def current_data_object
current_reminder
def load_form_if_exists
@form ||= AdditionalPaymentsForTeaching::FORMS.dig(
"reminders", params[:slug]
)&.new(reminder: 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 @@ -6,7 +6,7 @@ class EmailVerificationForm < Form
attribute :sent_one_time_password_at

# Required for shared partial in the view
delegate :email_address, to: :claim
delegate :email_address, to: :answers

validate :sent_one_time_password_must_be_valid
validate :otp_must_be_valid, if: :sent_one_time_password_at?
Expand All @@ -19,10 +19,19 @@ def self.model_name
self.one_time_password = one_time_password.gsub(/\D/, "")
end

attr_reader :reminder

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

assign_attributes(attributes_with_current_value)
end

def save
return false unless valid?

update!(email_verified: true)
reminder.update!(email_verified: true)
end

private
Expand All @@ -49,19 +58,7 @@ def sent_one_time_password_at?
end

def load_current_value(attribute)
# TODO: re-implement when the underlying claim and eligibility data sources
# are moved to an alternative place e.g. a session hash

# Some, but not all attributes are present directly on the claim record.
return claim.public_send(attribute) if claim.has_attribute?(attribute)

# At the moment, some attributes are unique to a policy eligibility record,
# so we need to loop through all the claims in the wrapper and check each
# eligibility individually; if the search fails, it should return `nil`.
claim.claims.each do |c|
return c.eligibility.public_send(attribute) if c.eligibility.has_attribute?(attribute)
end
nil
reminder.public_send(attribute) if reminder.has_attribute?(attribute)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,19 @@ def self.model_name
ActiveModel::Name.new(Form)
end

attr_reader :reminder

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

assign_attributes(attributes_with_current_value)
end

def save
return false unless valid?

update!(attributes)
reminder.update!(attributes)
end

private
Expand All @@ -29,19 +38,7 @@ def i18n_form_namespace
end

def load_current_value(attribute)
# TODO: re-implement when the underlying claim and eligibility data sources
# are moved to an alternative place e.g. a session hash

# Some, but not all attributes are present directly on the claim record.
return claim.public_send(attribute) if claim.has_attribute?(attribute)

# At the moment, some attributes are unique to a policy eligibility record,
# so we need to loop through all the claims in the wrapper and check each
# eligibility individually; if the search fails, it should return `nil`.
claim.claims.each do |c|
return c.eligibility.public_send(attribute) if c.eligibility.has_attribute?(attribute)
end
nil
reminder.public_send(attribute) if reminder.has_attribute?(attribute)
end
end
end
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
Loading