From d38cd585818ac831c9432e2502affcc8cca7d983 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Mon, 4 Sep 2023 18:10:26 +0100 Subject: [PATCH 1/2] Remove teaching authority fields from countries We're going to keep them only on regions to simplify the support console. --- app/models/country.rb | 26 +++++++---------- config/analytics.yml | 6 ---- ...emove_teaching_authority_from_countries.rb | 29 +++++++++++++++++++ db/schema.rb | 6 ---- spec/factories/countries.rb | 26 +++++++---------- spec/models/country_spec.rb | 26 +++++++---------- 6 files changed, 59 insertions(+), 60 deletions(-) create mode 100644 db/migrate/20230904170502_remove_teaching_authority_from_countries.rb diff --git a/app/models/country.rb b/app/models/country.rb index 24b24479c1..46ae727f68 100644 --- a/app/models/country.rb +++ b/app/models/country.rb @@ -2,22 +2,16 @@ # # Table name: countries # -# id :bigint not null, primary key -# code :string not null -# eligibility_enabled :boolean default(TRUE), not null -# eligibility_skip_questions :boolean default(FALSE), not null -# other_information :text default(""), not null -# qualifications_information :text default(""), not null -# sanction_information :string default(""), not null -# status_information :string default(""), not null -# teaching_authority_address :text default(""), not null -# teaching_authority_certificate :text default(""), not null -# teaching_authority_emails :text default([]), not null, is an Array -# teaching_authority_name :text default(""), not null -# teaching_authority_online_checker_url :string default(""), not null -# teaching_authority_websites :text default([]), not null, is an Array -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint not null, primary key +# code :string not null +# eligibility_enabled :boolean default(TRUE), not null +# eligibility_skip_questions :boolean default(FALSE), not null +# other_information :text default(""), not null +# qualifications_information :text default(""), not null +# sanction_information :string default(""), not null +# status_information :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null # # Indexes # diff --git a/config/analytics.yml b/config/analytics.yml index 142405a577..cb6645e4cd 100644 --- a/config/analytics.yml +++ b/config/analytics.yml @@ -125,12 +125,6 @@ - qualifications_information - sanction_information - status_information - - teaching_authority_address - - teaching_authority_certificate - - teaching_authority_emails - - teaching_authority_name - - teaching_authority_online_checker_url - - teaching_authority_websites - updated_at :documents: - available diff --git a/db/migrate/20230904170502_remove_teaching_authority_from_countries.rb b/db/migrate/20230904170502_remove_teaching_authority_from_countries.rb new file mode 100644 index 0000000000..9f71e6e27a --- /dev/null +++ b/db/migrate/20230904170502_remove_teaching_authority_from_countries.rb @@ -0,0 +1,29 @@ +class RemoveTeachingAuthorityFromCountries < ActiveRecord::Migration[7.0] + def change + change_table :countries, bulk: true do |t| + t.remove :teaching_authority_address, + type: :text, + default: "", + null: false + t.remove :teaching_authority_certificate, + type: :text, + default: "", + null: false + t.remove :teaching_authority_emails, + type: :text, + default: [], + null: false, + array: true + t.remove :teaching_authority_name, type: :text, default: "", null: false + t.remove :teaching_authority_online_checker_url, + type: :string, + default: "", + null: false + t.remove :teaching_authority_websites, + type: :text, + default: [], + null: false, + array: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4ca9121929..4aa291268f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -162,12 +162,6 @@ t.boolean "eligibility_enabled", default: true, null: false t.boolean "eligibility_skip_questions", default: false, null: false t.text "qualifications_information", default: "", null: false - t.text "teaching_authority_websites", default: [], null: false, array: true - t.string "teaching_authority_online_checker_url", default: "", null: false - t.text "teaching_authority_name", default: "", null: false - t.text "teaching_authority_emails", default: [], null: false, array: true - t.text "teaching_authority_certificate", default: "", null: false - t.text "teaching_authority_address", default: "", null: false t.index ["code"], name: "index_countries_on_code", unique: true end diff --git a/spec/factories/countries.rb b/spec/factories/countries.rb index eec697157f..9d00e2b925 100644 --- a/spec/factories/countries.rb +++ b/spec/factories/countries.rb @@ -2,22 +2,16 @@ # # Table name: countries # -# id :bigint not null, primary key -# code :string not null -# eligibility_enabled :boolean default(TRUE), not null -# eligibility_skip_questions :boolean default(FALSE), not null -# other_information :text default(""), not null -# qualifications_information :text default(""), not null -# sanction_information :string default(""), not null -# status_information :string default(""), not null -# teaching_authority_address :text default(""), not null -# teaching_authority_certificate :text default(""), not null -# teaching_authority_emails :text default([]), not null, is an Array -# teaching_authority_name :text default(""), not null -# teaching_authority_online_checker_url :string default(""), not null -# teaching_authority_websites :text default([]), not null, is an Array -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint not null, primary key +# code :string not null +# eligibility_enabled :boolean default(TRUE), not null +# eligibility_skip_questions :boolean default(FALSE), not null +# other_information :text default(""), not null +# qualifications_information :text default(""), not null +# sanction_information :string default(""), not null +# status_information :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null # # Indexes # diff --git a/spec/models/country_spec.rb b/spec/models/country_spec.rb index 216a9269ca..ca1ac49712 100644 --- a/spec/models/country_spec.rb +++ b/spec/models/country_spec.rb @@ -2,22 +2,16 @@ # # Table name: countries # -# id :bigint not null, primary key -# code :string not null -# eligibility_enabled :boolean default(TRUE), not null -# eligibility_skip_questions :boolean default(FALSE), not null -# other_information :text default(""), not null -# qualifications_information :text default(""), not null -# sanction_information :string default(""), not null -# status_information :string default(""), not null -# teaching_authority_address :text default(""), not null -# teaching_authority_certificate :text default(""), not null -# teaching_authority_emails :text default([]), not null, is an Array -# teaching_authority_name :text default(""), not null -# teaching_authority_online_checker_url :string default(""), not null -# teaching_authority_websites :text default([]), not null, is an Array -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint not null, primary key +# code :string not null +# eligibility_enabled :boolean default(TRUE), not null +# eligibility_skip_questions :boolean default(FALSE), not null +# other_information :text default(""), not null +# qualifications_information :text default(""), not null +# sanction_information :string default(""), not null +# status_information :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null # # Indexes # From 633fdc3296514ca8e566f90a734e6c014db33be5 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Mon, 4 Sep 2023 19:09:52 +0100 Subject: [PATCH 2/2] Remove references to removed fields The teaching authority fields have been removed from countries so we can remove the references to them in various other places. --- .../support_interface/countries_controller.rb | 16 --- app/forms/support_interface/country_form.rb | 12 -- app/helpers/region_helper.rb | 20 ++- app/mailers/teaching_authority_mailer.rb | 4 +- .../teaching_authority_contactable.rb | 36 ------ app/models/country.rb | 6 - app/models/region.rb | 30 ++++- .../_professional_standing_summary.html.erb | 11 +- ...ing_authority_contact_information.html.erb | 24 +++- .../_teaching_authority_contactable.html.erb | 23 ---- .../forms/_status_sanctions_checks.html.erb | 13 -- .../_teaching_authority_form_fields.html.erb | 7 -- .../support_interface/countries/edit.html.erb | 1 - .../countries/preview.html.erb | 16 --- .../support_interface/regions/edit.html.erb | 44 ++++++- spec/factories/countries.rb | 6 - spec/factories/regions.rb | 6 - spec/helpers/region_helper_spec.rb | 23 +--- spec/models/country_spec.rb | 118 +----------------- .../support_interface/countries_spec.rb | 43 ++----- .../shared/eligible_region_content_spec.rb | 19 +-- ...hing_authority_contact_information_spec.rb | 56 ++------- .../teaching_authority_contactable_spec.rb | 37 ------ 23 files changed, 124 insertions(+), 447 deletions(-) delete mode 100644 app/models/concerns/teaching_authority_contactable.rb delete mode 100644 app/views/shared/_teaching_authority_contactable.html.erb delete mode 100644 app/views/shared/support_interface/forms/_status_sanctions_checks.html.erb delete mode 100644 app/views/shared/support_interface/forms/_teaching_authority_form_fields.html.erb delete mode 100644 spec/views/shared/teaching_authority_contactable_spec.rb diff --git a/app/controllers/support_interface/countries_controller.rb b/app/controllers/support_interface/countries_controller.rb index 17cc4993f0..d2a2c0856b 100644 --- a/app/controllers/support_interface/countries_controller.rb +++ b/app/controllers/support_interface/countries_controller.rb @@ -19,16 +19,6 @@ def edit region_names: country.regions.pluck(:name).join("\n"), sanction_information: country.sanction_information, status_information: country.status_information, - 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_websites_string: - country.teaching_authority_websites_string, ) end @@ -71,12 +61,6 @@ def country_params :region_names, :sanction_information, :status_information, - :teaching_authority_address, - :teaching_authority_certificate, - :teaching_authority_emails_string, - :teaching_authority_name, - :teaching_authority_online_checker_url, - :teaching_authority_websites_string, ) end end diff --git a/app/forms/support_interface/country_form.rb b/app/forms/support_interface/country_form.rb index 3f826e1476..c4d0e8d657 100644 --- a/app/forms/support_interface/country_form.rb +++ b/app/forms/support_interface/country_form.rb @@ -15,12 +15,6 @@ class SupportInterface::CountryForm attribute :region_names, :string attribute :sanction_information, :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_websites_string, :string validates :eligibility_enabled, inclusion: { in: [true, false] } validates :eligibility_skip_questions, inclusion: { in: [true, false] } @@ -55,12 +49,6 @@ def assign_country_attributes qualifications_information:, sanction_information:, status_information:, - teaching_authority_address:, - teaching_authority_certificate:, - teaching_authority_emails_string:, - teaching_authority_name:, - teaching_authority_online_checker_url:, - teaching_authority_websites_string:, ) end diff --git a/app/helpers/region_helper.rb b/app/helpers/region_helper.rb index 1df9edf7f7..6368cef49e 100644 --- a/app/helpers/region_helper.rb +++ b/app/helpers/region_helper.rb @@ -1,20 +1,18 @@ # frozen_string_literal: true module RegionHelper - def region_certificate_phrase(region) - certificate = region_certificate_name(region) - "#{certificate.indefinite_article} #{tag.span(certificate, lang: region.country.code)}".html_safe - end - def region_certificate_name(region) region.teaching_authority_certificate.presence || - region.country.teaching_authority_certificate.presence || "letter that proves you’re recognised as a teacher" end + def region_certificate_phrase(region) + certificate = region_certificate_name(region) + "#{certificate.indefinite_article} #{tag.span(certificate, lang: region.country.code)}".html_safe + end + def region_teaching_authority_name(region) - region.teaching_authority_name.presence || - region.country.teaching_authority_name.presence || "teaching authority" + region.teaching_authority_name.presence || "teaching authority" end def region_teaching_authority_name_phrase(region) @@ -22,10 +20,8 @@ def region_teaching_authority_name_phrase(region) end def region_teaching_authority_emails_phrase(region) - emails = - region.teaching_authority_emails + - region.country.teaching_authority_emails - emails + region + .teaching_authority_emails .map { |email| govuk_link_to email, "mailto:#{email}" } .to_sentence .html_safe diff --git a/app/mailers/teaching_authority_mailer.rb b/app/mailers/teaching_authority_mailer.rb index 3af67d078e..48e3851988 100644 --- a/app/mailers/teaching_authority_mailer.rb +++ b/app/mailers/teaching_authority_mailer.rb @@ -10,7 +10,7 @@ class TeachingAuthorityMailer < ApplicationMailer def application_submitted view_mail( GOVUK_NOTIFY_TEMPLATE_ID, - to: region.teaching_authority_emails + country.teaching_authority_emails, + to: region.teaching_authority_emails, subject: I18n.t( "mailer.teaching_authority.application_submitted.subject", @@ -25,7 +25,7 @@ def application_form params[:application_form] end - delegate :region, :country, to: :application_form + delegate :region, to: :application_form def set_application_form @application_form = application_form diff --git a/app/models/concerns/teaching_authority_contactable.rb b/app/models/concerns/teaching_authority_contactable.rb deleted file mode 100644 index 802fff520b..0000000000 --- a/app/models/concerns/teaching_authority_contactable.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module TeachingAuthorityContactable - extend ActiveSupport::Concern - - included do - validates :teaching_authority_name, - format: { - without: /\Athe.*\z/i, - message: "Teaching authority name shouldn't start with ‘the’.", - } - 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 -end diff --git a/app/models/country.rb b/app/models/country.rb index 46ae727f68..bdc731b17f 100644 --- a/app/models/country.rb +++ b/app/models/country.rb @@ -18,8 +18,6 @@ # index_countries_on_code (code) UNIQUE # class Country < ApplicationRecord - include TeachingAuthorityContactable - has_many :regions LOCATION_AUTOCOMPLETE_CANONICAL_LIST = @@ -40,8 +38,4 @@ class Country < ApplicationRecord CODES_ELIGIBLE_IN_FEBRUARY_2023 - %w[HK UA] validates :code, inclusion: { in: CODES } - - validates :teaching_authority_online_checker_url, url: { allow_blank: true } - - alias_method :country, :itself end diff --git a/app/models/region.rb b/app/models/region.rb index 2dc7d84824..bfe3e0d7e6 100644 --- a/app/models/region.rb +++ b/app/models/region.rb @@ -36,8 +36,6 @@ # fk_rails_... (country_id => countries.id) # class Region < ApplicationRecord - include TeachingAuthorityContactable - belongs_to :country has_many :eligibility_checks @@ -55,9 +53,37 @@ 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 end diff --git a/app/views/assessor_interface/assessment_sections/_professional_standing_summary.html.erb b/app/views/assessor_interface/assessment_sections/_professional_standing_summary.html.erb index 4bfa90c9f9..679a3bc2db 100644 --- a/app/views/assessor_interface/assessment_sections/_professional_standing_summary.html.erb +++ b/app/views/assessor_interface/assessment_sections/_professional_standing_summary.html.erb @@ -21,17 +21,10 @@ <% end %> <%= govuk_accordion do |accordion| %> - <% if region.teaching_authority_online_checker_url.present? || region.country.teaching_authority_online_checker_url.present? %> + <% if (online_checker_url = region.teaching_authority_online_checker_url).present? %> <% accordion.with_section(heading_text: "Online checker") do %>

This authority has an online checker for validating the supplied registration number:

- - <% if (online_checker_url = region.teaching_authority_online_checker_url).present? %> -

<%= govuk_link_to online_checker_url, online_checker_url, target: "_blank", rel: "noreferrer noopener" %>

- <% end %> - - <% if (online_checker_url = region.country.teaching_authority_online_checker_url).present? %> -

<%= govuk_link_to online_checker_url, online_checker_url, target: "_blank", rel: "noreferrer noopener" %>

- <% end %> +

<%= govuk_link_to online_checker_url, online_checker_url, target: "_blank", rel: "noreferrer noopener" %>

<% end %> <% end %> diff --git a/app/views/shared/_teaching_authority_contact_information.html.erb b/app/views/shared/_teaching_authority_contact_information.html.erb index 9b2e89f8d0..db2502695b 100644 --- a/app/views/shared/_teaching_authority_contact_information.html.erb +++ b/app/views/shared/_teaching_authority_contact_information.html.erb @@ -1,7 +1,23 @@ -<%= render "shared/teaching_authority_contactable", contactable: region.country %> +<% if region.teaching_authority_name.present? %> +

<%= region.teaching_authority_name %>

+<% end %> + +<% if region.teaching_authority_address.present? %> +

<%= region.teaching_authority_address %>

+<% end %> -<% if region.country.teaching_authority_present? && region.teaching_authority_present? %> -

or

+<% if region.teaching_authority_emails.present? %> +

+ <% region.teaching_authority_emails.each do |email| %> + <%= govuk_link_to email, "mailto:#{email}" %>
+ <% end %> +

<% end %> -<%= render "shared/teaching_authority_contactable", contactable: region %> +<% if region.teaching_authority_websites.present? %> +

+ <% region.teaching_authority_websites.each do |website| %> + <%= govuk_link_to website, website %>
+ <% end %> +

+<% end %> diff --git a/app/views/shared/_teaching_authority_contactable.html.erb b/app/views/shared/_teaching_authority_contactable.html.erb deleted file mode 100644 index 654e8c3906..0000000000 --- a/app/views/shared/_teaching_authority_contactable.html.erb +++ /dev/null @@ -1,23 +0,0 @@ -<% if contactable.teaching_authority_name.present? %> -

<%= contactable.teaching_authority_name %>

-<% end %> - -<% if contactable.teaching_authority_address.present? %> -

<%= contactable.teaching_authority_address %>

-<% end %> - -<% if contactable.teaching_authority_emails.present? %> -

- <% contactable.teaching_authority_emails.each do |email| %> - <%= govuk_link_to email, "mailto:#{email}" %>
- <% end %> -

-<% end %> - -<% if contactable.teaching_authority_websites.present? %> -

- <% contactable.teaching_authority_websites.each do |website| %> - <%= govuk_link_to website, website %>
- <% end %> -

-<% end %> diff --git a/app/views/shared/support_interface/forms/_status_sanctions_checks.html.erb b/app/views/shared/support_interface/forms/_status_sanctions_checks.html.erb deleted file mode 100644 index 063843a7b3..0000000000 --- a/app/views/shared/support_interface/forms/_status_sanctions_checks.html.erb +++ /dev/null @@ -1,13 +0,0 @@ -

Teacher status and sanctions checks

- -<%= f.govuk_collection_select :status_check, [ -OpenStruct.new(id: 'online', name: 'Online'), -OpenStruct.new(id: 'written', name: 'Written'), -OpenStruct.new(id: 'none', name: 'None') -], :id, :name, label: { text: "How will teacher status be checked?" } %> - -<%= f.govuk_collection_select :sanction_check, [ -OpenStruct.new(id: 'online', name: 'Online'), -OpenStruct.new(id: 'written', name: 'Written'), -OpenStruct.new(id: 'none', name: 'None') -], :id, :name, label: { text: "How will sanctions be checked?" } %> diff --git a/app/views/shared/support_interface/forms/_teaching_authority_form_fields.html.erb b/app/views/shared/support_interface/forms/_teaching_authority_form_fields.html.erb deleted file mode 100644 index f313dc231f..0000000000 --- a/app/views/shared/support_interface/forms/_teaching_authority_form_fields.html.erb +++ /dev/null @@ -1,7 +0,0 @@ -

Teaching authority details

-<%= f.govuk_text_field :teaching_authority_name, label: { text: "Name:" } %> -<%= f.govuk_text_area :teaching_authority_address, label: { text: "Address:" } %> -<%= f.govuk_text_area :teaching_authority_emails_string, label: { text: "Email:" }, hint: { text: "One email per line (if more than one)" } %> -<%= f.govuk_text_area :teaching_authority_websites_string, label: { text: "Websites:" }, hint: { text: "One website per line (if more than one)" } %> -<%= f.govuk_text_field :teaching_authority_certificate, label: { text: "Name given to describe the Letter of Professional standing: " } %> -<%= f.govuk_text_field :teaching_authority_online_checker_url, label: { text: "Online checker URL" } %> diff --git a/app/views/support_interface/countries/edit.html.erb b/app/views/support_interface/countries/edit.html.erb index aafd787033..4c42cdcb1f 100644 --- a/app/views/support_interface/countries/edit.html.erb +++ b/app/views/support_interface/countries/edit.html.erb @@ -31,7 +31,6 @@ legend: { text: "Eligibility checker" }, hint: { text: 'Which route will applicants take through the eligibility checker?' } %> - <%= render "shared/support_interface/forms/teaching_authority_form_fields", f: %> <%= render "shared/support_interface/forms/additional_information", f: %> <%= f.govuk_submit "Preview", name: "preview", value: "true" do %> diff --git a/app/views/support_interface/countries/preview.html.erb b/app/views/support_interface/countries/preview.html.erb index eacbc48a55..fb848e1add 100644 --- a/app/views/support_interface/countries/preview.html.erb +++ b/app/views/support_interface/countries/preview.html.erb @@ -33,16 +33,6 @@

<%= govuk_boolean_tag(@country.eligibility_enabled) %>

<% end %> -<% if @country.changes.keys.select { |k| k.starts_with?("teaching_authority_") }.any? %> -

Teaching authority

- - <%= render "shared/teaching_authority_contactable", contactable: @country %> - -

<%= @country.teaching_authority_certificate %>

- -

<%= @country.teaching_authority_online_checker_url %>

-<% end %> - <% if @country.sanction_information_changed? %>

Sanction information

<%= raw GovukMarkdown.render(@country.sanction_information) %> @@ -72,11 +62,5 @@ <%= f.hidden_field :region_names, value: @form.region_names %> <%= f.hidden_field :sanction_information, value: @form.sanction_information %> <%= 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_websites_string, value: @form.teaching_authority_websites_string %> <%= f.govuk_submit "Save", name: "preview", value: "false" %> <% end %> diff --git a/app/views/support_interface/regions/edit.html.erb b/app/views/support_interface/regions/edit.html.erb index 575ab1134f..fe41d76262 100644 --- a/app/views/support_interface/regions/edit.html.erb +++ b/app/views/support_interface/regions/edit.html.erb @@ -16,17 +16,49 @@ :label, legend: { text: "Will applications be subject to a preliminary check?", size: "s" } %> - <%= render "shared/support_interface/forms/status_sanctions_checks", f: %> - <%= render "shared/support_interface/forms/teaching_authority_form_fields", f: %> + <%= f.govuk_fieldset legend: { text: "Teacher status and sanctions checks" } do %> + <%= f.govuk_collection_select :status_check, [ + OpenStruct.new(id: 'online', name: 'Online'), + OpenStruct.new(id: 'written', name: 'Written'), + OpenStruct.new(id: 'none', name: 'None') + ], :id, :name, label: { text: "How will teacher status be checked?", size: "s" } %> - <%= f.govuk_radio_buttons_fieldset :teaching_authority_requires_submission_email, legend: { size: 'm', text: 'Automatically notify the teaching authority (via email) every time a QTS application is submitted?' } do %> - <%= f.govuk_radio_button :teaching_authority_requires_submission_email, 'yes', label: { text: 'Yes' }, link_errors: true %> - <%= f.govuk_radio_button :teaching_authority_requires_submission_email, 'no', label: { text: 'No' }%> + <%= f.govuk_collection_select :sanction_check, [ + OpenStruct.new(id: 'online', name: 'Online'), + OpenStruct.new(id: 'written', name: 'Written'), + OpenStruct.new(id: 'none', name: 'None') + ], :id, :name, label: { text: "How will sanctions be checked?", size: "s" } %> + <% end %> + + <%= f.govuk_fieldset legend: { text: "Teaching authority details" } do %> + <%= f.govuk_text_field :teaching_authority_name, label: { text: "Name" } %> + <%= f.govuk_text_area :teaching_authority_address, label: { text: "Address" } %> + <%= f.govuk_text_area :teaching_authority_emails_string, label: { text: "Email" }, hint: { text: "One email per line (if more than one)" } %> + <%= f.govuk_text_area :teaching_authority_websites_string, label: { text: "Websites" }, hint: { text: "One website per line (if more than one)" } %> + <%= f.govuk_text_field :teaching_authority_certificate, label: { text: "Name given to describe the Letter of Professional standing" } %> + <%= f.govuk_text_field :teaching_authority_online_checker_url, label: { text: "Online checker website" } %> + + <%= f.govuk_collection_radio_buttons :teaching_authority_requires_submission_email, + [ + OpenStruct.new(value: :true, label: "Yes"), + OpenStruct.new(value: :false, label: "No") + ], + :value, + :label, + legend: { text: "Automatically notify the teaching authority (via email) every time a QTS application is submitted?", size: "s" } %> + + <%= f.govuk_collection_radio_buttons :teaching_authority_provides_written_statement, + [ + OpenStruct.new(value: :true, label: "Yes"), + OpenStruct.new(value: :false, label: "No") + ], + :value, + :label, + legend: { text: "Will the teaching authority only send the letter of professional standing (LOPS) directly to the TRA?", size: "s" } %> <% end %> <%= f.govuk_check_box :reduced_evidence_accepted, 1, 0, multiple: false, link_errors: true, small: true, label: { text: "Accept reduced evidence" } %> <%= f.govuk_check_box :written_statement_optional, 1, 0, multiple: false, link_errors: true, small: true, label: { text: "Applicant can submit without uploading the written statement (if a written statement is requested)" } %> - <%= f.govuk_check_box :teaching_authority_provides_written_statement, 1, 0, multiple: false, link_errors: true, small: true, label: { text: "Will the teaching authority only send the letter of professional standing (LOPS) directly to the TRA?" } %> <%= render "shared/support_interface/forms/additional_information", f: %> diff --git a/spec/factories/countries.rb b/spec/factories/countries.rb index 9d00e2b925..b716fbeeba 100644 --- a/spec/factories/countries.rb +++ b/spec/factories/countries.rb @@ -39,11 +39,5 @@ create(:region, :national, country:) end end - - trait :with_teaching_authority do - teaching_authority_address { Faker::Address.street_address } - teaching_authority_emails { [Faker::Internet.email] } - teaching_authority_websites { [Faker::Internet.url] } - end end end diff --git a/spec/factories/regions.rb b/spec/factories/regions.rb index f53fc52ee0..2198c567bf 100644 --- a/spec/factories/regions.rb +++ b/spec/factories/regions.rb @@ -72,12 +72,6 @@ status_check { :none } end - trait :with_teaching_authority do - teaching_authority_address { Faker::Address.street_address } - teaching_authority_emails { [Faker::Internet.email] } - teaching_authority_websites { [Faker::Internet.url] } - end - trait :in_country do transient { country_code { "" } } country { Country.find_or_create_by(code: country_code) } diff --git a/spec/helpers/region_helper_spec.rb b/spec/helpers/region_helper_spec.rb index 24492b063d..ebd326f086 100644 --- a/spec/helpers/region_helper_spec.rb +++ b/spec/helpers/region_helper_spec.rb @@ -14,27 +14,10 @@ ) end - context "with a region certificate" do - before { region.teaching_authority_certificate = "region certificate" } + context "with custom value" do + before { region.teaching_authority_certificate = "certificate" } - it { is_expected.to eq("a region certificate") } - end - - context "with a country certificate" do - before do - region.country.teaching_authority_certificate = "country certificate" - end - - it { is_expected.to eq("a country certificate") } - end - - context "with a region and country certificate" do - before do - region.teaching_authority_certificate = "region certificate" - region.country.teaching_authority_certificate = "country certificate" - end - - it { is_expected.to eq("a region certificate") } + it { is_expected.to eq("a certificate") } end end end diff --git a/spec/models/country_spec.rb b/spec/models/country_spec.rb index ca1ac49712..013aaf568a 100644 --- a/spec/models/country_spec.rb +++ b/spec/models/country_spec.rb @@ -20,128 +20,12 @@ require "rails_helper" RSpec.describe Country, type: :model do - subject(:country) { build(:country, teaching_authority_name:) } - - let(:teaching_authority_name) { "" } + subject(:country) { build(:country) } describe "validations" do it { is_expected.to be_valid } it { is_expected.to validate_inclusion_of(:code).in_array(%w[GB-SCT FR]) } it { is_expected.to_not validate_inclusion_of(:code).in_array(%w[ABC]) } - 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 - country.teaching_authority_emails_string - end - - it { is_expected.to eq("") } - - context "when there are emails" do - before do - country.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) { country.teaching_authority_emails } - - it { is_expected.to eq([]) } - - context "when there are emails" do - before do - country.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 - country.teaching_authority_websites_string - end - - it { is_expected.to eq("") } - - context "when there are emails" do - before do - country.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) do - country.teaching_authority_websites - end - - it { is_expected.to eq([]) } - - context "when there are emails" do - before do - country.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?) do - country.teaching_authority_present? - end - - it { is_expected.to eq(false) } - - context "with a name" do - before { country.update(teaching_authority_name: "Name") } - - it { is_expected.to eq(true) } - end - - context "with an address" do - before { country.update(teaching_authority_address: "Address") } - - it { is_expected.to eq(true) } - end - - context "with an email address" do - before { country.update(teaching_authority_emails: ["test@example.com"]) } - - it { is_expected.to eq(true) } - end - - context "with a website" do - before do - country.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 536eee6a7f..0ab0a03186 100644 --- a/spec/system/support_interface/countries_spec.rb +++ b/spec/system/support_interface/countries_spec.rb @@ -19,19 +19,13 @@ when_i_click_on_a_country then_i_see_a_country - when_i_fill_teaching_authority_name - when_i_fill_teaching_authority_address - when_i_fill_teaching_authority_emails - when_i_fill_teaching_authority_websites when_i_fill_other_information when_i_fill_sanction_information when_i_fill_status_information - when_i_fill_teaching_authority_certificate - when_i_fill_teaching_authority_online_checker_url when_i_fill_qualifications_information when_i_fill_regions and_i_click_preview - then_i_see_country_contact_preview + then_i_see_country_changes_preview then_i_see_region_changes_confirmation and_i_click_save @@ -40,15 +34,15 @@ when_i_select_sanction_check when_i_select_status_check - when_i_fill_teaching_authority_name - when_i_fill_teaching_authority_address - when_i_fill_teaching_authority_emails - when_i_fill_teaching_authority_websites when_i_fill_other_information when_i_fill_sanction_information when_i_fill_status_information + when_i_fill_teaching_authority_address when_i_fill_teaching_authority_certificate + when_i_fill_teaching_authority_emails + when_i_fill_teaching_authority_name when_i_fill_teaching_authority_online_checker_url + when_i_fill_teaching_authority_websites when_i_select_yes_teaching_authority_requires_submission_email when_i_fill_qualifications_information when_i_check_written_statement_optional @@ -105,12 +99,7 @@ def then_i_see_a_country expect(page).to have_title("United States") end - def then_i_see_country_contact_preview - expect(page).to have_content("Name") - expect(page).to have_content("Address") - expect(page).to have_content("Email address") - expect(page).to have_content("Website") - expect(page).to have_content("Certificate") + def then_i_see_country_changes_preview expect(page).to have_content("Other") expect(page).to have_content("Qualifications information") end @@ -154,31 +143,19 @@ def when_i_select_status_check def when_i_fill_teaching_authority_name fill_in "region-teaching-authority-name-field", with: "Name" - rescue Capybara::ElementNotFound - fill_in "support-interface-country-form-teaching-authority-name-field", - with: "Name" end def when_i_fill_teaching_authority_address fill_in "region-teaching-authority-address-field", with: "Address" - rescue Capybara::ElementNotFound - fill_in "support-interface-country-form-teaching-authority-address-field", - with: "Address" end def when_i_fill_teaching_authority_emails fill_in "region-teaching-authority-emails-string-field", with: "Email address" - rescue Capybara::ElementNotFound - fill_in "support-interface-country-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" - rescue Capybara::ElementNotFound - fill_in "support-interface-country-form-teaching-authority-websites-string-field", - with: "Website" end def when_i_fill_other_information @@ -190,17 +167,11 @@ def when_i_fill_other_information def when_i_fill_teaching_authority_certificate fill_in "region-teaching-authority-certificate-field", with: "Certificate" - rescue Capybara::ElementNotFound - fill_in "support-interface-country-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", with: "https://www.example.com/checks" - rescue Capybara::ElementNotFound - fill_in "support-interface-country-form-teaching-authority-online-checker-url-field", - with: "https://www.example.com/checks" end def when_i_fill_sanction_information @@ -218,7 +189,7 @@ def when_i_fill_status_information end def when_i_select_yes_teaching_authority_requires_submission_email - choose "region-teaching-authority-requires-submission-email-yes-field", + choose "region-teaching-authority-requires-submission-email-true-field", visible: :all end diff --git a/spec/views/shared/eligible_region_content_spec.rb b/spec/views/shared/eligible_region_content_spec.rb index f82afa5d7d..a963f13040 100644 --- a/spec/views/shared/eligible_region_content_spec.rb +++ b/spec/views/shared/eligible_region_content_spec.rb @@ -24,17 +24,13 @@ let(:region) do create( :region, - country: - create( - :country, - code: "FR", - teaching_authority_certificate: "certificate", - ), + country: create(:country, code: "FR"), status_check: :written, sanction_check: :written, status_information: "Status information", sanction_information: "Sanction information", teaching_authority_address: "address", + teaching_authority_certificate: "certificate", qualifications_information: "qualifications info", ) end @@ -51,11 +47,10 @@ let(:region) do create( :region, - country: - create(:country, teaching_authority_certificate: "certificate"), status_check: :online, sanction_check: :written, teaching_authority_address: "address", + teaching_authority_certificate: "certificate", ) end @@ -71,15 +66,11 @@ let(:region) do create( :region, - country: - create( - :country, - code: "FR", - teaching_authority_certificate: "certificate", - ), + country: create(:country, code: "FR"), status_check: :written, sanction_check: :none, teaching_authority_address: "address", + teaching_authority_certificate: "certificate", ) end diff --git a/spec/views/shared/teaching_authority_contact_information_spec.rb b/spec/views/shared/teaching_authority_contact_information_spec.rb index d362d11ba3..acca7e448b 100644 --- a/spec/views/shared/teaching_authority_contact_information_spec.rb +++ b/spec/views/shared/teaching_authority_contact_information_spec.rb @@ -5,52 +5,16 @@ subject { rendered } - context "with a region with teaching authority" do - let(:region) do - create( - :region, - :with_teaching_authority, - teaching_authority_address: "1 Street, Region, Country", - ) - end - - it { is_expected.to_not include('

or

') } - it { is_expected.to include(region.teaching_authority_address) } - end - - context "with a country with teaching authority" do - let(:country) do - create( - :country, - :with_teaching_authority, - teaching_authority_address: "1 Street, Country", - ) - end - let(:region) { create(:region, country:) } - - it { is_expected.to_not include('

or

') } - it { is_expected.to include(country.teaching_authority_address) } + let(:region) do + create( + :region, + teaching_authority_address: "Address", + teaching_authority_emails: ["test@example.com"], + teaching_authority_websites: ["https://www.example.com"], + ) end - context "with a country and a region with teaching authority" do - let(:country) do - create( - :country, - :with_teaching_authority, - teaching_authority_address: "1 Street, Country", - ) - end - let(:region) do - create( - :region, - :with_teaching_authority, - teaching_authority_address: "1 Street, Region, Country", - country:, - ) - end - - it { is_expected.to include('

or

') } - it { is_expected.to include(region.teaching_authority_address) } - it { is_expected.to include(country.teaching_authority_address) } - end + it { is_expected.to match(/Address/) } + it { is_expected.to match(/test@example\.com/) } + it { is_expected.to match(/www\.example\.com/) } end diff --git a/spec/views/shared/teaching_authority_contactable_spec.rb b/spec/views/shared/teaching_authority_contactable_spec.rb deleted file mode 100644 index 6e067ba651..0000000000 --- a/spec/views/shared/teaching_authority_contactable_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -require "rails_helper" - -RSpec.describe "Teaching authority contactable", type: :view do - before { render "shared/teaching_authority_contactable", contactable: } - - subject { rendered } - - context "with a country" do - let(:contactable) do - create( - :country, - teaching_authority_address: "Address", - teaching_authority_emails: ["test@example.com"], - teaching_authority_websites: ["https://www.example.com"], - ) - end - - it { is_expected.to match(/Address/) } - it { is_expected.to match(/test@example\.com/) } - it { is_expected.to match(/www\.example\.com/) } - end - - context "with a region" do - let(:contactable) do - create( - :region, - teaching_authority_address: "Address", - teaching_authority_emails: ["test@example.com"], - teaching_authority_websites: ["https://www.example.com"], - ) - end - - it { is_expected.to match(/Address/) } - it { is_expected.to match(/test@example\.com/) } - it { is_expected.to match(/www\.example\.com/) } - end -end