Refactor factories

This refactors the factories used in the tests to improve the code
readability and remove as much duplication as possible.

app/forms/assessor_interface/assessment_section_form.rb

@@ -35,7 +35,7 @@ def selected_failure_reasons
       .each_with_object({}) do |failure_reason, memo|
         next unless send("#{failure_reason}_checked")
         memo[failure_reason] = { notes: send("#{failure_reason}_notes") }
-        next unless FailureReasons.chooses_work_history?(failure_reason:)
+        next unless FailureReasons.chooses_work_history?(failure_reason)
         memo[failure_reason][
           :work_histories
         ] = work_histories.filter do |work_history|
@@ -86,7 +86,7 @@ def self.name
           klass.validates "#{failure_reason}_checked", inclusion: [true, false]
         end
 
-        if FailureReasons.chooses_work_history?(failure_reason:)
+        if FailureReasons.chooses_work_history?(failure_reason)
           klass.attribute "#{failure_reason}_work_history_checked"
 
           klass.validates "#{failure_reason}_work_history_checked",
@@ -97,7 +97,7 @@ def self.name
         klass.attribute "#{failure_reason}_notes", :string
 
         if assessment_section.preliminary? ||
-             FailureReasons.decline?(failure_reason:)
+             FailureReasons.decline?(failure_reason)
           next
         end
         klass.validates "#{failure_reason}_notes",
@@ -124,7 +124,7 @@ def initial_attributes(assessment_section)
       selected_failure_reasons_hash.each do |key, notes|
         attributes["#{key}_checked"] = true
         attributes["#{key}_notes"] = notes[:assessor_feedback]
-        if FailureReasons.chooses_work_history?(failure_reason: key)
+        if FailureReasons.chooses_work_history?(key)
           attributes["#{key}_work_history_checked"] = notes[:work_history_ids]
         end
       end

app/lib/failure_reasons.rb

@@ -94,15 +94,19 @@ class FailureReasons
     WRITTEN_STATEMENT_RECENT => :written_statement,
   }.freeze
 
-  def self.decline?(failure_reason:)
+  def self.decline?(failure_reason)
     DECLINABLE.include?(failure_reason.to_s)
   end
 
-  def self.further_information_request_document_type(failure_reason:)
+  def self.further_information?(failure_reason)
+    FURTHER_INFORMATIONABLE.include?(failure_reason.to_s)
+  end
+
+  def self.further_information_request_document_type(failure_reason)
     DOCUMENT_FAILURE_REASONS[failure_reason.to_s]
   end
 
-  def self.chooses_work_history?(failure_reason:)
+  def self.chooses_work_history?(failure_reason)
     WORK_HISTORY_FAILURE_REASONS.include?(failure_reason.to_s)
   end
 end

app/lib/further_information_request_items_factory.rb

@@ -35,7 +35,7 @@ def build_further_information_request_item(
     if (
          document_type =
            FailureReasons.further_information_request_document_type(
-             failure_reason: failure_reason_key,
+             failure_reason_key,
            )
        ).present?
       [

app/view_objects/assessor_interface/assessment_section_view_object.rb

@@ -152,10 +152,10 @@ def highlighted_work_history_contact_emails
 
     def build_key(failure_reason, key_section)
       key =
-        if FailureReasons.decline?(failure_reason:)
+        if FailureReasons.decline?(failure_reason)
           "decline"
         elsif FailureReasons.further_information_request_document_type(
-              failure_reason:,
+              failure_reason,
             ).present?
           "document"
         else

app/view_objects/teacher_interface/application_form_view_object.rb

@@ -255,8 +255,7 @@ def assessment_declined_reasons
 
         sorted_reasons =
           section.selected_failure_reasons.sort_by do |failure_reason|
-            is_decline =
-              FailureReasons.decline?(failure_reason: failure_reason.key)
+            is_decline = FailureReasons.decline?(failure_reason.key)
 
             [is_decline ? 0 : 1, failure_reason.key]
           end
@@ -277,8 +276,7 @@ def assessment_declined_reasons
 
             if (
                  assessor_feedback = failure_reason.assessor_feedback
-               ).present? &&
-                 FailureReasons.decline?(failure_reason: failure_reason.key)
+               ).present? && FailureReasons.decline?(failure_reason.key)
               { name: title, assessor_note: assessor_feedback }
             else
               { name: title }

app/views/assessor_interface/assessment_recommendation_decline/edit.html.erb

@@ -40,7 +40,7 @@
               </h4>
 
               <% if (assessor_feedback = failure_reason.assessor_feedback).present? %>
-                <% if FailureReasons.decline?(failure_reason: failure_reason.key) %>
+                <% if FailureReasons.decline?(failure_reason.key) %>
                   <p class="govuk-body govuk-!-margin-bottom-2">Your note to the applicant:</p>
                 <% else %>
                   <p class="govuk-body govuk-!-margin-bottom-2">Your note (the applicant won’t see this):</p>

app/views/assessor_interface/assessment_sections/_form.html.erb

@@ -70,7 +70,7 @@
                                     label: { text: t(view_object.notes_label_key_for(failure_reason:)), size: "s" },
                                     hint: { text: t(view_object.notes_hint_key_for(failure_reason:)) },
                                     placeholder: t(view_object.notes_placeholder_key_for(failure_reason:)) %>
-              <% if FailureReasons::chooses_work_history?(failure_reason:) %>
+              <% if FailureReasons::chooses_work_history?(failure_reason) %>
                 <%= f.govuk_collection_check_boxes :"#{failure_reason}_work_history_checked", view_object.work_histories, :id, :school_name, multiple: false, legend: { size: "s" } %>
               <% end %>
             <% end %>

config/environments/test.rb

@@ -23,12 +23,12 @@
   }
 
   # Show full error reports and enable caching.
-  config.consider_all_requests_local = false
+  config.consider_all_requests_local = true
   config.action_controller.perform_caching = false
   config.cache_store = :memory_store
 
   # Render exception templates instead of raising exceptions.
-  config.action_dispatch.show_exceptions = :all
+  config.action_dispatch.show_exceptions = :none
 
   # Disable request forgery protection in test environment.
   config.action_controller.allow_forgery_protection = false

lib/tasks/example_data.rake

@@ -15,12 +15,7 @@ namespace :example_data do
     Faker::UniqueGenerator.clear
 
     staff_members.each do |staff|
-      FactoryBot.create(
-        :staff,
-        :confirmed,
-        :with_change_email_permission,
-        **staff,
-      )
+      FactoryBot.create(:staff, :with_change_email_permission, **staff)
     end
 
     create_application_forms
@@ -208,7 +203,7 @@ def create_application_forms
       if application_form.declined_at.present?
         FactoryBot.create(
           :selected_failure_reason,
-          :fi_requestable,
+          :further_informationable,
           assessment_section: assessment.sections.first,
           key: assessment.sections.first.failure_reasons.sample,
         )

spec/components/application_form_search_result_component_spec.rb

@@ -11,9 +11,9 @@
     create(
       :application_form,
       :submitted,
-      :with_reviewer,
       given_names: "Given",
       family_name: "Family",
+      reviewer: create(:staff),
     )
   end
   let(:current_staff) { create(:staff) }
@@ -51,7 +51,7 @@
       it { is_expected.to include("Reference") }
 
       context "where there is no reviewer assigned" do
-        before { application_form.update(reviewer: nil) }
+        before { application_form.update!(reviewer: nil) }
 
         it { is_expected.not_to include("Reviewer") }
       end

spec/controllers/assessor_interface/uploads_controller_spec.rb

@@ -3,7 +3,7 @@
 require "rails_helper"
 
 RSpec.describe AssessorInterface::UploadsController, type: :controller do
-  let(:staff) { create(:staff, :with_assess_permission, :confirmed) }
+  let(:staff) { create(:staff, :with_assess_permission) }
   let(:application_form) { create(:application_form) }
 
   before { sign_in staff, scope: :staff }

spec/factories/application_forms.rb

@@ -81,8 +81,9 @@
 FactoryBot.define do
   factory :application_form do
     sequence(:reference) { |n| n.to_s.rjust(7, "0") }
-    association :teacher
-    association :region
+
+    teacher
+    region
 
     needs_work_history { !region.application_form_skip_work_history }
     needs_written_statement do
@@ -102,16 +103,8 @@
       requires_preliminary_check { true }
     end
 
-    trait :completed do
-      personal_information_status { "completed" }
-      identification_document_status { "completed" }
-      qualifications_status { "completed" }
-      age_range_status { "completed" }
-      subjects_status { "completed" }
-      english_language_status { "completed" }
-      work_history_status { "completed" }
-      registration_number_status { "completed" }
-      written_statement_status { "completed" }
+    trait :teaching_authority_provides_written_statement do
+      teaching_authority_provides_written_statement { true }
     end
 
     trait :action_required_by_admin do
@@ -152,6 +145,7 @@
 
     trait :verification_stage do
       stage { "verification" }
+      action_required_by_admin
     end
 
     trait :review_stage do
@@ -185,7 +179,7 @@
     trait :preliminary_check do
       submitted
       pre_assessment_stage
-      requires_preliminary_check { true }
+      requires_preliminary_check
       statuses { %w[preliminary_check] }
     end
 
@@ -242,55 +236,51 @@
       end
     end
 
-    trait :with_age_range do
-      age_range_min { Faker::Number.between(from: 5, to: 11) }
-      age_range_max { Faker::Number.between(from: age_range_min, to: 18) }
-    end
-
-    trait :with_subjects do
-      transient { number_of_subjects { Faker::Number.between(from: 1, to: 3) } }
-
-      subjects do
-        ([-> { Faker::Educator.subject }] * number_of_subjects).map(&:call)
-      end
-    end
-
-    trait :with_identification_document do
-      after(:create) do |application_form, _evaluator|
-        create(:upload, document: application_form.identification_document)
-      end
-    end
-
     trait :with_personal_information do
       given_names { Faker::Name.name }
       family_name { Faker::Name.last_name }
       date_of_birth do
         Faker::Date.between(from: 65.years.ago, to: 21.years.ago)
       end
       has_alternative_name { false }
+      personal_information_status { "completed" }
+    end
+
+    trait :with_identification_document do
+      identification_document_status { "completed" }
+      after(:create) do |application_form, _evaluator|
+        create(:upload, document: application_form.identification_document)
+      end
     end
 
     trait :with_alternative_name do
       has_alternative_name { true }
       alternative_given_names { Faker::Name.name }
       alternative_family_name { Faker::Name.last_name }
-    end
-
-    trait :with_name_change_document do
       after(:create) do |application_form, _evaluator|
         create(:upload, document: application_form.name_change_document)
       end
     end
 
-    trait :with_registration_number do
-      needs_registration_number { true }
-      registration_number do
-        Faker::Number.unique.leading_zero_number(digits: 8)
+    trait :with_age_range do
+      age_range_min { Faker::Number.between(from: 5, to: 11) }
+      age_range_max { Faker::Number.between(from: age_range_min, to: 18) }
+      age_range_status { "completed" }
+    end
+
+    trait :with_subjects do
+      transient { number_of_subjects { Faker::Number.between(from: 1, to: 3) } }
+
+      subjects do
+        ([-> { Faker::Educator.subject }] * number_of_subjects).map(&:call)
       end
+
+      subjects_status { "completed" }
     end
 
     trait :with_teaching_qualification do
       teaching_qualification_part_of_degree { true }
+      qualifications_status { "completed" }
 
       after(:create) do |application_form, _evaluator|
         create(
@@ -311,6 +301,7 @@
 
     trait :with_english_language_medium_of_instruction do
       english_language_proof_method { "medium_of_instruction" }
+      english_language_status { "completed" }
 
       after(:create) do |application_form, _evaluator|
         create(
@@ -323,6 +314,7 @@
 
     trait :with_english_language_proficiency_document do
       with_english_language_provider
+      english_language_status { "completed" }
 
       after(:create) do |application_form, _evaluator|
         create(
@@ -338,11 +330,13 @@
         EnglishLanguageProvider.all.sample || create(:english_language_provider)
       end
       english_language_provider_reference { "reference" }
+      english_language_status { "completed" }
     end
 
     trait :with_english_language_other_provider do
       english_language_proof_method { "provider" }
       english_language_provider_other { true }
+      english_language_status { "completed" }
 
       after(:create) do |application_form, _evaluator|
         create(
@@ -354,15 +348,18 @@
 
     trait :with_english_language_exemption_by_citizenship do
       english_language_citizenship_exempt { true }
+      english_language_status { "completed" }
     end
 
     trait :with_english_language_exemption_by_qualification do
       english_language_qualification_exempt { true }
+      english_language_status { "completed" }
     end
 
     trait :with_work_history do
       needs_work_history { true }
       has_work_history { true }
+      work_history_status { "completed" }
 
       after(:create) do |application_form, _evaluator|
         create(:work_history, :completed, :still_employed, application_form:)
@@ -375,12 +372,9 @@
       end
     end
 
-    trait :teaching_authority_provides_written_statement do
-      teaching_authority_provides_written_statement { true }
-    end
-
     trait :with_written_statement do
       needs_written_statement { true }
+      written_statement_status { "completed" }
 
       after(:create) do |application_form, _evaluator|
         if application_form.teaching_authority_provides_written_statement
@@ -391,8 +385,12 @@
       end
     end
 
-    trait :with_reviewer do
-      association :reviewer, factory: :staff
+    trait :with_registration_number do
+      needs_registration_number { true }
+      registration_number do
+        Faker::Number.unique.leading_zero_number(digits: 8)
+      end
+      registration_number_status { "completed" }
     end
   end
 end