Merge pull request #2255 from DFE-Digital/refactor-support-region-form

Refactor support region and country forms

app/controllers/support_interface/countries_controller.rb

@@ -4,28 +4,17 @@ module SupportInterface
   class CountriesController < BaseController
     def index
       authorize [:support_interface, Country]
+
       @countries =
         Country
           .includes(:regions)
           .sort_by { |country| CountryName.from_country(country) }
+
       render layout: "full_from_desktop"
     end
 
     def edit
-      @form =
-        CountryForm.new(
-          country:,
-          eligibility_enabled: country.eligibility_enabled,
-          eligibility_skip_questions: country.eligibility_skip_questions,
-          has_regions: country.regions.count > 1,
-          other_information: country.other_information,
-          region_names: country.regions.pluck(:name).join("\n"),
-          sanction_information: country.sanction_information,
-          status_information: country.status_information,
-          subject_limited: country.subject_limited,
-          teaching_qualification_information:
-            country.teaching_qualification_information,
-        )
+      @form = CountryForm.for_existing_country(country)
     end
 
     def update
@@ -50,11 +39,10 @@ def preview
 
     def country
       @country ||=
-        begin
-          country = Country.includes(:regions).find(params[:id])
-          authorize [:support_interface, country]
-          country
-        end
+        authorize [
+                    :support_interface,
+                    Country.includes(:regions).find(params[:id]),
+                  ]
     end
 
     def country_params

app/controllers/support_interface/regions_controller.rb

@@ -1,55 +1,56 @@
 # frozen_string_literal: true
 
-class SupportInterface::RegionsController < SupportInterface::BaseController
-  before_action :load_region
-
-  def edit
-  end
+module SupportInterface
+  class RegionsController < BaseController
+    def edit
+      @form = RegionForm.for_existing_region(region)
+    end
 
-  def update
-    @region.assign_attributes(region_params)
-    if @region.invalid?
-      render :edit, status: :unprocessable_entity
-    elsif ActiveModel::Type::Boolean.new.cast(params[:preview])
-      session[:region] = region_params
-      redirect_to [:preview, :support_interface, @region]
-    else
-      @region.save!
-      redirect_to %i[support_interface countries]
+    def update
+      @form = RegionForm.new(region_params.merge(region:))
+      if @form.invalid?
+        render :edit, status: :unprocessable_entity
+      elsif ActiveModel::Type::Boolean.new.cast(params[:preview])
+        session[:region] = region_params
+        redirect_to [:preview, :support_interface, region]
+      else
+        @form.save!
+        redirect_to %i[support_interface countries]
+      end
     end
-  end
 
-  def preview
-    @region.assign_attributes(session[:region])
-  end
+    def preview
+      @form = RegionForm.new(session[:region].merge(region:))
+      @form.assign_region_attributes
+    end
 
-  private
+    private
 
-  def load_region
-    @region = Region.find(params[:id])
-    authorize [:support_interface, @region]
-  end
+    def region
+      @region ||= authorize [:support_interface, Region.find(params[:id])]
+    end
 
-  def region_params
-    params.require(:region).permit(
-      :all_sections_necessary,
-      :other_information,
-      :requires_preliminary_check,
-      :sanction_check,
-      :sanction_information,
-      :status_check,
-      :status_information,
-      :teaching_authority_address,
-      :teaching_authority_certificate,
-      :teaching_authority_emails_string,
-      :teaching_authority_name,
-      :teaching_authority_online_checker_url,
-      :teaching_authority_provides_written_statement,
-      :teaching_authority_requires_submission_email,
-      :teaching_authority_websites_string,
-      :teaching_qualification_information,
-      :work_history_section_to_omit,
-      :written_statement_optional,
-    )
+    def region_params
+      params.require(:support_interface_region_form).permit(
+        :all_sections_necessary,
+        :other_information,
+        :requires_preliminary_check,
+        :sanction_check,
+        :sanction_information,
+        :status_check,
+        :status_information,
+        :teaching_authority_address,
+        :teaching_authority_certificate,
+        :teaching_authority_emails_string,
+        :teaching_authority_name,
+        :teaching_authority_online_checker_url,
+        :teaching_authority_provides_written_statement,
+        :teaching_authority_requires_submission_email,
+        :teaching_authority_websites_string,
+        :teaching_qualification_information,
+        :work_history_section_to_omit,
+        :written_statement_optional,
+      )
+    end
   end
 end

app/forms/support_interface/country_form.rb

@@ -9,23 +9,23 @@ class SupportInterface::CountryForm
   validates :country, presence: true
 
   attribute :eligibility_enabled, :boolean
-  attribute :eligibility_skip_questions, :boolean
+  attribute :eligibility_route, :string
   attribute :has_regions, :boolean
   attribute :other_information, :string
   attribute :region_names, :string
   attribute :sanction_information, :string
   attribute :status_information, :string
-  attribute :subject_limited, :boolean
   attribute :teaching_qualification_information, :string
 
   validates :eligibility_enabled, inclusion: { in: [true, false] }
-  validates :eligibility_skip_questions, inclusion: { in: [true, false] }
+  validates :eligibility_route, inclusion: { in: %w[expanded reduced standard] }
   validates :has_regions, inclusion: { in: [true, false] }
   validates :region_names, presence: true, if: :has_regions
 
   def save!
+    assign_country_attributes
+
     ActiveRecord::Base.transaction do
-      assign_country_attributes
       country.save!
 
       diff_actions.each do |action|
@@ -46,8 +46,8 @@ def save!
   def assign_country_attributes
     country.assign_attributes(
       eligibility_enabled:,
-      eligibility_skip_questions:,
-      subject_limited:,
+      eligibility_skip_questions: eligibility_route == "reduced",
+      subject_limited: eligibility_route == "expanded",
     )
 
     if has_regions
@@ -88,27 +88,27 @@ def diff_actions
       end
   end
 
-  def eligibility_route
-    if subject_limited
-      "expanded"
-    elsif eligibility_skip_questions
-      "reduced"
-    else
-      "standard"
-    end
-  end
+  def self.for_existing_country(country)
+    eligibility_route =
+      if country.subject_limited
+        "expanded"
+      elsif country.eligibility_skip_questions
+        "reduced"
+      else
+        "standard"
+      end
 
-  def eligibility_route=(value)
-    case value
-    when "standard"
-      self.subject_limited = false
-      self.eligibility_skip_questions = false
-    when "reduced"
-      self.subject_limited = false
-      self.eligibility_skip_questions = true
-    when "expanded"
-      self.subject_limited = true
-      self.eligibility_skip_questions = false
-    end
+    new(
+      country:,
+      eligibility_enabled: country.eligibility_enabled,
+      eligibility_route:,
+      has_regions: country.regions.count > 1,
+      other_information: country.other_information,
+      region_names: country.regions.pluck(:name).join("\n"),
+      sanction_information: country.sanction_information,
+      status_information: country.status_information,
+      teaching_qualification_information:
+        country.teaching_qualification_information,
+    )
   end
 end

app/forms/support_interface/region_form.rb

@@ -0,0 +1,135 @@
+# frozen_string_literal: true
+
+class SupportInterface::RegionForm
+  include ActiveModel::Model
+  include ActiveModel::Attributes
+  include ActiveModel::Dirty
+
+  attr_accessor :region
+  validates :region, presence: true
+
+  attribute :all_sections_necessary, :boolean
+  attribute :other_information, :string
+  attribute :requires_preliminary_check, :boolean
+  attribute :sanction_check, :string
+  attribute :sanction_information, :string
+  attribute :status_check, :string
+  attribute :status_information, :string
+  attribute :teaching_authority_address, :string
+  attribute :teaching_authority_certificate, :string
+  attribute :teaching_authority_emails_string, :string
+  attribute :teaching_authority_name, :string
+  attribute :teaching_authority_online_checker_url, :string
+  attribute :teaching_authority_provides_written_statement, :boolean
+  attribute :teaching_authority_requires_submission_email, :boolean
+  attribute :teaching_authority_websites_string, :string
+  attribute :teaching_qualification_information, :string
+  attribute :work_history_section_to_omit, :string
+  attribute :written_statement_optional, :boolean
+
+  validates :all_sections_necessary, inclusion: { in: [true, false] }
+  validates :requires_preliminary_check, inclusion: { in: [true, false] }
+  validates :sanction_check, inclusion: { in: %w[online written none] }
+  validates :status_check, inclusion: { in: %w[online written none] }
+  validates :teaching_authority_name,
+            format: {
+              without: /\Athe.*\z/i,
+              message: "Teaching authority name shouldn't start with ‘the’.",
+            }
+  validates :teaching_authority_online_checker_url, url: { allow_blank: true }
+  validates :teaching_authority_provides_written_statement,
+            inclusion: {
+              in: [true, false],
+            }
+  validates :work_history_section_to_omit,
+            inclusion: {
+              in: %w[whole_section contact_details],
+            },
+            unless: :all_sections_necessary
+  validates :written_statement_optional, inclusion: { in: [true, false] }
+
+  def save!
+    assign_region_attributes
+
+    ActiveRecord::Base.transaction { region.save! }
+  end
+
+  def assign_region_attributes
+    region.assign_attributes(
+      application_form_skip_work_history:,
+      other_information:,
+      reduced_evidence_accepted:,
+      requires_preliminary_check:,
+      sanction_check:,
+      sanction_information:,
+      status_check:,
+      status_information:,
+      teaching_authority_address:,
+      teaching_authority_certificate:,
+      teaching_authority_emails:,
+      teaching_authority_name:,
+      teaching_authority_online_checker_url:,
+      teaching_authority_provides_written_statement:,
+      teaching_authority_requires_submission_email:,
+      teaching_authority_websites:,
+      teaching_qualification_information:,
+      written_statement_optional:,
+    )
+  end
+
+  def teaching_authority_emails
+    teaching_authority_emails_string.split("\n").map(&:chomp).compact_blank
+  end
+
+  def teaching_authority_websites
+    teaching_authority_websites_string.split("\n").map(&:chomp).compact_blank
+  end
+
+  def application_form_skip_work_history
+    !all_sections_necessary && work_history_section_to_omit == "whole_section"
+  end
+
+  def reduced_evidence_accepted
+    !all_sections_necessary && work_history_section_to_omit == "contact_details"
+  end
+
+  def self.for_existing_region(region)
+    all_sections_necessary =
+      !region.application_form_skip_work_history &&
+        !region.reduced_evidence_accepted
+
+    work_history_section_to_omit =
+      if region.application_form_skip_work_history
+        "whole_section"
+      elsif region.reduced_evidence_accepted
+        "contact_details"
+      end
+
+    new(
+      region:,
+      all_sections_necessary:,
+      other_information: region.other_information,
+      requires_preliminary_check: region.requires_preliminary_check,
+      sanction_check: region.sanction_check,
+      sanction_information: region.sanction_information,
+      status_check: region.status_check,
+      status_information: region.status_information,
+      teaching_authority_address: region.teaching_authority_address,
+      teaching_authority_emails_string:
+        region.teaching_authority_emails.join("\n"),
+      teaching_authority_name: region.teaching_authority_name,
+      teaching_authority_online_checker_url:
+        region.teaching_authority_online_checker_url,
+      teaching_authority_provides_written_statement:
+        region.teaching_authority_provides_written_statement,
+      teaching_authority_requires_submission_email:
+        region.teaching_authority_requires_submission_email,
+      teaching_authority_websites_string:
+        region.teaching_authority_websites.join("\n"),
+      teaching_qualification_information:
+        region.teaching_qualification_information,
+      work_history_section_to_omit:,
+      written_statement_optional: region.written_statement_optional,
+    )
+  end
+end