[dev] Re: [imp] whitelist patch...

[Chuck Hagenbuch]

Quoting Amith Varghese <amith at xalan.com>:

> Here's the patch to add whitelisting to the mailbox view as well.

Committed, thanks.

> One thing that I did notice while testing is that the error handling
> between imp and sam doesn't really work.  For example in 
> IMP::blacklistMessage() assumes automatically that the address was added 
> to the blacklist by pushing something on the notification stack.  However, 
> if in sam's API the address is already in the  blacklist, it returns a 
> PEAR_Error.  My question is this:
> 
> How is error handling performed between different applications?

Apparently, a bit less consistently than it should be? :)

> In this situation, if I choose to blacklist 10 addresses and two
> addresses are already in my blacklist, I would expect the application to 
> say these 8 have been added and these 2 are already in the list.  However
> the way sam is set up is that it fails immediately and returns a PEAR 
> Error.  I was thinking that it would make sense if the API returned an 
> array of objects and then you could do something like
> 
> foreach ($results as $address) {
>     if (is_a($address,'PEAR_Error') {
>         $notification->push($address->getMessage()); // Error message
> here
>     } else {
>         $notification->push(sprintf(_("The address %s has been added to
> your
> blacklist."), $address));
>     }
> }
> 
> Is this the best way to do this?  Any suggestions are welcome.

It'd be best, I think, to return an aggregate error message (single object)
listing the addresses that failed - but also containing an error code, as
"already in blacklist" isn't a failure, it's an informational message - so
we should display "x and y were already in your blacklist" as a status
message, not as a success or failure...

-chuck

--
Charles Hagenbuch, <chuck at horde.org>
must ... find ... acorns ... *thud*