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

Automation providers features changes to support De-explorization of Ansible Tower Explorer #21108

Merged
merged 4 commits into from
Apr 9, 2021

Conversation

h-kataria
Copy link
Contributor

@h-kataria h-kataria commented Mar 16, 2021

  • This PR has feature renaming of Automation Providers related features
  • Updates startpage URL for Ansible Tower explorer to point to the new location in second level menus
  • Updates OOTB user roles that by default had access to Ansible Tower explorer to have access to all three new sub menus.
  • Also includes changes to display count of Inventory Groups/Folders in Configured Systems list view as well as name of folder on the details view screen.

for ManageIQ/manageiq-ui-classic#6819

Data Migration PR
UI PR
Cross Repo PR

- Added an alias `ems_automation` to be consistent with Configuration Provider and features/controller/view naming.
- Added methods to show count of inventory groups on list view and inventory group name on summary details view.
@h-kataria h-kataria force-pushed the automation_providers_features branch from 457f3f2 to 7ea9c2a Compare March 16, 2021 18:51
- Updated startpage shortcut entry.
- Updated OOTB user roles to access to 3 product features after the Ansible Tower explorer split into 3 individual screens.
@h-kataria h-kataria force-pushed the automation_providers_features branch from 7ea9c2a to c0fdd25 Compare March 17, 2021 02:04
@@ -1,2 +1,7 @@
class ManageIQ::Providers::AutomationManager::ConfiguredSystem < ::ConfiguredSystem
virtual_column :inventory_root_group_name, :type => :string
Copy link
Member

Choose a reason for hiding this comment

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

Assuming inventory_root_group is a relationship, can you add a :uses => :inventory_root_group clause here for performance reasons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Fryguy please re-review

@h-kataria h-kataria assigned Fryguy and unassigned gtanzillo Apr 2, 2021
@h-kataria h-kataria requested review from Fryguy and removed request for gtanzillo April 2, 2021 15:00
@kavyanekkalapu
Copy link
Member

@Fryguy can you review this pr?

@Fryguy Fryguy merged commit 14682ff into ManageIQ:master Apr 9, 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.
jrafanie added a commit to jrafanie/manageiq-schema that referenced this pull request May 25, 2021
The product feature was renamed along with the other de-explorization of ansible
tower in: ManageIQ/manageiq#21108

The database migration in ManageIQ#564
handled most of the renames but one feature was missed.

This commit handles automation_manager_add_provider => ems_automation_add_provider.
jrafanie added a commit to jrafanie/manageiq-schema that referenced this pull request May 25, 2021
The product feature was renamed along with the others for de-explorization of ansible
tower in: ManageIQ/manageiq#21108

The database migration in ManageIQ#564
handled most of the renames but one feature was missed.

This commit handles automation_manager_add_provider => ems_automation_add_provider.
jrafanie added a commit to jrafanie/manageiq-schema that referenced this pull request Jun 8, 2021
The product feature was renamed along with the others for de-explorization of ansible
tower in: ManageIQ/manageiq#21108

The database migration in ManageIQ#564
handled most of the renames but one feature was missed.

This commit handles automation_manager_add_provider => ems_automation_add_provider.
jrafanie added a commit to jrafanie/manageiq-api that referenced this pull request Jul 12, 2021
ManageIQ#1033 was fine on master but relied
on a refactoring/rewording of product features and models in ManageIQ/manageiq#21108
and this refactoring did not get backported to lasker.

In other words, ems_automation_add_provider doesn't exist in lasker, but
automation_manager_add_provider does, so we'll use that.
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