[PATCH] issue#382: Fix wrong default behavior of add guest cdrom

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add function for cdrom and disk form validation, so that cdrom will not be created when no path given. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 28 ++++++++++++++++++++++++++++ ui/pages/i18n.json.tmpl | 7 +++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index 6af00cc..20e81ca 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -127,6 +127,25 @@ kimchi.guest_storage_add_main = function() { }); }); + var validateCDROM = function(settings) { + if (settings['path'] && settings['path'] != '') + return true; + else { + kimchi.message.error.code('KCHVMSTOR0001E'); + return false; + } + } + + var validateDisk = function(settings) { + if (settings['pool'] && settings['vol']) + return true; + else { + kimchi.message.error.code('KCHVMSTOR0002E'); + return false; + } + } + + validator = {cdrom: validateCDROM, disk: validateDisk}; var submitForm = function(event) { if (submitButton.prop('disabled')) { return false; @@ -147,6 +166,15 @@ kimchi.guest_storage_add_main = function() { settings[$(c).attr('name')] = $(c).val(); } }); + // Validate form for cdrom and disk + validateSpecifiedForm = validator[settings['type']]; + if (!validateSpecifiedForm(settings)) { + $(submitButton).prop('disabled', false); + $.each([submitButton, nameTextbox, pathTextbox, poolTextbox, volTextbox], function(i, c) { + $(c).prop('disabled', false); + }); + return false; + } $(submitButton).addClass('loading').text(i18n['KCHVMCD6003M']); kimchi.addVMStorage(settings, function(result) { diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index ce23bc4..477733f 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -162,5 +162,8 @@ "KCHPOOL6009E": "$_("This is not a valid Server Name or IP. please, modify it.")", "KCHPOOL6010M": "$_("Looking for available partitions ...")", "KCHPOOL6011M": "$_("No available partitions found.")", - "KCHPOOL6012M": "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")" -} \ No newline at end of file + "KCHPOOL6012M": "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")", + + "KCHVMSTOR0001E": "$_("CDROM path cannot be blank.")", + "KCHVMSTOR0002E": "$_("Disk pool or volume cannot be blank.")" +} -- 1.8.3.2

Just a minor inline comment below On 06/24/2014 04:52 PM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add function for cdrom and disk form validation, so that cdrom will not be created when no path given.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 28 ++++++++++++++++++++++++++++ ui/pages/i18n.json.tmpl | 7 +++++-- 2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index 6af00cc..20e81ca 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -127,6 +127,25 @@ kimchi.guest_storage_add_main = function() { }); });
+ var validateCDROM = function(settings) { + if (settings['path'] && settings['path'] != '') other path, such as pool dir path are absolute path. so should we keep consistency? + return true; + else { + kimchi.message.error.code('KCHVMSTOR0001E'); + return false; + } + } + + var validateDisk = function(settings) { + if (settings['pool'] && settings['vol']) + return true; + else { + kimchi.message.error.code('KCHVMSTOR0002E'); + return false; + } + } + + validator = {cdrom: validateCDROM, disk: validateDisk}; var submitForm = function(event) { if (submitButton.prop('disabled')) { return false; @@ -147,6 +166,15 @@ kimchi.guest_storage_add_main = function() { settings[$(c).attr('name')] = $(c).val(); } }); + // Validate form for cdrom and disk + validateSpecifiedForm = validator[settings['type']]; + if (!validateSpecifiedForm(settings)) { + $(submitButton).prop('disabled', false); + $.each([submitButton, nameTextbox, pathTextbox, poolTextbox, volTextbox], function(i, c) { + $(c).prop('disabled', false); + }); + return false; + } $(submitButton).addClass('loading').text(i18n['KCHVMCD6003M']);
kimchi.addVMStorage(settings, function(result) { diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index ce23bc4..477733f 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -162,5 +162,8 @@ "KCHPOOL6009E": "$_("This is not a valid Server Name or IP. please, modify it.")", "KCHPOOL6010M": "$_("Looking for available partitions ...")", "KCHPOOL6011M": "$_("No available partitions found.")", - "KCHPOOL6012M": "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")" -} \ No newline at end of file + "KCHPOOL6012M": "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")", + + "KCHVMSTOR0001E": "$_("CDROM path cannot be blank.")", + "KCHVMSTOR0002E": "$_("Disk pool or volume cannot be blank.")" +}
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

On 06/24/2014 11:00 AM, Sheldon wrote:
Just a minor inline comment below
On 06/24/2014 04:52 PM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add function for cdrom and disk form validation, so that cdrom will not be created when no path given.
Question: does the backend already do that? Or we still need to add some logic to it too?
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 28 ++++++++++++++++++++++++++++ ui/pages/i18n.json.tmpl | 7 +++++-- 2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index 6af00cc..20e81ca 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -127,6 +127,25 @@ kimchi.guest_storage_add_main = function() { }); });
+ var validateCDROM = function(settings) { + if (settings['path'] && settings['path'] != '')
other path, such as pool dir path are absolute path. so should we keep consistency?
+ return true; + else { + kimchi.message.error.code('KCHVMSTOR0001E'); + return false; + } + } + + var validateDisk = function(settings) { + if (settings['pool'] && settings['vol']) + return true; + else { + kimchi.message.error.code('KCHVMSTOR0002E'); + return false; + } + } + + validator = {cdrom: validateCDROM, disk: validateDisk}; var submitForm = function(event) { if (submitButton.prop('disabled')) { return false; @@ -147,6 +166,15 @@ kimchi.guest_storage_add_main = function() { settings[$(c).attr('name')] = $(c).val(); } }); + // Validate form for cdrom and disk + validateSpecifiedForm = validator[settings['type']]; + if (!validateSpecifiedForm(settings)) { + $(submitButton).prop('disabled', false); + $.each([submitButton, nameTextbox, pathTextbox, poolTextbox, volTextbox], function(i, c) { + $(c).prop('disabled', false); + }); + return false; + } $(submitButton).addClass('loading').text(i18n['KCHVMCD6003M']);
kimchi.addVMStorage(settings, function(result) { diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index ce23bc4..477733f 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -162,5 +162,8 @@ "KCHPOOL6009E": "$_("This is not a valid Server Name or IP. please, modify it.")", "KCHPOOL6010M": "$_("Looking for available partitions ...")", "KCHPOOL6011M": "$_("No available partitions found.")", - "KCHPOOL6012M": "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")" -} \ No newline at end of file + "KCHPOOL6012M": "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")", + + "KCHVMSTOR0001E": "$_("CDROM path cannot be blank.")", + "KCHVMSTOR0002E": "$_("Disk pool or volume cannot be blank.")" +}

On 2014年06月25日 22:12, Aline Manera wrote:
On 06/24/2014 11:00 AM, Sheldon wrote:
Just a minor inline comment below
On 06/24/2014 04:52 PM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add function for cdrom and disk form validation, so that cdrom will not be created when no path given.
Question: does the backend already do that? Or we still need to add some logic to it too? Backend logic validated it. This is due to UI fills the first volume of default pool when path is blank, so this default behavior is wrong. I will update commit mesg to make it clear.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 28 ++++++++++++++++++++++++++++ ui/pages/i18n.json.tmpl | 7 +++++-- 2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index 6af00cc..20e81ca 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -127,6 +127,25 @@ kimchi.guest_storage_add_main = function() { }); });
+ var validateCDROM = function(settings) { + if (settings['path'] && settings['path'] != '')
other path, such as pool dir path are absolute path. so should we keep consistency? ACK
+ return true; + else { + kimchi.message.error.code('KCHVMSTOR0001E'); + return false; + } + } + + var validateDisk = function(settings) { + if (settings['pool'] && settings['vol']) + return true; + else { + kimchi.message.error.code('KCHVMSTOR0002E'); + return false; + } + } + + validator = {cdrom: validateCDROM, disk: validateDisk}; var submitForm = function(event) { if (submitButton.prop('disabled')) { return false; @@ -147,6 +166,15 @@ kimchi.guest_storage_add_main = function() { settings[$(c).attr('name')] = $(c).val(); } }); + // Validate form for cdrom and disk + validateSpecifiedForm = validator[settings['type']]; + if (!validateSpecifiedForm(settings)) { + $(submitButton).prop('disabled', false); + $.each([submitButton, nameTextbox, pathTextbox, poolTextbox, volTextbox], function(i, c) { + $(c).prop('disabled', false); + }); + return false; + } $(submitButton).addClass('loading').text(i18n['KCHVMCD6003M']);
kimchi.addVMStorage(settings, function(result) { diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index ce23bc4..477733f 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -162,5 +162,8 @@ "KCHPOOL6009E": "$_("This is not a valid Server Name or IP. please, modify it.")", "KCHPOOL6010M": "$_("Looking for available partitions ...")", "KCHPOOL6011M": "$_("No available partitions found.")", - "KCHPOOL6012M": "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")" -} \ No newline at end of file + "KCHPOOL6012M": "$_("This storage pool is not persistent. Instead of deactivate, this action will permanently delete it. Would you like to continue?")", + + "KCHVMSTOR0001E": "$_("CDROM path cannot be blank.")", + "KCHVMSTOR0002E": "$_("Disk pool or volume cannot be blank.")" +}
participants (4)
-
Aline Manera
-
lvroyce@linux.vnet.ibm.com
-
Royce Lv
-
Sheldon