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

AutomationManagerController & ProviderForemanController - unify model_to_type_name, fixing toolbars #350

Merged
merged 1 commit into from
Feb 10, 2017

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Feb 10, 2017

Both controllers are insanely similar, but both use a ManagerControllerMixin.

Foreman controller shows configured systems of all kinds, not just Foreman.

But trying to show a detail of any such non-foreman system fails with an exception trying to load the right toolbar.

Moved the relevant functionality to the mixin and merged so that both can find the right toolbar for either kind of system.

Testing:

Go to Configuration > Management - Configured Systems tab
Click on a system of type Ansible Tower

Before:

Error caught: [NameError] uninitialized constant ApplicationHelper::Toolbar::XConfiguredSystemCenter
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:270:in `const_get'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:270:in `block in constantize'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `each'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `inject'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `constantize'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/core_ext/string/inflections.rb:66:in `constantize'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper/toolbar_builder.rb:54:in `predefined_toolbar_class'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper/toolbar_builder.rb:63:in `toolbar_class'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper/toolbar_builder.rb:17:in `build_toolbar'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper/toolbar_builder.rb:6:in `call'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper.rb:381:in `build_toolbar'
/home/himdel/manageiq-ui-classic/app/controllers/mixins/manager_controller_mixin.rb:395:in `rebuild_toolbars'
/home/himdel/manageiq-ui-classic/app/controllers/mixins/manager_controller_mixin.rb:213:in `replace_right_cell'
/home/himdel/manageiq-ui-classic/app/controllers/mixins/manager_controller_mixin.rb:178:in `tree_select'
/home/himdel/manageiq-ui-classic/app/controllers/application_controller/explorer.rb:193:in `block (2 levels) in generic_x_show'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/actionpack-5.0.1/lib/action_controller/metal/mime_responds.rb:201:in `respond_to'
/home/himdel/manageiq-ui-classic/app/controllers/application_controller/explorer.rb:186:in `generic_x_show'
/home/himdel/manageiq-ui-classic/app/controllers/provider_foreman_controller.rb:194:in `x_show'

After:
it works :)

…_to_type_name, fixing toolbars

Both controllers are insanely similar, but both use a ManagerControllerMixin.

Foreman controller shows configured systems of all kinds, not just Foreman.

But trying to show a detail of any such non-foreman system fails with an exception trying to load the right toolbar.

Moved the relevant functionality to the mixin and merged so that both can find the right toolbar for either kind of system.

Testing:

Go to Configuration > Management - Configured Systems tab
Click on a system of type Ansible Tower

Before:

```
Error caught: [NameError] uninitialized constant ApplicationHelper::Toolbar::XConfiguredSystemCenter
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:270:in `const_get'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:270:in `block in constantize'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `each'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `inject'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `constantize'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/core_ext/string/inflections.rb:66:in `constantize'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper/toolbar_builder.rb:54:in `predefined_toolbar_class'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper/toolbar_builder.rb:63:in `toolbar_class'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper/toolbar_builder.rb:17:in `build_toolbar'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper/toolbar_builder.rb:6:in `call'
/home/himdel/manageiq-ui-classic/app/helpers/application_helper.rb:381:in `build_toolbar'
/home/himdel/manageiq-ui-classic/app/controllers/mixins/manager_controller_mixin.rb:395:in `rebuild_toolbars'
/home/himdel/manageiq-ui-classic/app/controllers/mixins/manager_controller_mixin.rb:213:in `replace_right_cell'
/home/himdel/manageiq-ui-classic/app/controllers/mixins/manager_controller_mixin.rb:178:in `tree_select'
/home/himdel/manageiq-ui-classic/app/controllers/application_controller/explorer.rb:193:in `block (2 levels) in generic_x_show'
/home/himdel/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/actionpack-5.0.1/lib/action_controller/metal/mime_responds.rb:201:in `respond_to'
/home/himdel/manageiq-ui-classic/app/controllers/application_controller/explorer.rb:186:in `generic_x_show'
/home/himdel/manageiq-ui-classic/app/controllers/provider_foreman_controller.rb:194:in `x_show'
```

After:
it works :)
@himdel himdel added the bug label Feb 10, 2017
@himdel
Copy link
Contributor Author

himdel commented Feb 10, 2017

Cc @lgalis

@miq-bot
Copy link
Member

miq-bot commented Feb 10, 2017

Checked commit https://github.com/himdel/manageiq-ui-classic/commit/657276c6775e753adc2c6f82c43896d0a37bd78c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks good. 🍪

@martinpovolny martinpovolny merged commit f3d208b into ManageIQ:master Feb 10, 2017
@martinpovolny martinpovolny added this to the Sprint 54 Ending Feb 13, 2017 milestone Feb 10, 2017
@lgalis
Copy link
Contributor

lgalis commented Feb 10, 2017

@himdel - The foreman provider controller should not display Ansible Configured Systems. I will post a PR shortly to only display Foreman Configured Systems.

@himdel himdel deleted the foreman-ansible-toolbar branch February 10, 2017 19:16
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