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

Feature changes to support moving Configured Systems out of Ansible Tower Explorer #21076

Merged

Conversation

h-kataria
Copy link
Contributor

@h-kataria h-kataria commented Feb 23, 2021

…Tower Explorer

  • Added new alias to get Automation Manager Configured Systems

UI PR
Cross repo PR

This PR does not need data migration as features were not renamed, features are only moved to be on folder level and 2 new features for show_list and show are added in this PR

@h-kataria h-kataria force-pushed the configured_system_features_changes branch from 7d37363 to 4e57e2d Compare February 24, 2021 19:55
@miq-bot
Copy link
Member

miq-bot commented Feb 24, 2021

Checked commits h-kataria/manageiq@cf33f61~...4e57e2d with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@@ -0,0 +1 @@
::AutomationManagerConfiguredSystem = ::ManageIQ::Providers::AnsibleTower::AutomationManager::ConfiguredSystem
Copy link
Member

Choose a reason for hiding this comment

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

Note the hardcoding of AnsibleTower here for the future.

Copy link
Member

@Fryguy Fryguy left a comment

Choose a reason for hiding this comment

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

LGTM, but want another set of eyes. I think automation_manager_configured_system_show_list and automation_manager_configured_system_show are net-new

@h-kataria
Copy link
Contributor Author

LGTM, but want another set of eyes. I think automation_manager_configured_system_show_list and automation_manager_configured_system_show are net-new

@Fryguy automation_manager_configured_system_show_list and automation_manager_configured_system_show are new features added under an existing feature automation_manager_configured_system_view to give more granular access to list and summary screens. If a user already had an access to the parent feature automation_manager_configured_system_view they will be good and this does not require any data migration.

@gtanzillo gtanzillo merged commit f13fcc8 into ManageIQ:master Mar 1, 2021
jrafanie added a commit to jrafanie/manageiq that referenced this pull request May 5, 2021
These were added/edited in ManageIQ#21108 and ManageIQ#21076

All other MiqProductFeature has the feature_type field:
irb(main):004:0> MiqProductFeature.where(:feature_type => nil).count
   (0.7ms)  SELECT COUNT(*) FROM "miq_product_features" WHERE "miq_product_features"."feature_type" IS NULL
=> 2

Because this was missing, the sort_by in MiqProductFeature.sort_children was
failing with the error below when you try to login as a non-admin user:

```
[----] F, [2021-05-05T12:03:59.696562 ManageIQ#8905:3fcc63630bcc] FATAL -- : Error caught: [ActionView::Template::Error] comparison of Array with Array failed
/Users/joerafaniello/Code/manageiq/app/models/miq_product_feature.rb:145:in `sort_by'
/Users/joerafaniello/Code/manageiq/app/models/miq_product_feature.rb:145:in `sort_children'
/Users/joerafaniello/Code/manageiq/app/models/miq_product_feature.rb:76:in `feature_children'
/Users/joerafaniello/Code/manageiq/app/models/miq_user_role.rb:55:in `block in allows_any?'
/Users/joerafaniello/Code/manageiq/app/models/miq_user_role.rb:55:in `map'
/Users/joerafaniello/Code/manageiq/app/models/miq_user_role.rb:55:in `allows_any?'
/Users/joerafaniello/Code/manageiq/app/models/miq_user_role.rb:57:in `allows_any?'
/Users/joerafaniello/Code/manageiq/lib/rbac/authorizer.rb:62:in `user_role_allows_any?'
/Users/joerafaniello/Code/manageiq/lib/rbac/authorizer.rb:28:in `role_allows?'
/Users/joerafaniello/Code/manageiq/lib/rbac/authorizer.rb:6:in `role_allows?'
/Users/joerafaniello/Code/manageiq/lib/rbac.rb:29:in `role_allows?'
/Users/joerafaniello/Code/manageiq/app/models/user.rb:212:in `role_allows_any?'
/Users/joerafaniello/Code/manageiq-ui-classic/app/presenters/menu/section.rb:38:in `visible?'
```

Admin users bypass some of the feature checks so was able to login without
issue.
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.

4 participants