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

[Petr Vobornik]

On 08/11/2011 06:16 PM, Endi Sukma Dewata wrote:
> 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.
done
>> The 'retry' flag in IPA.command can be renamed to 'show_error' too.
sending without this (not essential part of this patch - not 
batch_command). But I agree it should be probably changed - to be 
consistent.
>>
>> 2. The 'partial_success_message' probably can be renamed intofile:
>> 'error_message' which will say something like 'Some operations failed.'
done
>>
>> 3. Instead of a checkbox for show_errors_checkbox, it might be better to
>> use 'Show details' and 'Hide details' links.
done
>>
>> 4. In ipa.js:510 instead of repeating the error message the
>> error_thrown.name could say something like 'Batch Error' or 'Operations
>> Error'.
used 'Operations Error' - batch is internal naming and probably 
confusing to user
>>
>> 5. The add_error() could be moved into IPA.error_dialog so the
>> IPA.batch_command doesn't need to hold the 'errors' list.
left in batch_command for possible future use in custom handlers
>>
>> 6. The list of errors should be displayed as a list (with bullets) like
>> in the deleter dialog.
done
> 7. Just for consistency, the Retry label declaration in ipa.js:737 could
> be moved into the if-statement like the other labels.
>
done


-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0004-1-error-dialog-for-batch-command.patch
Type: text/x-patch
Size: 12492 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110815/8117ece8/attachment.bin>