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

Fixes to roles_controller update & delete action. Changes need to be reflected in SiteSetting DefaultRole setting. #5951

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/assets/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,8 @@
"signin_required": "You must be signed in to access this page.",
"malware_detected": "Malware Detected! The file you uploaded may contain malware. Please check your file and try again.",
"roles": {
"role_assigned": "This role can't be deleted because it is assigned to at least one user."
"role_assigned": "This role can't be deleted because it is assigned to at least one user.",
"role_set_as_default": "This role can't be deleted because it is set as Default Role in Site Setting."
},
"users": {
"signup_error": "You can't be authenticated. Please contact your administrator.",
Expand Down
17 changes: 16 additions & 1 deletion app/controllers/api/v1/admin/roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,31 @@ def create
# POST /api/v1/:id/roles.json
# Updates a role
def update
old_role_name = @role.name

return render_error errors: @role.errors.to_a, status: :bad_request unless @role.update role_params

# update the 'DefaultRole' site setting value, if it used to be the current role.
default_role_site_setting = SiteSetting.joins(:setting).find_by(provider: current_provider, setting: { name: 'DefaultRole' })

default_role_site_setting.update(value: @role.name) if default_role_site_setting.value == old_role_name

render_data status: :ok
end

# DELETE /api/v1/admin/roles.json
# Deletes a role
def destroy
undeletable_roles = %w[User Administrator Guest]
return render_error errors: @role.errors.to_a, status: :method_not_allowed if undeletable_roles.include?(@role.name)
if undeletable_roles.include?(@role.name) || User.find_by(role_id: @role.id)
return render_error errors: @role.errors.to_a,
status: :method_not_allowed
end

# prevent role from being deleted if its the default role in site setting
default_role_site_setting = SiteSetting.joins(:setting).find_by(provider: current_provider, setting: { name: 'DefaultRole' })

return render_error status: :forbidden if default_role_site_setting.value == @role.name

@role.destroy!

Expand Down
2 changes: 2 additions & 0 deletions app/javascript/hooks/mutations/admin/roles/useDeleteRole.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export default function useDeleteRole({ role, onSettled }) {
onError: (error) => {
if (error.response?.status === 405) {
toast.error(t('toast.error.roles.role_assigned'));
} else if (error.response?.status === 403) {
toast.error(t('toast.error.roles.role_set_as_default'));
} else {
toast.error(t('toast.error.problem_completing_action'));
}
Expand Down
Loading