-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Removal of UI blockage to access the changeOfferingForVolume
API
#10135
base: main
Are you sure you want to change the base?
Removal of UI blockage to access the changeOfferingForVolume
API
#10135
Conversation
changeOfferingForVolume
API
ui/src/config/section/storage.js
Outdated
@@ -223,7 +223,7 @@ export default { | |||
label: 'label.change.offering.for.volume', | |||
args: ['id', 'diskofferingid', 'size', 'miniops', 'maxiops', 'automigrate'], | |||
dataView: true, | |||
show: (record, store) => { return ['Allocated', 'Ready'].includes(record.state) && ['Admin'].includes(store.userInfo.roletype) }, | |||
show: (record, store) => { return ['Allocated', 'Ready'].includes(record.state) && (['Admin'].includes(store.userInfo.roletype) || !!store.apis.changeOfferingForVolume) }, |
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.
Should we remove the roletype check here?
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.
+1
normally the checks use
'listXXX' in store.getters.apis
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.
Hi @shwstppr ,
I really can't think of any scenario where checking for admin makes sense.
I'll make a commit to remove the check.
thanks.
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.
Hi @weizhouapache ,
At first I tried to do it the way you explained, but I didn't succeed. So I left it at that.
@shwstppr a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10135 +/- ##
============================================
- Coverage 16.06% 16.06% -0.01%
Complexity 12872 12872
============================================
Files 5642 5642
Lines 493973 493973
Branches 59895 59895
============================================
- Hits 79351 79350 -1
Misses 405837 405837
- Partials 8785 8786 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
UI build: ✔️ |
Co-authored-by: dahn <[email protected]>
Description
This PR corrects an incorrect behavior in the UI where users who have access to the
changeOfferingForVolume
API, but are not admins, cannot use it.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
A role was created (based on the default user role) with permission to access the
changeOfferingForVolume
API.An account and a user were created using this role.
The
changeOfferingForVolume
API icon appeared correctly in the UI (npm run server
).The change in the volume offer was made correctly