[Kimchi-devel] [PATCH V4 4/5] Storagepool SCSI/FC: Modifies UI flow to select a LUN to new VM

[Aline Manera]

When I select the text associated to the radio box it does not select 
the option. Need to fix it

More comments below

On 02/05/2014 12:18 PM, Rodrigo Trujillo wrote:
> This patch implements the UI functions and API calls to show to user
> the list of volumes (LUNs) of a SCSI FC storagepools. The user can then
> select the LUN when creating a new VM.
>
> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
> ---
>   ui/js/src/kimchi.api.js            | 13 +++++++
>   ui/js/src/kimchi.guest_add_main.js | 73 +++++++++++++++++++++++++++++++++++---
>   ui/pages/i18n.html.tmpl            |  1 +
>   3 files changed, 83 insertions(+), 4 deletions(-)
>
> diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js
> index 66fc41e..4597c5d 100644
> --- a/ui/js/src/kimchi.api.js
> +++ b/ui/js/src/kimchi.api.js
> @@ -155,6 +155,19 @@ var kimchi = {
>           });
>       },
>
> +    /*
> +    *   Retrieve the information of a storage pool by the given name.
> +    */
> +    retrieveStoragePool : function(storagePoolName, suc, err) {
> +        kimchi.requestJSON({
> +            url : kimchi.url + "storagepools/" +
> +                  encodeURIComponent(storagePoolName),
> +            type : 'GET',
> +            contentType : 'application/json',
> +            dataType : 'json'
> +        }).done(suc);
> +    },
> +
>       /**
>        * Retrieve the information of a template by the given name.
>        */
> diff --git a/ui/js/src/kimchi.guest_add_main.js b/ui/js/src/kimchi.guest_add_main.js
> index 2085562..6b8fc38 100644
> --- a/ui/js/src/kimchi.guest_add_main.js
> +++ b/ui/js/src/kimchi.guest_add_main.js
> @@ -62,9 +62,7 @@ kimchi.guest_add_main = function() {
>           }
>       });
>
> -    var addGuest = function(event) {
> -        var formData = $('#form-vm-add').serializeObject();
> -
> +    var addGuest = function(formData) {
>           kimchi.createVM(formData, function() {
>               kimchi.listVmsAuto();
>               kimchi.window.close();
> @@ -79,8 +77,75 @@ kimchi.guest_add_main = function() {
>           return false;
>       };
>
> +    // This function is used to select a lun for new vm disk if template has
> +    // a SCSI storagepool associated.
> +    function getLun() {
> +        var formData = $('#form-vm-add').serializeObject();
> +        var templateName = formData.template.substring(11);

> +        kimchi.retrieveTemplate(templateName,

> function(templateInfo) {
> +            var poolName = templateInfo.storagepool.substring(14);
> +            kimchi.retrieveStoragePool(poolName, function(poolInfo){
> +                if (poolInfo.type === "scsi") {
> +                    kimchi.listStorageVolumes(poolInfo.name, function(lunsList) {
> +                        if (lunsList.length == 0) {
> +                            kimchi.message.error('There are not volumes for this pool');

You need to add the message to i18n.html.tmpl and use it here, otherwise 
this message will not be translated.

> +                            return false;
> +                        }
> +                        var popUpList = '<section class="form-section">' +
> +                                           '<h2>1. Storage Pool: ' +  poolInfo.name + '</h2>' +
> +                                           '<div class="lun_radios">';

We need to improve the message to the user.

"You select a Template associated to a SCSI Storage Pool so you need to 
select which LUN you wan to use as primary disk to guest."

Or something like it

> +                        $.each(lunsList, function(index, value) {
> +                            popUpList += '<div class="field">' +
> +                                             '<input type="radio" id="lun-' + value.name + '" name="lun" value="' + value.name + '">' +
> +                                             '<label for="lun-' + value.name + '">' + value.name + '</label></div>';
> +                        });
> +                        popUpList += '</div></section>';
> +                        console.log(popUpList)
> +                        var popup = $(popUpList);
> +                        popup.dialog({
> +                            autoOpen : true,
> +                            modal : true,
> +                            width : 400,
> +                            draggable : false,
> +                            resizable : false,
> +                            closeText: "X",
> +                            dialogClass : "network-config",
> +                            title: "Please, select a LUN",
> +                            close: function( event, ui ) { $('input[name=lun]').attr('checked',false); },
> +                            buttons : [ {
> +                                text : i18n.action_select_lun,
> +                                class: "ui-button-primary",
> +                                click : function() {
> +                                    var lunName = $('input:radio[name=lun]:checked').val();
> +                                    if (lunName === undefined) {
> +                                        kimchi.message.error('You must select a LUN');
> +                                    } else {
> +                                        formData.volumes = new Array(lunName);
> +                                        addGuest(formData);
> +                                    }
> +                                    $( this ).dialog( "close" );
> +                                }
> +                            }]
> +                        });

What about split this big function into smaller ones?
It is hard to read it that way

Example:

1) function to get template
2) function to get storage associate to template
3) function to get type of storage pool

get template;
     get storage pool
         get storage pool type

if pool type != scsi:
     return

4) function to display new dialog

> +                    },function() {
> +                        // listStorageVolumes error handler
> +                        kimchi.message.error(i18n['msg.kimchi.list.volume.fail']);
> +                    });
> +                }
> +                else { addGuest(formData); }
> +            }, function() {
> +                // retrieveStoragePool error handler
> +                kimchi.message.error(i18n['msg.kimchi.retrieve.pool.fail']);
> +            });
> +        }, function() {
> +            // retrieveStoragePool error handler
> +            kimchi.message.error(i18n['msg.fail.template.retrieve']);
> +        });
> +        return false;
> +    }
> +
>       $('#form-vm-add').on('submit', addGuest);
> -    $('#vm-doAdd').on('click', addGuest);
> +    $('#vm-doAdd').on('click', getLun);
>
>       showTemplates();
>   };
> diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl
> index a4c3ccb..f635fdf 100644
> --- a/ui/pages/i18n.html.tmpl
> +++ b/ui/pages/i18n.html.tmpl
> @@ -123,6 +123,7 @@ var i18n = {
>       'network_dialog_ok': "$_("OK")",
>       'network_dialog_cancel': "$_("Cancel")",
>       'action_create': "$_("Create")",
> +    'action_select_lun': "$_("Select")",
>       'msg_warning': "$_("Warning")",
>       'msg.logicalpool.confirm.delete': "$_("It will format your disk and you will loose any data in"
>                                         " there, are you sure to continue? ")",