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/factories/case_log.rb b/spec/factories/case_log.rb index e869d8e7f..1ee6130ea 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -129,13 +129,13 @@ FactoryBot.define do ppostc1 { "w3" } ppostc2 { "w3" } property_relet { 0 } - mrcdate { Time.utc(2020, 0o5, 0o5, 10, 36, 49) } + mrcdate { Time.utc(2020, 5, 0o5, 10, 36, 49) } mrcday { mrcdate.day } mrcmonth { mrcdate.month } mrcyear { mrcdate.year } incref { 0 } sale_completion_date { nil } - startdate { Time.utc(2022, 0o2, 0o2, 10, 36, 49) } + startdate { Time.utc(2022, 2, 2, 10, 36, 49) } day { startdate.day } month { startdate.month } year { startdate.year } @@ -148,7 +148,7 @@ FactoryBot.define do la_known { 1 } declaration { 1 } end - created_at { Time.utc(2022, 0o2, 8, 16, 52, 15) } - updated_at { Time.utc(2022, 0o2, 8, 16, 52, 15) } + created_at { Time.utc(2022, 2, 8, 16, 52, 15) } + updated_at { Time.utc(2022, 2, 8, 16, 52, 15) } end end 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