[Bps-public-commit] rt-extension-nonwatcherrecipients branch, master, updated. 0.01-3-g4dee9a2

[Thomas Sibley]

The branch, master has been updated
       via  4dee9a239a3488b9eb2c5f10a1e363f85597180f (commit)
       via  709c31e6905728372b2638518b0425cfa11e15f7 (commit)
       via  fb2a9c2c554ab6fc940836d26d3bcb26d128c8df (commit)
      from  4a41aac3fb7ce4a2014ed3a5ac19d207bc22f6ee (commit)

Summary of changes:
 lib/RT/Extension/NonWatcherRecipients.pm |  7 +++--
 xt/basic.t                               | 49 ++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 18 deletions(-)

- Log -----------------------------------------------------------------
commit fb2a9c2c554ab6fc940836d26d3bcb26d128c8df
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Feb 19 17:39:21 2013 -0800

    Our IsWatcher method takes a ticket object as the second argument
    
    This meant the FindRecipients function failed if any addresses were
    included in the ticket update which also existed in the Users table.  A
    test case is included.
    
    Addresses which don't have an associated User record short-circuit
    IsWatcher earlier enough, so the previous test didn't trigger this bug.

diff --git a/lib/RT/Extension/NonWatcherRecipients.pm b/lib/RT/Extension/NonWatcherRecipients.pm
index e98a409..37ba50b 100644
--- a/lib/RT/Extension/NonWatcherRecipients.pm
+++ b/lib/RT/Extension/NonWatcherRecipients.pm
@@ -125,7 +125,7 @@ sub FindRecipients {
 
     # Show any extra recipients
     for my $hdr (qw(From To Cc RT-Send-Cc RT-Send-Bcc)) {
-        my @new = grep { not $self->IsWatcher($_->address) } @{$addr{$hdr} || []};
+        my @new = grep { not $self->IsWatcher($_->address, $Ticket) } @{$addr{$hdr} || []};
         $recipients .= "   $hdr: " . $self->Format(\@new) . "\n"
             if @new;
     }
diff --git a/xt/basic.t b/xt/basic.t
index f3d6da1..c821fd0 100644
--- a/xt/basic.t
+++ b/xt/basic.t
@@ -5,27 +5,46 @@ use RT::Extension::NonWatcherRecipients::Test tests => undef;
 
 use_ok('RT::Extension::NonWatcherRecipients');
 
+my $user = RT::User->new(RT->SystemUser);
+$user->Load('root');
+$user->SetEmailAddress('root at example.com');
+
+my $t = RT::Ticket->new($user);
+my ($id, $msg) = $t->Create(
+    Queue => 'General',
+    Subject => 'Test ticket',
+    Content => 'This is a test',
+    Requestor => ['root'],
+    );
+
+ok( $t->id, 'Create test ticket: ' . $t->id);
+
+diag "Unknown user without a record and filtering of existing watcher";
 {
+    my ($txn_id, $txn_msg, $txn_obj) = $t->Correspond(
+         CcMessageTo => 'sharkbanana at bestpractical.com, root at example.com',
+         Content => 'This is a test' );
 
-    my $user = RT::User->new(RT->SystemUser);
-    $user->Load('root');
-    $user->SetEmailAddress('root at example.com');
+    ok( $txn_id, "Created transaction: $txn_id $txn_msg");
 
-    my $curr_user = RT::CurrentUser->new($user);
+    my $message = RT::Extension::NonWatcherRecipients->FindRecipients(
+        Transaction => $txn_obj,
+        Ticket => $t);
 
-    my $t = RT::Ticket->new($curr_user);
-    my ($id, $msg) = $t->Create(
-        Queue => 'General',
-        Subject => 'Test ticket',
-        Content => 'This is a test',
-        Requestor => ['root'],
-        );
+    like( $message, qr/The following people received a copy/, 'Got message');
+    like( $message, qr/sharkbanana\@bestpractical\.com/, 'Got email address');
+    unlike( $message, qr/root\@example\.com/, "root's email address is excluded");
+}
 
-    ok( $t->id, 'Create test ticket: ' . $t->id);
+diag "Existing user record, but not a watcher";
+{
+    my $foo = RT::User->new( RT->SystemUser );
+    my ($ok, $msg) = $foo->Create( Name => 'foo', EmailAddress => 'foo at example.com' );
+    ok $ok, "Created user foo: $msg";
 
     my ($txn_id, $txn_msg, $txn_obj) = $t->Correspond(
-         CcMessageTo => 'sharkbanana at bestpractical.com',
-         Content => 'This is a test' );
+         CcMessageTo => 'foo at example.com',
+         Content => 'This is another test' );
 
     ok( $txn_id, "Created transaction: $txn_id $txn_msg");
 
@@ -34,7 +53,7 @@ use_ok('RT::Extension::NonWatcherRecipients');
         Ticket => $t);
 
     like( $message, qr/The following people received a copy/, 'Got message');
-    like( $message, qr/sharkbanana\@bestpractical\.com/, 'Got email address');
+    like( $message, qr/foo\@example\.com/, "foo's email address is included");
 }
 
 done_testing;

commit 709c31e6905728372b2638518b0425cfa11e15f7
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Feb 19 17:41:26 2013 -0800

    Return the empty string instead of undef for friendlier use in templates
    
    Avoids warnings

diff --git a/lib/RT/Extension/NonWatcherRecipients.pm b/lib/RT/Extension/NonWatcherRecipients.pm
index 37ba50b..818c52b 100644
--- a/lib/RT/Extension/NonWatcherRecipients.pm
+++ b/lib/RT/Extension/NonWatcherRecipients.pm
@@ -110,7 +110,7 @@ sub FindRecipients {
     my $Transaction = $args{Transaction};
     my $Ticket = $args{Ticket};
     my $recipients; # List of recipients
-    my $message;  # Message for template
+    my $message = ""; # Message for template
 
     unless ( $Transaction->Id and $Ticket->Id ){
         RT::Logger->error("Transaction and Ticket objects are required. "
@@ -118,7 +118,7 @@ sub FindRecipients {
                           . " and Ticket Id: " . $Ticket->Id);
     }
 
-    return unless my $att = $Transaction->Attachments->First;
+    return "" unless my $att = $Transaction->Attachments->First;
 
     my %addr = %{ $att->Addresses };
     my $creator = $Transaction->CreatorObj->RealName || '';

commit 4dee9a239a3488b9eb2c5f10a1e363f85597180f
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Feb 19 17:43:05 2013 -0800

    Abort early if we don't have a ticket and transaction

diff --git a/lib/RT/Extension/NonWatcherRecipients.pm b/lib/RT/Extension/NonWatcherRecipients.pm
index 818c52b..6bbabdf 100644
--- a/lib/RT/Extension/NonWatcherRecipients.pm
+++ b/lib/RT/Extension/NonWatcherRecipients.pm
@@ -116,6 +116,7 @@ sub FindRecipients {
         RT::Logger->error("Transaction and Ticket objects are required. "
                           . "Received Transaction Id: " . $Transaction->Id
                           . " and Ticket Id: " . $Ticket->Id);
+        return "";
     }
 
     return "" unless my $att = $Transaction->Attachments->First;

-----------------------------------------------------------------------