[Bps-public-commit] rt-extension-notificationmatrix branch, master, updated. 1.9-2-g404990e

[Thomas Sibley]

The branch, master has been updated
       via  404990e6ca4bb2fe6baf874ee3166075e6f25695 (commit)
      from  5404636aa126a3171a6ef9bddf679554c9defa9b (commit)

Summary of changes:
 lib/RT/Extension/NotificationMatrix/Rule.pm |   53 +++++++++++++++++++++++----
 t/basic.t                                   |    6 +---
 2 files changed, 47 insertions(+), 12 deletions(-)

- Log -----------------------------------------------------------------
commit 404990e6ca4bb2fe6baf874ee3166075e6f25695
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Sep 13 13:21:45 2011 -0400

    Suppress duplicate notifications across the internal/external email boundary
    
    External email is email to Requestors and Ccs, while internal email is to
    everyone else (Owners and AdminCcs).  Before this, "external" notifications
    were sent independently of "internal" notifications.
    
    Duplicate email resulted when someone occupied both an internal and external
    ticket/queue role, e.g. a Requestor & Owner or ticket Cc & queue AdminCc.
    
    Note that external has slightly different meanings when sending vs. receiving
    email.  When sending, external means to Requestors and Ccs.  When receiving,
    external means from Requestors and anyone else not a Cc or Owner.
    
    Remove one place in the tests where this duplicate email behaviour was
    tested: On Create, a user who is a queue AdminCc and Requestor got both
    the internal Transaction: Created mail and an Autoreply.  With this
    commit, only the internal Transaction: Created mail is sent.

diff --git a/lib/RT/Extension/NotificationMatrix/Rule.pm b/lib/RT/Extension/NotificationMatrix/Rule.pm
index 32099fc..c60a94f 100644
--- a/lib/RT/Extension/NotificationMatrix/Rule.pm
+++ b/lib/RT/Extension/NotificationMatrix/Rule.pm
@@ -176,13 +176,6 @@ sub _PrepareSendEmail {
     }
 
     $email->{__ref} = $ref;
-    $email->Prepare;
-
-    if ($self->SendAsComment) {
-        $email->TemplateObj->MIMEObj->head->set('From', '');
-        $email->TemplateObj->MIMEObj->head->set('Reply-To', '');
-        $email->SetReturnAddress( is_comment => 1 );
-    }
 
     return $email;
 }
@@ -191,11 +184,57 @@ sub Prepare {
     my $self = shift;
 
     $self->{__email} = [($self->PrepareInternal(), $self->PrepareExternal())];
+
+    # XXX: This hints key is never used.  What's the purpose?
     $self->{hints} = { class => 'SendEmail',
                        recipients => { To =>  [ map { @{$_->{To}} } @{$self->{__email}} ],
                                        Cc =>  [ map { @{$_->{Cc}} } @{$self->{__email}} ],
                                        Bcc => [ map { @{$_->{Bcc}} } @{$self->{__email}} ],
                                    } };
+
+    # Remove any internal recipients from external notifications by cascading
+    # down the array of emails
+    for my $i (0 .. $#{$self->{__email}}) {
+        my $current = $self->{__email}[$i];
+
+        # Mark as seen all addresses at the current level.  Any addresses left
+        # in the current level are already not duplicates.
+        my %seen = map { lc($_) => 1 }
+                   map { @{$current->{$_}} }
+                       qw(To Cc Bcc);
+
+        # For all mail below the current, remove seen addresses
+        for my $j (++$i .. $#{$self->{__email}}) {
+            my $mail = $self->{__email}[$j];
+            for my $type (qw(To Cc Bcc)) {
+                my @new;
+
+                for my $addr (@{$mail->{$type}}) {
+                    # We don't increment $seen here because SendEmail
+                    # suppresses duplicates within headers in a single email
+                    # later.  We just want to avoid duplicates across mail.
+                    if (defined $addr and length $addr and not $seen{lc $addr}) {
+                        push @new, $addr;
+                    } else {
+                        $RT::Logger->info("Removing '$addr' from $type of notification #$j (@{[ref $self]}) because the address was already included in a higher notification");
+                    }
+                }
+                $mail->{$type} = \@new;
+            }
+        }
+    }
+
+    # Prepare all the email actions now that recipients are ready
+    for my $email (@{$self->{__email}}) {
+        $email->Prepare;
+
+        if ($self->SendAsComment) {
+            $email->TemplateObj->MIMEObj->head->set('From', '');
+            $email->TemplateObj->MIMEObj->head->set('Reply-To', '');
+            $email->SetReturnAddress( is_comment => 1 );
+        }
+    }
+
     return 1;
 }
 
diff --git a/t/basic.t b/t/basic.t
index c16bfcd..0da18d9 100644
--- a/t/basic.t
+++ b/t/basic.t
@@ -10,7 +10,7 @@ BEGIN {
 }
 
 use RT;
-use RT::Test tests => 29;
+use RT::Test tests => 28;
 use RT::Test::Email;
 RT->Config->Set( LogToScreen => 'debug' );
 RT->Config->Set('Plugins',qw(RT::Extension::NotificationMatrix));
@@ -138,10 +138,6 @@ mail_ok {
     bcc => 'user_a at example.com, user_b at example.com', # from ticket created: group_a
     subject => qr/a test/,
     body => qr/Transaction: Ticket created by USER_B/,
-}, { from => qr'USER_B via RT',
-    to => 'user_b at example.com',
-    subject => qr/a test/,
-    body => qr/automatically generated in response/,
 };
 
 mail_ok {

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