[OGUI-1892] Disable level buttons for shifter users on page load and after#3438
[OGUI-1892] Disable level buttons for shifter users on page load and after#3438graduta wants to merge 16 commits into
Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
…OGUI-1892-disable-level-buttons-for-shifter-users
…-users' of github.com:AliceO2Group/WebUi into feature/ILG/OGUI-1892-disable-level-buttons-for-shifter-users
| label, | ||
| { | ||
| id: `level-${index}`, | ||
| title: available ? `Filter level ≤ ${index}` : `You don't have access to level ${label}`, |
There was a problem hiding this comment.
I think the tooltip will show Filter level ≤ null for Trace as it doesn't have an index.
| className: model.log.filter.criterias.severity.in.includes(value) ? 'active' : '', | ||
| onclick: (e) => { | ||
| model.log.setCriteria('severity', 'in', value); | ||
| e.target.blur(); // remove focus so user can 'enter' without actually toggle again the button |
There was a problem hiding this comment.
Was e.target.blur() intentionally removed?
| { | ||
| title: 'Reset date, time, matches, excludes, log levels', | ||
| isActive: false, | ||
| onclick: () => logModel.filter.resetCriteria(), |
There was a problem hiding this comment.
Reset filters will allow resetting to a default, more permissive criteria set.
| this.parseLocation(params); | ||
| } | ||
| if (hasShifterButNoAdminRole(this.session.access)) { | ||
| this.log.filter.setCriteria('level', 'max', 1); |
There was a problem hiding this comment.
Could use infologger-level.const.js?
| }, | ||
| ); | ||
|
|
||
| it('should disable buttons for level filter if user is shifter but not admin', async () => { |
There was a problem hiding this comment.
A couple of edge cases should there be a test for checking what happens if user has both admin and shifter roles?
What should happen if user doesn't have shifter or admin roles?
| * @param {string[]} access - array of user roles email groups affiliation | ||
| * @returns {{label: string, index:number}[]} - filter levels allowed for filtering | ||
| */ | ||
| export function getFilterLevelsAllowed(access = []) { |
There was a problem hiding this comment.
I comment here, but the change would be elsewhere The access is not checked server-side? If the user changes it locally they can still query things they aren't allowed to. Are we okay with this level of security?
| * Delegates sub-model actions depending new location of the page | ||
| * If user is shifter but not admin, set Ops as maximum level for filtering | ||
| */ | ||
| handleLocationChange() { |
There was a problem hiding this comment.
Overwriting on each navigation call could be unnecessary and setCriteria could be a better more central place for it to be checked and set to a default.
isaachilly
left a comment
isaachilly
left a comment
There was a problem hiding this comment.
Please see my thoughts and comments.
2 participants
I have JIRA issue created
PR which: