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

Join object and block storage under ems_storage in the features tree #17512

Merged
merged 1 commit into from
Jun 5, 2018

Conversation

skateman
Copy link
Member

@skateman skateman commented Jun 4, 2018

I'm trying to refactor something in the UI. For that I would like to have a way to determine if a feature related to a given model is available for a user. However, there are some inconsistencies between the features and models of storage providers.

There are three feature subtrees for storage providers:

  • ems_storage
  • ems_object_storage
  • ems_block_storage

When displaying a list of storage manager, the system is using the ems_{block,object}_storage route and I'm assuming also the feature with the same name from the RBAC tree. However, when opening a provider's summary screen, the route changes to ems_storage. This, together with the lack of separate object/block storage provider model classes is causing a lot of struggle with decorators/quadicons.

To solve this problem, we had this idea with @PanSpagetka to join the two specific features under the generic one. This would solve us the problem of sub-features when deciding if we want to expose the quadicon settings to a given user for the storage providers.

I was clicking in the UI a little and it seems working, also tested our case by creating a user with access only to ems_object_storage_view:

Rbac.role_allows?(:user => u, :feature => :ems_storage, :any => true) # => true
Rbac.role_allows?(:user => u, :feature => :ems_block_storage, :any => true) # => true
Rbac.role_allows?(:user => u, :feature => :ems_block_storage_view, :any => true) # => true
Rbac.role_allows?(:user => u, :feature => :ems_object_storage, :any => true) # => false
Rbac.role_allows?(:user => u, :feature => :ems_object_storage_view, :any => true) # => false

However, I think this is too good to be true and I am afraid I don't see the whole picture and/or I'm breaking something 😕

@miq-bot
Copy link
Member

miq-bot commented Jun 4, 2018

Checked commit skateman@3b671a1 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

Change makes sense

Don't think this will effect anything other than a change in the product feature tree in settings config

@kbrock kbrock self-assigned this Jun 5, 2018
@kbrock kbrock merged commit eebcb1a into ManageIQ:master Jun 5, 2018
@skateman skateman deleted the rbac-storage branch June 5, 2018 17:47
@agrare agrare added this to the Sprint 88 Ending Jun 18, 2018 milestone Jun 15, 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.

5 participants