-
Notifications
You must be signed in to change notification settings - Fork 356
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
Reused obsolete RBAC method for fetching single record #1636
Reused obsolete RBAC method for fetching single record #1636
Conversation
app/controllers/vm_common.rb
Outdated
@@ -539,7 +539,7 @@ def policy_options | |||
|
|||
# Set right_size selected db records | |||
def right_size(record = nil) | |||
@record ||= record ? record : find_records_with_rbac(params[:id]).first | |||
@record ||= record ? record : find_record_with_rbac(params[:id]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PanSpagetka should this be Vm
or VmOrTemplate
, any idea? (56724b1c#diff-5910cce3bec4cd624ba288de86215b4cR542)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@romanblanco There should be Vm
because you can't do Right-size recommendation on Template
.
@PanSpagetka please review |
@miq-bot add_label wip, rbac |
@@ -123,9 +123,7 @@ def find_record_with_rbac(klass, id, options = {}) | |||
# either sets flash or raises exception | |||
# | |||
def find_records_with_rbac(klass, ids, options = {}) | |||
filtered = Rbac.filtered(klass.where(:id => ids), | |||
:user => current_user, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current_user
(nor User.current_user
) is not necessary, as it loaded in https://github.com/ManageIQ/manageiq/blob/master/lib/rbac/filterer.rb#L496 by default
thanks @lpichler
@@ -525,7 +525,7 @@ def date_time_running_msg(typ, timestamp) | |||
|
|||
# Send error message if record is found and authorized, else return the record | |||
def perf_menu_record_valid(model, id) | |||
record = find_records_with_rbac(model.constantize, id) | |||
record = find_record_with_rbac(model.constantize, id) | |||
if record.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use record.present?
1698584
to
f362fd5
Compare
Checked commits romanblanco/manageiq-ui-classic@0f2c3dd~...b35c591 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
@PanSpagetka or @martinpovolny, please review |
Reusing obsolete
find_record_with_rbac
method to be used in case, when only one record is wanted.