Merge pull request #1807 from alphagov/show-none-of-the-above-question-for-autocomplete

Show none of the above question on a separate page for long list selection questions

Author: stephencdaly

app/components/check_your_answers_component/view.rb

@@ -29,7 +29,7 @@ def step_to_row(step)
     def none_of_the_above_answer_row(step)
       question_name = step.question.none_of_the_above_question_text
       answer = step.question.none_of_the_above_answer
-      row(question_name:, answer:, change_link: change_link(step))
+      row(question_name:, answer:, change_link: none_of_the_above_change_link(step))
     end
 
     def row(question_name:, answer:, change_link:)
@@ -55,5 +55,13 @@ def change_link(step)
         form_change_answer_path(mode: @mode, form_id: @form.id, form_slug: @form.form_slug, page_slug: step.id)
       end
     end
+
+    def none_of_the_above_change_link(step)
+      if step.autocomplete_selection_question?
+        change_selection_none_of_the_above_path(mode: @mode, form_id: @form.id, form_slug: @form.form_slug, page_slug: step.id)
+      else
+        form_change_answer_path(mode: @mode, form_id: @form.id, form_slug: @form.form_slug, page_slug: step.id)
+      end
+    end
   end
 end

app/controllers/forms/page_controller.rb

@@ -10,18 +10,22 @@ def set_request_logging_attributes
 
     def show
       return redirect_to form_page_path(@form.id, @form.form_slug, current_context.next_page_slug) unless current_context.can_visit?(@step.id)
-      return redirect_to review_file_page if answered_file_question?
+      return redirect_to review_file_page if @step.answered_file_question?
 
-      back_link(@step.id)
       setup_instance_vars_for_view
     end
 
     def save
       page_params = params.fetch(:question, {}).permit(*@step.params)
-      @step.update!(page_params)
+      @step.assign_question_attributes(page_params)
 
-      if current_context.save_step(@step)
-        current_context.clear_submission_details if is_first_page?
+      current_context.clear_submission_details if is_first_page?
+
+      validation_context = @step.autocomplete_selection_question? ? :skip_none_of_the_above_question_validation : nil
+      if current_context.save_step(@step, context: validation_context)
+        # Redirect before logging when the question has multiple pages so that we don't send multiple form started
+        # metrics to CloudWatch if this is the first question.
+        return redirect_to selection_none_of_the_above_page if redirect_to_none_of_the_above_page?
 
         unless mode.preview?
           LogEventService.new(current_context, @step, request, changing_existing_answer, page_params).log_page_save
@@ -56,46 +60,61 @@ def answer_index
     end
 
     def setup_instance_vars_for_view
-      @is_question = true
-      @question_edit_link = "#{Settings.forms_admin.base_url}/forms/#{@form.id}/pages-by-external-id/#{@step.id}/edit-question"
+      @question_edit_link = question_edit_link
       @save_url = save_url
+      @back_link = back_link(@step.id)
+    end
+
+    def question_edit_link
+      "#{Settings.forms_admin.base_url}/forms/#{@form.id}/pages-by-external-id/#{@step.id}/edit-question"
+    end
+
+    def save_url
+      save_form_page_path(@form.id, @form.form_slug, @step.id, changing_existing_answer: @changing_existing_answer, answer_index:)
     end
 
     def changing_existing_answer
       @changing_existing_answer = ActiveModel::Type::Boolean.new.cast(params[:changing_existing_answer])
     end
 
     def back_link(page_slug)
+      return check_your_answers_path(form_id: current_context.form.id) if changing_existing_answer
+
       previous_step = current_context.previous_step(page_slug)
+      return nil unless previous_step
 
-      if changing_existing_answer
-        @back_link = check_your_answers_path(form_id: current_context.form.id)
-      elsif previous_step
-        @back_link = previous_step.repeatable? ? add_another_answer_path(form_id: current_context.form.id, form_slug: current_context.form.form_slug, page_slug: previous_step.id) : form_page_path(@form.id, @form.form_slug, previous_step.page_id)
+      if previous_step.repeatable?
+        add_another_answer_path(form_id: current_context.form.id, form_slug: current_context.form.form_slug, page_slug: previous_step.id)
+      else
+        form_page_path(@form.id, @form.form_slug, previous_step.page_id)
       end
     end
 
     def redirect_post_save
-      return redirect_to review_file_page, success: t("banner.success.file_uploaded") if answered_file_question?
+      return redirect_to review_file_page, success: t("banner.success.file_uploaded") if @step.answered_file_question?
       return redirect_to exit_page_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id) if @step.exit_page_condition_matches?
 
       redirect_to next_page
     end
 
     def redirect_if_not_answered_file_question
-      unless @step.question.is_a?(Question::File) && @step.question.file_uploaded?
+      unless @step.answered_file_question?
         redirect_to form_page_path(@form.id, @form.form_slug, @step.id)
       end
     end
 
-    def answered_file_question?
-      @step.question.is_a?(Question::File) && @step.question.file_uploaded?
+    def redirect_to_none_of_the_above_page?
+      @step.autocomplete_selection_question? && @step.question.show_none_of_the_above_question?
     end
 
     def review_file_page
       review_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, changing_existing_answer:)
     end
 
+    def selection_none_of_the_above_page
+      selection_none_of_the_above_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id)
+    end
+
     def next_page
       if changing_existing_answer
         return next_step_changing
@@ -166,9 +185,5 @@ def admin_edit_condition_url(form_id, page_id)
     def is_first_page?
       current_context.form.start_page.to_s == @step.id
     end
-
-    def save_url
-      save_form_page_path(@form.id, @form.form_slug, @step.id, changing_existing_answer: @changing_existing_answer, answer_index:)
-    end
   end
 end

app/controllers/forms/selection_none_of_the_above_controller.rb

@@ -0,0 +1,49 @@
+module Forms
+  class SelectionNoneOfTheAboveController < PageController
+    before_action :redirect_if_not_show_none_of_the_above_question
+
+    def show
+      return redirect_to form_page_path(@form.id, @form.form_slug, current_context.next_page_slug) unless current_context.can_visit?(@step.id)
+
+      setup_instance_vars_for_view
+    end
+
+    def save
+      page_params = params.fetch(:question, {}).permit(*@step.params)
+      @step.question.with_none_of_the_above_selected
+      @step.assign_question_attributes(page_params)
+
+      if current_context.save_step(@step)
+        unless mode.preview?
+          LogEventService.new(current_context, @step, request, changing_existing_answer, page_params).log_page_save
+        end
+
+        redirect_to next_page
+      else
+        setup_instance_vars_for_view
+        render :show, status: :unprocessable_content
+      end
+    end
+
+    def redirect_if_not_show_none_of_the_above_question
+      unless @step.question.try(:has_none_of_the_above_question?) && @step.question.try(:autocomplete_component?)
+        redirect_to form_page_path(@form.id, @form.form_slug, @step.id)
+      end
+    end
+
+  private
+
+    def setup_instance_vars_for_view
+      @back_link = back_link
+      @question_edit_link = question_edit_link
+    end
+
+    def back_link
+      if changing_existing_answer
+        check_your_answers_path(form_id: current_context.form.id)
+      else
+        form_page_path(@form.id, @form.form_slug, @step.id)
+      end
+    end
+  end
+end

app/lib/flow/context.rb

@@ -14,8 +14,8 @@ def initialize(form:, store:)
     delegate :clear_stored_answer, :clear, :form_submitted?, :answers, to: :answer_store
     delegate :save_submission_details, :get_submission_reference, :requested_email_confirmation?, :clear_submission_details, to: :confirmation_details_store
 
-    def save_step(step)
-      return false unless step.valid?
+    def save_step(step, context: nil)
+      return false unless step.valid?(context)
 
       step.save_to_store(@answer_store)
     end

app/models/question/selection.rb

@@ -10,14 +10,28 @@ class Selection < QuestionBase
     validate :selection, :validate_radio, unless: :allow_multiple_answers?
     validates :none_of_the_above_answer, length: { maximum: 499 }
 
-    with_options unless: :autocomplete_component? do
-      validates :none_of_the_above_answer, presence: true, if: :validate_none_of_the_above_answer_presence?
-    end
+    # In the case of a selection component that uses an autocomplete UI component, we show the input for the none
+    # of the above question on a separate page, so we provide a context to skip the validation of
+    # none_of_the_above_answer for the first page.
+    validates :none_of_the_above_answer,
+              presence: true,
+              if: :validate_none_of_the_above_answer_presence?,
+              unless: -> { validation_context == :skip_none_of_the_above_question_validation }
 
     def allow_multiple_answers?
       answer_settings.only_one_option != "true"
     end
 
+    def with_none_of_the_above_selected
+      self.selection = allow_multiple_answers? ? [I18n.t("page.none_of_the_above")] : I18n.t("page.none_of_the_above")
+    end
+
+    def answered?
+      return false if show_none_of_the_above_question? && none_of_the_above_answer.nil?
+
+      super()
+    end
+
     def show_answer
       return selection_without_blanks.map { |selected| name_from_value(selected) }.join(", ") if allow_multiple_answers?
 

app/models/repeatable_step.rb

@@ -59,8 +59,8 @@ def max_answers?
     questions.length >= MAX_ANSWERS
   end
 
-  def valid?
-    questions.all?(&:valid?)
+  def valid?(context = nil)
+    questions.all? { |q| q.valid?(context) }
   end
 
   def show_answer

app/models/step.rb

@@ -48,9 +48,8 @@ def load_from_store(answer_store)
     self
   end
 
-  def update!(params)
+  def assign_question_attributes(params)
     question.assign_attributes(params)
-    question.valid?
   end
 
   def params
@@ -119,6 +118,14 @@ def exit_page_condition_matches?
     first_condition_matches? && has_exit_page_condition?
   end
 
+  def answered_file_question?
+    question.is_a?(Question::File) && question.file_uploaded?
+  end
+
+  def autocomplete_selection_question?
+    question.is_a?(Question::Selection) && question.autocomplete_component?
+  end
+
 private
 
   def goto_condition_page_slug(condition)

app/views/forms/selection_none_of_the_above/show.html.erb

@@ -0,0 +1,28 @@
+<% set_page_title(form_title(form_name: @current_context.form.name, page_name: @step.question.none_of_the_above_question_text, mode: @mode, error: @step.question&.errors&.any?)) %>
+
+<% content_for :back_link do %>
+  <% if @back_link.present? %>
+    <%= link_to t("forms.back"), @back_link, class: "govuk-back-link" %>
+  <% end %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <%= form_with model: @step.question, url: save_selection_none_of_the_above_path, scope: :question, method: :post do |f| %>
+      <% if @step.question&.errors&.any? %>
+        <%= f.govuk_error_summary(t("error_summary_title")) %>
+      <% end %>
+
+      <%= f.govuk_text_field :none_of_the_above_answer,
+                                        label: {
+                                          text: question_text_with_hidden_mode(@step.question.none_of_the_above_question_text, @mode),
+                                          tag: "h1",
+                                          size: "l",
+                                        } %>
+
+      <%= f.govuk_submit(t("continue")) %>
+    <% end %>
+
+    <%= render SupportDetailsComponent::View.new(@support_details) %>
+  </div>
+</div>

config/routes.rb

@@ -66,6 +66,19 @@
              as: :remove_file,
              constraints: page_constraints
 
+      # We don't currently support adding another answer for selection questions, so these routes don't include an
+      # `answer_index` param
+      get "/:page_slug/none-of-the-above/change" => "forms/selection_none_of_the_above#show",
+          as: :change_selection_none_of_the_above,
+          constraints: page_constraints,
+          defaults: { changing_existing_answer: true }
+      get "/:page_slug/none-of-the-above" => "forms/selection_none_of_the_above#show",
+          as: :selection_none_of_the_above,
+          constraints: page_constraints
+      post "/:page_slug/none-of-the-above" => "forms/selection_none_of_the_above#save",
+           as: :save_selection_none_of_the_above,
+           constraints: page_constraints
+
       get "/:page_slug/(/:answer_index)/change" => "forms/page#show",
           as: :form_change_answer,
           defaults: page_answer_defaults.merge(changing_existing_answer: true),

spec/components/check_your_answers_component/view_spec.rb

@@ -85,11 +85,13 @@
     let(:selection) { nil }
     let(:none_of_the_above_answer) { nil }
     let(:none_of_the_above_question_is_optional) { "false" }
+    let(:selection_options) { [DataStruct.new(name: "Option 1", value: "Option 1"), DataStruct.new(name: "Option 2", value: "Option 2")] }
     let(:question) do
       build(
         :single_selection_question,
         :with_none_of_the_above_question,
         question_text:,
+        selection_options:,
         none_of_the_above_question_text:,
         selection:,
         none_of_the_above_answer:,
@@ -116,6 +118,10 @@
           expect(page).to have_css(".govuk-summary-list__key", text: none_of_the_above_question_text)
           expect(page).to have_css(".govuk-summary-list__value", text: none_of_the_above_answer)
         end
+
+        it "has a change link for the 'None of the above' question" do
+          expect(page).to have_link("Change", href: form_change_answer_path(mode: mode, form_id: form.id, form_slug: form.form_slug, page_slug: steps[0].id))
+        end
       end
 
       context "when 'None of the above' question is optional" do
@@ -131,6 +137,14 @@
           end
         end
       end
+
+      context "when selection question uses an autocomplete component" do
+        let(:selection_options) { Array.new(31).map { |i| OpenStruct.new(name: "Option #{i}", value: "Option #{i}") } }
+
+        it "has a change link to the 'None of the above' specific path" do
+          expect(page).to have_link("Change", href: change_selection_none_of_the_above_path(mode: mode, form_id: form.id, form_slug: form.form_slug, page_slug: steps[0].id))
+        end
+      end
     end
   end
 end