Hi! I tested this patch and here's my overall feedback: - When the button "OK" is clicked and there is some error (e.g. the URL field is empty, the URL text doesn't start with a correct protocol), the button gets disabled and it never gets enabled again unless the user closes and reopens the dialog. - When I click on the button "OK" to upload a file, the dialog doesn't close and nothing is shown. This is not the same behaviour when downloading a file: the dialog is closed immediately as soon as the submit button is clicked. - When a download finishes, the progress box doesn't update the message "Downloading...". That message should disappear and the storage volume box should look like the other existing volumes in that pool. - The correct text when uploading a file should be "Upload a file" (instead of "...an file"). - IMO the submit button's label should be a verb (e.g. "Add") instead of "OK". - IMO the menu item "Add Volume" should be the first element on the menu. The feedback related to the code is below:
- {'fc_host': libvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST, + {#'fc_host': libvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST,
If you're not using that code anymore, remove it instead of commenting it. Git keeps track of the code history.
+.volume-progress .progress-status { +} +
What's this for? It's an empty declaration.
+ var onTaskResponse = function(result) { + var taskStatus = result['status']; + switch(taskStatus) { + case 'running': + onProgress(result); + setTimeout(function() { + trackTask(result['id']); + }, 1500); + break; + case 'finished': + taskID = -1; + var volumeName = result['target_url'].split('/').pop(); + var volumeBox = $('#volume' + kimchi.selectedSP + ' [data-volume-name="' + volumeName + '"]'); + $('.progress-status', volumeBox).text(i18n['KCHPOOL6015M']); + break; + case 'failed': + taskID = -1; + onError(result); + break; + default: + break; + } + }; + + var trackTask = function(task) { + kimchi.getTask(task, onTaskResponse, function(resp) {}); + };
Does this tracking approach work for multiple simultaneous downloads/uploads?
+ + var uploadFile = function() { + var blobFile = $('#volume-input-file')[0].files[0]; + var fileName = blobFile.name; + var fd = new FormData(); + fd.append('name', fileName); + fd.append('file', blobFile); + kimchi.uploadVolumeToSP({ + sp: kimchi.selectedSP, + formData: fd + }, function(resp) { + }); Why isn't an error handler used here as well, just like the download function call?
I tested your patches on top of my [still] local branch with the patch "storagevolume: Use default value for param 'name' when appropriate" which I sent today for review on this mailing list. Without it, the download and upload functions don't work because the parameter 'name' is not passed correctly to the backend. That's why I sent this patch today.