-
Notifications
You must be signed in to change notification settings - Fork 356
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
Add available product features local storage. #4756
Conversation
I don't really like that we bring down a (deprecated) term (super-admin) from the backend to the frontend. What about just caching the list of features for the given group? This is-super-admin thing is highly specific to the custom button events, but the features list can be useful elsewhere. |
@skateman ok i will store all the permissions for the current group. |
4aade29
to
a04dd50
Compare
@@ -696,6 +706,9 @@ function miqAjaxAuth(url) { | |||
.then(function() { | |||
return vanillaJsAPI.ws_init(); | |||
}) | |||
.then(function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bad indent here
(otherwise lgtm)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you have too many commits here 😉
app/javascript/http_api/api.js
Outdated
@@ -70,6 +70,8 @@ API.ws_destroy = function() { | |||
}; | |||
|
|||
API.logout = function() { | |||
// delete superadmin flag from local storage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
superadmin?
a04dd50
to
faa86fb
Compare
faa86fb
to
724680d
Compare
Checked commit Hyperkid123@724680d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, would merge :) but looks like it depends on ManageIQ/manageiq-api#490, so added pending core instead |
@himdel @martinpovolny ManageIQ/manageiq-api#490 has been merged can you merge this as well? @miq-bot remove_label pending core |
I am wondering if the user can gain something by manipulating the content of the local store. But don't see what that would be. Also I am wondering if all the groups should be stored and only the active one used as opposed to reloading the perms when you switch group in the top right drop-down. But this way, to reload features when playing with some settings it's enough to change group where if we stored all the groups a log in/log out would be needed. So it's probably a good thing as it is in this PR. I am going to merge this one and if what I wrote above makes someone rethink something we can address that in a follow up PR. |
Hammer? |
@martinpovolny can't touch this |
@Hyperkid123 is this being used anywhere? Or is there a follow up PR somewhere? It looks like there's no code touching (noticed in https://github.com/ManageIQ/manageiq-ui-classic/pull/5164/files#r248013681) |
@himdel this was my idea for something that got cancelled/postponed (not sure which) |
@himdel its postponed right now it was done because of custom buttons for user management. I would like to return to it sometimes but i can't say when i will find the time for it (too much stuff right now) |
Based on this https://bugzilla.redhat.com/show_bug.cgi?id=1634053, there is a requirement to show custom buttons events on certain screens. Only super admin should be able to view these events, but the API endpoint (ManageIQ/manageiq-api#464) will always return an array.
If you are not authorized, you will receive an empty array of events and nothing should be rendered on screen. That is the same result if user is authorized, but there are no custom button events. In this case there the ui should render message that no events are present.
To prevent bugs information about
the user identity (if is super admin)user product_features is required.Using this pr ManageIQ/manageiq-api#490 we can get this information and store it to localStorage
features are checked when:
localStorage is reset when: