diff --git a/.review_apps/ecs_task_definition.tf b/.review_apps/ecs_task_definition.tf index 04d521cb3b..6b15ee2b11 100644 --- a/.review_apps/ecs_task_definition.tf +++ b/.review_apps/ecs_task_definition.tf @@ -25,7 +25,8 @@ locals { { name = "SETTINGS__AUTH_PROVIDER", value = "developer" }, { name = "SETTINGS__FORMS_ENV", value = "review" }, { name = "SETTINGS__FORMS_RUNNER__URL", value = "https://forms.service.gov.uk" }, - { name = "SETTINGS__FEATURES__SEND_FILLER_ANSWERS", value = "true" } + { name = "SETTINGS__FEATURES__SEND_FILLER_ANSWERS", value = "true" }, + { name = "SETTINGS__FEATURES__SHOW_RELEVANT_ORGANISATIONS", value = "true" } ] } diff --git a/app/controllers/account/organisations_controller.rb b/app/controllers/account/organisations_controller.rb index 30d215e0c2..462385a712 100644 --- a/app/controllers/account/organisations_controller.rb +++ b/app/controllers/account/organisations_controller.rb @@ -6,25 +6,28 @@ class OrganisationsController < WebController skip_before_action :redirect_if_account_not_completed def edit - @organisation_input = OrganisationInput.new(user: current_user).assign_form_values + @organisation_input = OrganisationInput.new(user: current_user, allowed_organisations:).assign_form_values end def update - @organisation_input = OrganisationInput.new(account_organisation_input_params(current_user)) + @organisation_input = OrganisationInput.new(account_organisation_input_params) - if @organisation_input.submit + if @organisation_input.not_listed_selected? + render :not_listed + elsif @organisation_input.submit redirect_to next_path else render :edit, status: :unprocessable_content end end - def account_organisation_input_params(user) - params.require(:account_organisation_input).permit(:organisation_id).merge(user:) - end - private + def account_organisation_input_params + params.fetch(:account_organisation_input, {}).permit(:organisation_id) + .merge({ user: current_user, allowed_organisations: }) + end + def redirect_if_organisation_exists redirect_to root_path if current_user.organisation.present? end @@ -32,5 +35,13 @@ def redirect_if_organisation_exists def next_path after_sign_in_next_path end + + def allowed_organisations + if FeatureService.enabled?(:show_relevant_organisations) + Organisation.not_closed.for_domain(current_user.email_domain).order(:name) + else + Organisation.not_closed.order(:name) + end + end end end diff --git a/app/input_objects/account/organisation_input.rb b/app/input_objects/account/organisation_input.rb index a3cc8b6381..190194b12d 100644 --- a/app/input_objects/account/organisation_input.rb +++ b/app/input_objects/account/organisation_input.rb @@ -1,7 +1,9 @@ class Account::OrganisationInput < BaseInput - attr_accessor :user, :organisation_id + attr_accessor :user, :allowed_organisations, :organisation_id - validates :organisation_id, presence: true + validates :organisation_id, presence: true, inclusion: { in: ->(record) { record.allowed_organisation_ids } } + + NOT_LISTED_OPTION_VALUE = "not_listed".freeze def submit return false if invalid? @@ -19,6 +21,18 @@ def assign_form_values self end + def radios? + FeatureService.enabled?(:show_relevant_organisations) && allowed_organisations.size <= 30 + end + + def not_listed_selected? + organisation_id == NOT_LISTED_OPTION_VALUE + end + + def allowed_organisation_ids + allowed_organisations.pluck(:id).map(&:to_s) + end + private def log_organisation_chosen_event diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 15de4899fa..73e57e0c5a 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -38,6 +38,11 @@ class Organisation < ApplicationRecord end } + scope :for_domain, lambda { |domain| + joins(:organisation_domains) + .where(organisation_domains: { domain: domain }) + } + scope :order_by_user_count, lambda { order(Arel.sql("(SELECT COUNT(*) FROM users WHERE users.organisation_id = organisations.id) DESC")) .order(:name) diff --git a/app/models/user.rb b/app/models/user.rb index d2692b23dc..95708570de 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -152,6 +152,10 @@ def as_json(options = {}) super(options) end + def email_domain + email.split("@").last.downcase.strip + end + private def requires_name? diff --git a/app/views/account/organisations/edit.html.erb b/app/views/account/organisations/edit.html.erb index 66261814e3..3137f892bd 100644 --- a/app/views/account/organisations/edit.html.erb +++ b/app/views/account/organisations/edit.html.erb @@ -1,29 +1,67 @@ <% set_page_title(title_with_error_prefix(t('page_titles.account_organisation'), @organisation_input.errors.any?)) %>
<%= t("account.organisation.you_created_account_with", email_domain: @organisation_input.user.email_domain) %>
+ <% end %> + + <% if @organisation_input.radios? %> + <%= f.govuk_radio_buttons_fieldset(:organisation_id, legend: { size: 'm', tag: 'h2' }) do %> + <% @organisation_input.allowed_organisations.each.with_index do |organisation, index| %> + <%= f.govuk_radio_button( + :organisation_id, + organisation.id, + label: { text: organisation.name_with_abbreviation }, + link_errors: index.zero?) %> + <% end %> + + <%= f.govuk_radio_divider %> + <%= f.govuk_radio_button( + :organisation_id, + Account::OrganisationInput::NOT_LISTED_OPTION_VALUE, + label: { text: t("account.organisation.not_listed_option") }, + link_errors: false + ) %> + <% end %> + <% else %> + <% label_options = if show_relevant_organisations_enabled + { size: 'm', tag: 'h2', text: t("helpers.legend.account_organisation_input.organisation_id") } + else + { size: 'xl', tag: 'h1' } + end %> + + <%= render DfE::Autocomplete::View.new( + f, + attribute_name: :organisation_id, + form_field: f.govuk_collection_select( + :organisation_id, + @organisation_input.allowed_organisations, + :id, + :name_with_abbreviation, + class: ['govuk-!-width-three-quarters'], + options: { prompt: t('users.edit.organisation_prompt') }, + label: label_options, + ), + html_attributes: { 'data-show-all-values' => 'true' }, + ) %> + <% end %> <%= f.govuk_submit t("save_and_continue") %> <% end %> - <%= govuk_section_break(visible:true, size: "l") %> - <%= govuk_details(summary_text: t('account.organisation_details_summary'), text: t('account.organisation_text_html', contact_link: contact_link)) %> + <% unless @organisation_input.radios? %> + <%= govuk_section_break(visible: true, size: "l") %> + <%= govuk_details(summary_text: t('account.organisation.details_summary'), text: t('account.organisation.not_listed_html', contact_link: contact_link)) %> + <% end %>If you’re from a central government organisation that publishes content on the GOV.UK website but your organisation is not listed, please %{contact_link}.
-If you’re from a public sector organisation that does not publish content on GOV.UK you cannot use GOV.UK Forms yet. You can read about our forthcoming features and sign up to our mailing list for updates.
+ organisation: + details_summary: If your organisation is not listed + not_listed_html: | +If you’re from a central government organisation that’s not listed, %{contact_link}.
+At the moment, GOV.UK Forms is only available to central government organisations. If you’re from an organisation outside central government, you can read about future plans for GOV.UK Forms.
+ not_listed_option: The organisation I work for is not listed + you_created_account_with: You created your account using an @%{email_domain} email address. terms_of_use: accessibility: heading: Make forms that are easy to use and accessible @@ -68,6 +71,7 @@ en: attributes: organisation_id: blank: Select your organisation + inclusion: Select your organisation account/terms_of_use_input: attributes: agreed: @@ -964,8 +968,6 @@ en: hint: account_name_input: name: You do not need to include a title or any middle names. - account_organisation_input: - organisation_id: Currently, GOV.UK Forms is only available for central government organisations that publish content on the GOV.UK website. forms_copy_input: name: The form name will be shown at the top of each page of the form. Use a name that describes what the form will help people to do. For example ‘Apply for a juggling licence’. page: @@ -1203,6 +1205,8 @@ en: 'false': One or more options 'true': One option only legend: + account_organisation_input: + organisation_id: Which of these organisations do you work for? account_terms_of_use_input: agreed: Do you agree to these terms? forms_batch_submissions_input: @@ -1713,6 +1717,7 @@ en: non_crown_agreement_confirmation: You’ve agreed to the GOV.UK Forms agreement non_crown_agreement_new: GOV.UK Forms agreement not_found: Page not found + organisation_not_listed: What to do if your organisation is not listed organisations: Organisations payment_link: Add a link to a payment page on GOV.UK Pay privacy_policy: Provide a link to privacy information for this form diff --git a/config/settings.yml b/config/settings.yml index 64913349eb..e8b7d6a272 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -8,6 +8,7 @@ features: enabled_by_group: true send_filler_answers: enabled_by_group: true + show_relevant_organisations: false forms_api: # Authentication key to authenticate with forms-api diff --git a/spec/config/settings_spec.rb b/spec/config/settings_spec.rb index 2ec934a9ae..fffa8c83af 100644 --- a/spec/config/settings_spec.rb +++ b/spec/config/settings_spec.rb @@ -25,6 +25,7 @@ include_examples expected_value_test, :exit_pages, features, { "enabled_by_group" => true } include_examples expected_value_test, :multiple_branches, features, { "enabled_by_group" => true } include_examples expected_value_test, :send_filler_answers, features, { "enabled_by_group" => true } + include_examples expected_value_test, :show_relevant_organisations, features, false end describe "forms_api" do diff --git a/spec/factories/models/organisations.rb b/spec/factories/models/organisations.rb index 197d57d5dd..c4be3ff096 100644 --- a/spec/factories/models/organisations.rb +++ b/spec/factories/models/organisations.rb @@ -5,6 +5,7 @@ name { slug.titleize } abbreviation { name.split.collect(&:chr).join } internal { false } + closed { false } trait :with_signed_mou do after(:build) do |organisation| diff --git a/spec/input_objects/account/organisation_input_spec.rb b/spec/input_objects/account/organisation_input_spec.rb index d658fe7820..1243f8860a 100644 --- a/spec/input_objects/account/organisation_input_spec.rb +++ b/spec/input_objects/account/organisation_input_spec.rb @@ -1,14 +1,15 @@ require "rails_helper" describe Account::OrganisationInput do - subject(:organisation_input) { described_class.new(user:) } + subject(:organisation_input) { described_class.new(user:, allowed_organisations:) } let(:user) { create(:user, :with_no_org) } let(:organisation) { create(:organisation) } + let(:allowed_organisations) { [organisation, *create_list(:organisation, 2)] } describe "validations" do it "is valid with a valid organisation_id" do - organisation_input.organisation_id = organisation.id + organisation_input.organisation_id = organisation.id.to_s expect(organisation_input).to be_valid end @@ -18,12 +19,19 @@ expect(organisation_input).to be_invalid expect(organisation_input.errors[:organisation_id]).to include(error_message) end + + it "is invalid when the organisation is not in the allowed_organisations list" do + organisation_input.organisation_id = create(:organisation).id.to_s + error_message = I18n.t("activemodel.errors.models.account/organisation_input.attributes.organisation_id.inclusion") + expect(organisation_input).to be_invalid + expect(organisation_input.errors[:organisation_id]).to include(error_message) + end end describe "#submit" do context "with valid attributes" do before do - organisation_input.organisation_id = organisation.id + organisation_input.organisation_id = organisation.id.to_s end it "updates the user organisation_id" do @@ -36,7 +44,7 @@ it "logs the organisation_chosen event" do expect(Rails.logger).to receive(:info).with("User chose their organisation", { - organisation_id: organisation.id, + organisation_id: organisation.id.to_s, }) organisation_input.submit end diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 87e75be395..b023bd09ac 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -130,6 +130,37 @@ end end + describe ".for_domain" do + it "returns all organisations with matching domain" do + matched_org = create(:organisation, organisation_domains: [ + create(:organisation_domain, domain: "example.com"), + create(:organisation_domain, domain: "example.gov.uk"), + ]) + other_matched_org = create(:organisation, organisation_domains: [ + create(:organisation_domain, domain: "example.com"), + ]) + create(:organisation, organisation_domains: [ + create(:organisation_domain, domain: "example.gov.uk"), + ]) + create(:organisation) + + expect(described_class.for_domain("example.com")).to contain_exactly(matched_org, other_matched_org) + end + + it "does not return organisations with no domains set" do + create(:organisation) + expect(described_class.for_domain("example.com")).to be_empty + end + + it "does not match an email with a subdomain of a domain associated with an organisation" do + create(:organisation, organisation_domains: [ + create(:organisation_domain, domain: "example.com"), + ]) + + expect(described_class.for_domain("subdomain.example.com")).to be_empty + end + end + describe ".order_by_live_form_count" do let!(:organisation_with_two_live_forms) { create :organisation, slug: "org-with-two-live-forms" } let!(:organisation_with_one_live_form) { create :organisation, slug: "org-with-one-live-form" } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5429038ee2..5ef62d2fea 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -597,4 +597,16 @@ }.to change(user, :last_signed_in_at).to(Time.zone.now) end end + + describe "#email_domain" do + it "returns the domain part of the email" do + user = create(:user, email: "user@example.com") + expect(user.email_domain).to eq("example.com") + end + + it "strips whitespace" do + user = create(:user, email: " user@example.com ") + expect(user.email_domain).to eq("example.com") + end + end end diff --git a/spec/requests/account/organisations_controller_spec.rb b/spec/requests/account/organisations_controller_spec.rb index 0c6bf3ff1b..e8d625c1fd 100644 --- a/spec/requests/account/organisations_controller_spec.rb +++ b/spec/requests/account/organisations_controller_spec.rb @@ -1,7 +1,8 @@ require "rails_helper" describe Account::OrganisationsController do - let(:user) { create(:user, :with_no_org) } + let(:domain) { "example.gov.uk" } + let(:user) { create(:user, :with_no_org, email: Faker::Internet.email(domain: domain)) } before do login_as user @@ -14,9 +15,34 @@ expect(response).to render_template(:edit) end - it "assigns a new OrganisationForm to @organisation_input" do - get edit_account_organisation_path - expect(assigns(:organisation_input)).to be_a(Account::OrganisationInput) + context "when the show_relevant_organisations feature is enabled", :feature_show_relevant_organisations do + it "assign the input object with allowed organisations limited by the user's domain" do + matching_orgs = [ + create(:organisation, organisation_domains: [create(:organisation_domain, domain: domain)]), + create(:organisation, organisation_domains: [create(:organisation_domain, domain: domain)]), + ] + create(:organisation, organisation_domains: [create(:organisation_domain, domain: "example.com")]) + create(:organisation, closed: true, organisation_domains: [create(:organisation_domain, domain: domain)]) + + get edit_account_organisation_path + + input_object = assigns(:organisation_input) + expect(input_object).to be_a(Account::OrganisationInput) + expect(input_object.allowed_organisations).to match_array(matching_orgs) + end + end + + context "when the show_relevant_organisations feature is disabled", feature_show_relevant_organisations: false do + it "assigns the input object with all not closed organisations" do + orgs = create_list(:organisation, 2) + create(:organisation, closed: true) + + get edit_account_organisation_path + + input_object = assigns(:organisation_input) + expect(input_object).to be_a(Account::OrganisationInput) + expect(input_object.allowed_organisations).to match_array(orgs) + end end end @@ -32,7 +58,7 @@ describe "PUT #update" do context "with valid parameters" do - let(:organisation) { create(:organisation) } + let(:organisation) { create(:organisation, organisation_domains: [create(:organisation_domain, domain: domain)]) } let(:valid_params) { { account_organisation_input: { organisation_id: organisation.id } } } before do @@ -52,6 +78,21 @@ end end + context "when the not listed radio option is selected" do + let(:params) { { account_organisation_input: { organisation_id: Account::OrganisationInput::NOT_LISTED_OPTION_VALUE } } } + + it "renders the not_listed template" do + put account_organisation_path, params: params + expect(response).to render_template(:not_listed) + end + + it "does not update the user's organisation" do + expect { + put account_organisation_path, params: params + }.not_to(change { user.reload.organisation }) + end + end + context "with invalid parameters" do let(:invalid_params) { { account_organisation_input: { organisation_id: nil } } } @@ -67,5 +108,36 @@ expect(response).to render_template(:edit) end end + + context "when the show_relevant_organisations feature is enabled", :feature_show_relevant_organisations do + context "when the selected organisation does not match the user's email domain" do + let(:organisation) { create(:organisation, organisation_domains: [create(:organisation_domain, domain: "other.gov.uk")]) } + let(:invalid_params) { { account_organisation_input: { organisation_id: organisation.id } } } + + it "does not update the user's organisation" do + expect { + put account_organisation_path, params: invalid_params + }.not_to(change { user.reload.organisation }) + end + + it "re-renders the edit template" do + put account_organisation_path, params: invalid_params + expect(response).to have_http_status(:unprocessable_content) + expect(response).to render_template(:edit) + end + end + end + + context "when the show_relevant_organisations feature is disabled", feature_show_relevant_organisations: false do + context "when the selected organisation does not match the user's email domain" do + let(:organisation) { create(:organisation, organisation_domains: [create(:organisation_domain, domain: "other.gov.uk")]) } + let(:params) { { account_organisation_input: { organisation_id: organisation.id } } } + + it "updates the user's organisation" do + put account_organisation_path, params: params + expect(user.reload.organisation).to eq(organisation) + end + end + end end end diff --git a/spec/views/account/organisations/edit.html.erb_spec.rb b/spec/views/account/organisations/edit.html.erb_spec.rb index b9e866f79a..3e6721fc9b 100644 --- a/spec/views/account/organisations/edit.html.erb_spec.rb +++ b/spec/views/account/organisations/edit.html.erb_spec.rb @@ -1,12 +1,13 @@ require "rails_helper" describe "account/organisations/edit.html.erb" do - let(:organisation_input) { Account::OrganisationInput.new } + let(:organisation_input) { Account::OrganisationInput.new(allowed_organisations: organisations, user:) } let(:contact_href) { "https://example.com/contact" } - let!(:organisations) do + let(:user) { build(:user, email: "user@example.com") } + let(:organisations) do [ - create(:organisation, slug: "test-org"), - create(:organisation, slug: "department-for-testing", name: "Department for Testing"), + build_stubbed(:organisation, slug: "test-org"), + build_stubbed(:organisation, slug: "department-for-testing", name: "Department for Testing"), ] end @@ -20,31 +21,100 @@ render end + it "sets the page title" do + expect(view.content_for(:title)).to eq(t("page_titles.account_organisation")) + end + it "displays the form" do expect(rendered).to have_selector('form[action="/account/organisation"][method="post"]') expect(rendered).to have_field("_method", with: "patch", type: :hidden) expect(rendered).to have_button(I18n.t("save_and_continue")) end - it "renders the organisation select field for autocomplete" do - expect(rendered).to have_selector('select[name="account_organisation_input[organisation_id]"]', visible: :all) - organisations.each do |organisation| - expect(rendered).to have_selector("option[value='#{organisation.id}']", text: organisation.name) + context "when the show_relevant_organisations feature is disabled", feature_show_relevant_organisations: false do + it "has the expected h1" do + expect(rendered).to have_selector("h1", text: "Select your organisation") end - end - it "has organisation fields with abbreviations" do - expect(rendered).to have_select( - "Select your organisation", - with_options: [ - "Department for Testing (DfT)", - "Test Org (TO)", - ], - ) + it "renders the organisation select field for autocomplete" do + expect(rendered).to have_selector('select[name="account_organisation_input[organisation_id]"]', visible: :all) + organisations.each do |organisation| + expect(rendered).to have_selector("option[value='#{organisation.id}']", text: organisation.name) + end + end + + it "has organisation fields with abbreviations" do + expect(rendered).to have_select( + "Select your organisation", + with_options: [ + "Department for Testing (DfT)", + "Test Org (TO)", + ], + ) + end end - it "sets the page title" do - expect(view.content_for(:title)).to eq(t("page_titles.account_organisation")) + context "when the show_relevant_organisations feature is enabled", :feature_show_relevant_organisations do + it "has the expected h1" do + expect(rendered).to have_selector("h1", text: t("page_titles.account_organisation")) + end + + it "displays the user's email domain" do + expect(rendered).to have_content(I18n.t("account.organisation.you_created_account_with", email_domain: "example.com")) + end + + context "when there are fewer than 30 organisations" do + it "has the expected h2" do + expect(rendered).to have_selector("h2", text: "Which of these organisations do you work for?") + end + + it "renders the organisation radio buttons" do + expect(rendered).to have_selector('input[type="radio"][name="account_organisation_input[organisation_id]"]', count: organisations.size + 1) + organisations.each do |organisation| + expect(rendered).to have_selector("label[for='account-organisation-input-organisation-id-#{organisation.id}-field']", text: organisation.name_with_abbreviation) + end + end + + it "has a divider" do + expect(rendered).to have_selector(".govuk-radios__divider", text: "or") + end + + it "has a radio button for organisation not listed" do + expect(rendered).to have_selector("input[type='radio'][value='not_listed'][name='account_organisation_input[organisation_id]']") + expect(rendered).to have_selector("label[for='account-organisation-input-organisation-id-not-listed-field']", text: I18n.t("account.organisation.not_listed_option")) + end + + it "does not render the not listed details component" do + expect(rendered).not_to have_selector("details.govuk-details") + end + end + + context "when there are more than 30 organisations" do + let(:organisations) { build_stubbed_list(:organisation, 31) } + + it "has the expected h2" do + expect(rendered).to have_selector("h2", text: "Which of these organisations do you work for?") + end + + it "renders the organisation select field for autocomplete" do + expect(rendered).to have_selector('select[name="account_organisation_input[organisation_id]"]', visible: :all) + organisations.each do |organisation| + expect(rendered).to have_selector("option[value='#{organisation.id}']", text: organisation.name) + end + end + + it "has organisation fields with abbreviations" do + expect(rendered).to have_select( + "Which of these organisations do you work for?", + with_options: organisations.map(&:name_with_abbreviation), + ) + end + + it "renders the not listed details component" do + expect(rendered).to have_selector("details.govuk-details") + expect(rendered).to have_selector("summary.govuk-details__summary", text: I18n.t("account.organisation.details_summary")) + end + end end end @@ -62,23 +132,4 @@ expect(view.content_for(:title)).to eq(title_with_error_prefix(t("page_titles.account_organisation"), true)) end end - - context "when there are closed organisations" do - before do - create(:organisation, slug: "closed-org", closed: true) - - render - end - - it "only shows the organisations that are not closed" do - expect(rendered).to have_select( - "Select your organisation", - options: [ - "Select an organisation", - "Department for Testing (DfT)", - "Test Org (TO)", - ], - ) - end - end end