diff --git a/CHANGELOG.md b/CHANGELOG.md index b47f2cc67..ad239da06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ [Full changelog][unreleased] +## Release 157 - 2024-12-16 + +[Full changelog][157] + +- Dropdown switcher component for users with multiple organisations + ## Release 156 - 2024-12-12 [Full changelog][156] @@ -1741,7 +1747,8 @@ - Planned start and end dates are mandatory - Actual start and end dates must not be in the future -[unreleased]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-156...HEAD +[unreleased]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-157...HEAD +[157]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-156...release-157 [156]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-155...release-156 [155]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-154...release-155 [154]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-153...release-154 diff --git a/Gemfile.lock b/Gemfile.lock index fd329a753..d512b9f74 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -71,7 +71,7 @@ GEM activerecord (>= 5.0, < 7.2) request_store (~> 1.2) aws-eventstream (1.3.0) - aws-partitions (1.1017.0) + aws-partitions (1.1022.0) aws-sdk-core (3.214.0) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.992.0) @@ -80,7 +80,7 @@ GEM aws-sdk-kms (1.96.0) aws-sdk-core (~> 3, >= 3.210.0) aws-sigv4 (~> 1.5) - aws-sdk-s3 (1.176.0) + aws-sdk-s3 (1.176.1) aws-sdk-core (~> 3, >= 3.210.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.5) @@ -155,10 +155,10 @@ GEM rotp (~> 6.0) diff-lcs (1.5.1) docile (1.4.0) - dotenv (2.8.1) - dotenv-rails (2.8.1) - dotenv (= 2.8.1) - railties (>= 3.2) + dotenv (3.1.6) + dotenv-rails (3.1.6) + dotenv (= 3.1.6) + railties (>= 6.1) encryptor (3.0.0) erubi (1.13.0) erubis (2.7.0) @@ -285,14 +285,14 @@ GEM net-smtp (0.5.0) net-protocol nio4r (2.7.4) - nokogiri (1.16.8) + nokogiri (1.17.2) mini_portile2 (~> 2.8.2) racc (~> 1.4) - nokogiri (1.16.8-arm64-darwin) + nokogiri (1.17.2-arm64-darwin) racc (~> 1.4) - nokogiri (1.16.8-x86_64-darwin) + nokogiri (1.17.2-x86_64-darwin) racc (~> 1.4) - nokogiri (1.16.8-x86_64-linux) + nokogiri (1.17.2-x86_64-linux) racc (~> 1.4) notifications-ruby-client (6.2.0) jwt (>= 1.5, < 3) @@ -354,7 +354,7 @@ GEM activesupport (>= 5.0.0) minitest nokogiri (>= 1.6) - rails-html-sanitizer (1.6.1) + rails-html-sanitizer (1.6.2) loofah (~> 2.21) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) rails-i18n (7.0.10) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ce001987c..66f7e62c1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,8 +2,19 @@ class ApplicationController < ActionController::Base include Auth include Ip + before_action :set_organisation_list_and_current_organisation + private + def set_organisation_list_and_current_organisation + return unless user_signed_in? + + @organisation_list = current_user.all_organisations + if session[:current_user_organisation] + Current.user_organisation = session[:current_user_organisation] + end + end + def add_breadcrumb(name, path, options = {}) super(name, path, options.merge(title: name)) end diff --git a/app/controllers/users/organisation_session_controller.rb b/app/controllers/users/organisation_session_controller.rb new file mode 100644 index 000000000..33b56a610 --- /dev/null +++ b/app/controllers/users/organisation_session_controller.rb @@ -0,0 +1,14 @@ +class Users::OrganisationSessionController < ApplicationController + include Secured + + def update + desired_organisation_id = params[:current_user_organisation] + + if desired_organisation_id + if current_user.all_organisations.pluck(:id).include?(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/models/user.rb b/app/models/user.rb index 0e73e2c58..f3c674d61 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -38,6 +38,10 @@ def additional_organisations? additional_organisations.any? end + def current_organisation_id + Current.user_organisation || organisation.id + end + def active_for_authentication? active 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..46f367d6e --- /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: users_session_organisation_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.current_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..06eb732fb --- /dev/null +++ b/config/locales/views/organisation_switcher.en.yml @@ -0,0 +1,5 @@ +--- +en: + organisation_switcher: + label: "Available organisations" + submit: "Set organisation" diff --git a/config/routes.rb b/config/routes.rb index 8ace6b8d5..d69f9da33 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -32,6 +32,10 @@ resources :users, except: [:index] resources :activities, only: [:index] + namespace :users do + patch "session/organisation" => "organisation_session#update", :as => :session_organisation + end + 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/models/user_spec.rb b/spec/models/user_spec.rb index 00333f501..d81d3f336 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -88,6 +88,7 @@ user = create(:administrator) expect(user.organisation).to eq(current_organisation) + expect(user.current_organisation_id).to eq(current_organisation.id) expect(user.primary_organisation).not_to eq(current_organisation) end @@ -95,6 +96,13 @@ Current.user_organisation = nil end end + + context "when the current organisation has not been set" do + it "returns the primary organisation" do + user = create(:administrator) + expect(user.current_organisation_id).to eq(user.organisation.id) + end + end end describe "delegations" do 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