Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove organization from superuser edit/index of users #4453

Merged
merged 13 commits into from
Jan 16, 2025
Merged
27 changes: 9 additions & 18 deletions app/controllers/admin/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class Admin::UsersController < AdminController

def index
@filterrific = initialize_filterrific(
User.includes(:organization).alphabetized,
User.includes(roles: :resource).alphabetized,
params[:filterrific],
available_filters: [:search_name, :search_email]
) || return
Expand Down Expand Up @@ -38,15 +38,16 @@ def edit
end

def create
@user = User.new(user_params)
UserInviteService.invite(name: user_params[:name],
email: user_params[:email],
roles: [Role::ORG_USER],
resource: Organization.find(user_params[:organization_id]))
resource: Organization.find(organization_id_param))
flash[:notice] = "Created a new user!"
redirect_to admin_users_path
rescue => e
flash[:error] = "Failed to create user: #{e}"
render "admin/users/new"
render :new
end

def resource_ids
Expand Down Expand Up @@ -91,25 +92,15 @@ def remove_role
private

def user_params
organization_id = params[:user][:organization_id]

raise "Please select an organization for the user." if organization_id.blank?

user_params = params.require(:user).permit(:name, :organization_id, :email, :password, :password_confirmation)
user_params[:organization_role_join_attributes] = { role_id: updated_role_id(organization_id) }

user_params
rescue => e
redirect_back(fallback_location: edit_admin_user_path, error: e.message)
params.require(:user).permit(:name, :email)
end

def updated_role_id(organization_id)
user_role_title = Role::TITLES[Role::ORG_USER]
user_role_type = Role::ORG_USER
def organization_id_param
organization_id = params[:user][:organization_id]

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

role&.id || raise("Error finding a role within the provided organization")
organization_id
end

def load_organizations
Expand Down
6 changes: 4 additions & 2 deletions app/views/admin/users/_list.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<table class="table">
<thead>
<tr>
<th>Organization</th>
<th>Banks and Partners</th>
<th>Name</th>
<th>Email</th>
<th class="text-right">Actions</th>
Expand All @@ -10,7 +10,9 @@
<tbody>
<% @users.each do |user| %>
<tr>
<td><%= user.organization&.name %></td>
<td>
<%= user.roles.map { |role| role.resource&.name }.compact.uniq.sort.join(', ') %>
</td>
<td><%= user.display_name %></td>
<td><%= user.email %></td>
<td class="text-right">
Expand Down
4 changes: 0 additions & 4 deletions app/views/admin/users/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@
<div class="card-body">
<%= simple_form_for @user, url: admin_user_path, html: {id: "quickForm"} do |f| %>
<div class="form-inputs">
<%= f.input :organization_id, label: "Organization", wrapper: :input_group, required: true, autofocus: true do %>
<span class="input-group-text"><%= fa_icon "building" %></span>
<%= f.input_field :organization_id, collection: @organizations, class: "form-control" %>
<% end %>
<%= render 'admin/users/user_form_fields', f: f %>
</div>
</div>
Expand Down
58 changes: 33 additions & 25 deletions spec/requests/admin/users_requests_spec.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,29 @@
RSpec.describe "Admin::UsersController", type: :request do
let(:organization) { create(:organization) }
let(:user) { create(:user, organization: organization) }
let(:organization) { create(:organization, name: "Org ABC") }
let(:user) { create(:user, organization: organization, name: "User 123") }
let(:organization_admin) { create(:organization_admin, organization: organization) }
let(:super_admin) { create(:super_admin, organization: organization) }

let(:default_params) do
{ organization_name: organization.id }
end

let(:org) { create(:organization, name: 'Org ABC') }
let(:partner) { create(:partner, name: 'Partner XYZ', organization: org) }
let(:user) { create(:user, organization: org, name: 'User 123') }
let(:partner) { create(:partner, name: 'Partner XYZ', organization: organization) }

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

describe "GET #index" do
it "renders index template and shows banks and partners correctly" do
AddRoleService.call(user_id: user.id, resource_type: Role::ORG_ADMIN, resource_id: organization.id)
get admin_users_path

expect(response).to render_template(:index)

page = Nokogiri::HTML(response.body)
banks_and_partners = page.at_xpath("//*[contains(text(), \"#{user.email}\")]/../td[1]").text.strip
expect(banks_and_partners).to eq("Org ABC, Partner XYZ")
end
end

describe "GET #edit" do
it "renders edit template and shows roles" do
get edit_admin_user_path(user)
Expand All @@ -31,24 +37,17 @@
describe "PATCH #update" do
context 'with no errors' do
it "renders index template with a successful update flash message" do
patch admin_user_path(user), params: { user: default_params.merge(organization_id: user.organization.id,
name: 'New User 123', email: '[email protected]') }
patch admin_user_path(user), params: { user: { name: 'New User 123', email: '[email protected]' } }
expect(response).to redirect_to admin_users_path
expect(flash[:notice]).to eq("New User 123 updated!")
end
end

context 'with errors' do
it "redirects back with no organization_id flash message" do
patch admin_user_path(user), params: { user: { name: 'New User 123' } }
it "redirects back with flash message" do
patch admin_user_path(user), params: { user: { name: 'New User 123', email: "invalid_email" } }
expect(response).to redirect_to(edit_admin_user_path)
expect(flash[:error]).to eq('Please select an organization for the user.')
end

it "redirects back with no role found flash message" do
patch admin_user_path(user), params: { user: { name: 'New User 123', organization_id: -1 } }
expect(response).to redirect_to(edit_admin_user_path)
expect(flash[:error]).to eq('Error finding a role within the provided organization')
expect(flash[:error]).to eq("Something didn't work quite right -- try again?")
end
end
end
Expand All @@ -59,11 +58,11 @@
allow(AddRoleService).to receive(:call)
post admin_user_add_role_path(user_id: user.id,
resource_type: Role::ORG_ADMIN,
resource_id: org.id),
resource_id: organization.id),
headers: { 'HTTP_REFERER' => '/back/url'}
expect(AddRoleService).to have_received(:call).with(user_id: user.id.to_s,
resource_type: Role::ORG_ADMIN.to_s,
resource_id: org.id.to_s)
resource_id: organization.id.to_s)
expect(flash[:notice]).to eq('Role added!')
expect(response).to redirect_to('/back/url')
end
Expand All @@ -74,11 +73,11 @@
allow(AddRoleService).to receive(:call).and_raise('OH NOES')
post admin_user_add_role_path(user_id: user.id,
resource_type: Role::ORG_ADMIN,
resource_id: org.id),
resource_id: organization.id),
headers: { 'HTTP_REFERER' => '/back/url'}
expect(AddRoleService).to have_received(:call).with(user_id: user.id.to_s,
resource_type: Role::ORG_ADMIN.to_s,
resource_id: org.id.to_s)
resource_id: organization.id.to_s)
expect(flash[:alert]).to eq('OH NOES')
expect(response).to redirect_to('/back/url')
end
Expand Down Expand Up @@ -135,6 +134,15 @@
post admin_users_path, params: { user: { organization_id: organization.id } }
expect(assigns(:organizations)).to eq(Organization.all.alphabetized)
end

context "with missing organization id" do
it "redirects back with flash message" do
post admin_users_path, params: { user: { name: "ABC", email: organization.email } }

expect(response).to render_template("admin/users/new")
expect(flash[:error]).to eq("Failed to create user: Please select an organization for the user.")
end
end
end
end

Expand Down
12 changes: 6 additions & 6 deletions spec/system/admin/users_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@
expect(page).to have_content("Update AAlphabetically First User")

fill_in "user_name", with: "TestUser"
select(organization.name, from: 'user_organization_id')
click_on "Save"

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

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

it 'adds a role' do
Expand Down
Loading