Merge pull request #3516 from DFE-Digital/rm-callbacks

[CAPT-2145] Refactor controller callbacks

app/controllers/claims_form_callbacks.rb

@@ -1,50 +1,8 @@
 module ClaimsFormCallbacks
-  def current_school_before_show
-    set_backlink_override_to_current_slug if on_school_search_results?
-  end
-
-  def claim_school_before_show
-    set_backlink_override_to_current_slug if on_school_search_results?
-  end
-
   def teaching_subject_now_before_show
     redirect_to_slug("eligible-itt-subject") if no_eligible_itt_subject?
   end
 
-  def qualification_details_before_show
-    redirect_to_next_slug if no_dqt_data?
-  end
-
-  def address_before_show
-    set_backlink_override(slug: "postcode-search") if no_postcode?
-  end
-
-  def select_home_address_before_show
-    set_backlink_override(slug: "postcode-search")
-  end
-
-  def personal_bank_account_before_update
-    inject_hmrc_validation_attempt_count_into_the_form
-  end
-
-  def information_provided_before_update
-    return unless journey.requires_student_loan_details?
-
-    retrieve_student_loan_details if on_tid_route?
-  end
-
-  def personal_details_after_form_save_success
-    return redirect_to_next_slug unless journey.requires_student_loan_details?
-
-    retrieve_student_loan_details
-    redirect_to_next_slug
-  end
-
-  def personal_bank_account_after_form_save_failure
-    increment_hmrc_validation_attempt_count if hmrc_api_validation_attempted?
-    render_template_for_current_slug
-  end
-
   def check_your_email_after_form_save_success
     @email_resent = true
     render("check_your_email")
@@ -54,59 +12,9 @@ def check_your_answers_after_form_save_success
     create_and_save_claim_form
   end
 
-  def employee_email_after_form_save_failure
-    session[:slugs].delete("employee-email")
-    render_template_for_current_slug
-  end
-
   private
 
-  def set_backlink_override_to_current_slug
-    set_backlink_override(slug: current_slug)
-  end
-
-  def set_backlink_override(slug:)
-    @backlink_path = claim_path(current_journey_routing_name, slug) if page_sequence.in_sequence?(slug)
-  end
-
-  def on_school_search_results?
-    params[:school_search]&.present?
-  end
-
   def no_eligible_itt_subject?
     !journey_session.answers.eligible_itt_subject
   end
-
-  def no_dqt_data?
-    journey_session.answers.has_no_dqt_data_for_claim?
-  end
-
-  def no_postcode?
-    !journey_session.answers.postcode
-  end
-
-  def retrieve_student_loan_details
-    journey::AnswersStudentLoansDetailsUpdater.call(journey_session)
-  end
-
-  def hmrc_api_validation_attempted?
-    @form&.hmrc_api_validation_attempted?
-  end
-
-  def inject_hmrc_validation_attempt_count_into_the_form
-    params[:claim][:hmrc_validation_attempt_count] = current_hmrc_validation_attempt_count
-  end
-
-  def increment_hmrc_validation_attempt_count
-    journey_session.answers.hmrc_validation_attempt_count = current_hmrc_validation_attempt_count + 1
-    journey_session.save!
-  end
-
-  def current_hmrc_validation_attempt_count
-    journey_session.answers.hmrc_validation_attempt_count || 0
-  end
-
-  def on_tid_route?
-    journey_session.answers.logged_in_with_tid? && journey_session.answers.all_personal_details_same_as_tid?
-  end
 end

app/forms/bank_details_form.rb

@@ -3,7 +3,6 @@ class BankDetailsForm < Form
   MAX_HMRC_API_VALIDATION_ATTEMPTS = 3
   BANKING_NAME_REGEX_FILTER = /\A[0-9A-Za-z .\/&-]*\z/
 
-  attribute :hmrc_validation_attempt_count, :integer
   attribute :banking_name, :string
   attribute :bank_sort_code, :string
   attribute :bank_account_number, :string
@@ -48,6 +47,10 @@ def hmrc_api_validation_succeeded?
 
   private
 
+  def hmrc_validation_attempt_count
+    journey_session.answers.hmrc_validation_attempt_count
+  end
+
   def normalised_bank_detail(bank_detail)
     bank_detail&.gsub(/\s|-/, "")
   end
@@ -61,6 +64,7 @@ def bank_sort_code_must_be_six_digits
   end
 
   def bank_account_is_valid
+    return if @bank_account_is_valid_processed
     return unless can_validate_with_hmrc_api?
 
     response = nil
@@ -71,20 +75,28 @@ def bank_account_is_valid
       @hmrc_api_validation_attempted = true
       @hmrc_api_validation_succeeded = true if response.success?
 
+      if !response.success?
+        journey_session.answers.increment_hmrc_validation_attempt_count
+      end
+
       unless met_maximum_attempts?
         errors.add(:bank_sort_code, i18n_errors_path(:invalid_sort_code)) unless response.sort_code_correct?
         errors.add(:bank_account_number, i18n_errors_path(:invalid_account_number)) if response.sort_code_correct? && !response.account_exists?
         errors.add(:banking_name, i18n_errors_path(:invalid_banking_name)) if response.sort_code_correct? && response.account_exists? && !response.name_match?
       end
     rescue Hmrc::ResponseError => e
+      journey_session.answers.increment_hmrc_validation_attempt_count
       response = e.response
       @hmrc_api_response_error = true
     ensure
       new_hmrc_bank_validation_responses_value = journey_session.answers.hmrc_bank_validation_responses.dup << {code: response.code, body: response.body}
       journey_session.answers.assign_attributes(
         hmrc_bank_validation_responses: new_hmrc_bank_validation_responses_value
       )
+
       journey_session.save!
+
+      @bank_account_is_valid_processed = true
     end
   end
 
@@ -93,10 +105,10 @@ def can_validate_with_hmrc_api?
   end
 
   def within_maximum_attempts?
-    hmrc_validation_attempt_count + 1 <= MAX_HMRC_API_VALIDATION_ATTEMPTS
+    hmrc_validation_attempt_count <= MAX_HMRC_API_VALIDATION_ATTEMPTS
   end
 
   def met_maximum_attempts?
-    hmrc_validation_attempt_count + 1 >= MAX_HMRC_API_VALIDATION_ATTEMPTS
+    hmrc_validation_attempt_count >= MAX_HMRC_API_VALIDATION_ATTEMPTS
   end
 end

app/forms/information_provided_form.rb

@@ -0,0 +1,19 @@
+class InformationProvidedForm < Form
+  def save
+    if journey.requires_student_loan_details? && on_tid_route?
+      retrieve_student_loan_details
+    end
+
+    true
+  end
+
+  private
+
+  def on_tid_route?
+    journey_session.answers.logged_in_with_tid? && journey_session.answers.all_personal_details_same_as_tid?
+  end
+
+  def retrieve_student_loan_details
+    journey::AnswersStudentLoansDetailsUpdater.call(journey_session)
+  end
+end

app/forms/personal_details_form.rb

@@ -49,7 +49,7 @@ def date_of_birth
   end
 
   def save
-    return false unless valid?
+    return false if invalid?
 
     journey_session.answers.assign_attributes(
       first_name:,
@@ -60,8 +60,11 @@ def save
     )
 
     reset_dependent_answers_attributes
-
     journey_session.save!
+
+    if journey.requires_student_loan_details?
+      journey::AnswersStudentLoansDetailsUpdater.call(journey_session)
+    end
   end
 
   def show_name_section?

app/models/journeys/answers_student_loans_details_updater.rb

@@ -1,7 +1,8 @@
 module Journeys
   class AnswersStudentLoansDetailsUpdater
     def self.call(journey_session)
-      new(journey_session).save!
+      instance = new(journey_session)
+      instance.save!
     end
 
     def initialize(journey_session)

app/models/journeys/base.rb

@@ -5,6 +5,7 @@ module Base
         "sign-in" => SignInForm,
         "sign-in-or-continue" => SignInOrContinueForm,
         "current-school" => CurrentSchoolForm,
+        "information-provided" => InformationProvidedForm,
         "gender" => GenderForm,
         "personal-details" => PersonalDetailsForm,
         "select-email" => SelectEmailForm,

app/models/journeys/session_answers.rb

@@ -60,14 +60,18 @@ class SessionAnswers
     attribute :student_loan_plan, :string, pii: false
     attribute :submitted_using_slc_data, :boolean, pii: false
     attribute :sent_one_time_password_at, :datetime, pii: false
-    attribute :hmrc_validation_attempt_count, :integer, pii: false
+    attribute :hmrc_validation_attempt_count, :integer, default: 0, pii: false
     attribute :reminder_id, :string, pii: false
 
     attribute :reminder_full_name, :string, pii: true
     attribute :reminder_email_address, :string, pii: true
     attribute :reminder_otp_secret, :string, pii: true
     attribute :reminder_otp_confirmed, :boolean, default: false, pii: false # whether or not they have confirmed email via otp
 
+    def increment_hmrc_validation_attempt_count
+      self.hmrc_validation_attempt_count = attributes["hmrc_validation_attempt_count"] + 1
+    end
+
     def has_attribute?(name)
       attribute_names.include?(name.to_s)
     end

app/views/claims/address.html.erb

@@ -4,6 +4,10 @@
   <%= govuk_notification_banner(title_text: "Notice") { |nb| nb.with_heading(text: "Please enter your address manually") } %>
 <% end %>
 
+<% if !journey_session.answers.postcode %>
+  <% @backlink_path = claim_path(current_journey_routing_name, "postcode-search") %>
+<% end %>
+
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <%= form_for @form, url: claim_path(current_journey_routing_name), builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %>

app/views/claims/current_school.html.erb

@@ -1,3 +1,7 @@
+<% if params[:school_search]&.present? %>
+  <% @backlink_path = claim_path(current_journey_routing_name, @page_sequence.current_slug) %>
+<% end %>
+
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <% if @form.schools.blank? %>

app/views/student_loans/claims/claim_school.html.erb

@@ -1,3 +1,7 @@
+<% if params[:school_search]&.present? %>
+  <% @backlink_path = claim_path(current_journey_routing_name, @page_sequence.current_slug) %>
+<% end %>
+
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <% if @form.schools.blank? %>

spec/features/hmrc_bank_validation_spec.rb

@@ -118,13 +118,10 @@ def get_to_bank_details_page
         fill_in "Name on your account", with: bank_name
         fill_in "Sort code", with: sort_code
         fill_in "Account number", with: "1111111" # Invalid number
-
         click_on "Continue"
 
         expect(page).to have_text "Account number must be 8 digits"
-
         fill_in "Account number", with: account_number
-
         click_on "Continue"
 
         expect(page).to have_text(I18n.t("forms.gender.questions.payroll_gender"))
@@ -146,15 +143,12 @@ def get_to_bank_details_page
         fill_in "Name on your account", with: bank_name
         fill_in "Sort code", with: sort_code
         fill_in "Account number", with: account_number
-
         click_on "Continue"
 
         expect(page).to have_text "Enter the account number associated with the name on the account and/or sort code"
-
         click_on "Continue"
 
         expect(page).to have_text "Enter the account number associated with the name on the account and/or sort code"
-
         click_on "Continue"
 
         # Third attempt succeeds.

spec/forms/bank_details_form_spec.rb

@@ -7,11 +7,14 @@
       create(:journey_configuration, :additional_payments)
     }
 
+    let(:hmrc_validation_attempt_count) { 0 }
+
     let(:journey_session) do
       create(
         :"#{journey::I18N_NAMESPACE}_session",
         answers: attributes_for(
-          :"#{journey::I18N_NAMESPACE}_answers"
+          :"#{journey::I18N_NAMESPACE}_answers",
+          hmrc_validation_attempt_count:
         )
       )
     end
@@ -21,8 +24,7 @@
       {
         banking_name:,
         bank_sort_code:,
-        bank_account_number:,
-        hmrc_validation_attempt_count:
+        bank_account_number:
       }
     end
 
@@ -37,7 +39,6 @@
     let(:banking_name) { "Jo Bloggs" }
     let(:bank_sort_code) { rand(100000..999999) }
     let(:bank_account_number) { rand(10000000..99999999) }
-    let(:hmrc_validation_attempt_count) { 0 }
 
     describe "#valid?" do
       context "banking name with invalid characters" do

spec/forms/personal_details_form_spec.rb

@@ -420,12 +420,10 @@
         journey_session.save!
       end
 
-      it "resets tslr specific depenent answers" do
+      it "resets tslr specific dependent answers" do
         form.save
 
-        answers = journey_session.answers
-
-        expect(answers.student_loan_repayment_amount).to be nil
+        expect(journey_session.reload.answers.student_loan_repayment_amount).to be_zero
       end
     end
   end