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

Reused obsolete RBAC method for fetching single record #1636

Merged

Conversation

romanblanco
Copy link
Member

@romanblanco romanblanco commented Jun 30, 2017

Reusing obsolete find_record_with_rbac method to be used in case, when only one record is wanted.

@@ -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])
Copy link
Member Author

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)

Copy link
Contributor

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.

@romanblanco
Copy link
Member Author

@PanSpagetka please review
/cc @martinpovolny @lpichler

@romanblanco
Copy link
Member Author

@miq-bot add_label wip, rbac
@miq-bot assign @PanSpagetka

@@ -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,
Copy link
Member Author

@romanblanco romanblanco Jun 30, 2017

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?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use record.present?

@romanblanco romanblanco force-pushed the rbac_method_find_single_record branch from 1698584 to f362fd5 Compare July 12, 2017 13:06
@romanblanco romanblanco changed the title [WIP] Reused obsolete RBAC method for fetching single record Reused obsolete RBAC method for fetching single record Jul 12, 2017
@miq-bot miq-bot removed the wip label Jul 12, 2017
@miq-bot
Copy link
Member

miq-bot commented Jul 12, 2017

Checked commits romanblanco/manageiq-ui-classic@0f2c3dd~...b35c591 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
9 files checked, 0 offenses detected
Everything looks fine. 🏆

@romanblanco
Copy link
Member Author

@PanSpagetka or @martinpovolny, please review

@martinpovolny martinpovolny merged commit 91d3b9b into ManageIQ:master Jul 13, 2017
@martinpovolny martinpovolny added this to the Sprint 65 Ending Jul 24, 2017 milestone Jul 13, 2017
@romanblanco romanblanco deleted the rbac_method_find_single_record branch July 13, 2017 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants