[Rt-commit] r14754 - in rt/3.8/trunk: . lib/RT/Interface t/ticket

[falcone at bestpractical.com]

Author: falcone
Date: Mon Aug  4 13:06:10 2008
New Revision: 14754

Modified:
   rt/3.8/trunk/   (props changed)
   rt/3.8/trunk/lib/RT/EmailParser.pm
   rt/3.8/trunk/lib/RT/Interface/Email.pm
   rt/3.8/trunk/lib/RT/Ticket_Overlay.pm
   rt/3.8/trunk/t/ticket/add-watchers.t

Log:
 r37622 at ketch:  falcone | 2008-08-04 11:41:43 -0400
 * work around Email::Address not handling local-only emails with
   a band-aid for now.  Fix AddWatcher to use the new code,
   which closes a number of holes.
 r37623 at ketch:  falcone | 2008-08-04 12:23:16 -0400
 * rename Email::Address wrapper method
 * return the User Name if we have one to help build better addresses
 r37624 at ketch:  falcone | 2008-08-04 12:23:45 -0400
 * change method name


Modified: rt/3.8/trunk/lib/RT/EmailParser.pm
==============================================================================
--- rt/3.8/trunk/lib/RT/EmailParser.pm	(original)
+++ rt/3.8/trunk/lib/RT/EmailParser.pm	Mon Aug  4 13:06:10 2008
@@ -512,6 +512,47 @@
 
 }
 
+=head2 ParseEmailAddress string
+
+Returns a list of Email::Address objects
+Works around the bug that Email::Address 1.889 and earlier
+doesn't handle local-only email addresses (when users pass
+in just usernames on the RT system in fields that expect
+Email Addresses)
+
+We don't handle the case of 
+bob, fred at bestpractical.com 
+because we don't want to fail parsing
+bob, "Falcone, Fred" <fred at bestpractical.com>
+The next release of Email::Address will have a new method
+we can use that removes the bandaid
+
+=cut
+
+sub ParseEmailAddress {
+    my $self = shift;
+    my $address_string = shift;
+
+    $address_string =~ s/^\s+|\s+$//g;
+
+    my @addresses;
+    # if it looks like a username / local only email
+    if ($address_string !~ /@/ && $address_string =~ /^\w+$/) {
+        my $user = RT::User->new( $RT::SystemUser );
+        my ($id, $msg) = $user->Load($address_string);
+        if ($id) {
+            push @addresses, Email::Address->new($user->Name,$user->EmailAddress);
+        } else {
+            $RT::Logger->error("Unable to parse an email address from $address_string: $msg");
+        }
+    } else {
+        @addresses = Email::Address->parse($address_string);
+    }
+
+    return @addresses;
+
+}
+
 
 sub DESTROY {
     my $self = shift;

Modified: rt/3.8/trunk/lib/RT/Interface/Email.pm
==============================================================================
--- rt/3.8/trunk/lib/RT/Interface/Email.pm	(original)
+++ rt/3.8/trunk/lib/RT/Interface/Email.pm	Mon Aug  4 13:06:10 2008
@@ -917,7 +917,7 @@
 
     # Some broken mailers send:  ""Vincent, Jesse"" <jesse at fsck.com>. Hate
     $Addr =~ s/\"\"(.*?)\"\"/\"$1\"/g;
-    my @Addresses = Email::Address->parse($Addr);
+    my @Addresses = RT::EmailParser->ParseEmailAddress($Addr);
 
     my ($AddrObj) = grep ref $_, @Addresses;
     unless ( $AddrObj ) {

Modified: rt/3.8/trunk/lib/RT/Ticket_Overlay.pm
==============================================================================
--- rt/3.8/trunk/lib/RT/Ticket_Overlay.pm	(original)
+++ rt/3.8/trunk/lib/RT/Ticket_Overlay.pm	Mon Aug  4 13:06:10 2008
@@ -1037,7 +1037,7 @@
     return $self->_AddWatcher( %args )
         if $self->CurrentUserHasRight('ModifyTicket');
     if ( $args{'Email'} ) {
-        my ($addr) = Email::Address->parse( $args{'Email'} );
+        my ($addr) = RT::EmailParser->ParseEmailAddress( $args{'Email'} );
         return (0, $self->loc("Couldn't parse address from '[_1]' string", $args{'Email'} ))
             unless $addr;
 

Modified: rt/3.8/trunk/t/ticket/add-watchers.t
==============================================================================
--- rt/3.8/trunk/t/ticket/add-watchers.t	(original)
+++ rt/3.8/trunk/t/ticket/add-watchers.t	Mon Aug  4 13:06:10 2008
@@ -45,7 +45,7 @@
 # 
 # END BPS TAGGED BLOCK }}}
 
-use Test::More tests => 28;
+use Test::More tests => 32;
 use RT::Test;
 
 use strict;
@@ -123,6 +123,18 @@
 ($rv, $msg) = $ticket2->AddWatcher( Type => 'Requestor', Email => $user->EmailAddress );
 ok(  $rv, "user can add self as Requestor by Email" );
 
+# remove user and try adding by username
+# This worked in 3.6 and is a regression in 3.8
+($rv, $msg) = $ticket->DeleteWatcher( Type => 'Cc', Email => $user->EmailAddress );
+ok( $rv, "watcher removed by Email" );
+($rv, $msg) = $ticket->DeleteWatcher( Type => 'Requestor', Email => $user->EmailAddress );
+ok( $rv, "watcher removed by Email" );
+
+($rv, $msg) = $ticket2->AddWatcher( Type => 'Cc', Email => $user->Name );
+ok(  $rv, "user can add self as Cc by username" );
+($rv, $msg) = $ticket2->AddWatcher( Type => 'Requestor', Email => $user->Name );
+ok(  $rv, "user can add self as Requestor by username" );
+
 # Queue watcher tests
 $principal->RevokeRight( Right => 'Watch'  , Object => $queue );
 ok( !$user->HasRight( Right => 'Watch',        Object => $queue ), "user queue watch right revoked" );
@@ -154,4 +166,3 @@
 ($rv, $msg) = $queue2->AddWatcher( Type => 'Requestor', Email => $user->EmailAddress );
 ok(  $rv, "user can add self as Requestor by Email" );
 
-