Refactor check errors page

app/components/check_answers_summary_list_card_component.html.erb

@@ -36,7 +36,7 @@
             <% if @log.collection_period_open_for_editing? %>
               <% row.with_action(
                 text: question.action_text(log, correcting_hard_validation: @correcting_hard_validation),
-                href: action_href(question, log, correcting_hard_validation: @correcting_hard_validation),
+                href: @correcting_hard_validation ? correct_validation_action_href(question, log, applicable_questions.map(&:id)) : action_href(question, log),
                 visually_hidden_text: question.check_answer_label.to_s.downcase,
               ) %>
             <% end %>

app/components/check_answers_summary_list_card_component.rb

@@ -29,16 +29,16 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base
     "Person #{question.check_answers_card_number}"
   end
 
-  def action_href(question, log, correcting_hard_validation: false)
+  def action_href(question, log)
-    if correcting_hard_validation
+    referrer = question.displayed_as_answered?(log) ? "check_answers" : "check_answers_new_answer"
-      if question.displayed_as_answered?(log)
+    send("#{log.model_name.param_key}_#{question.page.id}_path", log, referrer:)
-        lettings_log_confirm_clear_answer_path(log, question_id: question.id, related_question_ids: request.query_parameters["related_question_ids"], original_page_id: request.query_parameters["original_page_id"])
+  end
-      else
+
-        send("#{log.model_name.param_key}_#{question.page.id}_path", log, referrer: "check_your_errors", related_question_ids: request.query_parameters["related_question_ids"], original_page_id: request.query_parameters["original_page_id"])
+  def correct_validation_action_href(question, log, related_question_ids)
-      end
+    if question.displayed_as_answered?(log)
+      lettings_log_confirm_clear_answer_path(log, question_id: question.id)
     else
-      referrer = question.displayed_as_answered?(log) ? "check_answers" : "check_answers_new_answer"
+      send("#{log.model_name.param_key}_#{question.page.id}_path", log, referrer: "check_errors", related_question_ids: request.query_parameters["related_question_ids"], original_page_id: request.query_parameters["original_page_id"])
-      send("#{log.model_name.param_key}_#{question.page.id}_path", log, referrer:)
     end
   end
 

app/controllers/form_controller.rb

@@ -30,10 +30,16 @@ class FormController < ApplicationController
         mandatory_questions_with_no_response.map do |question|
           @log.errors.add question.id.to_sym, question.unanswered_error_message, category: :not_answered
         end
-        Rails.logger.info "User triggered validation(s) on: #{@log.errors.map(&:attribute).join(', ')}"
+        error_attributes = @log.errors.map(&:attribute)
+        Rails.logger.info "User triggered validation(s) on: #{error_attributes.join(', ')}"
         @subsection = form.subsection_for_page(@page)
         restore_error_field_values(@page&.questions)
-        render "form/page"
+        if params["check_errors"]
+          @questions = @log.form.questions.select { |q| error_attributes.include?(q.id.to_sym) }
+          render "form/check_errors"
+        else
+          render "form/page"
+        end
       end
     else
       render_not_found

app/helpers/form_page_error_helper.rb

@@ -7,8 +7,4 @@ module FormPageErrorHelper
   def all_questions_affected_by_errors(log)
     log.errors.map(&:attribute) - [:base]
   end
-
-  def check_your_errors_link(log, related_question_ids, original_page_id)
-    govuk_link_to "See all related answers", lettings_log_check_your_errors_path(log, related_question_ids:, original_page_id:), method: :post
-  end
 end

app/views/form/check_errors.html.erb

@@ -1,11 +1,15 @@
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-three-quarters-from-desktop">
+      <%= form_with model: @log, url: request.original_url, method: "post", local: true do |f| %>
+          <%= f.govuk_error_summary %>
+    <% end %>
+
     <h1 class="govuk-heading-m govuk-!-margin-bottom-6">
       <span class="govuk-grid-column-two-thirds">
         Make sure these answers are correct:
       </span>
       <span class="govuk-body govuk-!-text-align-right govuk-grid-column-one-third">
-        <%= govuk_link_to "Clear all", lettings_log_confirm_clear_all_answers_path(@log, related_question_ids: request.query_parameters["related_question_ids"], original_page_id: request.query_parameters["original_page_id"]) %>
+        <%= govuk_link_to "Clear all", lettings_log_confirm_clear_all_answers_path(@log) %>
       </span>
     </h1>
 

app/views/form/page.html.erb

@@ -73,7 +73,7 @@
 
       <% if all_questions_with_errors.count > 1 %>
         <div class="govuk-button-group">
-          <%= check_your_errors_link(@log, all_questions_with_errors, @page.id) %>
+          <%= f.govuk_submit "See all related answers", :name => "check_errors", secondary: true %>
         </div>
       <% end %>
 

config/routes.rb

@@ -212,9 +212,9 @@ Rails.application.routes.draw do
     get "delete-confirmation", to: "lettings_logs#delete_confirmation"
     get "duplicate-logs", to: "duplicate_logs#show"
     get "delete-duplicates", to: "duplicate_logs#delete_duplicates"
-    get "check-your-errors", to: "check_your_errors#index"
+    post "confirm-clear-answer", to: "check_errors#confirm_clear_answer"
-    get "confirm-clear-answer", to: "check_your_errors#confirm_clear_answer"
+    post "confirm-clear-all-answers", to: "check_errors#confirm_clear_all_answers"
-    get "confirm-clear-all-answers", to: "check_your_errors#confirm_clear_all_answers"
+    post "clear-answer", to: "check_errors#clear_answer"
 
     collection do
       get "csv-download", to: "lettings_logs#download_csv"

spec/requests/check_errors_controller_spec.rb

@@ -0,0 +1,108 @@
+require "rails_helper"
+
+RSpec.describe CheckErrorsController, type: :request do
+  let(:page) { Capybara::Node::Simple.new(response.body) }
+  let(:user) { create(:user, :data_coordinator) }
+  let(:lettings_log) { create(:lettings_log, :duplicate, assigned_to: user) }
+
+  describe "check errors page" do
+    context "when user is not signed in" do
+      it "redirects to sign in page" do
+        post "/lettings-logs/#{lettings_log.id}/net-income", params: {}
+        expect(response).to redirect_to("/account/sign-in")
+      end
+    end
+
+    context "when the user is from different organisation" do
+      let(:other_user) { create(:user) }
+
+      before do
+        sign_in other_user
+      end
+
+      it "renders page not found" do
+        post "/lettings-logs/#{lettings_log.id}/net-income", params: {}
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "when user is signed in" do
+      let(:params) do
+        {
+          id: lettings_log.id,
+          lettings_log: {
+            page: "income_amount",
+            earnings: "100000",
+            incfreq: "1",
+          },
+          check_errors: "",
+        }
+      end
+
+      before do
+        lettings_log.update(needstype: 1, declaration: 1, ecstat1: 10, hhmemb: 2, net_income_known: 0, incfreq: 1, earnings: 1000)
+      end
+
+      context "with multiple error fields and answered questions" do
+        before do
+          sign_in user
+          post "/lettings-logs/#{lettings_log.id}/income-amount", params: params
+        end
+
+        it "displays correct clear links" do
+          expect(page).to have_link("Clear", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?question_id=hhmemb")
+          expect(page).to have_link("Clear", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?question_id=ecstat1")
+          expect(page).to have_link("Clear", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?question_id=earnings")
+          expect(page).to have_link("Clear all", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-all-answers")
+        end
+      end
+    end
+  end
+
+  describe "confirm clear answer page" do
+    context "when user is not signed in" do
+      it "redirects to sign in page" do
+        get "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?original_page_id=income_amount&question_id=hhmemb&related_question_ids%5B%5D=hhmemb&related_question_ids%5B%5D=ecstat1&related_question_ids%5B%5D=earnings", params: {}
+        expect(response).to redirect_to("/account/sign-in")
+      end
+    end
+
+    context "when the user is from different organisation" do
+      let(:other_user) { create(:user) }
+
+      before do
+        sign_in other_user
+      end
+
+      it "renders page not found" do
+        get "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?original_page_id=income_amount&question_id=hhmemb&related_question_ids%5B%5D=hhmemb&related_question_ids%5B%5D=ecstat1&related_question_ids%5B%5D=earnings", params: {}
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "when user is signed in" do
+      context "with multiple error fields and answered questions" do
+        before do
+          sign_in user
+          get "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?original_page_id=income_amount&question_id=hhmemb&related_question_ids%5B%5D=hhmemb&related_question_ids%5B%5D=ecstat1&related_question_ids%5B%5D=earnings", params: {}
+        end
+
+        it "displays correct clear links" do
+          expect(page).to have_content("Are you sure you want to clear Number of household members answer?")
+          expect(page).to have_content("This action is permanent")
+          expect(page).to have_button("Cancel", href: "/lettings-logs/#{lettings_log.id}/income-amount")
+          expect(page).to have_button("Confirm and continue", href: "/lettings-logs/#{lettings_log.id}/clear-answer?original_page_id=income_amount&question_id=hhmemb&related_question_ids%5B%5D=hhmemb&related_question_ids%5B%5D=ecstat1&related_question_ids%5B%5D=earnings")
+        end
+      end
+    end
+  end
+
+  describe "confirm clear all answers page" do
+  end
+
+  describe "clear answer" do
+  end
+
+  describe "clear all answers" do
+  end
+end

spec/requests/check_your_errors_controller_spec.rb

@@ -1,69 +0,0 @@
-require "rails_helper"
-
-RSpec.describe CheckYourErrorsController, type: :request do
-  let(:page) { Capybara::Node::Simple.new(response.body) }
-  let(:user) { create(:user, :data_coordinator) }
-  let(:lettings_log) { create(:lettings_log, :duplicate, assigned_to: user) }
-
-  describe "check your errors page" do
-    context "when user is not signed in" do
-      it "redirects to sign in page" do
-        get "/lettings-logs/#{lettings_log.id}/check-your-errors"
-        expect(response).to redirect_to("/account/sign-in")
-      end
-    end
-
-    context "when the user is from different organisation" do
-      let(:other_user) { create(:user) }
-
-      before do
-        sign_in other_user
-      end
-
-      it "renders page not found" do
-        get "/lettings-logs/#{lettings_log.id}/check-your-errors"
-        expect(response).to have_http_status(:not_found)
-      end
-    end
-
-    context "when user is signed in" do
-      context "with multiple error fields and answered questions" do
-        before do
-          sign_in user
-          get "/lettings-logs/#{lettings_log.id}/check-your-errors?related_question_ids[]=startdate&related_question_ids[]=needstype&original_page_id=tenancy_start_date"
-        end
-
-        it "displays correct clear links" do
-          expect(page).to have_link("Clear", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?original_page_id=tenancy_start_date&question_id=startdate&related_question_ids%5B%5D=startdate&related_question_ids%5B%5D=needstype")
-          expect(page).to have_link("Clear", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?original_page_id=tenancy_start_date&question_id=needstype&related_question_ids%5B%5D=startdate&related_question_ids%5B%5D=needstype")
-          expect(page).to have_link("Clear all", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-all-answers?original_page_id=tenancy_start_date&related_question_ids%5B%5D=startdate&related_question_ids%5B%5D=needstype")
-        end
-      end
-
-      context "with multiple error fields and unanswered questions" do
-        before do
-          lettings_log.update!(needstype: nil, startdate: nil)
-          sign_in user
-          get "/lettings-logs/#{lettings_log.id}/check-your-errors?related_question_ids[]=startdate&related_question_ids[]=needstype&original_page_id=tenancy_start_date"
-        end
-
-        it "displays correct clear links" do
-          expect(page).to have_link("Answer", href: "/lettings-logs/#{lettings_log.id}/needs-type?original_page_id=tenancy_start_date&referrer=check_your_errors&related_question_ids%5B%5D=startdate&related_question_ids%5B%5D=needstype")
-          expect(page).to have_link("Answer", href: "/lettings-logs/#{lettings_log.id}/tenancy-start-date?original_page_id=tenancy_start_date&referrer=check_your_errors&related_question_ids%5B%5D=startdate&related_question_ids%5B%5D=needstype")
-        end
-      end
-    end
-  end
-
-  describe "confirm clear answer page" do
-  end
-
-  describe "confirm clear all answers page" do
-  end
-
-  describe "clear answer" do
-  end
-
-  describe "clear all answers" do
-  end
-end