You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/09/15 14:30:46 UTC

[GitHub] [cloudstack] SadiJr opened a new pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

SadiJr opened a new pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454


   ### Description
   
   1. In UI, with Backup Plugin enabled, a new tab inside Service Offerings, called Backup Offerings, appears. In this tab, users can import a Backup Offering. However, regardless of what the user chooses, the `Allow User Driven Backups` parameter is always send as true
   
   2. In UI, with Quota Plugin enabled, a new tab, called Quota, appears. In this tab there is a component called Tariff, which is responsible for displaying the tariffs of each resource. In addition, in this view there is an edit button that, when clicked, does nothing.
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [x] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   1.
   ![image](https://user-images.githubusercontent.com/31869303/133452664-241492ab-81ca-418a-9426-1e58ee59870a.png)
   
   2. 
   ![image](https://user-images.githubusercontent.com/31869303/133452145-b4bda529-f4be-4429-b828-0e507baca586.png)
   
   
   ### How Has This Been Tested?
   1. I imported a Backup Offering with `Allow User Driven Backups` parameter as `false` and checked, using cloud-monkey and in the DB if this parameter as correctly set.
   2. I edited a Quota Tariff, waited until the effective start day, and check if, in UI, the new value appears.
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] rhtyd commented on pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#issuecomment-920119948


   @blueorangutan ui


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] SadiJr commented on a change in pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
SadiJr commented on a change in pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#discussion_r711245966



##########
File path: ui/src/views/offering/ImportBackupOffering.vue
##########
@@ -167,7 +167,7 @@ export default {
             params[key] = input
           }
         }
-        params.allowuserdrivenbackups = values.allowuserdrivenbackups ? values.allowuserdrivenbackups : true
+        params.allowuserdrivenbackups = values.allowuserdrivenbackups == null ? true : values.allowuserdrivenbackups

Review comment:
       @utchoang:
   Done, and working fine. Thanks for the suggestion. 
   
   @rhtyd:
   Can you test and review the new code? The default behaviour, for my tests, don't changed.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#issuecomment-920126070


   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/5454 (SL-JID-651)


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] utchoang commented on a change in pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
utchoang commented on a change in pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#discussion_r710625835



##########
File path: ui/src/views/offering/ImportBackupOffering.vue
##########
@@ -167,7 +167,7 @@ export default {
             params[key] = input
           }
         }
-        params.allowuserdrivenbackups = values.allowuserdrivenbackups ? values.allowuserdrivenbackups : true
+        params.allowuserdrivenbackups = values.allowuserdrivenbackups == null ? true : values.allowuserdrivenbackups

Review comment:
       @SadiJr You can edit the following instead of your condition:
   1. `v-decorator="['allowuserdrivenbackups', { initialValue: true }]"` in template `<a-form-item>`
   2. `params.allowuserdrivenbackups = values.allowuserdrivenbackups` in `params`




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] SadiJr commented on a change in pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
SadiJr commented on a change in pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#discussion_r711243888



##########
File path: ui/src/views/plugins/quota/EditTariffValueWizard.vue
##########
@@ -112,7 +112,7 @@ export default {
 
         this.loading = true
 
-        api('quotaTariffUpdate', {}, 'POST', params).then(json => {
+        api('quotaTariffUpdate', params, 'GET').then(json => {

Review comment:
       @rhtyd @utchoang 
   
   Done. I tested with POST, and works fine. Thanks for the suggestion. 




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] rhtyd commented on pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#issuecomment-920119865


   @blueorangutan package 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] SadiJr commented on a change in pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
SadiJr commented on a change in pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#discussion_r710109613



##########
File path: ui/src/views/offering/ImportBackupOffering.vue
##########
@@ -167,7 +167,7 @@ export default {
             params[key] = input
           }
         }
-        params.allowuserdrivenbackups = values.allowuserdrivenbackups ? values.allowuserdrivenbackups : true
+        params.allowuserdrivenbackups = values.allowuserdrivenbackups == null ? true : values.allowuserdrivenbackups

Review comment:
       This change was made to fix the reported bug of `Allow User Driven Backups` parameter.
   
   In the old code, when user set this parameter as `false`, this check, as it is a ternary operation, it always ended up sending the parameter as `true`.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] rhtyd merged pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454


   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] DaanHoogland commented on pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#issuecomment-922828402


   @blueorangutan ui


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] rhtyd commented on a change in pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#discussion_r709784226



##########
File path: ui/src/views/offering/ImportBackupOffering.vue
##########
@@ -167,7 +167,7 @@ export default {
             params[key] = input
           }
         }
-        params.allowuserdrivenbackups = values.allowuserdrivenbackups ? values.allowuserdrivenbackups : true
+        params.allowuserdrivenbackups = values.allowuserdrivenbackups == null ? true : values.allowuserdrivenbackups

Review comment:
       This may change the default behaviour, needs checking




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] utchoang commented on a change in pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
utchoang commented on a change in pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#discussion_r710625835



##########
File path: ui/src/views/offering/ImportBackupOffering.vue
##########
@@ -167,7 +167,7 @@ export default {
             params[key] = input
           }
         }
-        params.allowuserdrivenbackups = values.allowuserdrivenbackups ? values.allowuserdrivenbackups : true
+        params.allowuserdrivenbackups = values.allowuserdrivenbackups == null ? true : values.allowuserdrivenbackups

Review comment:
       @SadiJr You can edit the following instead of your condition:
   1. `v-decorator="['allowuserdrivenbackups', { initialValue: true }]"` in template `form-item`
   2. `params.allowuserdrivenbackups = values.allowuserdrivenbackups` in `params`




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#issuecomment-922829642


   @DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#issuecomment-922834631


   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/5454 (SL-JID-670)


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#issuecomment-920120164


   @rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] utchoang commented on a change in pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
utchoang commented on a change in pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#discussion_r710626828



##########
File path: ui/src/views/plugins/quota/EditTariffValueWizard.vue
##########
@@ -112,7 +112,7 @@ export default {
 
         this.loading = true
 
-        api('quotaTariffUpdate', {}, 'POST', params).then(json => {
+        api('quotaTariffUpdate', params, 'GET').then(json => {

Review comment:
       The same idea, the POST method is not supported here?




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] rhtyd commented on a change in pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#discussion_r709784409



##########
File path: ui/src/views/plugins/quota/EditTariffValueWizard.vue
##########
@@ -112,7 +112,7 @@ export default {
 
         this.loading = true
 
-        api('quotaTariffUpdate', {}, 'POST', params).then(json => {
+        api('quotaTariffUpdate', params, 'GET').then(json => {

Review comment:
       Isn't Post desirable? 




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] rhtyd commented on pull request #5454: [UI] Fixes: edit tariff quota and allow user driven backups parameter in Import Backup Offering

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5454:
URL: https://github.com/apache/cloudstack/pull/5454#issuecomment-923706461


   @SadiJr suggestion - you may send another PR to actually display whether an offering allows user driven backups, in the UI when backup offering is listed; the details view doesn't show the information.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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