Distinguishing of external values in association tables

Problem:
Rule association widget was displaying standard records with external records in one table. User couldn't distinguish the values. When clicking on the external record link it navigated to appropriate page for that entity. But for external value there is no record to show so it displayed error.

Solution:
* For tables with possible external values a 'external' column was added. It displays "True" if the value is external and nothing if not. Displaying nothing is intentional. If user sees some text in external column he imidiately knows that the record is external without even reading the "True" text.
* Rows with external values don't have a link for navigating to record page. This prevents showing the error as no record exists.

Additional changes:
* Association table widget was stripped of get_records method. Loading records isn't its resposibility it's a resposibility of field.
* Column was extended by possible suppressing of link creation. It's done by optional suppress_link argument in setup method.
* To allow setting suppress_link attribute in inherited tables a new overridable method was created - setup_column.

Posible future improvements:
* Table is using dynamic setting of width for columns. Each column has the same width. For flag columns such as 'external' the width of the column is too big. It would be better to be able to set smaller fixed width and the rest of the columns width (without the width set) would be computed (to fit the table).
* When a table has displayed buttons in its last column header the cells in column header have different vertical alignmnent. It should be united.

https://fedorahosted.org/freeipa/ticket/1993

3 files changed, 75 insertions, 54 deletions

diff --git a/install/ui/association.js b/install/ui/association.js
index 708b23723..553e2bce1 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -322,10 +322,7 @@ IPA.association_table_widget = function (spec) {
         return column;
     };
 
-    that.create = function(container) {
-
-        var column;
-
+    that.create_columns = function() {
         // create a column if none defined
         if (!that.columns.length) {
             that.create_column({
@@ -336,7 +333,10 @@ IPA.association_table_widget = function (spec) {
                 link: true
             });
         }
+    };
 
+    that.init_columns = function() {
+        var column;
         var columns = that.columns.values;
         for (var i=0; i<columns.length; i++) {
             column = columns[i];
@@ -349,12 +349,27 @@ IPA.association_table_widget = function (spec) {
                 };
             }
         }
+    };
 
+    that.init_adder_columns = function() {
+        var column;
         var adder_columns = that.adder_columns.values;
         for (var j=0; j<adder_columns.length; j++) {
             column = adder_columns[j];
             column.entity_name = that.other_entity;
         }
+    };
+
+    that.init = function() {
+
+        that.create_columns();
+        that.init_columns();
+        that.init_adder_columns();
+    };
+
+    that.create = function(container) {
+
+        that.init();
 
         that.table_create(container);
 
@@ -450,40 +465,6 @@ IPA.association_table_widget = function (spec) {
         }
     };
 
-    that.get_records = function(on_success, on_error) {
-
-        var length = that.values.length;
-        if (!length) return;
-
-        if (length > 100) {
-            length = 100;
-        }
-
-        var batch = IPA.batch_command({
-            name: that.entity.name+'_'+that.name,
-            on_success: on_success,
-            on_error: on_error
-        });
-
-        for (var i=0; i<length; i++) {
-            var value = that.values[i];
-
-            var command = IPA.command({
-                entity: that.other_entity,
-                method: 'show',
-                args: [value],
-                options: {
-                    all: true,
-                    rights: true
-                }
-            });
-
-            batch.add_command(command);
-        }
-
-        batch.execute();
-    };
-
     that.load = function(result) {
         that.values = result[that.name] || [];
         that.update();
@@ -492,29 +473,23 @@ IPA.association_table_widget = function (spec) {
 
     that.update = function(values) {
 
-        if(values) that.values = values;
+        if (values) that.values = values;
 
         that.empty();
 
+        var i;
         var columns = that.columns.values;
         if (columns.length == 1) { // show pkey only
             var name = columns[0].name;
-            for (var i=0; i<that.values.length; i++) {
+            for (i=0; i<that.values.length; i++) {
                 var record = {};
                 record[name] = that.values[i];
                 that.add_record(record);
             }
-
-        } else { // get and show additional fields
-            that.get_records(
-                function(data, text_status, xhr) {
-                    var results = data.result.results;
-                    for (var i=0; i<results.length; i++) {
-                        var record = results[i].result;
-                        that.add_record(record);
-                    }
-                }
-            );
+        } else {
+            for (i=0; i<that.values.length; i++) {
+                that.add_record(that.values[i]);
+            }
         }
     };
 
@@ -666,6 +641,7 @@ IPA.association_table_widget = function (spec) {
     }
 
     // methods that should be invoked by subclasses
+    that.association_table_widget_create_columns = that.create_columns;
     that.association_table_widget_show_add_dialog = that.show_add_dialog;
     that.association_table_widget_show_remove_dialog = that.show_remove_dialog;
 
diff --git a/install/ui/rule.js b/install/ui/rule.js
index d36cd3d3b..3b1308bc9 100644
--- a/install/ui/rule.js
+++ b/install/ui/rule.js
@@ -105,6 +105,29 @@ IPA.rule_association_table_widget = function(spec) {
 
     that.enabled = spec.enabled !== undefined ? spec.enabled : true;
 
+    that.setup_column = function(column, div, record) {
+        var suppress_link = false;
+        if (that.external) {
+            suppress_link = record[that.external] === 'true';
+        }
+        column.setup(div, record, suppress_link);
+    };
+
+    that.create_columns = function() {
+
+        if (!that.columns.length) {
+            that.association_table_widget_create_columns();
+            if (that.external) {
+                that.create_column({
+                    name: that.external,
+                    label: IPA.messages.objects.sudorule.external,
+                    entity_name: that.other_entity,
+                    format: IPA.boolean_format
+                });
+            }
+        }
+    };
+
     that.create_add_dialog = function() {
 
         var entity_label = that.entity.metadata.label_singular;
@@ -138,12 +161,30 @@ IPA.rule_association_table_field = function(spec) {
 
     that.external = spec.external;
 
+    that.set_values_external = function(values, external) {
+
+        for (var i=0; i<values.length; i++) {
+
+            var record = values[i];
+
+            if (typeof record !== 'object') {
+                record = {};
+                record[that.name] = values[i];
+            }
+
+            record[that.external] = external;
+
+            values[i] = record;
+        }
+    };
 
     that.load = function(result) {
         that.values = result[that.name] || [];
 
         if (that.external) {
+            that.set_values_external(that.values, '');
             var external_values = result[that.external] || [];
+            that.set_values_external(external_values, 'true');
             $.merge(that.values, external_values);
         }
 
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 69fe704b8..ab39b24e0 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -948,7 +948,7 @@ IPA.column = function (spec) {
         throw except;
     }
 
-    that.setup = function(container, record) {
+    that.setup = function(container, record, suppress_link) {
 
         container.empty();
 
@@ -958,7 +958,7 @@ IPA.column = function (spec) {
         }
         value = value ? value.toString() : '';
 
-        if (that.link) {
+        if (that.link && !suppress_link) {
             $('<a/>', {
                 href: '#'+value,
                 text: value,
@@ -1378,10 +1378,14 @@ IPA.table_widget = function (spec) {
 
             var div = $('div[name="'+column.name+'"]', tr);
 
-            column.setup(div, record);
+            that.setup_column(column, div, record);
         }
     };
 
+    that.setup_column = function(column, div, record) {
+        column.setup(div, record);
+    };
+
     that.add_rows = function(rows) {
         for (var i=0; i<rows.length; i++) {
             var tr = rows[i];