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

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

Reply via email to