Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-24 Thread Petr Vobornik

On 08/23/2011 11:09 PM, Endi Sukma Dewata wrote:

On 8/23/2011 6:34 AM, Petr Vobornik wrote:

Please take a look at the attached patch. This should be applied on top
of your patch. It does several things:

1. As mentioned over IRC, we should be treating these partial failure as
a success (the command does return a success). This way it's not going
to show the Retry button.

2. Instead of concatenating the messages, they are now added into the
error list. This way they will appear nicely as a list.

3. If the error dialog appears, it will wait until you click OK before
continuing.

4. Since some of the membership operations are done using serial
associator you might get multiple dialogs, but this should be gone once
we fix #1688.

Please feel free to merge this patch into yours if you want to make
further modifications. Or we can push both patches if you think it's
good enough.

It's good enough for #1628 so we can push both patches.


I can think of some more improvements, but it can be done separately.


I'm not sure if we need to show the completed operations because we
should be able to infer that from the command we're trying to execute
and the error message we're getting. No error means it's completed.

Maybe we should try to provide a better error message, e.g. Some
failures occurred when removing users from group editors.
This isn't probably important. If user is removing some users from group 
he should know that message Some parts of operation failed is related 
to his action.



Also, we
might want to change the 'Operations Error' title because it's actually
a success. How about 'Operation Completed'? This can be done separately.

Agree



If you think showing the completed operations would be useful please
file a ticket and we'll discuss it. We might be able to show the
completed operations under 'Show details'.
It can be useful, but I think it isn't high priority. I filed 
enhancement ticket https://fedorahosted.org/freeipa/ticket/1702 .



Other problem in error dialog is that there are used untranslated
strings. We should modify it to use translated and as fallback (like in
init method) untranslated.


Let's put the locations of any untranslated messages we find into this
ticket: https://fedorahosted.org/freeipa/ticket/1701




--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-24 Thread Endi Sukma Dewata

On 8/24/2011 3:40 AM, Petr Vobornik wrote:

It's good enough for #1628 so we can push both patches.


Pushed to master and ipa-2-1.

--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-23 Thread Petr Vobornik

On 08/22/2011 07:05 PM, Endi Sukma Dewata wrote:

On 8/22/2011 10:06 AM, Petr Vobornik wrote:

'Failed' moved to command. On 'failed' success is transformed to error -
can be change behaviour of serial associator in some commands
(previously some commands were executed even after 'failed' of
previous). It isn't probably big issue because they fail probably from
the same reason (consequent commands would fail to).


This will be addressed in ticket #1688.


- 'failed' message is extended by related object (so user would know for
which command in the batch it is related to).


Just to be consistent, I think we should format the message like this:
primary key: error message.


OK

- there is still the problem ('no modifications to be performed') I
wrote about on Aug 18. I think it should be fixed before commiting this
path.


This will be addressed in ticket #1692.

One more issue, a command could return multiple error messages in each
failure, but right now the get_failed() only reads the first message in
each failure. Try adding several users into a group, but remove some of
them just before adding it, only the first missing user is reported. I
think the code in ipa.js:395-401 should iterate through all messages.


Reworked.


I'm thinking that we should show only notification dialog (like in batch 
error for 'failed' commands. The reason is that part of the command can 
be successfully executed, so offering retry is wrong because it may 
cause other error (try it in the example above). Second reason is that 
if we want to show all errors we have to concatenate error messages with 
some separator (quite easy, current implementation) or to pass array of 
error messages  to error dialog  like in batch error (needs to add 
suppor for it in command).


I'm thinking about dialog with this content:

dialog-titleOperation Error/dialog-title

p
Some parts of operation failed. Completed: $completed.
/p
show-hide-link /
ul
li $key: $message /li
li $key2: $message2 /li
/ul
 ok-button

The 'Completed' part would be shown only if present.

Other problem in error dialog is that there are used untranslated 
strings. We should modify it to use translated and as fallback (like in 
init method) untranslated.


--
Petr Vobornik
From 7baa345b59ef61e6539f745bfea5a701cfcff549 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 adding associations

https://fedorahosted.org/freeipa/ticket/1628
---
 install/ui/ipa.js|  112 +++--
 install/ui/test/association_tests.js |4 +-
 2 files changed, 93 insertions(+), 23 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 9a252f1e50fdaf544cb872fe0dbed9377e791559..c6c8ef2fef9d99642e3695d08811d27587f9fa55 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -331,6 +331,7 @@ IPA.command = function(spec) {
 }
 
 function success_handler(data, text_status, xhr) {
+var failed;
 
 if (!data) {
 // error_handler() calls IPA.hide_activity_icon()
@@ -347,7 +348,18 @@ IPA.command = function(spec) {
 message: data.error.message,
 data: data
 });
+} else if ((failed = that.get_failed(that, data.result, text_status, xhr)) 
+!failed.is_empty()) {
+var message = failed.errors[0].message;
+for(var i = 1; i  failed.errors.length; i++) {
+message += '\n' + failed.errors[i].message;
+}
 
+error_handler.call(this, xhr, text_status,  /* error_thrown */ {
+name: failed.errors[0].name,
+message: message,
+data: data
+});
 } else if (that.on_success) {
 IPA.hide_activity_icon();
 //custom success handling, maintaining AJAX call's context
@@ -379,6 +391,27 @@ IPA.command = function(spec) {
 $.ajax(request);
 };
 
+that.get_failed = function(command, result, text_status, xhr) {
+var errors = IPA.error_list();
+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];
+for(var i = 0; i  member.length; i++) {
+if(member[i].length  1) {
+var name = 'IPA Error';
+var message = member[i][1];
+if(member[i][0])
+message = member[i][0] + ': ' + message;
+errors.add(command, name, message, text_status);
+}
+}
+}
+}
+}
+return 

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-23 Thread Endi Sukma Dewata

On 8/23/2011 6:34 AM, Petr Vobornik wrote:

On 08/22/2011 07:05 PM, Endi Sukma Dewata wrote:

One more issue, a command could return multiple error messages in each
failure, but right now the get_failed() only reads the first message in
each failure. Try adding several users into a group, but remove some of
them just before adding it, only the first missing user is reported. I
think the code in ipa.js:395-401 should iterate through all messages.


Reworked.

I'm thinking that we should show only notification dialog (like in batch
error for 'failed' commands. The reason is that part of the command can
be successfully executed, so offering retry is wrong because it may
cause other error (try it in the example above). Second reason is that
if we want to show all errors we have to concatenate error messages with
some separator (quite easy, current implementation) or to pass array of
error messages to error dialog like in batch error (needs to add suppor
for it in command).


Please take a look at the attached patch. This should be applied on top 
of your patch. It does several things:


1. As mentioned over IRC, we should be treating these partial failure as 
a success (the command does return a success). This way it's not going 
to show the Retry button.


2. Instead of concatenating the messages, they are now added into the 
error list. This way they will appear nicely as a list.


3. If the error dialog appears, it will wait until you click OK before 
continuing.


4. Since some of the membership operations are done using serial 
associator you might get multiple dialogs, but this should be gone once 
we fix #1688.


Please feel free to merge this patch into yours if you want to make 
further modifications. Or we can push both patches if you think it's 
good enough.


I can think of some more improvements, but it can be done separately.


I'm thinking about dialog with this content:

dialog-titleOperation Error/dialog-title

p
Some parts of operation failed. Completed: $completed.
/p
show-hide-link /
ul
li $key: $message /li
li $key2: $message2 /li
/ul
ok-button

The 'Completed' part would be shown only if present.


I'm not sure if we need to show the completed operations because we 
should be able to infer that from the command we're trying to execute 
and the error message we're getting. No error means it's completed.


Maybe we should try to provide a better error message, e.g. Some 
failures occurred when removing users from group editors. Also, we 
might want to change the 'Operations Error' title because it's actually 
a success. How about 'Operation Completed'? This can be done separately.


If you think showing the completed operations would be useful please 
file a ticket and we'll discuss it. We might be able to show the 
completed operations under 'Show details'.



Other problem in error dialog is that there are used untranslated
strings. We should modify it to use translated and as fallback (like in
init method) untranslated.


Let's put the locations of any untranslated messages we find into this 
ticket: https://fedorahosted.org/freeipa/ticket/1701


--
Endi S. Dewata
From fd441b13c995e81301cccd08d410980015853632 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 23 Aug 2011 11:27:41 -0500
Subject: [PATCH] Fixed command partial failure handling.

When a command returns a partial failure it should be treated as a
success but the failures should still be displayed.

Ticket #1628
---
 install/ui/ipa.js |   86 
 1 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index c6c8ef2fef9d99642e3695d08811d27587f9fa55..decf93f34bf012b25fc9cef259f77f39b84f0fc8 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -242,6 +242,9 @@ IPA.command = function(spec) {
 
 that.retry = typeof spec.retry == 'undefined' ? true : spec.retry;
 
+that.error_message = spec.error_message || (IPA.messages.dialogs ?
+IPA.messages.dialogs.batch_error_message : 'Some operations failed.');
+
 that.get_command = function() {
 return (that.entity ? that.entity+'_' : '') + that.method;
 };
@@ -331,7 +334,6 @@ IPA.command = function(spec) {
 }
 
 function success_handler(data, text_status, xhr) {
-var failed;
 
 if (!data) {
 // error_handler() calls IPA.hide_activity_icon()
@@ -348,22 +350,37 @@ IPA.command = function(spec) {
 message: data.error.message,
 data: data
 });
-} else if ((failed = that.get_failed(that, data.result, text_status, xhr)) 
-!failed.is_empty()) {
-var message = failed.errors[0].message;
-for(var i = 1; i  failed.errors.length; i++) {
-message += '\n' + failed.errors[i].message;
-}
 
-error_handler.call(this, xhr, 

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-22 Thread Petr Vobornik

On 08/18/2011 06:25 PM, Endi Sukma Dewata wrote:

On 8/17/2011 10:38 AM, Petr Vobornik wrote:

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.


Currently with serial associator if there's a failure the rest of the
commands will not be executed, so it's an existing problem. You can test
this by adding a user into multiple groups via UI but remove one of the
groups via CLI just before adding. The user will not be added into the
remaining groups. Bulk associator doesn't have this problem because it's
executed as a single command.

I think eventually we want to convert the serial associator to use batch
commands (no need to do it now, but you can if you want). Once it's
converted, all error checking (including result.failed) should be done
in IPA.command so it can be captured by the batch handler.

I'm not sure about other scenarios that will return result.failed, but I
wouldn't assume it's limited to associations. So I think it should be
handled in a more generic way in IPA.command.

One other thing, I think we should avoid using plural class name (i.e.
IPA.errors) because suppose we have a collection of instances of this
class the variable name could become confusing (e.g. that.errorss :) ).
IPA.error_list might be better.



Reworked.

'Failed' moved to command. On 'failed' success is transformed to error - 
can be change behaviour of serial associator in some commands 
(previously some commands were executed even after 'failed' of 
previous). It isn't probably big issue because they fail probably from 
the same reason (consequent commands would fail to).


- 'failed' message is extended by related object (so user would know for 
which command in the batch it is related to).


- there is still the problem ('no modifications to be performed') I 
wrote about on Aug 18. I think it should be fixed before commiting this 
path.


--
Petr Vobornik
From f758ddbcec44c1b2f39d7de84ae216611c03cd43 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 adding associations

https://fedorahosted.org/freeipa/ticket/1628
---
 install/ui/ipa.js|  100 +++---
 install/ui/test/association_tests.js |4 +-
 2 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 9a252f1e50fdaf544cb872fe0dbed9377e791559..eebb9f2f14f542f85c35e47c55d51b54f321de62 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -331,6 +331,7 @@ IPA.command = function(spec) {
 }
 
 function success_handler(data, text_status, xhr) {
+var failed;
 
 if (!data) {
 // error_handler() calls IPA.hide_activity_icon()
@@ -347,7 +348,13 @@ IPA.command = function(spec) {
 message: data.error.message,
 data: data
 });
-
+} else if ((failed = that.get_failed(that, data.result, text_status, xhr)) 
+!failed.is_empty()) {
+error_handler.call(this, xhr, text_status,  /* error_thrown */ {
+name: failed.errors[0].name,
+message: failed.errors[0].message,
+data: data
+});
 } else if (that.on_success) {
 IPA.hide_activity_icon();
 //custom success handling, maintaining AJAX call's context
@@ -379,6 +386,25 @@ IPA.command = function(spec) {
 $.ajax(request);
 };
 
+that.get_failed = function(command, result, text_status, xhr) {
+var errors = IPA.error_list();
+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 = 'IPA Error';
+var message = member[0][1];
+if(member[0][0])
+message += ' 

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-22 Thread Endi Sukma Dewata

On 8/22/2011 10:06 AM, Petr Vobornik wrote:

'Failed' moved to command. On 'failed' success is transformed to error -
can be change behaviour of serial associator in some commands
(previously some commands were executed even after 'failed' of
previous). It isn't probably big issue because they fail probably from
the same reason (consequent commands would fail to).


This will be addressed in ticket #1688.


- 'failed' message is extended by related object (so user would know for
which command in the batch it is related to).


Just to be consistent, I think we should format the message like this: 
primary key: error message.



- there is still the problem ('no modifications to be performed') I
wrote about on Aug 18. I think it should be fixed before commiting this
path.


This will be addressed in ticket #1692.

One more issue, a command could return multiple error messages in each 
failure, but right now the get_failed() only reads the first message in 
each failure. Try adding several users into a group, but remove some of 
them just before adding it, only the first missing user is reported. I 
think the code in ipa.js:395-401 should iterate through all messages.


--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-18 Thread Petr Vobornik

On 08/17/2011 05:38 PM, Petr Vobornik wrote:

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.



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

Modified to work with bulk association.

--
Petr Vobornik
From cd0afe270583ce5b90317d7479412a9547048c94 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 adding associations

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

diff --git a/install/ui/association.js b/install/ui/association.js
index 2c6a1d2003be0668b61b230b8317263b06a822a5..d8722fac36f781563dfc040b9ecdf0d3a80ed646 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -41,9 +41,41 @@ IPA.associator = function (spec) {
 that.on_success = spec.on_success;
 that.on_error = spec.on_error;
 
+that.errors = IPA.errors();
+
 that.execute = function() {
 };
 
+that.check_for_errors = 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.show_errors = 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();
+}
+};
+
 return that;
 };
 
@@ -60,12 +92,14 @@ IPA.serial_associator = function(spec) {
 that.execute = function() {
 
 if (!that.values || !that.values.length) {
+that.show_errors();
 that.on_success();
 return;
 }
 
 var value = that.values.shift();
 if (!value) {
+that.show_errors();
 that.on_success();
 return;
 }
@@ -79,7 +113,10 @@ IPA.serial_associator = function(spec) {
 method: that.method,
 args: args,
 options: options,
-on_success: that.execute,
+on_success: function(data, text_status, xhr) {
+that.check_for_errors(command, data, text_status, xhr);
+that.execute();
+},
 on_error: that.on_error
 });
 
@@ -104,12 +141,14 @@ IPA.bulk_associator = function(spec) {
 that.execute = function() {
 
 if (!that.values || !that.values.length) {
+that.show_errors();
 that.on_success();
 return;
 }
 
 var value = that.values.shift();
 if (!value) {
+that.show_errors();
 that.on_success();
 return;
 }
@@ -127,7 +166,11 @@ IPA.bulk_associator = function(spec) {
 method: that.method,
 args: args,
 options: options,
-on_success: that.on_success,
+on_success: function(data, text_status, xhr) {
+that.check_for_errors(command, data, text_status, xhr);
+that.show_errors();
+that.on_success();
+},
 on_error: 

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-18 Thread Petr Vobornik

On 08/18/2011 10:28 AM, Petr Vobornik wrote:

On 08/17/2011 05:38 PM, Petr Vobornik wrote:

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.



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

Modified to work with bulk association.



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


After implementation error notification in associations. I noticed one 
'bug?' :


After adding users to hbac rule, batch error notification is shown 
saying 'no modifications to be performed'.


Reproduce:
- create hbacrule named 'aa'
- add several users - in example 'admin' and 'ttest'

Request:
{method:batch,params:[[{method:hbacrule_mod,params:[[aa],{all:true,rights:true,usercategory:}]},{method:hbacrule_add_user,params:[[aa],{user:admin,ttest}]}],{}]}

Response:

{
error: null,
id: null,
result: {
count: 2,
results: [
{
error: no modifications to be performed
},
{
completed: 2,
error: null,
failed: {
memberuser: {
group: [],
user: []
}
},
result: {
cn: [
aa
],
dn: 
ipauniqueid=cfb492f2-c8dc-11e0-9504-00163e06af05,cn=hbac,dc=vm-021,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com, 


ipaenabledflag: [
TRUE
],
memberuser_group: [
admins
],
memberuser_user: [
admin,
ttest
]
}
}
]
}
}




I think the problem is that the first command should be included only if 
something changed.


It isn't a bug in this patch, but with it it is a new annoyance (you 
have to click OK).


--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-17 Thread Petr Vobornik

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();