4:39 a.m.
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.