CLDC-1117: Allow data providers to see but not edit users in their org (#442)

* Allow data providers to see but not edit users in their org

* Don't actually need octal literals

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
   end
 
   def details

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

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

app/views/organisations/users.html.erb

@@ -4,7 +4,9 @@
   <%= "Users" %>
 <% end %>
 
+<% 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|

app/views/users/show.html.erb

@@ -14,13 +14,21 @@
       <%= summary_list.row do |row|
         row.key { 'Name' }
         row.value { @user.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 }
+        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|

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

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

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

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")

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
 

spec/requests/users_controller_spec.rb

@@ -138,6 +138,25 @@ RSpec.describe UsersController, type: :request do
           get "/users/#{other_user.id}", headers: headers, params: {}
         end
 
+        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
+
+        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
@@ -147,6 +166,7 @@ RSpec.describe UsersController, type: :request do
           end
         end
       end
+    end
 
     describe "#edit" do
       context "when the current user matches the user ID" do