diff --git a/app/admin/users.rb b/app/admin/users.rb index 75575402d..781ef046c 100644 --- a/app/admin/users.rb +++ b/app/admin/users.rb @@ -1,5 +1,5 @@ ActiveAdmin.register User do - permit_params :name, :email, :password, :password_confirmation, :organisation_id + permit_params :name, :email, :password, :password_confirmation, :organisation_id, :role controller do def update_resource(object, attributes) @@ -14,6 +14,7 @@ ActiveAdmin.register User do column :name column :email column :organisation + column(:role) { |u| u.role.to_s.humanize } column :current_sign_in_at column :sign_in_count column :created_at @@ -34,6 +35,7 @@ ActiveAdmin.register User do f.input :password f.input :password_confirmation f.input :organisation + f.input :role end f.actions end diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 87b075e15..dcf1aa34a 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -2,8 +2,20 @@ class OrganisationsController < ApplicationController before_action :authenticate_user! before_action :find_organisation + def show + redirect_to details_organisation_path(@organisation) + end + def users - render "users" + if current_user.data_coordinator? + render "users" + else + head :unauthorized + end + end + + def details + render "show" end private diff --git a/app/helpers/tab_nav_helper.rb b/app/helpers/tab_nav_helper.rb new file mode 100644 index 000000000..c8d8e83b9 --- /dev/null +++ b/app/helpers/tab_nav_helper.rb @@ -0,0 +1,20 @@ +module TabNavHelper + include GovukLinkHelper + + def user_cell(user) + [govuk_link_to(user.name, user), user.email].join("\n") + end + + def org_cell(user) + role = "#{user.role.to_s.humanize}" + [user.organisation.name, role].join("\n") + 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 + end +end diff --git a/app/helpers/user_table_helper.rb b/app/helpers/user_table_helper.rb deleted file mode 100644 index 867108556..000000000 --- a/app/helpers/user_table_helper.rb +++ /dev/null @@ -1,12 +0,0 @@ -module UserTableHelper - include GovukLinkHelper - - def user_cell(user) - [govuk_link_to(user.name, user), user.email].join("\n") - end - - def org_cell(user) - role = "#{user.role}" - [user.organisation.name, role].join("\n") - end -end diff --git a/app/models/constants/user.rb b/app/models/constants/user.rb new file mode 100644 index 000000000..d0a24cbeb --- /dev/null +++ b/app/models/constants/user.rb @@ -0,0 +1,7 @@ +module Constants::User + ROLES = { + "data_accessor" => 0, + "data_provider" => 1, + "data_coordinator" => 2, + }.freeze +end diff --git a/app/models/user.rb b/app/models/user.rb index 81d3d2d9c..8e18a55db 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,4 +1,6 @@ class User < ApplicationRecord + include Constants::User + # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable and :omniauthable devise :database_authenticatable, :recoverable, :rememberable, :validatable, @@ -8,6 +10,8 @@ class User < ApplicationRecord has_many :owned_case_logs, through: :organisation has_many :managed_case_logs, through: :organisation + enum role: ROLES + def case_logs CaseLog.for_organisation(organisation) end diff --git a/app/views/layouts/organisations.html.erb b/app/views/layouts/organisations.html.erb index 4be6b090b..b316bf64f 100644 --- a/app/views/layouts/organisations.html.erb +++ b/app/views/layouts/organisations.html.erb @@ -3,10 +3,9 @@ Your organisation - <%= render TabNavigationComponent.new(items: [ - { name: t('Details'), url: details_organisation_path(@organisation) }, - { name: t('Users'), url: users_organisation_path(@organisation) }, - ]) %> + <% items = tab_items(current_user) %> + + <%= render TabNavigationComponent.new(items: items) %>

<%= content_for(:tab_title) %>

diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 7b2dae5c2..97fa5963c 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -34,7 +34,7 @@ <%= summary_list.row do |row| row.key { 'Role' } - row.value { current_user.role } + row.value { current_user.role.humanize } row.action() end %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index c7be56592..72e1ef208 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,7 +22,7 @@ Rails.application.routes.draw do resources :organisations do member do - get "details", to: "organisations#show" + get "details", to: "organisations#details" get "users", to: "organisations#users" get "users/invite", to: "users/account#new" end diff --git a/db/migrate/20211202124802_change_user_role_to_enum.rb b/db/migrate/20211202124802_change_user_role_to_enum.rb new file mode 100644 index 000000000..7eaa5ecd9 --- /dev/null +++ b/db/migrate/20211202124802_change_user_role_to_enum.rb @@ -0,0 +1,15 @@ +class ChangeUserRoleToEnum < ActiveRecord::Migration[6.1] + def up + change_table :users, bulk: true do |t| + t.remove :role + t.column :role, :integer + end + end + + def down + change_table :users, bulk: true do |t| + t.remove :role + t.column :role, :string + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 8b85217f5..fe6ed0955 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_12_01_144335) do +ActiveRecord::Schema.define(version: 2021_12_02_124802) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -198,13 +198,13 @@ ActiveRecord::Schema.define(version: 2021_12_01_144335) do t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.string "name" - t.string "role" t.bigint "organisation_id" t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" + t.integer "role" t.index ["email"], name: "index_users_on_email", unique: true t.index ["organisation_id"], name: "index_users_on_organisation_id" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true diff --git a/db/seeds.rb b/db/seeds.rb index 6ff200682..a59808694 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -16,5 +16,18 @@ org = Organisation.create!( other_stock_owners: "None", managing_agents: "None", ) -User.create!(email: "test@example.com", password: "password", organisation: org) +User.create!( + email: "test@example.com", + password: "password", + organisation: org, + role: "data_provider", +) + +User.create!( + email: "coordinator@example.com", + password: "password", + organisation: org, + role: "data_coordinator", +) + AdminUser.create!(email: "admin@example.com", password: "password") diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index a7615adba..bad96a9e6 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -30,6 +30,7 @@ describe Admin::UsersController, type: :controller do name: "Jane", password: "pAssword1", organisation_id: organisation.id, + role: "data_coordinator", }, } end @@ -49,6 +50,7 @@ describe Admin::UsersController, type: :controller do expect(page).to have_field("user_email") expect(page).to have_field("user_name") expect(page).to have_field("user_organisation_id") + expect(page).to have_field("user_role") expect(page).to have_field("user_password") expect(page).to have_field("user_password_confirmation") end diff --git a/spec/factories/user.rb b/spec/factories/user.rb index 36789715a..aa08b1d99 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -4,7 +4,10 @@ FactoryBot.define do name { "Danny Rojas" } password { "pAssword1" } organisation - role { "Data Provider" } + role { "data_provider" } + trait :data_coordinator do + role { "data_coordinator" } + end created_at { Time.zone.now } updated_at { Time.zone.now } end diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 3a3d97b7e..dd6d120e8 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -3,7 +3,6 @@ require_relative "form/helpers" RSpec.describe "User Features" do include Helpers - let!(:user) { FactoryBot.create(:user) } let(:organisation) { user.organisation } let(:org_id) { organisation.id } @@ -11,33 +10,50 @@ RSpec.describe "User Features" do sign_in user end - context "Organisation page" do - it "default to organisation details" do - visit("/case-logs") - click_link("Your organisation") - expect(page).to have_content(user.organisation.name) + context "User is a data coordinator" do + let!(:user) { FactoryBot.create(:user, :data_coordinator) } + + context "Organisation page" do + it "defaults to organisation details" do + visit("/case-logs") + click_link("Your organisation") + expect(page).to have_content(user.organisation.name) + end + + it "can switch tabs" do + visit("/organisations/#{org_id}") + click_link("Users") + expect(page).to have_current_path("/organisations/#{org_id}/users") + click_link("Details") + expect(page).to have_current_path("/organisations/#{org_id}/details") + end end - it "can switch tabs" do - visit("/organisations/#{org_id}") - click_link("Users") - expect(page).to have_current_path("/organisations/#{org_id}/users") - click_link("Details") - expect(page).to have_current_path("/organisations/#{org_id}/details") + context "Organisation users" do + it "users can be added" do + visit("/organisations/#{org_id}") + click_link("Users") + click_link("Invite user") + expect(page).to have_current_path("/users/new") + expect(page).to have_content("Invite user to submit CORE data") + fill_in("user[name]", with: "New User") + fill_in("user[email]", with: "new_user@example.com") + expect { click_button("Continue") }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect(page).to have_current_path("/organisations/#{org_id}/users") + end end end - context "Organisation users" do - it "users can be added" do - visit("/organisations/#{org_id}") - click_link("Users") - click_link("Invite user") - expect(page).to have_current_path("/users/new") - expect(page).to have_content("Invite user to submit CORE data") - fill_in("user[name]", with: "New User") - fill_in("user[email]", with: "new_user@example.com") - expect { click_button("Continue") }.to change { ActionMailer::Base.deliveries.count }.by(1) - expect(page).to have_current_path("/organisations/#{org_id}/users") + context "User is a data provider" do + let!(:user) { FactoryBot.create(:user) } + + context "Organisation page" do + it "can only see the details tab" do + visit("/case-logs") + click_link("Your organisation") + expect(page).to have_current_path("/organisations/#{org_id}/details") + expect(page).to have_no_link("Users") + end end end end diff --git a/spec/helpers/tab_nav_helper_spec.rb b/spec/helpers/tab_nav_helper_spec.rb new file mode 100644 index 000000000..ad68dd1a1 --- /dev/null +++ b/spec/helpers/tab_nav_helper_spec.rb @@ -0,0 +1,40 @@ +require "rails_helper" + +RSpec.describe TabNavHelper do + let(:organisation) { FactoryBot.create(:organisation) } + let(:user) { FactoryBot.build(:user, organisation: organisation) } + + describe "#user_cell" do + it "returns user link and email separated by a newline character" do + expected_html = "Danny Rojas\n#{user.email}" + expect(user_cell(user)).to match(expected_html) + end + end + + describe "#org_cell" do + it "returns the users org name and role separated by a newline character" do + expected_html = "DLUHC\nData provider" + expect(org_cell(user)).to match(expected_html) + end + end + + describe "#tab_items" do + context "user is a data_coordinator" do + let(:user) { FactoryBot.build(:user, :data_coordinator, organisation: organisation) } + it "returns details and user tabs" do + result = tab_items(user).map { |i| i[:name] } + expect(result.count).to eq(2) + expect(result.first).to match("Details") + expect(result.second).to match("Users") + end + end + + context "user is a data_provider" do + it "returns details tab only" do + result = tab_items(user).map { |i| i[:name] } + expect(result.count).to eq(1) + expect(result.first).to match("Details") + end + end + end +end diff --git a/spec/helpers/user_table_helper_spec.rb b/spec/helpers/user_table_helper_spec.rb deleted file mode 100644 index 6d8921a0f..000000000 --- a/spec/helpers/user_table_helper_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require "rails_helper" - -RSpec.describe UserTableHelper do - let(:user) { FactoryBot.build(:user) } - - describe "#user_cell" do - it "returns user link and email separated by a newline character" do - expected_html = "Danny Rojas\n#{user.email}" - expect(user_cell(user)).to match(expected_html) - end - end - - describe "#org_cell" do - it "returns the users org name and role separated by a newline character" do - expected_html = "DLUHC\nData Provider" - expect(org_cell(user)).to match(expected_html) - end - end -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index bec457e7f..3195b73a0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -40,5 +40,11 @@ RSpec.describe User, type: :model do expect(user.completed_case_logs.to_a).to eq([owned_case_log]) expect(user.not_completed_case_logs.to_a).to eq([managed_case_log]) end + + it "has a role" do + expect(user.role).to eq("data_provider") + expect(user.data_provider?).to be true + expect(user.data_coordinator?).to be false + end end end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index cd07845ac..be49b8c9d 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -1,58 +1,112 @@ require "rails_helper" RSpec.describe OrganisationsController, type: :request do - let(:user) { FactoryBot.create(:user) } let(:organisation) { user.organisation } let(:headers) { { "Accept" => "text/html" } } let(:page) { Capybara::Node::Simple.new(response.body) } - context "details tab" do + describe "#show" do + let(:user) { FactoryBot.create(:user, :data_coordinator) } + before do sign_in user get "/organisations/#{organisation.id}", headers: headers, params: {} end - it "shows the tab navigation" do - expected_html = "