Merge pull request #2030 from govuk-forms/firebreak-build-journey-using-form-document-resource

Only consume StepResources when building the Journey

Author: stephencdaly

app/controllers/forms/base_controller.rb

@@ -1,6 +1,6 @@
 module Forms
   class BaseController < ApplicationController
-    prepend_before_action :set_form
+    prepend_before_action :prepare_context
     around_action :set_locale
 
     def redirect_to_friendly_url_start
@@ -15,7 +15,6 @@ def redirect_to_friendly_url_start
     end
 
     def error_repeat_submission
-      @current_context = Flow::Context.new(form: @form, store: session)
       render template: "errors/repeat_submission", locals: { form: @form }
     end
 
@@ -27,9 +26,7 @@ def set_request_logging_attributes
 
   private
 
-    def current_context
-      @current_context ||= Flow::Context.new(form: @form, store: session)
-    end
+    attr_reader :current_context
 
     def mode
       @mode ||= Mode.new(params[:mode])
@@ -43,29 +40,39 @@ def set_available_languages
       @available_languages = current_context.form.available_languages if current_context.form.multilingual?
     end
 
-    def set_form
-      form_id = params.require(:form_id)
-      @form = Api::V2::FormDocumentRepository.find_with_mode(form_id:, mode:, language: locale)
-
-      if @form.blank?
-        I18n.with_locale(locale) do
-          if locale == "cy"
-            archived_welsh_version = Api::V2::FormDocumentRepository.find(form_id:, tag: :archived, language: :cy)
-            live_english_version = Api::V2::FormDocumentRepository.find(form_id:, tag: :live, language: :en)
-
-            if archived_welsh_version.present? && live_english_version.present?
-              return render template: "forms/archived_welsh/show",
-                            locals: { form: live_english_version },
-                            status: :not_found
-            end
-          end
+    def prepare_context
+      form_document = Api::V2::FormDocumentRepository.find_with_mode(form_id:, mode:, language: locale)
+
+      return handle_form_not_found if form_document.blank?
+
+      @form = Form.new(form_document)
+      raise ActiveResource::ResourceNotFound, "Form has no steps" unless @form.start_page
+
+      @current_context = Flow::Context.new(form: @form, form_document:, store: session)
+    end
 
-          archived_form = Api::V2::FormDocumentRepository.find(form_id:, tag: :archived)
-          return render template: "forms/archived/show", locals: { form_name: archived_form.name }, status: :not_found if archived_form.present?
+    def handle_form_not_found
+      I18n.with_locale(locale) do
+        if locale == "cy"
+          archived_welsh_form_document = Api::V2::FormDocumentRepository.find(form_id:, tag: :archived, language: :cy)
+          live_english_form_document = Api::V2::FormDocumentRepository.find(form_id:, tag: :live, language: :en)
+
+          if archived_welsh_form_document.present? && live_english_form_document.present?
+            return render template: "forms/archived_welsh/show",
+                          locals: { form: Form.new(live_english_form_document) },
+                          status: :not_found
+          end
         end
+
+        archived_form_document = Api::V2::FormDocumentRepository.find(form_id:, tag: :archived)
+        return render template: "forms/archived/show", locals: { form_name: archived_form_document.name }, status: :not_found if archived_form_document.present?
       end
 
-      raise ActiveResource::ResourceNotFound, "Not Found" unless @form.present? && @form.start_page
+      raise ActiveResource::ResourceNotFound, "Not Found"
+    end
+
+    def form_id
+      params.require(:form_id)
     end
   end
 end

app/controllers/forms/step_controller.rb

@@ -51,7 +51,7 @@ def save
     def prepare_step
       step_slug = params.require(:step_slug)
       begin
-        @step = current_context.find_or_create(step_slug)
+        @step = current_context.step_by_id(step_slug)
       rescue Flow::StepFactory::StepNotFoundError
         return redirect_to form_step_path(@form.id, @form.form_slug, current_context.next_step_slug)
       end
@@ -185,7 +185,7 @@ def check_goto_page_routing_error
       EventLogger.log_page_event(event_name, @step.question.question_text, nil)
 
       routes_page_id = first_condition_with_error.check_page_id
-      routes_page = @current_context.find_or_create(routes_page_id)
+      routes_page = @current_context.step_by_id(routes_page_id)
 
       render template: "errors/goto_page_routing_error", locals: {
         error_name: first_goto_error_name,

app/lib/flow/context.rb

@@ -2,15 +2,15 @@ module Flow
   class Context
     attr_reader :form, :journey
 
-    def initialize(form:, store:)
+    def initialize(form:, form_document:, store:)
       @form = form
       @answer_store = Store::SessionAnswerStore.new(store, form.id)
       @confirmation_details_store = Store::ConfirmationDetailsStore.new(store, form.id)
-      @journey = Journey.new(answer_store: @answer_store, form:)
+      @journey = Journey.new(answer_store: @answer_store, form_document:)
     end
 
     delegate :support_details, to: :form
-    delegate :find_or_create, :previous_step, :next_step_slug, :next_step, :can_visit?, :completed_steps, :all_steps, to: :journey
+    delegate :step_by_id, :previous_step, :next_step_slug, :next_step, :can_visit?, :completed_steps, :all_steps, to: :journey
     delegate :clear_stored_answer, :clear, :form_submitted?, :answers, :locales_used, to: :answer_store
     delegate :save_submission_details, :get_submission_reference, :requested_email_confirmation?, :clear_submission_details, :save_copy_of_answers_preference, :wants_copy_of_answers?, to: :confirmation_details_store
 

app/lib/flow/errors.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Flow
+  module Errors
+    class StepNotFoundError < StandardError
+      def initialize(msg = "Step not found.")
+        super
+      end
+    end
+  end
+end

app/lib/flow/journey.rb

@@ -17,20 +17,23 @@ module Flow
   # array will be empty.
 
   class Journey
-    attr_reader :completed_steps
+    include Flow::Errors
 
-    def initialize(answer_store:, form:)
+    attr_reader :completed_steps, :all_steps
+
+    def initialize(answer_store:, form_document:)
       @answer_store = answer_store
-      @form = form
-      @step_factory = StepFactory.new(form:)
+      @form_document = form_document
+      @step_factory = StepFactory.new(form_document:)
+      # generate completed_steps first to load answers only for steps that will be visited taking routing into account
       @completed_steps = generate_completed_steps
+      @all_steps = generate_all_steps
 
       populate_file_suffixes
     end
 
-    def find_or_create(step_slug)
-      step = completed_steps.find { |s| s.id == step_slug }
-      step || @step_factory.create_step(step_slug)
+    def step_by_id(id)
+      @all_steps.find { |s| s.id.to_s == id.to_s } || raise(StepNotFoundError, "Can't find step #{id}")
     end
 
     def previous_step(step_slug)
@@ -52,10 +55,6 @@ def can_visit?(step_slug)
       (completed_steps.map(&:id).include? step_slug) || step_slug == next_step_slug
     end
 
-    def all_steps
-      @form.form_document_steps.map { |form_document_step| find_or_create(form_document_step.id.to_s) }
-    end
-
     def completed_file_upload_questions
       completed_steps
               .select { |step| step.question.is_a?(Question::File) && step.question.file_uploaded? }
@@ -125,5 +124,14 @@ def safe_load_from_store(step)
         original_step
       end
     end
+
+    def generate_all_steps
+      @form_document.steps.map { |form_document_step| find_or_create(form_document_step.id.to_s) }
+    end
+
+    def find_or_create(step_slug)
+      step = completed_steps.find { |s| s.id == step_slug }
+      step || @step_factory.create_step(step_slug)
+    end
   end
 end

app/lib/flow/step_factory.rb

@@ -1,26 +1,22 @@
 module Flow
   class StepFactory
-    START_PAGE = "_start".freeze
+    include Flow::Errors
 
-    class StepNotFoundError < StandardError
-      def initialize(msg = "Step not found.")
-        super
-      end
-    end
+    START_PAGE = "_start".freeze
 
-    def initialize(form:)
-      @form = form
+    def initialize(form_document:)
+      @form_document = form_document
     end
 
     def create_step(step_slug_or_start)
       # Normalize the id or constant passed in
-      step_slug = step_slug_or_start.to_s == START_PAGE ? @form.start_page : step_slug_or_start
+      step_slug = step_slug_or_start.to_s == START_PAGE ? @form_document.start_page : step_slug_or_start
       step_slug = step_slug.to_s
 
       return CheckYourAnswersStep.new if step_slug == CheckYourAnswersStep::CHECK_YOUR_ANSWERS_STEP_SLUG
 
       # for now, we use the step id as slug
-      form_document_step = @form.form_document_steps.find { |s| s.id.to_s == step_slug }
+      form_document_step = @form_document.steps.find { |s| s.id.to_s == step_slug }
       raise StepNotFoundError, "Can't find step #{step_slug}" if form_document_step.nil?
 
       question = QuestionRegister.from_form_document_step(form_document_step)

app/models/form.rb

@@ -1,13 +1,10 @@
 class Form
-  attr_reader :document_json
   private attr_reader :form_document
 
-  def initialize(form_document, form_document_json = nil)
+  def initialize(form_document)
     @form_document = form_document
-    @document_json = form_document_json
   end
 
-  delegate :steps, to: :form_document, prefix: true
   delegate :declaration_text,
            :form_id,
            :form_slug,
@@ -31,8 +28,8 @@ def initialize(form_document, form_document_json = nil)
 
   alias_method :id, :form_id
 
-  def step_by_id(step_id)
-    form_document_steps.find { |s| s.id == step_id }
+  def document_json
+    form_document.as_json
   end
 
   def payment_url_with_reference(reference)

app/models/submission.rb

@@ -25,7 +25,7 @@ class Submission < ApplicationRecord
   encrypts :answers
 
   def journey
-    @journey ||= Flow::Journey.new(answer_store:, form:)
+    @journey ||= Flow::Journey.new(answer_store:, form_document: form_document_resource)
   end
 
   def form
@@ -60,7 +60,10 @@ def answer_store
   end
 
   def form_from_document
-    form_document_resource = Api::V2::FormDocumentResource.new(form_document, true)
-    Form.new(form_document_resource, form_document)
+    Form.new(form_document_resource)
+  end
+
+  def form_document_resource
+    @form_document_resource ||= Api::V2::FormDocumentResource.new(form_document, true)
   end
 end

app/services/api/v2/form_document_repository.rb

@@ -5,8 +5,7 @@ def find(form_id:, tag:, language: :en)
 
       begin
         form_document_json = Api::V2::FormDocumentResource.get(form_id, tag, **options_for_language(language))
-        form_document = Api::V2::FormDocumentResource.new(form_document_json)
-        Form.new(form_document, form_document_json)
+        Api::V2::FormDocumentResource.new(form_document_json)
       rescue ActiveResource::ResourceNotFound
         nil
       end

app/services/form_submission_service.rb

@@ -51,9 +51,11 @@ def ensure_form_english
   end
 
   def fetch_english_language_form
-    @form = Api::V2::FormDocumentRepository.find_with_mode(form_id: form.id, mode:)
+    english_form_document = Api::V2::FormDocumentRepository.find_with_mode(form_id: form.id, mode:)
 
-    raise ActiveResource::ResourceNotFound.new(404, "Not Found") if @form.nil?
+    raise ActiveResource::ResourceNotFound.new(404, "Not Found") if english_form_document.nil?
+
+    @form = Form.new(english_form_document)
   end
 
   def validate_submission