Skip to content

Commit 95da0b3

Browse files
canceleicoalest
andauthored
Remove organization from superuser edit/index of users (#4453)
* Update user table to display organization, banks, and partners in the admin users list view * revert new.html * update users controller and _list to match use case * fix linter * Revert changes to create/new and fix specs * Fix failing system spec * Remove unneeded user params * Show banks and partners as unique sorted list * Fix linting --------- Co-authored-by: Cory Streiff <[email protected]>
1 parent 1c9182e commit 95da0b3

File tree

5 files changed

+52
-55
lines changed

5 files changed

+52
-55
lines changed

app/controllers/admin/users_controller.rb

+9-18
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ class Admin::UsersController < AdminController
55

66
def index
77
@filterrific = initialize_filterrific(
8-
User.includes(:organization).alphabetized,
8+
User.includes(roles: :resource).alphabetized,
99
params[:filterrific],
1010
available_filters: [:search_name, :search_email]
1111
) || return
@@ -38,15 +38,16 @@ def edit
3838
end
3939

4040
def create
41+
@user = User.new(user_params)
4142
UserInviteService.invite(name: user_params[:name],
4243
email: user_params[:email],
4344
roles: [Role::ORG_USER],
44-
resource: Organization.find(user_params[:organization_id]))
45+
resource: Organization.find(organization_id_param))
4546
flash[:notice] = "Created a new user!"
4647
redirect_to admin_users_path
4748
rescue => e
4849
flash[:error] = "Failed to create user: #{e}"
49-
render "admin/users/new"
50+
render :new
5051
end
5152

5253
def resource_ids
@@ -91,25 +92,15 @@ def remove_role
9192
private
9293

9394
def user_params
94-
organization_id = params[:user][:organization_id]
95-
96-
raise "Please select an organization for the user." if organization_id.blank?
97-
98-
user_params = params.require(:user).permit(:name, :organization_id, :email, :password, :password_confirmation)
99-
user_params[:organization_role_join_attributes] = { role_id: updated_role_id(organization_id) }
100-
101-
user_params
102-
rescue => e
103-
redirect_back(fallback_location: edit_admin_user_path, error: e.message)
95+
params.require(:user).permit(:name, :email)
10496
end
10597

106-
def updated_role_id(organization_id)
107-
user_role_title = Role::TITLES[Role::ORG_USER]
108-
user_role_type = Role::ORG_USER
98+
def organization_id_param
99+
organization_id = params[:user][:organization_id]
109100

110-
role = Role.find_by(resource_type: user_role_title, resource_id: organization_id, name: user_role_type)
101+
raise "Please select an organization for the user." if organization_id.blank?
111102

112-
role&.id || raise("Error finding a role within the provided organization")
103+
organization_id
113104
end
114105

115106
def load_organizations

app/views/admin/users/_list.html.erb

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<table class="table">
22
<thead>
33
<tr>
4-
<th>Organization</th>
4+
<th>Banks and Partners</th>
55
<th>Name</th>
66
<th>Email</th>
77
<th class="text-right">Actions</th>
@@ -10,7 +10,9 @@
1010
<tbody>
1111
<% @users.each do |user| %>
1212
<tr>
13-
<td><%= user.organization&.name %></td>
13+
<td>
14+
<%= user.roles.map { |role| role.resource&.name }.compact.uniq.sort.join(', ') %>
15+
</td>
1416
<td><%= user.display_name %></td>
1517
<td><%= user.email %></td>
1618
<td class="text-right">

app/views/admin/users/edit.html.erb

-4
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,6 @@
3636
<div class="card-body">
3737
<%= simple_form_for @user, url: admin_user_path, html: {id: "quickForm"} do |f| %>
3838
<div class="form-inputs">
39-
<%= f.input :organization_id, label: "Organization", wrapper: :input_group, required: true, autofocus: true do %>
40-
<span class="input-group-text"><%= fa_icon "building" %></span>
41-
<%= f.input_field :organization_id, collection: @organizations, class: "form-control" %>
42-
<% end %>
4339
<%= render 'admin/users/user_form_fields', f: f %>
4440
</div>
4541
</div>

spec/requests/admin/users_requests_spec.rb

+33-25
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,29 @@
11
RSpec.describe "Admin::UsersController", type: :request do
2-
let(:organization) { create(:organization) }
3-
let(:user) { create(:user, organization: organization) }
2+
let(:organization) { create(:organization, name: "Org ABC") }
3+
let(:user) { create(:user, organization: organization, name: "User 123") }
44
let(:organization_admin) { create(:organization_admin, organization: organization) }
55
let(:super_admin) { create(:super_admin, organization: organization) }
6-
7-
let(:default_params) do
8-
{ organization_name: organization.id }
9-
end
10-
11-
let(:org) { create(:organization, name: 'Org ABC') }
12-
let(:partner) { create(:partner, name: 'Partner XYZ', organization: org) }
13-
let(:user) { create(:user, organization: org, name: 'User 123') }
6+
let(:partner) { create(:partner, name: 'Partner XYZ', organization: organization) }
147

158
context "When logged in as a super admin" do
169
before do
1710
sign_in(super_admin)
1811
AddRoleService.call(user_id: user.id, resource_type: Role::PARTNER, resource_id: partner.id)
1912
end
2013

14+
describe "GET #index" do
15+
it "renders index template and shows banks and partners correctly" do
16+
AddRoleService.call(user_id: user.id, resource_type: Role::ORG_ADMIN, resource_id: organization.id)
17+
get admin_users_path
18+
19+
expect(response).to render_template(:index)
20+
21+
page = Nokogiri::HTML(response.body)
22+
banks_and_partners = page.at_xpath("//*[contains(text(), \"#{user.email}\")]/../td[1]").text.strip
23+
expect(banks_and_partners).to eq("Org ABC, Partner XYZ")
24+
end
25+
end
26+
2127
describe "GET #edit" do
2228
it "renders edit template and shows roles" do
2329
get edit_admin_user_path(user)
@@ -31,24 +37,17 @@
3137
describe "PATCH #update" do
3238
context 'with no errors' do
3339
it "renders index template with a successful update flash message" do
34-
patch admin_user_path(user), params: { user: default_params.merge(organization_id: user.organization.id,
35-
name: 'New User 123', email: '[email protected]') }
40+
patch admin_user_path(user), params: { user: { name: 'New User 123', email: '[email protected]' } }
3641
expect(response).to redirect_to admin_users_path
3742
expect(flash[:notice]).to eq("New User 123 updated!")
3843
end
3944
end
4045

4146
context 'with errors' do
42-
it "redirects back with no organization_id flash message" do
43-
patch admin_user_path(user), params: { user: { name: 'New User 123' } }
47+
it "redirects back with flash message" do
48+
patch admin_user_path(user), params: { user: { name: 'New User 123', email: "invalid_email" } }
4449
expect(response).to redirect_to(edit_admin_user_path)
45-
expect(flash[:error]).to eq('Please select an organization for the user.')
46-
end
47-
48-
it "redirects back with no role found flash message" do
49-
patch admin_user_path(user), params: { user: { name: 'New User 123', organization_id: -1 } }
50-
expect(response).to redirect_to(edit_admin_user_path)
51-
expect(flash[:error]).to eq('Error finding a role within the provided organization')
50+
expect(flash[:error]).to eq("Something didn't work quite right -- try again?")
5251
end
5352
end
5453
end
@@ -59,11 +58,11 @@
5958
allow(AddRoleService).to receive(:call)
6059
post admin_user_add_role_path(user_id: user.id,
6160
resource_type: Role::ORG_ADMIN,
62-
resource_id: org.id),
61+
resource_id: organization.id),
6362
headers: { 'HTTP_REFERER' => '/back/url'}
6463
expect(AddRoleService).to have_received(:call).with(user_id: user.id.to_s,
6564
resource_type: Role::ORG_ADMIN.to_s,
66-
resource_id: org.id.to_s)
65+
resource_id: organization.id.to_s)
6766
expect(flash[:notice]).to eq('Role added!')
6867
expect(response).to redirect_to('/back/url')
6968
end
@@ -74,11 +73,11 @@
7473
allow(AddRoleService).to receive(:call).and_raise('OH NOES')
7574
post admin_user_add_role_path(user_id: user.id,
7675
resource_type: Role::ORG_ADMIN,
77-
resource_id: org.id),
76+
resource_id: organization.id),
7877
headers: { 'HTTP_REFERER' => '/back/url'}
7978
expect(AddRoleService).to have_received(:call).with(user_id: user.id.to_s,
8079
resource_type: Role::ORG_ADMIN.to_s,
81-
resource_id: org.id.to_s)
80+
resource_id: organization.id.to_s)
8281
expect(flash[:alert]).to eq('OH NOES')
8382
expect(response).to redirect_to('/back/url')
8483
end
@@ -135,6 +134,15 @@
135134
post admin_users_path, params: { user: { organization_id: organization.id } }
136135
expect(assigns(:organizations)).to eq(Organization.all.alphabetized)
137136
end
137+
138+
context "with missing organization id" do
139+
it "redirects back with flash message" do
140+
post admin_users_path, params: { user: { name: "ABC", email: organization.email } }
141+
142+
expect(response).to render_template("admin/users/new")
143+
expect(flash[:error]).to eq("Failed to create user: Please select an organization for the user.")
144+
end
145+
end
138146
end
139147
end
140148

spec/system/admin/users_system_spec.rb

+6-6
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,15 @@
2828
expect(page).to have_content("Update AAlphabetically First User")
2929

3030
fill_in "user_name", with: "TestUser"
31-
select(organization.name, from: 'user_organization_id')
3231
click_on "Save"
3332

34-
expect(page.find(".alert")).to have_content "TestUser updated"
33+
# Check if redirected to index page with successful flash message
34+
expect(page).to have_current_path(admin_users_path)
35+
expect(page).to have_css(".alert", text: "TestUser updated")
3536

36-
# Check if the organization role has been updated
37-
tbody = find('#filterrific_results table tbody')
38-
first_row = tbody.find('tr', text: 'TestUser')
39-
expect(first_row).to have_text(organization.name)
37+
# Check if user name has changed to TestUser
38+
users_table = find('#filterrific_results table tbody')
39+
expect(users_table).to have_text("TestUser")
4040
end
4141

4242
it 'adds a role' do

0 commit comments

Comments
 (0)