Fixed delegation UI issues

This patch fixes several issues in delegation UI:

When adding a new delegation, only the first attribute selected
was saved. Now all attributes will be saved properly.

When loading the details page, the custom widgets did not store
the original values properly so is_dirty() did not work correctly.
Now this has been fixed except for the memberof attribute because
of these issues:
 - https://fedorahosted.org/freeipa/ticket/869
 - https://fedorahosted.org/freeipa/ticket/870

When saving the details page, the attrs were saved as an array
which was rejected by the server. Now it is stored as comma-
separated list.

6 files changed, 80 insertions, 69 deletions

diff --git a/install/ui/aci.js b/install/ui/aci.js
index 742a52c78..92440dfa5 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -81,35 +81,27 @@ IPA.attribute_table_widget= function (spec){
     };
 
     that.save = function(){
-        var attrs_boxes  =  $('table#'+id+" td :checked");
-        if (!attrs_boxes.length){
-            return [];
-        }
-        var retval = "";
-        for (var i = 0; i < attrs_boxes.length; i += 1){
-            if (i > 0){
-                retval += ',';
-            }
-            retval += attrs_boxes[i].id.substring("aciattr-".length);
+        var values = [];
+
+        var attrs_boxes =  $('table#'+id+" td :checked");
+
+        for (var i = 0; i < attrs_boxes.length; i += 1) {
+            var value = attrs_boxes[i].id.substring("aciattr-".length);
+            values.push(value);
         }
 
-        return [retval];
+        return values;
     };
 
-    var attrs = [];
     that.reset =function(){
         $('input[type=checkbox]', table).attr('checked','');
-        for (var i = 0; i < attrs.length; i+=1){
-            $(attrs[i], table).attr('checked','checked');
+        for (var i = 0; i < that.values.length; i+=1){
+            $('#aciattr-'+that.values[i], table).attr('checked','checked');
         }
     };
 
     that.load = function(record){
-        if (!record.attrs) return;
-        attrs = [];
-        for (var i = 0; i < record.attrs.length; i+=1){
-            attrs.push('#aciattr-' +record.attrs[i]);
-        }
+        that.values = record[that.name] || [];
         that.reset();
     };
 
@@ -122,7 +114,6 @@ IPA.entity_select_widget = function(spec){
     var entity = spec.entity || 'group';
 
     function populate_select(value){
-        var selected = value;
         function find_success(result){
             $('option', that.entity_select).remove();
             var entities = result.result.result;
@@ -132,7 +123,7 @@ IPA.entity_select_widget = function(spec){
                         text:entities[i].cn[0],
                         value:entities[i].cn[0]
                     }));
-                if (selected === entities[i].cn[0]){
+                if (value === entities[i].cn[0]){
                     option.attr('selected','selected');
                 }
             }
@@ -180,14 +171,18 @@ IPA.entity_select_widget = function(spec){
         }).appendTo(dd);
         populate_select();
     };
-    var value = '';
     that.reset = function(){
-        that.entity_filter.val(value );
-        populate_select(value);
+        that.entity_filter.val(that.values[0]);
+        populate_select(that.values[0]);
 
     };
     that.load = function(record){
-        value = record[that.name];
+        var value = record[that.name];
+        if (value instanceof Array) {
+            that.values = value;
+        } else {
+            that.values = value ? [value] : [''];
+        }
         that.reset();
     };
 
@@ -735,13 +730,13 @@ IPA.entity_factories.delegation =  function() {
                         custom_input(IPA.entity_select_widget(
                             {name:'group', entity:'group'})).
                         custom_input(IPA.entity_select_widget(
-                            {name:'memberof', entity:'group'})).
+                            {name:'memberof', entity:'group', join: true})).
                         custom_input(
                             IPA.rights_widget({
                                 id:'delegation_rights'})).
                         custom_input(
                             IPA.attribute_table_widget({
-                                name:'attrs'})))).
+                                name:'attrs', join: true})))).
         add_dialog(IPA.add_dialog({
             name: 'add',
             title: 'Add Delegation'
@@ -750,8 +745,8 @@ IPA.entity_factories.delegation =  function() {
                    field(IPA.entity_select_widget({name:'group',
                                                    entity:'group'})).
                    field(IPA.entity_select_widget({name:'memberof',
-                                                   entity:'group'})).
-                   field(IPA.attribute_table_widget({ name: 'attrs'}))).
+                                                   entity:'group', join: true})).
+                   field(IPA.attribute_table_widget({ name: 'attrs', join: true}))).
         standard_associations();
     return that;
 
diff --git a/install/ui/add.js b/install/ui/add.js
index 89527ba23..33712b45e 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -36,7 +36,8 @@ IPA.add_dialog = function (spec) {
     that.init = function() {
 
         that.add_button('Add', function() {
-            var record = that.get_record();
+            var record = {};
+            that.save(record);
             that.add(
                 record,
                 function() {
@@ -51,7 +52,8 @@ IPA.add_dialog = function (spec) {
 
 
         that.add_button('Add and Add Another', function() {
-            var record = that.get_record();
+            var record = {};
+            that.save(record);
             that.add(
                 record,
                 function() {
@@ -65,7 +67,8 @@ IPA.add_dialog = function (spec) {
         });
 
         that.add_button('Add and Edit', function() {
-            var record = that.get_record();
+            var record = {};
+            that.save(record);
             that.add(
                 record,
                 function() {
@@ -113,6 +116,8 @@ IPA.add_dialog = function (spec) {
             }
         }
 
+        //alert(JSON.stringify(command.to_json()));
+
         command.execute();
     };
 
diff --git a/install/ui/details.js b/install/ui/details.js
index dc3aaa53f..c5006c710 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -781,7 +781,7 @@ IPA.details_update = function (on_win, on_fail)
     var that = this;
     var entity_name = that.entity_name;
 
-    function update_on_win(data, text_status, xhr) {
+    function on_success(data, text_status, xhr) {
         if (on_win)
             on_win(data, text_status, xhr);
         if (data.error)
@@ -791,7 +791,7 @@ IPA.details_update = function (on_win, on_fail)
         that.load(result);
     }
 
-    function update_on_fail(xhr, text_status, error_thrown) {
+    function on_error(xhr, text_status, error_thrown) {
         if (on_fail)
             on_fail(xhr, text_status, error_thrown);
     }
@@ -822,8 +822,12 @@ IPA.details_update = function (on_win, on_fail)
                 if (param_info['primary_key']) continue;
                 if (values.length === 1) {
                     modlist[field.name] = values[0];
-                }else if (values.length > 1){
-                    modlist[field.name] = values;
+                } else if (values.length > 1){
+                    if (field.join) {
+                        modlist[field.name] = values.join(',');
+                    } else {
+                        modlist[field.name] = values;
+                    }
                 } else if (param_info['multivalue']){
                     modlist[field.name] = [];
                 }
@@ -841,14 +845,21 @@ IPA.details_update = function (on_win, on_fail)
         }
     }
 
-    var pkey = that.get_primary_key() ;
-    if (pkey){
-        pkey = [pkey];
-    }else{
-        pkey = [];
-    }
+    var pkey = that.get_primary_key();
+
+    var args = pkey ? [pkey] : [];
+
+    var command = IPA.command({
+        method: entity_name+'_mod',
+        args: args,
+        options: modlist,
+        on_success: on_success,
+        on_error: on_error
+    });
+
+    //alert(JSON.stringify(command.to_json()));
 
-    IPA.cmd('mod', pkey, modlist, update_on_win, null, entity_name);
+    command.execute();
 };
 
 
diff --git a/install/ui/service.js b/install/ui/service.js
index 69b4777ca..f67d755de 100644
--- a/install/ui/service.js
+++ b/install/ui/service.js
@@ -163,8 +163,7 @@ IPA.service_add_dialog = function (spec) {
         field.create(span);
     };
 
-    that.get_record = function() {
-        var record = {};
+    that.save = function(record) {
 
         var field = that.get_field('service');
         var service = field.save()[0];
@@ -175,11 +174,7 @@ IPA.service_add_dialog = function (spec) {
         record['krbprincipalname'] = service+'/'+host;
 
         field = that.get_field('force');
-        var force = field.save()[0];
-
-        record['force'] = force;
-
-        return record;
+        record['force'] = field.save()[0];
     };
 
     return that;
diff --git a/install/ui/test/data/delegation_show.json b/install/ui/test/data/delegation_show.json
index 9444c8750..0b0474d6e 100644
--- a/install/ui/test/data/delegation_show.json
+++ b/install/ui/test/data/delegation_show.json
@@ -8,8 +8,10 @@
                 "cn"
             ],
             "filter": "(memberOf=cn=ipausers,cn=groups,cn=accounts,dc=ayoung,dc=boston,dc=devel,dc=redhat,dc=com)",
-            "group": "muppets",
-            "membergroup": "cn=ipausers,cn=groups,cn=accounts,dc=ayoung,dc=boston,dc=devel,dc=redhat,dc=com",
+            "group": "admins",
+            "membergroup": [
+                "admins"
+            ],
             "permissions": [
                 "write"
             ]
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 9f7c8cae8..3bb5c0f5a 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -38,6 +38,7 @@ IPA.widget = function(spec) {
     that.height = spec.height;
 
     that.undo = typeof spec.undo == 'undefined' ? true : spec.undo;
+    that.join = spec.join;
 
     that.init = spec.init || init;
     that.create = spec.create || create;
@@ -110,6 +111,7 @@ IPA.widget = function(spec) {
 
     function load(record) {
         that.record = record;
+        that.values = record[that.name];
         that.reset();
     }
 
@@ -126,36 +128,39 @@ IPA.widget = function(spec) {
     }
 
     that.is_dirty = function() {
+
         if (that.read_only) {
             return false;
         }
+
         var values = that.save();
 
-        if (!that.values){
+        if (!that.values) {
+
             if (!values) {
                 return false;
             }
-            if ( values === "" ){
-                return false;
-            }
-            if ((values instanceof Array) &&
-                (values.length ===1) &&
-                (values[0] === "")){
-                return false;
-            }
 
-            if ((values instanceof Array) &&
-                (values.length === 0)){
+            if (values === '') {
                 return false;
             }
 
-            if (values) {
-                return true;
+            if (values instanceof Array) {
+
+                if ((values.length === 0) ||
+                    (values.length === 1) &&
+                    (values[0] === '')) {
+                    return false;
+                }
             }
+
+            return true;
         }
+
         if (values.length != that.values.length) {
             return true;
         }
+
         for (var i=0; i<values.length; i++) {
             if (values[i] != that.values[i]) {
                 return true;
@@ -1017,14 +1022,12 @@ IPA.dialog = function(spec) {
         that.container.dialog('option', name, value);
     };
 
-    that.get_record = function() {
-        var record = {};
+    that.save = function(record) {
         for (var i=0; i<that.fields.length; i++) {
             var field = that.fields[i];
             var values = field.save();
-            record[field.name] = values[0];
+            record[field.name] = values.join(',');
         }
-        return record;
     };
 
     that.close = function() {