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

[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-title>Operation 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-edewata-0253-Fixed-command-partial-failure-handling.patch
Type: text/x-patch
Size: 6923 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110823/593ee3c8/attachment.bin>