Ticket #1628 - https://fedorahosted.org/freeipa/ticket/1628
Unreported insufficient access error

This patch is dependant on freeipa-pvoborni-0004-1-error-dialog-for-batch-command.patch.

This may be only a checking if approach of this patch is good.

I was not sure if this type of error message (result.failed property) is more general or it only appears in adding members. So I put error handling in serial_associator instead of command. If it would be put in command and success will be transformed to error, it will change the behaviour of executing commands - other commands after error won't be executed. If the approach is good, it could be probably better to change it a little and offer same logic for batch_associator.

It should be working for adding users to groups, netgroups, roles and assigning hbac rules (tested as non admin user).


Modified association test - data in success handler should not be undefined.

--
Petr Vobornik
From 7670faa42492e8850cffd7c54d60f6f6eaa7c51b Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Wed, 17 Aug 2011 15:06:41 +0200
Subject: [PATCH] Show error in serial association

https://fedorahosted.org/freeipa/ticket/1628
---
 install/ui/association.js            |   43 ++++++++++++++++++++++++++++++--
 install/ui/ipa.js                    |   45 +++++++++++++++++++--------------
 install/ui/test/association_tests.js |    2 +-
 3 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index 2c6a1d2003be0668b61b230b8317263b06a822a5..ebc8ab3924633d038a6d21af50c9e8865968aeec 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -57,16 +57,18 @@ IPA.serial_associator = function(spec) {
 
     var that = IPA.associator(spec);
 
+    that.errors = IPA.errors();
+
     that.execute = function() {
 
         if (!that.values || !that.values.length) {
-            that.on_success();
+            that.post_execute();
             return;
         }
 
         var value = that.values.shift();
         if (!value) {
-            that.on_success();
+            that.post_execute();
             return;
         }
 
@@ -79,7 +81,9 @@ IPA.serial_associator = function(spec) {
             method: that.method,
             args: args,
             options: options,
-            on_success: that.execute,
+            on_success: function(data, text_status, xhr) {
+                that.command_success.call(this, command, data, text_status, xhr);
+            },
             on_error: that.on_error
         });
 
@@ -88,6 +92,39 @@ IPA.serial_associator = function(spec) {
         command.execute();
     };
 
+    that.command_success = function(command, data, text_status, xhr) {
+        var result = data.result;
+        if(result && result.failed) {
+            for(var association in result.failed) {
+                for(var member_name in result.failed[association]) {
+                    var member = result.failed[association][member_name];
+                    if(member.length > 0 && member[0].length > 1) {
+                        var name = 'Association Error';
+                        var message = member[0][1];
+                        that.errors.add_error(command, name, message, text_status);
+                    }
+                }
+            }
+        }
+        that.execute();
+    };
+
+    that.post_execute = function () {
+        if(that.errors.errors.length > 0) {
+            var dialog = IPA.error_dialog({
+                error_thrown: {
+                    name: IPA.messages.dialogs.batch_error_title,
+                    message: IPA.messages.dialogs.batch_error_message
+                },
+                errors: that.errors.errors,
+                visible_buttons: ['ok']
+            });
+            dialog.open();
+        }
+
+        that.on_success();
+    };
+
     return that;
 };
 
diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 9a252f1e50fdaf544cb872fe0dbed9377e791559..142a3210a3af16df415c693277135fed1625eeeb 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -417,7 +417,7 @@ IPA.batch_command = function (spec) {
     var that = IPA.command(spec);
 
     that.commands = [];
-    that.errors = [];
+    that.errors = IPA.errors();
     that.error_message = spec.error_message || (IPA.messages.dialogs ?
             IPA.messages.dialogs.batch_error_message : 'Some operations failed.');
     that.show_error = typeof spec.show_error == 'undefined' ?
@@ -434,21 +434,8 @@ IPA.batch_command = function (spec) {
         }
     };
 
-    var clear_errors = function() {
-        that.errors = [];
-    };
-
-    var add_error = function(command, name, message, status) {
-        that.errors.push({
-            command: command,
-            name: name,
-            message: message,
-            status: status
-        });
-    };
-
     that.execute = function() {
-        clear_errors();
+        that.errors.clear_errors();
 
         IPA.command({
             name: that.name,
@@ -470,7 +457,7 @@ IPA.batch_command = function (spec) {
                         name = 'Internal Error '+xhr.status;
                         message = result ? xhr.statusText : "Internal error";
 
-                        add_error(command, name, message, text_status);
+                        that.errors.add_error(command, name, message, text_status);
 
                         if (command.on_error) command.on_error.call(
                             this,
@@ -486,7 +473,7 @@ IPA.batch_command = function (spec) {
                         name = 'IPA Error ' + (result.error.code || '');
                         message = result.error.message || result.error;
 
-                        add_error(command, name, message, text_status);
+                        that.errors.add_error(command, name, message, text_status);
 
                         if (command.on_error) command.on_error.call(
                             this,
@@ -503,7 +490,7 @@ IPA.batch_command = function (spec) {
                     }
                 }
                 //check for partial errors and show error dialog
-                if(that.show_error && that.errors.length > 0) {
+                if(that.show_error && that.errors.errors.length > 0) {
                     var dialog = IPA.error_dialog({
                         xhr: xhr,
                         text_status: text_status,
@@ -513,7 +500,7 @@ IPA.batch_command = function (spec) {
                             message: that.error_message
                         },
                         command: that,
-                        errors: that.errors,
+                        errors: that.errors.errors,
                         visible_buttons: ['ok']
                     });
                     dialog.open();
@@ -785,3 +772,23 @@ IPA.error_dialog = function(spec) {
 
     return that;
 };
+
+IPA.errors = function() {
+    var that = {};
+
+    that.clear_errors = function() {
+        that.errors = [];
+    };
+
+    that.add_error = function(command, name, message, status) {
+        that.errors.push({
+            command: command,
+            name: name,
+            message: message,
+            status: status
+        });
+    };
+
+    that.clear_errors();
+    return that;
+};
diff --git a/install/ui/test/association_tests.js b/install/ui/test/association_tests.js
index 769355ae8bd4356d0243450b9664d33271948d57..6647084ecf20bd326c7dcbb2bada1f1b7bef58a8 100644
--- a/install/ui/test/association_tests.js
+++ b/install/ui/test/association_tests.js
@@ -60,7 +60,7 @@ test("Testing serial_associator().", function() {
                 'Checking IPA.command() parameter: primary key'
             );
 
-            that.on_success();
+            that.on_success({});
         };
 
         return that;
-- 
1.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to