From e7a049036c69ea77bd91f2c28f5ec330cb366b47 Mon Sep 17 00:00:00 2001 From: Ben Shimmin Date: Fri, 13 Dec 2024 16:27:58 +0000 Subject: [PATCH] Front-end work for allowing users to switch organisations We have a new front-end component, a dropdown menu organisation switcher, which will be displayed for a user with a non-zero number of `addtional_organisations` and will allow them to switch their organisation. (There is no functionality currently to populate a given user's `additional_organisations` - this would need to be done manually. By default a user will have zero `additional_organisations` and this component will be hidden.) Once switched, the user will see reports, exports, and activities for that organisation. They can switch back to their original organisation in the same way. We store the user's switched organisation in the session as `current_user_organisation`, and we also write this into `Current.user_organisation` so we can easily access it in the `User` model where we override the `#organisation` method in order to retrieve the data. --- app/controllers/application_controller.rb | 9 ++ .../organisation_session_controller.rb | 15 +++ app/views/layouts/application.html.haml | 3 + app/views/shared/_navigation.html.haml | 8 +- .../shared/_organisation_switcher.html.haml | 7 ++ .../views/organisation_switcher.en.yml | 5 + config/routes.rb | 2 + .../controllers/activities_controller_spec.rb | 1 + .../application_controller_spec.rb | 47 +++++++ .../organisation_session_controller.rb | 47 +++++++ .../features/users_can_edit_a_comment_spec.rb | 6 +- .../users_can_switch_organisation_spec.rb | 116 ++++++++++++++++++ spec/rails_helper.rb | 2 + spec/views/shared/navigation_spec.rb | 10 +- 14 files changed, 270 insertions(+), 8 deletions(-) create mode 100644 app/controllers/organisation_session_controller.rb create mode 100644 app/views/shared/_organisation_switcher.html.haml create mode 100644 config/locales/views/organisation_switcher.en.yml create mode 100644 spec/controllers/organisation_session_controller.rb create mode 100644 spec/features/users_can_switch_organisation_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ce001987c..5af81b87b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,6 +2,15 @@ class ApplicationController < ActionController::Base include Auth include Ip + before_action -> { + return unless user_signed_in? + + @organisation_list = current_user.all_organisations + if session[:current_user_organisation] + Current.user_organisation = session[:current_user_organisation] + end + } + private def add_breadcrumb(name, path, options = {}) diff --git a/app/controllers/organisation_session_controller.rb b/app/controllers/organisation_session_controller.rb new file mode 100644 index 000000000..a9e9d7c46 --- /dev/null +++ b/app/controllers/organisation_session_controller.rb @@ -0,0 +1,15 @@ +class OrganisationSessionController < ApplicationController + include Secured + + def update + desired_organisation_id = params[:current_user_organisation] + + if desired_organisation_id + if current_user.additional_organisations.pluck(:id).include?(desired_organisation_id) || + current_user.primary_organisation.id == desired_organisation_id + session[:current_user_organisation] = desired_organisation_id + end + end + redirect_to current_user.service_owner? ? request.referer : root_path + end +end diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 2d8c1eaba..dc461efbf 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -62,6 +62,9 @@ = t("header.feedback.html") = render 'layouts/messages' + + = render "shared/organisation_switcher" + = breadcrumb_tags = yield diff --git a/app/views/shared/_navigation.html.haml b/app/views/shared/_navigation.html.haml index ea4a7e0e6..fbbf2da8e 100644 --- a/app/views/shared/_navigation.html.haml +++ b/app/views/shared/_navigation.html.haml @@ -9,8 +9,8 @@ %li{ class: navigation_item_class(reports_path) } = link_to t("page_title.report.index"), reports_path, class: "govuk-header__link" - %li{ class: navigation_item_class(organisation_activities_path(organisation_id: current_user.organisation_id)) } - = link_to t("page_title.activity.index"), organisation_activities_path(organisation_id: current_user.organisation_id), class: "govuk-header__link" + %li{ class: navigation_item_class(organisation_activities_path(organisation_id: current_user.organisation.id)) } + = link_to t("page_title.activity.index"), organisation_activities_path(organisation_id: current_user.organisation.id), class: "govuk-header__link" - if policy(:level_b).budget_upload? %li{ class: navigation_item_class(new_level_b_budgets_upload_path) } @@ -20,8 +20,8 @@ %li{ class: navigation_item_class(exports_path) } = link_to t("page_title.export.index"), exports_path, class: "govuk-header__link" - elsif policy([:export, current_user.organisation]).show? - %li{ class: navigation_item_class(exports_organisation_path(id: current_user.organisation_id)) } - = link_to t("page_title.export.index"), exports_organisation_path(id: current_user.organisation_id), class: "govuk-header__link" + %li{ class: navigation_item_class(exports_organisation_path(id: current_user.organisation.id)) } + = link_to t("page_title.export.index"), exports_organisation_path(id: current_user.organisation.id), class: "govuk-header__link" - if policy(Organisation).index? %li{ class: navigation_item_class(organisations_path) } diff --git a/app/views/shared/_organisation_switcher.html.haml b/app/views/shared/_organisation_switcher.html.haml new file mode 100644 index 000000000..ba5b5fa6c --- /dev/null +++ b/app/views/shared/_organisation_switcher.html.haml @@ -0,0 +1,7 @@ +- if authenticated? && current_user.additional_organisations? + .govuk-grid-row + .govuk-grid-column-full{:class => "govuk-!-padding-top-4"} + =form_with url: organisation_session_path, method: :patch do |form| + =form.label t("organisation_switcher.label"), class: "govuk-label", for: "current_user_organisation" + =form.select :current_user_organisation, options_for_select(@organisation_list.pluck(:name, :id), Current.user_organisation || current_user.organisation.id), {}, class: "govuk-select" + =form.submit t("organisation_switcher.submit"), class: "govuk-button govuk-!-margin-bottom-1", "data-module": "govuk-button" diff --git a/config/locales/views/organisation_switcher.en.yml b/config/locales/views/organisation_switcher.en.yml new file mode 100644 index 000000000..e85b06435 --- /dev/null +++ b/config/locales/views/organisation_switcher.en.yml @@ -0,0 +1,5 @@ +--- +en: + organisation_switcher: + label: "Available organisations" + submit: "Set organisation" \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 8ace6b8d5..7dd9a5b6b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -32,6 +32,8 @@ resources :users, except: [:index] resources :activities, only: [:index] + patch "/organisation_session" => "organisation_session#update" + roles = %w[implementing_organisations partner_organisations matched_effort_providers external_income_providers] constraints role: /#{roles.join("|")}/ do get "organisations/(:role)", to: "organisations#index", defaults: {role: "partner_organisations"}, as: :organisations diff --git a/spec/controllers/activities_controller_spec.rb b/spec/controllers/activities_controller_spec.rb index 9cf536412..cd9b71ad7 100644 --- a/spec/controllers/activities_controller_spec.rb +++ b/spec/controllers/activities_controller_spec.rb @@ -184,6 +184,7 @@ before do allow(controller).to receive(:current_user).and_return(user) allow(ActivityPolicy).to receive(:new).and_return(policy) + allow(user).to receive(:all_organisations).and_return([]) end describe "#confirm_destroy" do diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index d81dbea5c..a8aeb03ed 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -1,5 +1,7 @@ require "rails_helper" +class DummyController < ApplicationController; end + RSpec.describe ApplicationController, type: :controller do describe "#request_ip" do it "returns the anonymized v4 IP with the last octet zero padded" do @@ -27,4 +29,49 @@ end end end + + describe "before_action" do + controller DummyController do + def custom_action + head 200 + end + end + + before(:each) do + routes.draw do + get "custom_action" => "dummy#custom_action" + end + end + + context "user is not signed in" do + it "does not set Current.user_organisation" do + get "custom_action" + + expect(Current.user_organisation).to be(nil) + end + end + + context "user is signed in" do + let(:user) { create(:partner_organisation_user) } + + before do + allow(controller).to receive(:current_user).and_return(user) + end + + it "does not set Current.user_organisation if `current_user_organisation` is not in the session" do + get "custom_action" + + expect(Current.user_organisation).to be(nil) + end + + it "sets Current.user_organisation if `current_user_organisation` is in the session" do + id = SecureRandom.uuid + session[:current_user_organisation] = id + + get "custom_action" + + expect(Current.user_organisation).to be(id) + end + end + end end diff --git a/spec/controllers/organisation_session_controller.rb b/spec/controllers/organisation_session_controller.rb new file mode 100644 index 000000000..8044017b0 --- /dev/null +++ b/spec/controllers/organisation_session_controller.rb @@ -0,0 +1,47 @@ +require "rails_helper" + +RSpec.describe OrganisationSessionController do + let(:user) { create(:partner_organisation_user, organisation: organisation) } + let(:organisation) { create(:partner_organisation) } + let(:other_organisation) { create(:partner_organisation) } + + before do + allow(controller).to receive(:current_user).and_return(user) + + user.additional_organisations << [create(:partner_organisation), create(:partner_organisation)] + end + + describe "#update" do + it "sets the session `current_user_organisation` to the user's primary organisation's ID" do + put :update, params: build_params(user.primary_organisation.id) + + expect(session[:current_user_organisation]).to eq(user.primary_organisation.id) + end + + it "sets the session `current_user_organisation` to an ID from the user's additional organisations" do + id = user.additional_organisations.pluck(:id).sample + + put :update, params: build_params(id) + + expect(session[:current_user_organisation]).to eq(id) + end + + it "does not set the session `current_user_organisation` to a random ID" do + random_id = SecureRandom.uuid + + put :update, params: build_params(random_id) + + expect(session[:current_user_organisation]).not_to eq(random_id) + end + + it "does not set the session `current_user_organisation` to an organisation ID not in the user's additional organisations" do + put :update, params: build_params(other_organisation.id) + + expect(session[:current_user_organisation]).not_to eq(other_organisation.id) + end + + def build_params(id) + {current_user_organisation: id} + end + end +end diff --git a/spec/features/users_can_edit_a_comment_spec.rb b/spec/features/users_can_edit_a_comment_spec.rb index 6b52d236d..4465ab2c5 100644 --- a/spec/features/users_can_edit_a_comment_spec.rb +++ b/spec/features/users_can_edit_a_comment_spec.rb @@ -1,4 +1,6 @@ RSpec.describe "Users can edit a comment" do + include HideFromBullet + let(:beis_user) { create(:beis_user) } let(:partner_org_user) { create(:partner_organisation_user) } @@ -77,7 +79,9 @@ scenario "the user can edit comments on actuals belonging to the same organisation" do actual = create(:actual, :with_comment, report: project_activity_report, parent_activity: project_activity) - visit organisation_activity_comments_path(project_activity_comment.commentable.organisation, project_activity_comment.commentable) + skip_bullet do + visit organisation_activity_comments_path(project_activity_comment.commentable.organisation, project_activity_comment.commentable) + end expect(page).to have_link("Edit", href: edit_activity_actual_path(project_activity, actual)) end diff --git a/spec/features/users_can_switch_organisation_spec.rb b/spec/features/users_can_switch_organisation_spec.rb new file mode 100644 index 000000000..aeea8a793 --- /dev/null +++ b/spec/features/users_can_switch_organisation_spec.rb @@ -0,0 +1,116 @@ +RSpec.feature "Users can switch organisation" do + context "when the user has additional organisations" do + let(:user_with_additional_organisations) do + org1 = create(:partner_organisation) + org2 = create(:partner_organisation) + user = create(:partner_organisation_user) + user.additional_organisations << [org1, org2] + user + end + + before do + authenticate!(user: user_with_additional_organisations) + visit root_path + end + after { logout } + + scenario "the organisation switcher dropdown is shown" do + expect(page).to have_content t("organisation_switcher.label") + end + + scenario "the organisation switcher dropdown is populated correctly" do + user_with_additional_organisations.all_organisations.each do |org| + expect(page).to have_content org.name + end + end + + scenario "the user can switch organisation" do + expect(page).to have_select("current_user_organisation", selected: user_with_additional_organisations.primary_organisation.name) + + additional_org_name = user_with_additional_organisations.additional_organisations.first.name + + select(additional_org_name, from: "current_user_organisation") + click_on t("organisation_switcher.submit") + + expect(page).to have_select("current_user_organisation", selected: additional_org_name) + end + + scenario "the nav links have the correct organisation ID when the user has switched organisation" do + additional_org = user_with_additional_organisations.additional_organisations.first + + activities_href = organisation_activities_path(organisation_id: user_with_additional_organisations.primary_organisation.id) + exports_href = exports_organisation_path(id: user_with_additional_organisations.primary_organisation.id) + + expect(page).to have_link(href: activities_href) + expect(page).to have_link(href: exports_href) + + select(additional_org.name, from: "current_user_organisation") + click_on t("organisation_switcher.submit") + + updated_activities_href = organisation_activities_path(organisation_id: additional_org.id) + updated_exports_href = exports_organisation_path(id: additional_org.id) + + expect(page).to have_link(href: updated_activities_href) + expect(page).to have_link(href: updated_exports_href) + end + + scenario "the Activities page shows the correct content when the user has switched organisation" do + additional_org = user_with_additional_organisations.additional_organisations.first + + select(additional_org.name, from: "current_user_organisation") + click_on t("organisation_switcher.submit") + + updated_activities_href = organisation_activities_path(organisation_id: additional_org.id) + + visit(updated_activities_href) + + expect(page).to have_content(t("page_title.activity.index")) + end + + scenario "the Exports page shows the correct content when the user has switched organisation" do + additional_org = user_with_additional_organisations.additional_organisations.first + + select(additional_org.name, from: "current_user_organisation") + click_on t("organisation_switcher.submit") + + updated_exports_href = exports_organisation_path(id: additional_org.id) + + visit(updated_exports_href) + + expect(page).to have_content(t("page_title.export.organisation.show", name: additional_org.name)) + end + + scenario "the nav links have the primary organisation ID when the user has switched organisation and switched back" do + activities_href = organisation_activities_path(organisation_id: user_with_additional_organisations.primary_organisation.id) + exports_href = exports_organisation_path(id: user_with_additional_organisations.primary_organisation.id) + + additional_org = user_with_additional_organisations.additional_organisations.first + + select(additional_org.name, from: "current_user_organisation") + click_on t("organisation_switcher.submit") + + updated_exports_href = exports_organisation_path(id: additional_org.id) + + visit(updated_exports_href) + + select(user_with_additional_organisations.primary_organisation.name, from: "current_user_organisation") + click_on t("organisation_switcher.submit") + + expect(page).to have_link(href: activities_href) + expect(page).to have_link(href: exports_href) + end + end + + context "when the user has no additional organisations" do + let(:user) { create(:partner_organisation_user) } + + before { authenticate!(user:) } + after { logout } + + scenario "the organisation switcher dropdown is not shown" do + visit root_path + + expect(page).not_to have_content t("organisation_switcher.label") + end + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index b83032d09..c423b8353 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -83,10 +83,12 @@ ActionMailer::Base.delivery_method = :test ActionMailer::Base.perform_deliveries = true ActionMailer::Base.deliveries = [] + ActiveSupport::CurrentAttributes.reset_all end config.after(:each) do |example| ActionMailer::Base.deliveries.clear + ActiveSupport::CurrentAttributes.reset_all end config.before(:each, type: :request) do diff --git a/spec/views/shared/navigation_spec.rb b/spec/views/shared/navigation_spec.rb index ea5894dd1..d851245ae 100644 --- a/spec/views/shared/navigation_spec.rb +++ b/spec/views/shared/navigation_spec.rb @@ -6,8 +6,10 @@ allow(view).to receive(:policy) do |record| Pundit.policy(user, record) end + allow(view).to receive(:current_user).and_return(user) - allow(user).to receive(:organisation_id).and_return(SecureRandom.uuid) + allow(user).to receive(:organisation).and_return(organisation) + allow(organisation).to receive(:id).and_return(SecureRandom.uuid) end render @@ -15,25 +17,27 @@ context "when the current user is a BEIS user" do let(:user) { build(:beis_user) } + let(:organisation) { build(:beis_organisation) } it "shows the link to the exports index" do expect(rendered).to have_link(t("page_title.export.index"), href: exports_path) end it "does not show the link to an organisation export page" do - expect(rendered).to_not have_link(t("page_title.export.index"), href: exports_organisation_path(id: user.organisation_id)) + expect(rendered).to_not have_link(t("page_title.export.index"), href: exports_organisation_path(id: user.organisation.id)) end end context "when the current user is a partner organisation user" do let(:user) { build(:partner_organisation_user) } + let(:organisation) { build(:partner_organisation) } it "does not show the link to the exports index" do expect(rendered).to_not have_link(t("page_title.export.index"), href: exports_path) end it "shows the link to the partner organisation's organisation export page" do - expect(rendered).to have_link(t("page_title.export.index"), href: exports_organisation_path(id: user.organisation_id)) + expect(rendered).to have_link(t("page_title.export.index"), href: exports_organisation_path(id: user.organisation.id)) end end end