CLDC-2670: Improve navigation flow when revisiting questions from CYA 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),
-                href: action_href(log, question.page.id),
+                href: action_href(question, log),
                 visually_hidden_text: question.check_answer_label.to_s.downcase,
               ) %>
             <% end %>

app/components/check_answers_summary_list_card_component.rb

@@ -28,8 +28,9 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base
     "Person #{question.check_answers_card_number}"
   end
 
-  def action_href(log, page_id, referrer = "check_answers")
-    send("#{log.model_name.param_key}_#{page_id}_path", log, referrer:)
+  def action_href(question, log)
+    referrer = question.displayed_as_answered?(log) ? "check_answers" : nil
+    send("#{log.model_name.param_key}_#{question.page.id}_path", log, referrer:)
   end
 
 private

app/controllers/form_controller.rb

@@ -9,6 +9,11 @@ class FormController < ApplicationController
   def submit_form
     if @log
       @page = form.get_page(params[@log.model_name.param_key][:page])
+      shown_page_ids_with_unanswered_questions_before_update = @page.subsection.pages
+                                                      .select { |page| page.routed_to?(@log, current_user) }
+                                                      .select { |page| page.has_unanswered_questions?(@log) }
+                                                      .map(&:id)
+
       responses_for_page = responses_for_page(@page)
       mandatory_questions_with_no_response = mandatory_questions_with_no_response(responses_for_page)
 
@@ -18,7 +23,9 @@ class FormController < ApplicationController
           updated_question_string = [updated_question&.question_number_string, updated_question&.check_answer_label.to_s.downcase].compact.join(": ")
           flash[:notice] = "You have successfully updated #{updated_question_string}"
         end
-        redirect_to(successful_redirect_path)
+
+        pages_requiring_update = pages_requiring_update(shown_page_ids_with_unanswered_questions_before_update)
+        redirect_to(successful_redirect_path(pages_requiring_update))
       else
         mandatory_questions_with_no_response.map do |question|
           @log.errors.add question.id.to_sym, question.unanswered_error_message, category: :not_answered
@@ -171,7 +178,7 @@ private
     params[@log.model_name.param_key]["interruption_page_referrer_type"].presence
   end
 
-  def successful_redirect_path
+  def successful_redirect_path(pages_to_check)
     if FeatureToggle.deduplication_flow_enabled?
       if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner")
         return correcting_duplicate_logs_redirect_path
@@ -195,7 +202,9 @@ private
       previous_page = form.previous_page_id(@page, @log, current_user)
 
       if next_page&.interruption_screen? || next_page_id == previous_page || CONFIRMATION_PAGE_IDS.include?(next_page_id)
-        return send("#{@log.class.name.underscore}_#{next_page_id}_path", @log, { referrer: "check_answers" })
+        return redirect_path_to_question(next_page, pages_to_check)
+      elsif pages_to_check.any?
+        return redirect_path_to_question(pages_to_check[0], pages_to_check)
       else
         return send("#{@log.model_name.param_key}_#{form.subsection_for_page(@page).id}_check_answers_path", @log)
       end
@@ -208,6 +217,24 @@ private
     send(redirect_path, @log)
   end
 
+  def redirect_path_to_question(page_to_show, unanswered_pages)
+    remaining_pages = unanswered_pages.excluding(page_to_show)
+    remaining_page_ids = remaining_pages.any? ? remaining_pages.map(&:id).join(",") : nil
+    send("#{@log.class.name.underscore}_#{page_to_show.id}_path", @log, { referrer: "check_answers", unanswered_pages: remaining_page_ids })
+  end
+
+  def pages_requiring_update(previously_visible_empty_page_ids)
+    return [] unless is_referrer_type?("check_answers")
+
+    currently_shown_pages = @page.subsection.pages
+                                    .select { |page| page.routed_to?(@log, current_user) }
+
+    existing_unanswered_pages = request.params["unanswered_pages"].nil? ? [] : request.params["unanswered_pages"].split(",")
+    currently_shown_pages
+                       .reject { |page| previously_visible_empty_page_ids.include?(page.id) && !existing_unanswered_pages.include?(page.id) }
+                       .select { |page| page.has_unanswered_questions?(@log) }
+  end
+
   def form
     @log&.form
   end

app/helpers/form_page_helper.rb

@@ -1,5 +1,10 @@
 module FormPageHelper
-  def action_href(log, page_id, referrer = "check_answers")
+  def check_answers_href(question, log)
+    referrer = question.displayed_as_answered?(log) ? "check_answers" : nil
+    action_href(log, question.page.id, referrer)
+  end
+
+  def action_href(log, page_id, referrer)
     send("#{log.model_name.param_key}_#{page_id}_path", log, referrer:)
   end
 

app/models/form/page.rb

@@ -36,6 +36,10 @@ class Form::Page
     end
   end
 
+  def has_unanswered_questions?(log)
+    questions.any? { |question| log[question.id].nil? }
+  end
+
   def interruption_screen?
     questions.all? { |question| question.type == "interruption_screen" }
   end

app/models/form/question.rb

@@ -110,12 +110,16 @@ class Form::Question
   end
 
   def action_text(log)
+    displayed_as_answered?(log) ? "Change" : "Answer"
+  end
+
+  def displayed_as_answered?(log)
     if is_derived_or_has_inferred_check_answers_value?(log)
-      "Change"
+      true
     elsif type == "checkbox"
-      answer_options.keys.any? { |key| value_is_yes?(log[key]) } ? "Change" : "Answer"
+      answer_options.keys.any? { |key| value_is_yes?(log[key]) }
     else
-      log[id].blank? ? "Answer" : "Change"
+      log[id].present?
     end
   end
 

app/views/form/_check_answers_summary_list.html.erb

@@ -28,7 +28,7 @@
       <% if @log.collection_period_open_for_editing? %>
         <% row.with_action(
           text: question.action_text(@log),
-          href: action_href(@log, question.page.id, referrer),
+          href: check_answers_href(question, @log),
           visually_hidden_text: question.check_answer_label.to_s.downcase,
         ) %>
       <% end %>

spec/helpers/form_page_helper_spec.rb

@@ -6,22 +6,36 @@ RSpec.describe FormPageHelper do
     let(:sales_log) { FactoryBot.create(:sales_log) }
 
     context "with a lettings log" do
-      it "has an update answer link href helper" do
-        expect(action_href(lettings_log, "net_income")).to eq("/lettings-logs/#{lettings_log.id}/net-income?referrer=check_answers")
+      let(:question) { lettings_log.form.questions.detect { |q| q.id == "needstype" } }
+
+      it "answer link href helper does not attach referrer when question not answered" do
+        expect(check_answers_href(question, lettings_log)).to eq("/lettings-logs/#{lettings_log.id}/needs-type")
+      end
+
+      it "answer link href helper attaches referrer when question already answered" do
+        lettings_log[question.id] = 1
+        expect(check_answers_href(question, lettings_log)).to eq("/lettings-logs/#{lettings_log.id}/needs-type?referrer=check_answers")
       end
 
-      it "returns a correct referrer in the url" do
-        expect(action_href(lettings_log, "retirement_value_check", "interruption_screen")).to eq("/lettings-logs/#{lettings_log.id}/retirement-value-check?referrer=interruption_screen")
+      it "has an action href helper" do
+        expect(action_href(lettings_log, "net_income", "interruption_screen")).to eq("/lettings-logs/#{lettings_log.id}/net-income?referrer=interruption_screen")
       end
     end
 
     context "with a sales log" do
-      it "has an update answer link href helper" do
-        expect(action_href(sales_log, "buyer_1_age")).to eq("/sales-logs/#{sales_log.id}/buyer-1-age?referrer=check_answers")
+      let(:question) { sales_log.form.questions.detect { |q| q.id == "ownershipsch" } }
+
+      it "answer link href helper does not attach referrer when question not answered" do
+        expect(check_answers_href(question, sales_log)).to eq("/sales-logs/#{sales_log.id}/ownership-scheme")
+      end
+
+      it "answer link href helper attaches referrer when question already answered" do
+        sales_log[question.id] = 1
+        expect(check_answers_href(question, sales_log)).to eq("/sales-logs/#{sales_log.id}/ownership-scheme?referrer=check_answers")
       end
 
-      it "returns a correct referrer in the url" do
-        expect(action_href(sales_log, "grant_value_check", "interruption_screen")).to eq("/sales-logs/#{sales_log.id}/grant-value-check?referrer=interruption_screen")
+      it "has an action href helper" do
+        expect(action_href(sales_log, "buyer_1_age", "interruption_screen")).to eq("/sales-logs/#{sales_log.id}/buyer-1-age?referrer=interruption_screen")
       end
     end
   end

spec/requests/form_controller_spec.rb

@@ -1036,6 +1036,13 @@ RSpec.describe FormController, type: :request do
             sales_log.saledate = Time.zone.local(2024, 12, 1)
             sales_log.save!(validate: false)
             sales_log.reload
+            Timecop.freeze(Time.zone.local(2024, 12, 1))
+            Singleton.__init__(FormHandler)
+          end
+
+          after do
+            Timecop.unfreeze
+            Singleton.__init__(FormHandler)
           end
 
           it "sets managing organisation to created by organisation" do
@@ -1326,6 +1333,57 @@ RSpec.describe FormController, type: :request do
       end
 
       context "when coming from check answers page" do
+        let(:sales_log) { create(:sales_log, ownershipsch: 3, created_by: user) }
+        let(:lettings_log_referrer) { "/lettings-logs/#{lettings_log.id}/needs-type?referrer=check_answers" }
+        let(:sales_log_referrer) { "/sales-logs/#{sales_log.id}/ownership-scheme?referrer=check_answers" }
+
+        let(:sales_log_form_ownership_params) do
+          {
+            id: sales_log.id,
+            sales_log: {
+              page: "ownership_scheme",
+              ownershipsch: 1,
+            },
+          }
+        end
+        let(:sales_log_form_shared_ownership_type_params) do
+          {
+            id: sales_log.id,
+            unanswered_pages: "joint_purchase",
+            sales_log: {
+              page: "shared_ownership_type",
+              type: 2,
+            },
+          }
+        end
+        let(:lettings_log_form_needs_type_params) do
+          {
+            id: lettings_log.id,
+            lettings_log: {
+              page: "needs_type",
+              needstype: 2,
+            },
+          }
+        end
+
+        context "and changing an answer" do
+          it "navigates to follow-up questions when required" do
+            post "/lettings-logs/#{lettings_log.id}/needs-type", params: lettings_log_form_needs_type_params, headers: headers.merge({ "HTTP_REFERER" => lettings_log_referrer })
+            expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/scheme?referrer=check_answers")
+          end
+
+          it "queues up additional follow-up questions if needed" do
+            post "/sales-logs/#{sales_log.id}/shared-ownership-type", params: sales_log_form_ownership_params, headers: headers.merge({ "HTTP_REFERER" => sales_log_referrer })
+            expect(response).to redirect_to("/sales-logs/#{sales_log.id}/shared-ownership-type?referrer=check_answers&unanswered_pages=joint_purchase")
+          end
+
+          it "moves to a queued up follow-up questions if one was provided" do
+            post "/sales-logs/#{sales_log.id}/shared-ownership-type", params: sales_log_form_ownership_params, headers: headers.merge({ "HTTP_REFERER" => sales_log_referrer })
+            post "/sales-logs/#{sales_log.id}/ownership-scheme", params: sales_log_form_shared_ownership_type_params, headers: headers.merge({ "HTTP_REFERER" => sales_log_referrer })
+            expect(response).to redirect_to("/sales-logs/#{sales_log.id}/joint-purchase?referrer=check_answers")
+          end
+        end
+
         context "and navigating to an interruption screen" do
           let(:interrupt_params) do
             {