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

[Endi Sukma Dewata]

On 8/11/2011 11:03 AM, Endi Sukma Dewata wrote:
> 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.

7. Just for consistency, the Retry label declaration in ipa.js:737 could 
be moved into the if-statement like the other labels.

-- 
Endi S. Dewata