diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 3f9682a60..3ba167453 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -8,11 +8,7 @@ class OrganisationsController < ApplicationController end def users - if current_user.data_coordinator? - render "users" - else - head :unauthorized - end + render "users" end def details diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 7c90ea738..e156fbffd 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -96,7 +96,7 @@ private else render_not_found and return unless current_user.organisation == @user.organisation render_not_found and return if action_name == "edit_password" && current_user != @user - render_not_found and return unless current_user.data_coordinator? || current_user == @user + render_not_found and return unless action_name == "show" || current_user.data_coordinator? || current_user == @user end end end diff --git a/app/helpers/tab_nav_helper.rb b/app/helpers/tab_nav_helper.rb index c8d8e83b9..e8ac7b3d4 100644 --- a/app/helpers/tab_nav_helper.rb +++ b/app/helpers/tab_nav_helper.rb @@ -11,10 +11,9 @@ module TabNavHelper end def tab_items(user) - items = [{ name: t("Details"), url: details_organisation_path(user.organisation) }] - if user.data_coordinator? - items << { name: t("Users"), url: users_organisation_path(user.organisation) } - end - items + [ + { name: t("Details"), url: details_organisation_path(user.organisation) }, + { name: t("Users"), url: users_organisation_path(user.organisation) }, + ] end end diff --git a/app/views/organisations/users.html.erb b/app/views/organisations/users.html.erb index 48b8e6846..33c7bc5d0 100644 --- a/app/views/organisations/users.html.erb +++ b/app/views/organisations/users.html.erb @@ -4,7 +4,9 @@ <%= "Users" %> <% end %> -<%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %> +<% if current_user.data_coordinator? %> + <%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %> +<% end %> <%= govuk_table do |table| %> <%= table.head do |head| %> <%= head.row do |row| diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 81265ba4b..319ab692d 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -14,13 +14,21 @@ <%= summary_list.row do |row| row.key { 'Name' } row.value { @user.name } - row.action(visually_hidden_text: 'name', href: edit_user_path, html_attributes: { 'data-qa': 'change-name' }) + if current_user == @user || current_user.data_coordinator? + row.action(visually_hidden_text: 'name', href: edit_user_path, html_attributes: { 'data-qa': 'change-name' }) + else + row.action() + end end %> <%= summary_list.row() do |row| row.key { 'Email address' } row.value { @user.email } - row.action(visually_hidden_text: 'email address', href: edit_user_path, html_attributes: { 'data-qa': 'change-email' }) + if current_user == @user || current_user.data_coordinator? + row.action(visually_hidden_text: 'email address', href: edit_user_path, html_attributes: { 'data-qa': 'change-email' }) + else + row.action() + end end %> <%= summary_list.row do |row| diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 5129d2588..3fa703c79 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -70,11 +70,12 @@ RSpec.describe "User Features" do let(:user) { FactoryBot.create(:user) } context "when viewing organisation page" do - it "can only see the details tab" do + it "can see the details tab and users tab" do visit("/logs") click_link("Your organisation") expect(page).to have_current_path("/organisations/#{org_id}/details") - expect(page).to have_no_link("Users") + expect(page).to have_link("Details") + expect(page).to have_link("Users") end end end diff --git a/spec/helpers/tab_nav_helper_spec.rb b/spec/helpers/tab_nav_helper_spec.rb index d427df4a6..2c8f54258 100644 --- a/spec/helpers/tab_nav_helper_spec.rb +++ b/spec/helpers/tab_nav_helper_spec.rb @@ -31,10 +31,11 @@ RSpec.describe TabNavHelper do end context "when user is a data_provider" do - it "returns details tab only" do + it "returns details and user tabs" do result = tab_items(user).map { |i| i[:name] } - expect(result.count).to eq(1) + expect(result.count).to eq(2) expect(result.first).to match("Details") + expect(result.second).to match("Users") end end end diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index bf5a4e50a..59e0f05d8 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe Organisation, type: :model do describe "#new" do let(:user) { FactoryBot.create(:user) } - let(:organisation) { user.organisation } + let!(:organisation) { user.organisation } it "has expected fields" do expect(organisation.attribute_names).to include("name", "phone", "provider_type") diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 5c077b3a7..c19568516 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -257,8 +257,8 @@ RSpec.describe OrganisationsController, type: :request do get "/organisations/#{organisation.id}/users", headers: headers, params: {} end - it "returns unauthorized 401" do - expect(response).to have_http_status(:unauthorized) + it "returns 200" do + expect(response).to have_http_status(:ok) end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 717301e7b..68ddc139d 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -138,12 +138,32 @@ RSpec.describe UsersController, type: :request do get "/users/#{other_user.id}", headers: headers, params: {} end - it "returns not found 404" do - expect(response).to have_http_status(:not_found) + context "when the user is part of the same organisation" do + let(:other_user) { FactoryBot.create(:user, organisation: user.organisation) } + + it "shows their details" do + expect(response).to have_http_status(:ok) + expect(page).to have_content("#{other_user.name}’s account") + end + + it "does not have edit links" do + expect(page).not_to have_link("Change", text: "name") + expect(page).not_to have_link("Change", text: "email address") + expect(page).not_to have_link("Change", text: "password") + expect(page).not_to have_link("Change", text: "role") + expect(page).not_to have_link("Change", text: "are you a data protection officer?") + expect(page).not_to have_link("Change", text: "are you a key contact?") + end end - it "shows the 404 view" do - expect(page).to have_content("Page not found") + context "when the user is not part of the same organisation" do + it "returns not found 404" do + expect(response).to have_http_status(:not_found) + end + + it "shows the 404 view" do + expect(page).to have_content("Page not found") + end end end end