You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/06/23 10:54:18 UTC

[GitHub] [cloudstack-primate] shwstppr commented on a change in pull request #446: Allow creating Compute/Disk offering as Domain admin

shwstppr commented on a change in pull request #446:
URL: https://github.com/apache/cloudstack-primate/pull/446#discussion_r444132985



##########
File path: src/config/section/offering.js
##########
@@ -27,7 +27,6 @@ export default {
       docHelp: 'adminguide/service_offerings.html#compute-and-disk-service-offerings',
       icon: 'cloud',
       permission: ['listServiceOfferings', 'listDomains'],
-      params: { isrecursive: 'true' },

Review comment:
       this is not correct. `isrecursive` must be passed to list offerings for domain and sub domain. `isrecursive=true` is inline with legacy UI
   ![Screenshot from 2020-06-23 16-14-39](https://user-images.githubusercontent.com/153340/85394897-eae20d80-b56c-11ea-8f75-786422f325fa.png)
   

##########
File path: src/config/section/offering.js
##########
@@ -109,7 +108,6 @@ export default {
       icon: 'hdd',
       docHelp: 'adminguide/service_offerings.html#compute-and-disk-service-offerings',
       permission: ['listDiskOfferings', 'listDomains'],
-      params: { isrecursive: 'true' },

Review comment:
       same as above

##########
File path: src/views/offering/AddDiskOffering.vue
##########
@@ -358,6 +358,9 @@ export default {
       }
     },
     isAdmin () {
+      if (!['Admin'].includes(this.$store.getters.userInfo.roletype)) {
+        this.isPublic = false
+      }

Review comment:
       Same as above.
   Instead setting isPublic to false here. `handleSubmit` should be checked for the value and visibility of `a-select` element for domain must be fixed for domain admin

##########
File path: src/views/offering/AddComputeOffering.vue
##########
@@ -606,6 +606,9 @@ export default {
       }
     },
     isAdmin () {
+      if (!['Admin'].includes(this.$store.getters.userInfo.roletype)) {
+        this.isPublic = false
+      }

Review comment:
       `isAdmin` is a getter function and should not be setting variable values.
   Also, this change does not solve root issue, ie, domain selection element is not visible to domain admin in create Compute Offering offering and isPublic is passed true in create API.
   ![Screenshot from 2020-06-23 16-19-04](https://user-images.githubusercontent.com/153340/85395300-88d5d800-b56d-11ea-9fa5-099cda2e94fa.png)
   ![Screenshot from 2020-06-23 16-19-30](https://user-images.githubusercontent.com/153340/85395312-8c695f00-b56d-11ea-9c70-fd4f9a852006.png)
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org