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

[WIP] Standalone ServiceUI product features require, updating affected roles #16329

Conversation

AllenBW
Copy link
Member

@AllenBW AllenBW commented Oct 27, 2017

https://bugzilla.redhat.com/show_bug.cgi?id=1507029

This will require discussion for sure, my understaind of EvmRole-user_self_service was all the sui has to offer (well that was made really easy).

My understanding of EvmRole-user_limited_self_service was everythign except ya cant change the service catalog

@AllenBW
Copy link
Member Author

AllenBW commented Oct 27, 2017

cc @chriskacerguis
@miq-bot add_label bug

@miq-bot miq-bot added the bug label Oct 27, 2017
@chessbyte chessbyte changed the title BZ#1507029-Standalone sui product features require, updating affected roles Standalone ServiceUI product features require, updating affected roles Oct 27, 2017
@AllenBW
Copy link
Member Author

AllenBW commented Oct 27, 2017

woops, working the test failures 😬

@AllenBW
Copy link
Member Author

AllenBW commented Oct 27, 2017

OK soooo I was talking with @imtayadeway and it looks like I have two options to fix the tests... either update https://github.com/ManageIQ/manageiq/blob/master/app/models/miq_user_role.rb#L61-L67
to fetch not by restrictions, or restore the restrictions... @dclarizio @h-kataria thoughts?

The restrictions no longer apply to the sui, cuz those product features have no power (there). But i defer to the pros 🙇‍♀️ 😍 🍭

@AllenBW AllenBW force-pushed the bz/#1507029-update-sui-roles-product-features branch 2 times, most recently from c91e323 to 9bfbbf9 Compare October 30, 2017 13:37
@miq-bot
Copy link
Member

miq-bot commented Oct 30, 2017

This pull request is not mergeable. Please rebase and repush.

@AllenBW AllenBW force-pushed the bz/#1507029-update-sui-roles-product-features branch from 9bfbbf9 to 6f3cbf6 Compare October 30, 2017 13:57
@AllenBW
Copy link
Member Author

AllenBW commented Oct 30, 2017

Ok giving up on what I said 3 days ago, added back restrictions, touching as little code as possible in hopes of getting this in ❤️ 🙇

@imtayadeway
Copy link
Contributor

Adding customary ✂️ 🔥 🚽

@gtanzillo
Copy link
Member

@AllenBW It's not clear if the features that are being removed were never supposed to be allowed for the roles EvmRole-user_limited_self_service and EvmRole-user_self_service. Users of these roles will no longer have those features when they log into the classic UI. Is that correct and ok?

@AllenBW
Copy link
Member Author

AllenBW commented Oct 30, 2017

@gtanzillo I can't comment on if the features removed were ever supposed to be allowed, and good point! It is my understanding that these roles are designed to only have sui access/privileges. If this is true, than removing all features when they log into ops sounds like the right call. Thoughts from the powers that be? @chriskacerguis @ohadlevy ?

@chriskacerguis
Copy link
Contributor

I'm on the same page as @AllenBW, but I don't have the history to comment on your question...BUT, like I said, I agree with Allen's assessment

@gtanzillo
Copy link
Member

Hmm, I know this was around long before SUI. And, I've seen prod DBs with this role in use. So I'm still not sure.

Does this PR need to be merged before the freeze tonight?

@chriskacerguis
Copy link
Contributor

@gtanzillo no, I don't think so

@dclarizio
Copy link

I think we need input from @Loicavenel. Those roles were originally for the self service user in OPS. The Service Catalog area still exists in the OPS UI and some, perhaps many, users/customers still use it for access.

The question for PM is should the existing roles allow both OPS and SUI access to the Services area or should we leave the existing roles as they are and add new OOTB roles for SUI access?

@Loicavenel
Copy link

We should keep these roles and update it with new SUI RBAC part to offer same functionalities..or close to. Then users can connect to OSP UI or SUI with the same Role and get the same experience

SUI product features are no longer embedded in overall product features
@AllenBW AllenBW force-pushed the bz/#1507029-update-sui-roles-product-features branch from 6f3cbf6 to 161acb8 Compare October 31, 2017 18:02
@AllenBW
Copy link
Member Author

AllenBW commented Oct 31, 2017

Updated as per your guidance @Loicavenel

@AllenBW
Copy link
Member Author

AllenBW commented Nov 1, 2017

@miq-bot add_label wip

so additional comments have been made in the bz... https://bugzilla.redhat.com/show_bug.cgi?id=1507029 and ask has been expanded.

For all existing roles that have SUI equivalent functionality, those SUI specific product features should be added to them.

I could use some help on this one 😕 @Loicavenel and anyone else with a firm grasp of roles and product features

@AllenBW AllenBW changed the title Standalone ServiceUI product features require, updating affected roles [WIP] Standalone ServiceUI product features require, updating affected roles Nov 1, 2017
@miq-bot miq-bot added the wip label Nov 1, 2017
Adds sui_services_view for all roles that contain service_view
Adds sui_services for all roles that contain service
Adds sui_vm_console where vm_vnc_console
Adds sui_vm_web_console where cockpit_console
Adds sui_vm_tags where vm_tag
Adds sui_vm_start/stop/suspend where vm_start/stop/suspend
Adds sui_ordres_operations where miq_request_admin
@miq-bot
Copy link
Member

miq-bot commented Nov 6, 2017

Checked commits AllenBW/manageiq@161acb8~...c087477 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@AllenBW
Copy link
Member Author

AllenBW commented Nov 6, 2017

@Loicavenel @chriskacerguis Using this mapping (OPS UI & SUI Roles - Sheet1.pdf) we have mapped sui and opsui product features. I know its tedious to review, but you're input on this and where we go from here is very much so appreciated

@Loicavenel
Copy link

I am good this PDF is my spreadsheet :)

@AllenBW
Copy link
Member Author

AllenBW commented Nov 9, 2017

@chriskacerguis @anyone ? Looking for some thoughts/eyes/guidance on what we are doing here ❓ 👍 cuzzz we all know 🐝 💤 don't 💀 till you 🔪 them 🤔

@AllenBW AllenBW changed the title [WIP] Standalone ServiceUI product features require, updating affected roles Standalone ServiceUI product features require, updating affected roles Nov 9, 2017
@AllenBW
Copy link
Member Author

AllenBW commented Nov 9, 2017

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Nov 9, 2017
@AllenBW
Copy link
Member Author

AllenBW commented Nov 14, 2017

Another 5 days has passed, just popping in to remind anyone that notices we have a high priority bz attached to this:

https://bugzilla.redhat.com/show_bug.cgi?id=1507029
cc @chriskacerguis

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

@AllenBW, looks good 👍 Sorry for the delay getting to this!

@gtanzillo gtanzillo added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 14, 2017
@gtanzillo gtanzillo merged commit e1b3984 into ManageIQ:master Nov 14, 2017
@AllenBW
Copy link
Member Author

AllenBW commented Nov 14, 2017

@gtanzillo No worries!! Apologies if this sounded impatient, know everyone us busy, just wanted to gently keep it on the radar 🍍 💌

thanks for the merge!!

@AllenBW AllenBW deleted the bz/#1507029-update-sui-roles-product-features branch November 14, 2017 16:15
@simaishi
Copy link
Contributor

gaprindashvili/yes ?

@AllenBW
Copy link
Member Author

AllenBW commented Nov 16, 2017

@simaishi yis yis yis yis shoulda added that as tag, target is 5.9 https://bugzilla.redhat.com/show_bug.cgi?id=1507029

simaishi pushed a commit that referenced this pull request Nov 17, 2017
…roduct-features

Standalone ServiceUI product features require, updating affected roles
(cherry picked from commit e1b3984)

https://bugzilla.redhat.com/show_bug.cgi?id=1514189
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 4dcdf3b666d5cf7d4efa5e697626c0a7bf10b4a8
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Tue Nov 14 11:12:40 2017 -0500

    Merge pull request #16329 from AllenBW/bz/#1507029-update-sui-roles-product-features
    
    Standalone ServiceUI product features require, updating affected roles
    (cherry picked from commit e1b3984004e49d540bb43b477f5e0d196fe8eaaa)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1514189

@martinpovolny
Copy link
Member

@AllenBW, @Loicavenel, @gtanzillo : why do we have sui_vm_start and vm_start? Do we really want that? It's the same functionality on a different place so why have 2 different identifiers? It's a potential source of bugs I am afraid.

@miq-bot miq-bot changed the title Standalone ServiceUI product features require, updating affected roles [WIP] Standalone ServiceUI product features require, updating affected roles Dec 2, 2017
@miq-bot miq-bot added wip and removed wip labels Dec 2, 2017
@Loicavenel
Copy link

@martinpovolny you want to be able to Service UI Role and Ops UI Role differently.

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.

10 participants