User roles (#138)

* Users have roles

* Add role to active admin

* Add role to users table

* Redirect organisation show to details

* Remove users tab for data providers

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

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

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 = "<span class='app-!-colour-muted'>#{user.role.to_s.humanize}</span>"
+    [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

app/helpers/user_table_helper.rb

@@ -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 = "<span class='app-!-colour-muted'>#{user.role}</span>"
-    [user.organisation.name, role].join("\n")
-  end
-end

app/models/constants/user.rb

@@ -0,0 +1,7 @@
+module Constants::User
+  ROLES = {
+    "data_accessor" => 0,
+    "data_provider" => 1,
+    "data_coordinator" => 2,
+  }.freeze
+end

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

app/views/layouts/organisations.html.erb

@@ -3,10 +3,9 @@
     Your organisation
   </h1>
 
-  <%= render TabNavigationComponent.new(items: [
+  <% items = tab_items(current_user) %>
-    { name: t('Details'), url: details_organisation_path(@organisation) },
+
-    { name: t('Users'), url: users_organisation_path(@organisation) },
+  <%= render TabNavigationComponent.new(items: items) %>
-  ]) %>
 
   <h2 class="govuk-visually-hidden"><%= content_for(:tab_title) %></h2>
 

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 %>

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

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

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

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

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

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

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
+  context "User is a data coordinator" do
-    it "default to organisation details" do
+    let!(:user) { FactoryBot.create(:user, :data_coordinator) }
-      visit("/case-logs")
+
-      click_link("Your organisation")
+    context "Organisation page" do
-      expect(page).to have_content(user.organisation.name)
+      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
+    context "Organisation users" do
-      visit("/organisations/#{org_id}")
+      it "users can be added" do
-      click_link("Users")
+        visit("/organisations/#{org_id}")
-      expect(page).to have_current_path("/organisations/#{org_id}/users")
+        click_link("Users")
-      click_link("Details")
+        click_link("Invite user")
-      expect(page).to have_current_path("/organisations/#{org_id}/details")
+        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
+  context "User is a data provider" do
-    it "users can be added" do
+    let!(:user) { FactoryBot.create(:user) }
-      visit("/organisations/#{org_id}")
+
-      click_link("Users")
+    context "Organisation page" do
-      click_link("Invite user")
+      it "can only see the details tab" do
-      expect(page).to have_current_path("/users/new")
+        visit("/case-logs")
-      expect(page).to have_content("Invite user to submit CORE data")
+        click_link("Your organisation")
-      fill_in("user[name]", with: "New User")
+        expect(page).to have_current_path("/organisations/#{org_id}/details")
-      fill_in("user[email]", with: "new_user@example.com")
+        expect(page).to have_no_link("Users")
-      expect { click_button("Continue") }.to change { ActionMailer::Base.deliveries.count }.by(1)
+      end
-      expect(page).to have_current_path("/organisations/#{org_id}/users")
     end
   end
 end

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 = "<a class=\"govuk-link\" href=\"/users\">Danny Rojas</a>\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\n<span class='app-!-colour-muted'>Data provider</span>"
+      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

spec/helpers/user_table_helper_spec.rb

@@ -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 = "<a class=\"govuk-link\" href=\"/users\">Danny Rojas</a>\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\n<span class='app-!-colour-muted'>Data Provider</span>"
-      expect(org_cell(user)).to match(expected_html)
-    end
-  end
-end

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

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
+    it "redirects to details" do
-      expected_html = "<nav class=\"app-tab-navigation\""
+      expect(response).to have_http_status(:redirect)
-      expect(response.body).to include(expected_html)
     end
+  end
+
+  context "As a data coordinator user" do
+    let(:user) { FactoryBot.create(:user, :data_coordinator) }
+
+    context "details tab" do
+      before do
+        sign_in user
+        get "/organisations/#{organisation.id}/details", headers: headers, params: {}
+      end
+
+      it "shows the tab navigation" do
+        expected_html = "<nav class=\"app-tab-navigation\""
+        expect(response.body).to include(expected_html)
+      end
 
-    it "shows a summary list of org details" do
+      it "shows a summary list of org details" do
-      expected_html = "<dl class=\"govuk-summary-list\""
+        expected_html = "<dl class=\"govuk-summary-list\""
-      expect(response.body).to include(expected_html)
+        expect(response.body).to include(expected_html)
-      expect(response.body).to include(organisation.name)
+        expect(response.body).to include(organisation.name)
+      end
+
+      it "has a hidden header title" do
+        expected_html = "<h2 class=\"govuk-visually-hidden\">  Details"
+        expect(response.body).to include(expected_html)
+      end
     end
 
-    it "has a hidden header title" do
+    context "users tab" do
-      expected_html = "<h2 class=\"govuk-visually-hidden\">  Details"
+      before do
-      expect(response.body).to include(expected_html)
+        sign_in user
+        get "/organisations/#{organisation.id}/users", headers: headers, params: {}
+      end
+
+      it "shows the tab navigation" do
+        expected_html = "<nav class=\"app-tab-navigation\""
+        expect(response.body).to include(expected_html)
+      end
+
+      it "shows a new user button" do
+        expect(page).to have_link("Invite user")
+      end
+
+      it "shows a table of users" do
+        expected_html = "<table class=\"govuk-table\""
+        expect(response.body).to include(expected_html)
+        expect(response.body).to include(user.email)
+      end
+
+      it "has a hidden header title" do
+        expected_html = "<h2 class=\"govuk-visually-hidden\">  Users"
+        expect(response.body).to include(expected_html)
+      end
     end
   end
 
-  context "users tab" do
+  context "As a data provider user" do
-    before do
+    let(:user) { FactoryBot.create(:user) }
-      sign_in user
-      get "/organisations/#{organisation.id}/users", headers: headers, params: {}
-    end
 
-    it "shows the tab navigation" do
+    context "details tab" do
-      expected_html = "<nav class=\"app-tab-navigation\""
+      before do
-      expect(response.body).to include(expected_html)
+        sign_in user
-    end
+        get "/organisations/#{organisation.id}/details", headers: headers, params: {}
+      end
 
-    it "shows a new user button" do
+      it "shows the tab navigation" do
-      expect(page).to have_link("Invite user")
+        expected_html = "<nav class=\"app-tab-navigation\""
-    end
+        expect(response.body).to include(expected_html)
+      end
 
-    it "shows a table of users" do
+      it "shows a summary list of org details" do
-      expected_html = "<table class=\"govuk-table\""
+        expected_html = "<dl class=\"govuk-summary-list\""
-      expect(response.body).to include(expected_html)
+        expect(response.body).to include(expected_html)
-      expect(response.body).to include(user.email)
+        expect(response.body).to include(organisation.name)
+      end
+
+      it "has a hidden header title" do
+        expected_html = "<h2 class=\"govuk-visually-hidden\">  Details"
+        expect(response.body).to include(expected_html)
+      end
     end
 
-    it "has a hidden header title" do
+    context "users tab" do
-      expected_html = "<h2 class=\"govuk-visually-hidden\">  Users"
+      before do
-      expect(response.body).to include(expected_html)
+        sign_in user
+        get "/organisations/#{organisation.id}/users", headers: headers, params: {}
+      end
+
+      it "should return unauthorised 401" do
+        expect(response).to have_http_status(:unauthorized)
+      end
     end
   end
 end