[Freeipa-devel] [PATCH] 004 error dialog for batch command

[Endi Sukma Dewata]

On 8/11/2011 9:01 AM, Petr Vobornik wrote:
> [PATCH] error dialog for batch command
>
> https://fedorahosted.org/freeipa/ticket/1597
> https://fedorahosted.org/freeipa/ticket/1592
>
> Added option to show multiple errors in error dialog.
>
> Notes:
> - also covering '[ipa webui] Does not return appropriate error when
> deleting an external host but checking update dns' (1592)
> - added support(element's classes) for css styling of aggregated errors
> - except search dialog delete (1592) - no other batch command uses this
> feature (has to be explicitly turned on).

Some issues:

1. I think by default all batch commands should use this feature. The 
batch command is used for various purposes, not just for deletion. 
Consider this scenario:

First, find a way to log in simultaneously using different accounts. You 
can use either multiple machines, accounts, or browsers, whichever is 
the easiest.

In the first session, log in as admin, create a test user, add it into 
the admins group.

Then in the second session, login as the test user, then edit a sudo 
rule. Modify the description and the enabled flag (this will be executed 
as separate operations in a single batch). Don't click Update yet.

Back to the first session, remove the test user from the admins group. 
Then go back to the second session, click Update.

Since the test user doesn't have admin rights anymore the operations 
will fail. However, currently these failures are not reported and the 
values simply revert back to the original. The error dialog should show 
these errors.

So in this case we don't really need the 'partial_success_notify' flag, 
or it can be renamed into 'show_error' which should be true by default. 
The 'retry' flag in IPA.command can be renamed to 'show_error' too.

2. The 'partial_success_message' probably can be renamed into 
'error_message' which will say something like 'Some operations failed.'

3. Instead of a checkbox for show_errors_checkbox, it might be better to 
use 'Show details' and 'Hide details' links.

4. In ipa.js:510 instead of repeating the error message the 
error_thrown.name could say something like 'Batch Error' or 'Operations 
Error'.

5. The add_error() could be moved into IPA.error_dialog so the 
IPA.batch_command doesn't need to hold the 'errors' list.

6. The list of errors should be displayed as a list (with bullets) like 
in the deleter dialog.

-- 
Endi S. Dewata