Merge pull request #1654 from DFE-Digital/support-console-preview

Improve support console region and country previews

app/controllers/support_interface/base_controller.rb

@@ -4,10 +4,5 @@ class SupportInterface::BaseController < ApplicationController
   include SupportCurrentNamespace
   include StaffAuthenticatable
 
-  before_action :authorize_support
   after_action :verify_authorized
-
-  def authorize_support
-    authorize :support
-  end
 end

app/controllers/support_interface/countries_controller.rb

@@ -1,83 +1,87 @@
 # frozen_string_literal: true
 
-class SupportInterface::CountriesController < SupportInterface::BaseController
-  skip_before_action :authorize_support, only: :confirm_edit
-  before_action :load_country_and_edit_actions, only: %w[confirm_edit update]
-
-  def index
-    @countries = Country.includes(:regions).order(:code)
-  end
-
-  def edit
-    @country = Country.includes(:regions).find(params[:id])
-    @all_regions = @country.regions.map(&:name).join("\n")
-  end
+module SupportInterface
+  class CountriesController < BaseController
+    def index
+      authorize [:support_interface, Country]
+      @countries = Country.includes(:regions).order(:code)
+    end
 
-  def confirm_edit
-    authorize :support, :edit?
-    @country.assign_attributes(country_params)
-  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,
+          qualifications_information: country.qualifications_information,
+          region_names: country.regions.pluck(:name).join("\n"),
+          requires_preliminary_check: country.requires_preliminary_check,
+          teaching_authority_address: country.teaching_authority_address,
+          teaching_authority_certificate:
+            country.teaching_authority_certificate,
+          teaching_authority_emails_string:
+            country.teaching_authority_emails_string,
+          teaching_authority_name: country.teaching_authority_name,
+          teaching_authority_online_checker_url:
+            country.teaching_authority_online_checker_url,
+          teaching_authority_other: country.teaching_authority_other,
+          teaching_authority_sanction_information:
+            country.teaching_authority_sanction_information,
+          teaching_authority_status_information:
+            country.teaching_authority_status_information,
+          teaching_authority_websites_string:
+            country.teaching_authority_websites_string,
+        )
+    end
 
-  def update
-    if @country.update(country_params)
-      @diff_actions.each do |action|
-        case action[:action]
-        when :create
-          @country.regions.create!(name: action[:name])
-        when :delete
-          region = @country.regions.find_by!(name: action[:name])
-          region.eligibility_checks.delete_all
-          region.destroy!
-        end
+    def update
+      @form = CountryForm.new(country_params.merge(country:))
+      if @form.invalid?
+        render :edit, status: :unprocessable_entity
+      elsif ActiveModel::Type::Boolean.new.cast(params[:preview])
+        session[:country] = country_params
+        redirect_to [:preview, :support_interface, country]
+      else
+        @form.save!
+        redirect_to %i[support_interface countries]
       end
-
-      flash[
-        :success
-      ] = "Successfully updated #{CountryName.from_country(@country)}"
-
-      redirect_to support_interface_countries_path
-    else
-      render :edit, status: :unprocessable_entity
     end
-  end
-
-  private
-
-  def load_country_and_edit_actions
-    @country = Country.includes(:regions).find(params[:id])
-    @all_regions = (params.dig(:country, :all_regions) || "").chomp
-    @diff_actions = calculate_diff_actions
-  end
-
-  def calculate_diff_actions
-    current_region_names = @country.regions.map(&:name)
-    new_region_names = @all_regions.split("\n").map(&:chomp)
-    new_region_names = [""] if new_region_names.empty?
 
-    regions_to_delete = current_region_names - new_region_names
-    regions_to_create = new_region_names - current_region_names
+    def preview
+      @form = CountryForm.new(session[:country].merge(country:))
+      @form.assign_country_attributes
+    end
 
-    delete_actions = regions_to_delete.map { |name| { action: :delete, name: } }
-    create_actions = regions_to_create.map { |name| { action: :create, name: } }
+    private
 
-    (delete_actions + create_actions).sort_by { |action| action[:name] }
-  end
+    def country
+      @country ||=
+        begin
+          country = Country.includes(:regions).find(params[:id])
+          authorize [:support_interface, country]
+          country
+        end
+    end
 
-  def country_params
-    params.require(:country).permit(
-      :eligibility_enabled,
-      :eligibility_skip_questions,
-      :qualifications_information,
-      :requires_preliminary_check,
-      :teaching_authority_name,
-      :teaching_authority_address,
-      :teaching_authority_emails_string,
-      :teaching_authority_websites_string,
-      :teaching_authority_certificate,
-      :teaching_authority_other,
-      :teaching_authority_sanction_information,
-      :teaching_authority_status_information,
-      :teaching_authority_online_checker_url,
-    )
+    def country_params
+      params.require(:support_interface_country_form).permit(
+        :has_regions,
+        :region_names,
+        :eligibility_enabled,
+        :eligibility_skip_questions,
+        :qualifications_information,
+        :requires_preliminary_check,
+        :teaching_authority_name,
+        :teaching_authority_address,
+        :teaching_authority_emails_string,
+        :teaching_authority_websites_string,
+        :teaching_authority_certificate,
+        :teaching_authority_other,
+        :teaching_authority_sanction_information,
+        :teaching_authority_status_information,
+        :teaching_authority_online_checker_url,
+      )
+    end
   end
 end

app/controllers/support_interface/english_language_providers_controller.rb

@@ -1,17 +1,17 @@
 # frozen_string_literal: true
 
 class SupportInterface::EnglishLanguageProvidersController < SupportInterface::BaseController
+  before_action :load_english_language_provider, except: :index
+
   def index
+    authorize [:support_interface, EnglishLanguageProvider]
     @english_language_providers = EnglishLanguageProvider.order(:created_at)
   end
 
   def edit
-    @english_language_provider = EnglishLanguageProvider.find(params[:id])
   end
 
   def update
-    @english_language_provider = EnglishLanguageProvider.find(params[:id])
-
     if @english_language_provider.update(english_language_provider_params)
       flash[
         :success
@@ -25,6 +25,11 @@ def update
 
   private
 
+  def load_english_language_provider
+    @english_language_provider = EnglishLanguageProvider.find(params[:id])
+    authorize [:support_interface, @english_language_provider]
+  end
+
   def english_language_provider_params
     params.require(:english_language_provider).permit(
       :name,

app/controllers/support_interface/feature_flags_controller.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class SupportInterface::FeatureFlagsController < SupportInterface::BaseController
+  before_action { authorize %i[support_interface feature_flag] }
+end

app/controllers/support_interface/regions_controller.rb

@@ -1,38 +1,35 @@
 # frozen_string_literal: true
 
 class SupportInterface::RegionsController < SupportInterface::BaseController
-  skip_before_action :authorize_support, only: :preview
+  before_action :load_region
 
   def edit
-    @region = Region.find(params[:id])
   end
 
   def update
-    @region = Region.find(params[:id])
-
-    if @region.update(region_params)
-      flash[
-        :success
-      ] = "Successfully updated #{CountryName.from_region(@region)}"
-
-      if params[:preview] == "preview"
-        redirect_to preview_support_interface_region_path(@region)
-      else
-        redirect_to support_interface_countries_path
-      end
-    else
+    @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]
     end
   end
 
   def preview
-    authorize :support, :show?
-
-    @region = Region.find(params[:id])
+    @region.assign_attributes(session[:region])
   end
 
   private
 
+  def load_region
+    @region = Region.find(params[:id])
+    authorize [:support_interface, @region]
+  end
+
   def region_params
     params.require(:region).permit(
       :application_form_skip_work_history,

app/controllers/support_interface/staff_controller.rb

@@ -1,17 +1,17 @@
 # frozen_string_literal: true
 
 class SupportInterface::StaffController < SupportInterface::BaseController
+  before_action :load_staff, except: :index
+
   def index
+    authorize [:support_interface, Staff]
     @staff = Staff.order(:name)
   end
 
   def edit
-    @staff = Staff.find(params[:id])
   end
 
   def update
-    @staff = Staff.find(params[:id])
-
     if @staff.update(staff_params)
       redirect_to %i[support_interface staff index]
     else
@@ -21,6 +21,11 @@ def update
 
   private
 
+  def load_staff
+    @staff = Staff.find(params[:id])
+    authorize [:support_interface, @staff]
+  end
+
   def staff_params
     params.require(:staff).permit(
       :award_decline_permission,

app/forms/support_interface/country_form.rb

@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+class SupportInterface::CountryForm
+  include ActiveModel::Model
+  include ActiveModel::Attributes
+
+  attr_accessor :country
+  validates :country, presence: true
+
+  attribute :eligibility_enabled, :boolean
+  attribute :eligibility_skip_questions, :boolean
+  attribute :has_regions, :boolean
+  attribute :qualifications_information, :string
+  attribute :region_names, :string
+  attribute :requires_preliminary_check, :boolean
+  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_other, :string
+  attribute :teaching_authority_sanction_information, :string
+  attribute :teaching_authority_status_information, :string
+  attribute :teaching_authority_websites_string, :string
+
+  validates :eligibility_enabled, inclusion: { in: [true, false] }
+  validates :eligibility_skip_questions, inclusion: { in: [true, false] }
+  validates :has_regions, inclusion: { in: [true, false] }
+  validates :region_names, presence: true, if: :has_regions
+  validates :requires_preliminary_check, inclusion: { in: [true, false] }
+
+  def save!
+    ActiveRecord::Base.transaction do
+      assign_country_attributes
+      country.save!
+
+      diff_actions.each do |action|
+        case action[:action]
+        when :create
+          @country.regions.create!(name: action[:name])
+        when :delete
+          region = @country.regions.find_by!(name: action[:name])
+          region.eligibility_checks.delete_all
+          region.destroy!
+        end
+      end
+    end
+
+    true
+  end
+
+  def assign_country_attributes
+    country.assign_attributes(
+      eligibility_enabled:,
+      eligibility_skip_questions:,
+      qualifications_information:,
+      requires_preliminary_check:,
+      teaching_authority_address:,
+      teaching_authority_certificate:,
+      teaching_authority_emails_string:,
+      teaching_authority_name:,
+      teaching_authority_online_checker_url:,
+      teaching_authority_other:,
+      teaching_authority_sanction_information:,
+      teaching_authority_status_information:,
+      teaching_authority_websites_string:,
+    )
+  end
+
+  def diff_actions
+    @diff_actions ||=
+      begin
+        current_region_names = country.regions.map(&:name)
+        new_region_names =
+          has_regions ? region_names.split("\n").map(&:chomp) : []
+
+        new_region_names = [""] if new_region_names.empty?
+
+        regions_to_delete = current_region_names - new_region_names
+        regions_to_create = new_region_names - current_region_names
+
+        delete_actions =
+          regions_to_delete.map { |name| { action: :delete, name: } }
+        create_actions =
+          regions_to_create.map { |name| { action: :create, name: } }
+
+        (delete_actions + create_actions).sort_by { |action| action[:name] }
+      end
+  end
+end