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

Fix load balancers access in API #13866

Merged
merged 4 commits into from Feb 13, 2017

Conversation

imtayadeway
Copy link
Contributor

@imtayadeway imtayadeway commented Feb 10, 2017

Fixes load balancer subcollection member read access.

@miq-bot add-label api, bug
@miq-bot assign @abellotti

@miq-bot
Copy link
Member

miq-bot commented Feb 10, 2017

Checked commits imtayadeway/manageiq@6415748~...91535c5 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🏆

:load_balancers_subcollection_actions:
:get:
- :name: show
:identifier: load_balancer_show
Copy link
Member

Choose a reason for hiding this comment

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

why moving these out of here ? keeping them here gives us finer granularity on sub*/roles based on collection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need that granularity right now - the identifiers are the same and based on load balancers, not the parent collection. If that changes they can always be broken out again - but I saw an opportunity to consolidate and simplify here.

Copy link
Member

Choose a reason for hiding this comment

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

ok, so I take it the portion of the PR fixing the bug was adding the subresource_actions get/read role ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abellotti yep, I added the refactoring as a separate commit last in 91535c5

@abellotti
Copy link
Member

🎶 LGTM!!

@abellotti abellotti merged commit fcdd9d2 into ManageIQ:master Feb 13, 2017
@abellotti abellotti added this to the Sprint 54 Ending Feb 13, 2017 milestone Feb 13, 2017
@imtayadeway imtayadeway deleted the api-load-balancers-access branch February 13, 2017 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants