diff --git a/app/models/user.rb b/app/models/user.rb index e6c6ebbe05..26c04f8498 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -111,13 +111,11 @@ def invitation_status "invited" if invitation_sent_at.present? end - def kind - return "super" if has_cached_role?(Role::SUPER_ADMIN) + def org_role return "admin" if has_cached_role?(Role::ORG_ADMIN, organization) return "normal" if has_cached_role?(Role::ORG_USER, organization) - return "partner" if has_cached_role?(Role::PARTNER, partner) - "normal" + "not a member" end def is_admin?(org) diff --git a/app/views/admin/users/_roles.html.erb b/app/views/admin/users/_roles.html.erb index 1d1d843da6..c1592a791d 100644 --- a/app/views/admin/users/_roles.html.erb +++ b/app/views/admin/users/_roles.html.erb @@ -24,7 +24,9 @@ <% user.roles.each do |role| %> <%= role.title %> - <%= role.resource.name %> (id: <%= role.resource.id %>) + + <%= role.resource&.name || "Super Admin" %> <%= role.resource&.id ? "(id: #{role.resource&.id})" : "" %> + <%= delete_button_to admin_user_remove_role_path(user, role_id: role.id), confirm: "Are you sure you want to remove this role?" %> diff --git a/app/views/users/_organization_user.html.erb b/app/views/users/_organization_user.html.erb index 6d07882d5a..5190838ff2 100644 --- a/app/views/users/_organization_user.html.erb +++ b/app/views/users/_organization_user.html.erb @@ -3,12 +3,12 @@ <%= user.display_name %> <%= user.email %> - <%= user.kind %> + <%= user.org_role %> <%= user.current_sign_in_at&.strftime('%Y/%m/%d') %> <%= user.invitation_status %> <%= reinvite_user_link(user) %> - <% unless user.has_cached_role?(Role::SUPER_ADMIN) || user.has_cached_role?(Role::ORG_ADMIN, current_organization) %> + <% unless user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
+ <% end %> <% if current_user.is_admin?(current_organization) && user.has_cached_role?(Role::ORG_ADMIN, current_organization) %> <%= edit_button_to demote_to_user_organization_path(user_id: user.id, organization_name: current_organization.short_name), @@ -39,5 +36,4 @@ {method: :post, rel: "nofollow", data: {confirm: 'This will demote the admin to user status. Are you sure that you want to submit this?', size: 'xs'}} unless user.id == current_user.id %> <% end %> - diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 9177937b23..99c78a6afc 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -76,6 +76,13 @@ resource_type: Role::ORG_ADMIN) end end + + factory :super_admin_org_admin do + name { "Administrative User And Org Admin" } + after(:create) do |user| + user.add_role(Role::SUPER_ADMIN) + end + end end factory :super_admin do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e6ea890636..1ac79e615b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -115,10 +115,12 @@ expect(build(:user, invitation_sent_at: Time.zone.parse("2018-10-10 00:00:00"), invitation_accepted_at: Time.zone.parse("2018-10-11 00:00:00"), current_sign_in_at: Time.zone.parse("2018-10-23 00:00:00")).invitation_status).to eq("joined") end - it "#kind" do - expect(create(:super_admin).kind).to eq("super") - expect(create(:organization_admin).kind).to eq("admin") - expect(create(:user).kind).to eq("normal") + it "#org_role" do + expect(create(:super_admin).org_role).to eq("normal") + expect(create(:super_admin_org_admin).org_role).to eq("admin") + expect(create(:organization_admin).org_role).to eq("admin") + expect(create(:user).org_role).to eq("normal") + expect(create(:partner_user).org_role).to eq("not a member") end it "#reinvitable?" do diff --git a/spec/requests/admin/organizations_requests_spec.rb b/spec/requests/admin/organizations_requests_spec.rb index db274795d0..7164a0b1d0 100644 --- a/spec/requests/admin/organizations_requests_spec.rb +++ b/spec/requests/admin/organizations_requests_spec.rb @@ -100,6 +100,10 @@ it "returns http success" do get admin_organizations_path expect(response).to be_successful + expect(response.body).to include(organization.name) + expect(response.body).to include(organization.email) + expect(response.body).to include(organization.created_at.strftime("%Y-%m-%d")) + expect(response.body).to include(organization.display_last_distribution_date) end end @@ -184,8 +188,11 @@ it "provides links to edit the user" do get admin_organization_path({ id: organization.id }) - expect(response.body).to include("Edit User") - expect(response.body).to include(edit_admin_user_path(user.id)) + expect(response.body).to include("Actions") + expect(response.body).to include('Promote to Admin') + expect(response.body).to include(promote_to_org_admin_organization_path(user_id: user.id)) + expect(response.body).to include('Remove User') + expect(response.body).to include(remove_user_organization_path(user_id: user.id)) end end end diff --git a/spec/requests/admin/users_requests_spec.rb b/spec/requests/admin/users_requests_spec.rb index b6e9add5b2..ad2b2a75a4 100644 --- a/spec/requests/admin/users_requests_spec.rb +++ b/spec/requests/admin/users_requests_spec.rb @@ -53,14 +53,16 @@ end describe '#add_role' do - context 'with no errors' do - it 'should call the service and redirect back' do + shared_examples "add role check" do |user_factory| + let!(:user_to_modify) { create(user_factory, name: "User to modify", organization: organization) } + + it "should call the service and redirect back", :aggregate_failures do allow(AddRoleService).to receive(:call) - post admin_user_add_role_path(user_id: user.id, + post admin_user_add_role_path(user_id: user_to_modify.id, resource_type: Role::ORG_ADMIN, resource_id: organization.id), headers: { 'HTTP_REFERER' => '/back/url'} - expect(AddRoleService).to have_received(:call).with(user_id: user.id.to_s, + expect(AddRoleService).to have_received(:call).with(user_id: user_to_modify.id.to_s, resource_type: Role::ORG_ADMIN.to_s, resource_id: organization.id.to_s) expect(flash[:notice]).to eq('Role added!') @@ -68,6 +70,13 @@ end end + context 'with no errors' do + include_examples "add role check", :user + context "modifying another super admin" do + include_examples "add role check", :super_admin + end + end + context 'with errors' do it 'should redirect back with error' do allow(AddRoleService).to receive(:call).and_raise('OH NOES') @@ -85,19 +94,29 @@ end describe '#remove_role' do - context 'with no errors' do - it 'should call the service and redirect back' do + shared_examples "remove role check" do |user_factory| + let!(:user_to_modify) { create(user_factory, name: "User to modify", organization: organization) } + + it "should call the service and redirect back", :aggregate_failures do + role_to_remove_id = user_to_modify.roles.find_by(name: Role::ORG_ADMIN, resource_id: organization.id).id allow(RemoveRoleService).to receive(:call) - delete admin_user_remove_role_path(user_id: user.id, - role_id: 123), + delete admin_user_remove_role_path(user_id: user_to_modify.id, + role_id: role_to_remove_id), headers: { 'HTTP_REFERER' => '/back/url'} - expect(RemoveRoleService).to have_received(:call).with(user_id: user.id.to_s, - role_id: '123') + expect(RemoveRoleService).to have_received(:call).with(user_id: user_to_modify.id.to_s, + role_id: role_to_remove_id.to_s) expect(flash[:notice]).to eq('Role removed!') expect(response).to redirect_to('/back/url') end end + context 'with no errors' do + include_examples "remove role check", :organization_admin + context 'modifying another super admin' do + include_examples "remove role check", :super_admin_org_admin + end + end + context 'with errors' do it 'should redirect back with error' do allow(RemoveRoleService).to receive(:call).and_raise('OH NOES') diff --git a/spec/requests/organization_requests_spec.rb b/spec/requests/organization_requests_spec.rb index a42c9c69f3..8e0b6444ff 100644 --- a/spec/requests/organization_requests_spec.rb +++ b/spec/requests/organization_requests_spec.rb @@ -6,6 +6,85 @@ let!(:unit) { create(:unit, name: "WolfPack", organization: organization) } let!(:store) { create(:storage_location, organization: organization) } let!(:ndbn_member) { create(:ndbn_member, ndbn_member_id: "50000", account_name: "Best Place") } + let!(:super_admin_org_admin) { create(:super_admin_org_admin, organization: organization) } + + shared_examples "promote to admin check" do |user_factory, current_user| + let!(:user_to_promote) { create(user_factory, name: "User to promote") } + let(:response_path) { + case current_user + when :super_admin + admin_organization_path(organization.id) + when :non_super_admin + organization_path + end + } + + it "runs correctly", :aggregate_failures do + # Explicitly specify the organization_name, as current_organization will not + # be set for super admins + post promote_to_org_admin_organization_path( + user_id: user_to_promote.id, + organization_name: organization.short_name + ) + expect(user_to_promote.reload.has_role?(Role::ORG_ADMIN, organization)).to be_truthy + # The user_update_redirect_path will vary based on whether the logged in + # user is a super admin or not + expect(response).to redirect_to(response_path) + expect(flash[:notice]).to eq("User has been promoted!") + end + end + + shared_examples "demote to user check" do |user_factory, current_user| + let!(:user_to_demote) { create(user_factory, name: "User to demote", organization: organization) } + let(:response_path) { + case current_user + when :super_admin + admin_organization_path(organization.id) + when :non_super_admin + organization_path + end + } + + it "runs correctly", :aggregate_failures do + # Explicitly specify the organization_name, as current_organization will not + # be set for super admins + post demote_to_user_organization_path( + user_id: user_to_demote.id, + organization_name: organization.short_name + ) + expect(user_to_demote.reload.has_role?(Role::ORG_ADMIN, organization)).to be_falsey + # The user_update_redirect_path will vary based on whether the logged in + # user is a super admin or not + expect(response).to redirect_to(response_path) + expect(flash[:notice]).to eq("User has been demoted!") + end + end + + shared_examples "remove user check" do |user_factory, current_user| + let!(:user_to_remove) { create(user_factory, name: "User to remove", organization: organization) } + let(:response_path) { + case current_user + when :super_admin + admin_organization_path(organization.id) + when :non_super_admin + organization_path + end + } + + it "runs correctly", :aggregate_failures do + # Explicitly specify the organization_name, as current_organization will not + # be set for super admins + post remove_user_organization_path( + user_id: user_to_remove.id, + organization_name: organization.short_name + ) + expect(user_to_remove.reload.has_role?(Role::ORG_USER, organization)).to be_falsey + # The user_update_redirect_path will vary based on whether the logged in + # user is a super admin or not + expect(response).to redirect_to(response_path) + expect(flash[:notice]).to eq("User has been removed!") + end + end context "While signed in as a normal user" do before do @@ -345,46 +424,39 @@ end describe "POST #promote_to_org_admin" do - subject { post promote_to_org_admin_organization_path(user_id: user.id) } + context "promoting a user" do + include_examples "promote to admin check", :user, :non_super_admin + end - it "runs successfully" do - subject - expect(user.has_role?(Role::ORG_ADMIN, organization)).to eq(true) - expect(response).to redirect_to(organization_path) - expect(flash[:notice]).to eq("User has been promoted!") + context "promoting a super admin user" do + include_examples "promote to admin check", :super_admin, :non_super_admin end end describe "POST #demote_to_user" do - subject { post demote_to_user_organization_path(user_id: admin_user.id) } + context "demoting a user" do + include_examples "demote to user check", :organization_admin, :non_super_admin + end - it "runs correctly" do - subject - expect(admin_user.reload.has_role?(Role::ORG_ADMIN, admin_user.organization)).to be_falsey - expect(response).to redirect_to(organization_path) - expect(flash[:notice]).to eq("User has been demoted!") + context "demoting a super admin user" do + include_examples "demote to user check", :super_admin_org_admin, :non_super_admin end end describe "POST #remove_user" do - subject { post remove_user_organization_path(user_id: user.id) } - - context "when user is org user" do - it "redirects after update" do - subject - expect(response).to redirect_to(organization_path) - end + context "removing a user" do + include_examples "remove user check", :user, :non_super_admin + end - it "removes the org user role" do - expect { subject }.to change { user.has_role?(Role::ORG_USER, organization) }.from(true).to(false) - end + context "removing a super admin user" do + include_examples "remove user check", :super_admin, :non_super_admin end context "when user is not an org user" do let(:user) { create(:user, organization: create(:organization)) } it 'raises an error' do - subject + post remove_user_organization_path(user_id: user.id) expect(response).to be_not_found end @@ -430,56 +502,46 @@ context 'When signed in as a super admin' do before do - sign_in(create(:super_admin, organization: organization)) + sign_in(super_admin_org_admin) end - describe "GET #show" do - before { get admin_organizations_path(id: organization.id) } - - it { expect(response).to be_successful } - - it 'organization details' do - expect(response.body).to include(organization.name) - expect(response.body).to include(organization.email) - expect(response.body).to include(organization.created_at.strftime("%Y-%m-%d")) - expect(response.body).to include(organization.display_last_distribution_date) + describe "POST #promote_to_org_admin" do + context "promoting a user" do + include_examples "promote to admin check", :user, :super_admin end - it "can see 'Edit User' button for users" do - within(".content") do - expect(response.body).to have_link("Actions") - end + context "promoting a super admin user" do + include_examples "promote to admin check", :super_admin, :super_admin + end + end - within "#dropdown-toggle" do - expect(response.body).to have_link("Edit User") - expect(response.body).to have_link("Remove User") - end + describe "POST #demote_to_user" do + context "demoting a user" do + include_examples "demote to user check", :organization_admin, :super_admin end - it "can see 'Demote User' button for organization admins" do - within(".content") do - expect(response.body).to have_link("Demote to User") - end + context "demoting a super admin user" do + include_examples "demote to user check", :super_admin_org_admin, :super_admin end end - describe "POST #promote_to_org_admin" do - before { post promote_to_org_admin_organization_path(user_id: user.id, organization_name: organization.short_name) } + describe "POST #remove_user" do + context "removing a user" do + include_examples "remove user check", :user, :super_admin + end - it "promotes the user to org_admin" do - expect(user.has_role?(Role::ORG_ADMIN, organization)).to eq(true) - expect(response).to redirect_to(admin_organization_path({ id: organization.id })) - expect(flash[:notice]).to eq("User has been promoted!") + context "removing a super admin user" do + include_examples "remove user check", :super_admin, :super_admin end - end - describe "POST #demote_to_user" do - before { post demote_to_user_organization_path(user_id: admin_user.id, organization_name: organization.short_name) } + context "when user is not an org user" do + let(:user) { create(:user, organization: create(:organization)) } + + it 'raises an error' do + post remove_user_organization_path(user_id: user.id) - it "demotes the org_admin to user" do - expect(admin_user.reload.has_role?(Role::ORG_ADMIN, admin_user.organization)).to be_falsey - expect(response).to redirect_to(admin_organization_path({ id: organization.id })) - expect(flash[:notice]).to eq("User has been demoted!") + expect(response).to be_not_found + end end end end diff --git a/spec/system/admin/users_system_spec.rb b/spec/system/admin/users_system_spec.rb index 581430e1bb..ed25878cad 100644 --- a/spec/system/admin/users_system_spec.rb +++ b/spec/system/admin/users_system_spec.rb @@ -39,18 +39,43 @@ expect(users_table).to have_text("TestUser") end - it 'adds a role' do - user = create(:user, name: 'User 123', organization: organization) - create(:partner, name: 'Partner ABC', organization: organization) - - visit edit_admin_user_path(user) - expect(page).to have_content('User 123') - select "Partner", from: "resource_type" - find("div.input-group:has(.select2-container)").click - find("li.select2-results__option", text: "Partner ABC").click - click_on 'Add Role' - - expect(page.find('.alert')).to have_content('Role added') + shared_examples "add role check" do |user_factory| + let!(:user_to_modify) { create(user_factory, name: "User to modify", organization: organization) } + + it "adds a role", :aggregate_failures do + create(:partner, name: 'Partner ABC', organization: organization) + visit edit_admin_user_path(user_to_modify) + expect(page).to have_content('User to modify') + select "Partner", from: "resource_type" + find("div.input-group:has(.select2-container)").click + find("li.select2-results__option", text: "Partner ABC").click + click_on 'Add Role' + + expect(page.find('.alert')).to have_content('Role added') + end + end + + include_examples "add role check", :user + context 'modifying another super admin' do + include_examples "add role check", :super_admin + end + + shared_examples "remove role check" do |user_factory| + let!(:user_to_modify) { create(user_factory, name: "User to modify", organization: organization) } + + it "removes a role", :aggregate_failures do + visit edit_admin_user_path(user_to_modify) + expect(page).to have_content('User to modify') + accept_confirm do + click_on 'Delete', match: :first # For users that have multiple roles + end + expect(page.find('.alert')).to have_content('Role removed!') + end + end + + include_examples "remove role check", :user + context 'modifying another super admin' do + include_examples "remove role check", :super_admin end it "filters users by name" do diff --git a/spec/system/organization_system_spec.rb b/spec/system/organization_system_spec.rb index 6316da2567..3f5cec01bb 100644 --- a/spec/system/organization_system_spec.rb +++ b/spec/system/organization_system_spec.rb @@ -2,9 +2,47 @@ let(:organization) { create(:organization) } let(:user) { create(:user, organization: organization) } let(:organization_admin) { create(:organization_admin, organization: organization) } + let(:super_admin_org_admin) { create(:super_admin_org_admin, organization: organization) } include ActionView::RecordIdentifier + shared_examples "organization role management checks" do |user_factory| + let!(:managed_user) { create(user_factory, name: "User to be managed", organization: organization) } + + it 'can remove that user from the organization' do + visit organization_path + accept_confirm do + click_button dom_id(managed_user, "dropdownMenu") + click_link "Remove User" + end + + expect(page).to have_content("User has been removed!") + expect(managed_user.has_role?(Role::ORG_USER)).to be false + end + + it "can promote that user from the organization" do + visit organization_path + accept_confirm do + click_button dom_id(managed_user, "dropdownMenu") + click_link "Promote to Admin" + end + + expect(page).to have_content("User has been promoted!") + expect(managed_user.has_role?(Role::ORG_ADMIN, organization)).to be true + end + + it "can demote that user from the organization" do + managed_user.add_role(Role::ORG_ADMIN, organization) + visit organization_path + accept_confirm do + click_link "Demote to User" + end + + expect(page).to have_content("User has been demoted!") + expect(managed_user.has_role?(Role::ORG_ADMIN, organization)).to be false + end + end + context "while signed in as an organization admin" do before do sign_in(organization_admin) @@ -21,16 +59,34 @@ expect(page).to have_content("invited to organization") end - it "can remove a user from the organization" do - user = create(:user, name: "User to be deactivated", organization: organization) - visit organization_path - accept_confirm do - click_button dom_id(user, "dropdownMenu") - click_link "Remove User" + context "managing a user from the organization" do + include_examples "organization role management checks", :user + end + + context "managing a super admin user from the organization" do + include_examples "organization role management checks", :super_admin + end + end + + context "while signed in as a super admin" do + before do + sign_in(super_admin_org_admin) + end + + before(:each) do + visit admin_dashboard_path + within ".main-header" do + click_on super_admin_org_admin.name.to_s end + click_link "Switch to: #{organization.name}" + end - expect(page).to have_content("User has been removed!") - expect(user.has_role?(Role::ORG_USER)).to be false + context "managing a user from the organization" do + include_examples "organization role management checks", :user + end + + context "managing a super admin user from the organization" do + include_examples "organization role management checks", :super_admin end end end