Skip to content

Commit

Permalink
Simplify Users::RolesController#user_params
Browse files Browse the repository at this point in the history
c.f. this commit [1] where we did the same for `Users::NamesController`.

I found the `UserParameterSanitiser` and associated logic that we
brought from the `UsersController` quite confusing and somewhat overkill
for this controller. This commit removes the use of
`UserParameterSanitiser` but retains the use of `.permitted_user_params`
on the `Role::Base` subclasses. Note that only
`Roles::Superadmin.permitted_user_params` returns an array including
`:role` which is the only parameter we're interested in in this
controller.

I think ideally we'd make use of Pundit's strong parameters
functionality [2] and move this logic out of the `Roles::Base`
subclasses into the relevant policy class.  However, that's a job for
another day!

[1]: 2c8010c
[2]: https://github.com/varvet/pundit/blob/4d8cdf1c10058c12f2c175f30b408f5d5532a00b/README.md#strong-parameters
  • Loading branch information
floehopper committed Nov 22, 2023
1 parent f6810b0 commit 306c27c
Showing 1 changed file with 2 additions and 8 deletions.
10 changes: 2 additions & 8 deletions app/controllers/users/roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,8 @@ def authorize_user
end

def user_params
UserParameterSanitiser.new(
user_params: permitted_user_params,
current_user_role: current_user.role.to_sym,
).sanitise
end

def permitted_user_params
@permitted_user_params ||= params.require(:user).permit(:role).to_h
permitted_user_params = current_user.role_class.permitted_user_params
params.require(:user).permit(*permitted_user_params.intersection([:role]))
end

def redirect_to_account_page_if_acting_on_own_user
Expand Down

0 comments on commit 306c27c

Please sign in to comment.