diff --git a/app/controllers/support_interface/countries_controller.rb b/app/controllers/support_interface/countries_controller.rb index 94153b4acc..121ed4843f 100644 --- a/app/controllers/support_interface/countries_controller.rb +++ b/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 diff --git a/app/controllers/support_interface/regions_controller.rb b/app/controllers/support_interface/regions_controller.rb index ad84a7f08e..3e62b6f951 100644 --- a/app/controllers/support_interface/regions_controller.rb +++ b/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 diff --git a/app/forms/support_interface/country_form.rb b/app/forms/support_interface/country_form.rb index 8918ad9c5e..5e1418cebe 100644 --- a/app/forms/support_interface/country_form.rb +++ b/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 diff --git a/app/forms/support_interface/region_form.rb b/app/forms/support_interface/region_form.rb new file mode 100644 index 0000000000..3c6900812f --- /dev/null +++ b/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 diff --git a/app/models/region.rb b/app/models/region.rb index 9f4715be1e..0321690579 100644 --- a/app/models/region.rb +++ b/app/models/region.rb @@ -53,74 +53,7 @@ class Region < ApplicationRecord validates :sanction_check, inclusion: { in: sanction_checks.values } validates :status_check, inclusion: { in: status_checks.values } - 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 } - def checks_available? !sanction_check_none? && !status_check_none? end - - def teaching_authority_emails_string - teaching_authority_emails.join("\n") - end - - def teaching_authority_emails_string=(string) - self.teaching_authority_emails = - string.split("\n").map(&:chomp).compact_blank - end - - def teaching_authority_websites_string - teaching_authority_websites.join("\n") - end - - def teaching_authority_websites_string=(string) - self.teaching_authority_websites = - string.split("\n").map(&:chomp).compact_blank - end - - def teaching_authority_present? - teaching_authority_name.present? || teaching_authority_address.present? || - teaching_authority_emails.present? || teaching_authority_websites.present? - end - - def all_sections_necessary - !application_form_skip_work_history && !reduced_evidence_accepted - end - - def all_sections_necessary=(value) - application_form_skip_work_history_will_change! - reduced_evidence_accepted_will_change! - - if value - self.application_form_skip_work_history = false - self.reduced_evidence_accepted = false - end - end - - def work_history_section_to_omit - if application_form_skip_work_history - "whole_section" - elsif reduced_evidence_accepted - "contact_details" - end - end - - def work_history_section_to_omit=(value) - return if all_sections_necessary - application_form_skip_work_history_will_change! - reduced_evidence_accepted_will_change! - - case value - when "whole_section" - self.application_form_skip_work_history = true - self.reduced_evidence_accepted = false - when "contact_details" - self.application_form_skip_work_history = false - self.reduced_evidence_accepted = true - end - end end diff --git a/app/views/support_interface/countries/preview.html.erb b/app/views/support_interface/countries/preview.html.erb index f6a28ffc55..011e65eb59 100644 --- a/app/views/support_interface/countries/preview.html.erb +++ b/app/views/support_interface/countries/preview.html.erb @@ -69,7 +69,6 @@ <%= form_with model: @form, url: [:support_interface, @country], method: :put do |f| %> <%= f.hidden_field :eligibility_enabled, value: @form.eligibility_enabled %> <%= f.hidden_field :eligibility_route, value: @form.eligibility_route %> - <%= f.hidden_field :eligibility_skip_questions, value: @form.eligibility_skip_questions %> <%= f.hidden_field :has_regions, value: @form.has_regions %> <%= f.hidden_field :other_information, value: @form.other_information %> <%= f.hidden_field :region_names, value: @form.region_names %> diff --git a/app/views/support_interface/regions/edit.html.erb b/app/views/support_interface/regions/edit.html.erb index 6e9a5cc6cf..a3b12d0346 100644 --- a/app/views/support_interface/regions/edit.html.erb +++ b/app/views/support_interface/regions/edit.html.erb @@ -1,6 +1,6 @@ <% content_for :page_title, CountryName.from_region(@region) %> -<%= form_with model: [:support_interface, @region] do |f| %> +<%= form_with model: @form, url: [:support_interface, @region], method: :put do |f| %> <%= f.govuk_error_summary %>

<%= CountryName.from_region(@region) %>

diff --git a/app/views/support_interface/regions/preview.html.erb b/app/views/support_interface/regions/preview.html.erb index eb29c27d15..21d2343875 100644 --- a/app/views/support_interface/regions/preview.html.erb +++ b/app/views/support_interface/regions/preview.html.erb @@ -10,24 +10,24 @@
-<%= form_with model: [:support_interface, @region] do |f| %> - <%= f.hidden_field :application_form_skip_work_history, value: @region.application_form_skip_work_history %> - <%= f.hidden_field :other_information, value: @region.other_information %> - <%= f.hidden_field :reduced_evidence_accepted, value: @region.reduced_evidence_accepted %> - <%= f.hidden_field :requires_preliminary_check, value: @region.requires_preliminary_check %> - <%= f.hidden_field :sanction_check, value: @region.sanction_check %> - <%= f.hidden_field :sanction_information, value: @region.sanction_information %> - <%= f.hidden_field :status_check, value: @region.status_check %> - <%= f.hidden_field :status_information, value: @region.status_information %> - <%= f.hidden_field :teaching_authority_address, value: @region.teaching_authority_address %> - <%= f.hidden_field :teaching_authority_certificate, value: @region.teaching_authority_certificate %> - <%= f.hidden_field :teaching_authority_emails_string, value: @region.teaching_authority_emails_string %> - <%= f.hidden_field :teaching_authority_name, value: @region.teaching_authority_name %> - <%= f.hidden_field :teaching_authority_online_checker_url, value: @region.teaching_authority_online_checker_url %> - <%= f.hidden_field :teaching_authority_provides_written_statement, value: @region.teaching_authority_provides_written_statement %> - <%= f.hidden_field :teaching_authority_requires_submission_email, value: @region.teaching_authority_requires_submission_email %> - <%= f.hidden_field :teaching_authority_websites_string, value: @region.teaching_authority_websites_string %> - <%= f.hidden_field :teaching_qualification_information, value: @region.teaching_qualification_information %> - <%= f.hidden_field :written_statement_optional, value: @region.written_statement_optional %> +<%= form_with model: @form, url: [:support_interface, @region], method: :put do |f| %> + <%= f.hidden_field :all_sections_necessary, value: @form.all_sections_necessary %> + <%= f.hidden_field :other_information, value: @form.other_information %> + <%= f.hidden_field :requires_preliminary_check, value: @form.requires_preliminary_check %> + <%= f.hidden_field :sanction_check, value: @form.sanction_check %> + <%= f.hidden_field :sanction_information, value: @form.sanction_information %> + <%= f.hidden_field :status_check, value: @form.status_check %> + <%= f.hidden_field :status_information, value: @form.status_information %> + <%= f.hidden_field :teaching_authority_address, value: @form.teaching_authority_address %> + <%= f.hidden_field :teaching_authority_certificate, value: @form.teaching_authority_certificate %> + <%= f.hidden_field :teaching_authority_emails_string, value: @form.teaching_authority_emails_string %> + <%= f.hidden_field :teaching_authority_name, value: @form.teaching_authority_name %> + <%= f.hidden_field :teaching_authority_online_checker_url, value: @form.teaching_authority_online_checker_url %> + <%= f.hidden_field :teaching_authority_provides_written_statement, value: @form.teaching_authority_provides_written_statement %> + <%= f.hidden_field :teaching_authority_requires_submission_email, value: @form.teaching_authority_requires_submission_email %> + <%= f.hidden_field :teaching_authority_websites_string, value: @form.teaching_authority_websites_string %> + <%= f.hidden_field :teaching_qualification_information, value: @form.teaching_qualification_information %> + <%= f.hidden_field :work_history_section_to_omit, value: @form.work_history_section_to_omit %> + <%= f.hidden_field :written_statement_optional, value: @form.written_statement_optional %> <%= f.govuk_submit "Save", name: "preview", value: "false" %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 90a7be2305..3b19cd9321 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -65,14 +65,6 @@ en: attributes: selected_failure_reasons: blank: Select the reasons for your recommendation. - country: - attributes: - teaching_authority_online_checker_url: - url: Enter a valid teaching authority online checker URL - region: - attributes: - teaching_authority_online_checker_url: - url: Enter a valid teaching authority online checker URL teacher: attributes: email: diff --git a/spec/models/region_spec.rb b/spec/models/region_spec.rb index 8da7c735a0..1a022d359b 100644 --- a/spec/models/region_spec.rb +++ b/spec/models/region_spec.rb @@ -38,9 +38,7 @@ require "rails_helper" RSpec.describe Region, type: :model do - subject(:region) { build(:region, teaching_authority_name:) } - - let(:teaching_authority_name) { "" } + subject(:region) { build(:region) } describe "validations" do it { is_expected.to be_valid } @@ -58,115 +56,5 @@ .with_prefix(:status_check) .backed_by_column_of_type(:string) end - it do - is_expected.to validate_url_of( - :teaching_authority_online_checker_url, - ).with_message("Enter a valid teaching authority online checker URL") - end - - context "with a teaching authority name that starts with the" do - let(:teaching_authority_name) { "the authority" } - it { is_expected.to_not be_valid } - end - end - - describe "#teaching_authority_emails_string" do - subject(:teaching_authority_emails_string) do - region.teaching_authority_emails_string - end - - it { is_expected.to eq("") } - - context "when there are emails" do - before do - region.update( - teaching_authority_emails: %w[a@example.com b@example.com], - ) - end - - it { is_expected.to eq("a@example.com\nb@example.com") } - end - end - - describe "#teaching_authority_emails_string=" do - subject(:teaching_authority_emails) { region.teaching_authority_emails } - - it { is_expected.to eq([]) } - - context "when there are emails" do - before do - region.update( - teaching_authority_emails_string: "a@example.com\nb@example.com\n", - ) - end - - it { is_expected.to eq(%w[a@example.com b@example.com]) } - end - end - - describe "#teaching_authority_websites_string" do - subject(:teaching_authority_websites_string) do - region.teaching_authority_websites_string - end - - it { is_expected.to eq("") } - - context "when there are emails" do - before do - region.update( - teaching_authority_websites: %w[example1.com example2.com], - ) - end - - it { is_expected.to eq("example1.com\nexample2.com") } - end - end - - describe "#teaching_authority_emails_string=" do - subject(:teaching_authority_websites) { region.teaching_authority_websites } - - it { is_expected.to eq([]) } - - context "when there are emails" do - before do - region.update( - teaching_authority_websites_string: "example1.com\nexample2.com\n", - ) - end - - it { is_expected.to eq(%w[example1.com example2.com]) } - end - end - - describe "#teaching_authority_present?" do - subject(:teaching_authority_present?) { region.teaching_authority_present? } - - it { is_expected.to eq(false) } - - context "with a name" do - before { region.update(teaching_authority_name: "Name") } - - it { is_expected.to eq(true) } - end - - context "with an address" do - before { region.update(teaching_authority_address: "Address") } - - it { is_expected.to eq(true) } - end - - context "with an email address" do - before { region.update(teaching_authority_emails: ["test@example.com"]) } - - it { is_expected.to eq(true) } - end - - context "with a website" do - before do - region.update(teaching_authority_websites: ["https://www.example.com"]) - end - - it { is_expected.to eq(true) } - end end end diff --git a/spec/system/support_interface/countries_spec.rb b/spec/system/support_interface/countries_spec.rb index 0dd208c1cd..ee88f9ac78 100644 --- a/spec/system/support_interface/countries_spec.rb +++ b/spec/system/support_interface/countries_spec.rb @@ -134,67 +134,74 @@ def when_i_fill_regions end def when_i_select_sanction_check - select "Online", from: "region-sanction-check-field" + select "Online", from: "support-interface-region-form-sanction-check-field" end def when_i_select_status_check - select "Online", from: "region-status-check-field" + select "Online", from: "support-interface-region-form-status-check-field" end def when_i_fill_teaching_authority_name - fill_in "region-teaching-authority-name-field", with: "Name" + fill_in "support-interface-region-form-teaching-authority-name-field", + with: "Name" end def when_i_fill_teaching_authority_address - fill_in "region-teaching-authority-address-field", with: "Address" + fill_in "support-interface-region-form-teaching-authority-address-field", + with: "Address" end def when_i_fill_teaching_authority_emails - fill_in "region-teaching-authority-emails-string-field", + fill_in "support-interface-region-form-teaching-authority-emails-string-field", with: "Email address" end def when_i_fill_teaching_authority_websites - fill_in "region-teaching-authority-websites-string-field", with: "Website" + fill_in "support-interface-region-form-teaching-authority-websites-string-field", + with: "Website" end def when_i_fill_other_information - fill_in "region-other-information-field", with: "Other" + fill_in "support-interface-region-form-other-information-field", + with: "Other" rescue Capybara::ElementNotFound fill_in "support-interface-country-form-other-information-field", with: "Other" end def when_i_fill_teaching_authority_certificate - fill_in "region-teaching-authority-certificate-field", with: "Certificate" + fill_in "support-interface-region-form-teaching-authority-certificate-field", + with: "Certificate" end def when_i_fill_teaching_authority_online_checker_url - fill_in "region-teaching-authority-online-checker-url-field", + fill_in "support-interface-region-form-teaching-authority-online-checker-url-field", with: "https://www.example.com/checks" end def when_i_fill_sanction_information - fill_in "region-sanction-information-field", with: "Sanction information" + fill_in "support-interface-region-form-sanction-information-field", + with: "Sanction information" rescue Capybara::ElementNotFound fill_in "support-interface-country-form-sanction-information-field", with: "Sanction information" end def when_i_fill_status_information - fill_in "region-status-information-field", with: "Status information" + fill_in "support-interface-region-form-status-information-field", + with: "Status information" rescue Capybara::ElementNotFound fill_in "support-interface-country-form-status-information-field", with: "Status information" end def when_i_select_yes_teaching_authority_requires_submission_email - choose "region-teaching-authority-requires-submission-email-true-field", + choose "support-interface-region-form-teaching-authority-requires-submission-email-true-field", visible: :all end def when_i_fill_teaching_qualification_information - fill_in "region-teaching-qualification-information-field", + fill_in "support-interface-region-form-teaching-qualification-information-field", with: "Qualifications information" rescue Capybara::ElementNotFound fill_in "support-interface-country-form-teaching-qualification-information-field", @@ -202,11 +209,13 @@ def when_i_fill_teaching_qualification_information end def when_i_check_written_statement_optional - choose "region-written-statement-optional-true-field", visible: false + choose "support-interface-region-form-written-statement-optional-true-field", + visible: false end def when_i_check_requires_preliminary_check - choose "region-requires-preliminary-check-true-field", visible: false + choose "support-interface-region-form-requires-preliminary-check-true-field", + visible: false end def and_i_click_save