[GitHub] rafaelweingartner commented on a change in pull request #2905: Fix delete domain button in the UI to update only after deleteDomain job finishes

2018-11-12 Thread GitBox
rafaelweingartner commented on a change in pull request #2905: Fix delete 
domain button in the UI to update only after deleteDomain job finishes
URL: https://github.com/apache/cloudstack/pull/2905#discussion_r232628359
 
 

 ##
 File path: ui/scripts/domains.js
 ##
 @@ -69,11 +69,23 @@
 async: false,
 success: function(json) {
 var jid = json.deletedomainresponse.jobid;
+var wasDomainDeletedWithSuccess = false;
+$.ajax({
+url: 
createURL("queryAsyncJobResult=" + jid),
+dataType: "json",
+async: false,
 
 Review comment:
   That is actually what is being used right now. I tried though to use the " 
args.response.success" with a custom function to render the HTML in case of 
success. However, I was not able to find a way to send a function there. The 
only way I have seen people using is the following:
   
   ```
   args.response.success({
   _custom: {
   jobId: jid
   }
   });
   ```
   Which takes only the jobid into account, and does not provide me a callback. 
   
   Would you like me to revert this one then?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2905: Fix delete domain button in the UI to update only after deleteDomain job finishes

2018-10-30 Thread GitBox
rafaelweingartner commented on a change in pull request #2905: Fix delete 
domain button in the UI to update only after deleteDomain job finishes
URL: https://github.com/apache/cloudstack/pull/2905#discussion_r229386718
 
 

 ##
 File path: ui/scripts/domains.js
 ##
 @@ -69,11 +69,23 @@
 async: false,
 success: function(json) {
 var jid = json.deletedomainresponse.jobid;
+var wasDomainDeletedWithSuccess = false;
+$.ajax({
+url: 
createURL("queryAsyncJobResult=" + jid),
+dataType: "json",
+async: false,
 
 Review comment:
   @rhtyd do you think this can be a problem?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2905: Fix delete domain button in the UI to update only after deleteDomain job finishes

2018-10-29 Thread GitBox
rafaelweingartner commented on a change in pull request #2905: Fix delete 
domain button in the UI to update only after deleteDomain job finishes
URL: https://github.com/apache/cloudstack/pull/2905#discussion_r228012074
 
 

 ##
 File path: ui/scripts/domains.js
 ##
 @@ -69,11 +69,23 @@
 async: false,
 success: function(json) {
 var jid = json.deletedomainresponse.jobid;
+var wasDomainDeletedWithSuccess = false;
+$.ajax({
+url: 
createURL("queryAsyncJobResult=" + jid),
+dataType: "json",
+async: false,
 
 Review comment:
   that is a good question. If you take a look, the upper method "deleteDomain" 
(at line 69), it is already an "async=false" call. I could for instance, move 
the code from lines 81-93 to the success function (line 78), but that can cause 
an odd behavior if the user clicks somewhere in the UI before the method 
finishes. That is why I decided to roll with the "standard" and use a "sync" 
call as well.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2905: Fix delete domain button in the UI to update only after deleteDomain job finishes

2018-10-29 Thread GitBox
rafaelweingartner commented on a change in pull request #2905: Fix delete 
domain button in the UI to update only after deleteDomain job finishes
URL: https://github.com/apache/cloudstack/pull/2905#discussion_r228012074
 
 

 ##
 File path: ui/scripts/domains.js
 ##
 @@ -69,11 +69,23 @@
 async: false,
 success: function(json) {
 var jid = json.deletedomainresponse.jobid;
+var wasDomainDeletedWithSuccess = false;
+$.ajax({
+url: 
createURL("queryAsyncJobResult=" + jid),
+dataType: "json",
+async: false,
 
 Review comment:
   that is a good question. If you take a look, the upper method "deleteDomain" 
(at line 69), it is already an "async=false" call. I could for instance, move 
the code from lines 81-93 to the success function (line 78), but that can cause 
an odd behavior if the user clicks somewhere in the UI before the method 
finishing. That is why I decided to roll with the "standard" and use a "sync" 
call as well.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2905: Fix delete domain button in the UI to update only after deleteDomain job finishes

2018-10-24 Thread GitBox
rafaelweingartner commented on a change in pull request #2905: Fix delete 
domain button in the UI to update only after deleteDomain job finishes
URL: https://github.com/apache/cloudstack/pull/2905#discussion_r228012074
 
 

 ##
 File path: ui/scripts/domains.js
 ##
 @@ -69,11 +69,23 @@
 async: false,
 success: function(json) {
 var jid = json.deletedomainresponse.jobid;
+var wasDomainDeletedWithSuccess = false;
+$.ajax({
+url: 
createURL("queryAsyncJobResult=" + jid),
+dataType: "json",
+async: false,
 
 Review comment:
   that is a good question. If you take a look, the upper method "deleteDomain" 
(at line 69), it is already an "async" call. I could for instance, move the 
code from lines 81-93 to the success function (line 78), but that can cause an 
odd behavior if the user clicks somewhere in the UI before the method 
finishing. That is why I decided to roll with the "standard" and use a "sync" 
call as well.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services