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

Remove openstack infra provider from network actions #3367

Merged
merged 1 commit into from Feb 6, 2018

Conversation

alexander-demicev
Copy link

@alexander-demicev alexander-demicev commented Feb 2, 2018

This PR removes infra provider from filtered result, because actions like creating networks, assigning FIP, etc. do not work for OpenStack infra provider.

Not sure if removing from filtered result should be done here or should be moved inside controllers.
ping @lpichler @aufi
https://github.com/ManageIQ/manageiq-ui-classic/pull/3367/files

@alexander-demicev alexander-demicev changed the title Remove infra provider from filtered result Remove openstack infra provider from network actions Feb 2, 2018
@@ -4,6 +4,7 @@ module ApplicationController::Network
def network_managers
openstack_network_manager = Rbac::Filterer.filtered(ManageIQ::Providers::Openstack::NetworkManager).select(:id, :name, :parent_ems_id)
redhat_network_manager = Rbac::Filterer.filtered(ManageIQ::Providers::Redhat::NetworkManager).select(:id, :name, :parent_ems_id)
openstack_network_manager = openstack_network_manager.select { |manager| EmsCloud.find_by_id(manager.parent_ems_id) }
Copy link
Contributor

@lpichler lpichler Feb 5, 2018

Choose a reason for hiding this comment

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

I am think you can change input query for RBAC of openstack_network_manager (and we will avoid N+1)

ManageIQ::Providers::Openstack::NetworkManager.joins(:parent_manager).where(:parent_managers_ext_management_systems=>{:type=>'ManageIQ::Providers::Openstack::CloudManager'})

please check whether stated type 'ManageIQ::Providers::Openstack::CloudManager' is enough.

Copy link
Author

Choose a reason for hiding this comment

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

Done :) Thanks for review.

@miq-bot
Copy link
Member

miq-bot commented Feb 5, 2018

Checked commit alexander-demicev@58e9f54 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

Copy link
Contributor

@lpichler lpichler left a comment

Choose a reason for hiding this comment

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

thanks also @alexander-demichev !

@mzazrivec mzazrivec self-assigned this Feb 6, 2018
@mzazrivec mzazrivec added this to the Sprint 79 Ending Feb 12, 2018 milestone Feb 6, 2018
@mzazrivec mzazrivec merged commit d2ce22d into ManageIQ:master Feb 6, 2018
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

4 participants