[Rt-commit] rt branch, 4.2/sendmail-action-logs, created. rt-4.0.5-289-gdc0481d
Alex Vandiver
alexmv at bestpractical.com
Fri Mar 23 17:08:59 EDT 2012
The branch, 4.2/sendmail-action-logs has been created
at dc0481db8a88fd2dfe0416e948229fbc4f6e8954 (commit)
- Log -----------------------------------------------------------------
commit b1c3da561156beb6ba29a7a2af7f84eee912785c
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Fri Sep 9 14:49:36 2011 +0400
save lenghty call into a variable
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index 8788417..af64e16 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -761,7 +761,9 @@ sub RemoveInappropriateRecipients {
# caused by one of the watcher addresses being broken.
# Default ("true") is to redistribute, for historical reasons.
- if ( !RT->Config->Get('RedistributeAutoGeneratedMessages') ) {
+ my $redistribute = RT->Config->Get('RedistributeAutoGeneratedMessages');
+
+ if ( !$redistribute ) {
# Don't send to any watchers.
@{ $self->{$_} } = () for (@EMAIL_RECIPIENT_HEADERS);
@@ -769,9 +771,7 @@ sub RemoveInappropriateRecipients {
. " The incoming message was autogenerated. "
. "Not redistributing this message based on site configuration."
);
- } elsif ( RT->Config->Get('RedistributeAutoGeneratedMessages') eq
- 'privileged' )
- {
+ } elsif ( $redistribute eq 'privileged' ) {
# Only send to "privileged" watchers.
foreach my $type (@EMAIL_RECIPIENT_HEADERS) {
commit 07addb195042b6520ab1d946f28080fc448125d2
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Fri Sep 9 14:50:21 2011 +0400
don't use split for addresses, use parser
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index af64e16..32ee023 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -789,7 +789,7 @@ sub RemoveInappropriateRecipients {
}
if ( my $squelch = $attachment->GetHeader('RT-Squelch-Replies-To') ) {
- push @blacklist, split( /,/, $squelch );
+ push @blacklist, map $_->address, Email::Address->parse( $squelch );
}
}
commit ca5172a467e9147971a4ff5f7357c01878a651ae
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Fri Sep 9 15:17:08 2011 +0400
turn blacklist into a hash with reasons
* to make log clearer about why we blacklist recipients
* make sure we use case insensitive email comparision
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index 32ee023..08d2111 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -747,7 +747,7 @@ Remove addresses that are RT addresses or that are on this transaction's blackli
sub RemoveInappropriateRecipients {
my $self = shift;
- my @blacklist = ();
+ my %blacklist = ();
# If there are no recipients, don't try to send the message.
# If the transaction has content and has the header RT-Squelch-Replies-To
@@ -778,7 +778,8 @@ sub RemoveInappropriateRecipients {
foreach my $addr ( @{ $self->{$type} } ) {
my $user = RT::User->new(RT->SystemUser);
$user->LoadByEmail($addr);
- push @blacklist, $addr unless $user->id && $user->Privileged;
+ $blacklist{ $addr } ||= 'not privileged'
+ unless $user->id && $user->Privileged;
}
}
$RT::Logger->info( $msgid
@@ -789,20 +790,27 @@ sub RemoveInappropriateRecipients {
}
if ( my $squelch = $attachment->GetHeader('RT-Squelch-Replies-To') ) {
- push @blacklist, map $_->address, Email::Address->parse( $squelch );
+ $blacklist{ $_->address } ||= 'squelch:attachment'
+ foreach Email::Address->parse( $squelch );
}
}
- # Let's grab the SquelchMailTo attributes and push those entries into the @blacklisted
- push @blacklist, map $_->Content, $self->TicketObj->SquelchMailTo, $self->TransactionObj->SquelchMailTo;
-
- # Cycle through the people we're sending to and pull out anyone on the
- # system blacklist
-
- # Trim leading and trailing spaces.
- @blacklist = map { RT::User->CanonicalizeEmailAddress( $_->address ) }
- Email::Address->parse( join ', ', grep defined, @blacklist );
+ # Let's grab the SquelchMailTo attributes and push those entries
+ # into the blacklisted
+ $blacklist{ $_->Content } ||= 'squelch:transaction'
+ foreach $self->TransactionObj->SquelchMailTo;
+ $blacklist{ $_->Content } ||= 'squelch:ticket'
+ foreach $self->TicketObj->SquelchMailTo;
+
+ # canonicalize emails
+ foreach my $address ( keys %blacklist ) {
+ my $reason = delete $blacklist{ $address };
+ $blacklist{ lc $_ } = $reason
+ foreach map RT::User->CanonicalizeEmailAddress( $_->address ),
+ Email::Address->parse( $address );
+ }
+ # Cycle through the people we're sending to and pull out anyone on the blacklist
foreach my $type (@EMAIL_RECIPIENT_HEADERS) {
my @addrs;
foreach my $addr ( @{ $self->{$type} } ) {
@@ -813,7 +821,7 @@ sub RemoveInappropriateRecipients {
$RT::Logger->info( $msgid . "$addr appears to point to this RT instance. Skipping" );
next;
}
- if ( grep $addr eq $_, @blacklist ) {
+ if ( $blacklist{ lc $addr } ) {
$RT::Logger->info( $msgid . "$addr was blacklisted for outbound mail on this transaction. Skipping");
next;
}
commit e1aa0b183fa555c2b3dc50e86359807ba876942f
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Fri Sep 9 16:58:12 2011 +0400
explain in logs why email is in blacklist
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index 08d2111..b068f2b 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -744,6 +744,17 @@ Remove addresses that are RT addresses or that are on this transaction's blackli
=cut
+my %squelch_reasons = (
+ 'not privileged' => "as user is not privileged, not redistributing autogenerated message",
+ 'squelch:attachment'
+ => "by RT-Squelch-Replies-To header in the incoming message",
+ 'squelch:transaction'
+ => "by squelch settings for this transaction only",
+ 'squelch:ticket'
+ => "by squelch settings for this ticket",
+);
+
+
sub RemoveInappropriateRecipients {
my $self = shift;
@@ -821,8 +832,8 @@ sub RemoveInappropriateRecipients {
$RT::Logger->info( $msgid . "$addr appears to point to this RT instance. Skipping" );
next;
}
- if ( $blacklist{ lc $addr } ) {
- $RT::Logger->info( $msgid . "$addr was blacklisted for outbound mail on this transaction. Skipping");
+ if ( my $reason = $blacklist{ lc $addr } ) {
+ $RT::Logger->info( $msgid . " $addr is blacklisted $squelch_reasons{ $reason }. Skipping" );
next;
}
push @addrs, $addr;
commit 012ee743c2db03b567e0fb80a07a12ee4c759458
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Mar 23 17:03:43 2012 -0400
Make messages more verbose and actionable
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index b068f2b..f990ece 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -745,13 +745,14 @@ Remove addresses that are RT addresses or that are on this transaction's blackli
=cut
my %squelch_reasons = (
- 'not privileged' => "as user is not privileged, not redistributing autogenerated message",
+ 'not privileged'
+ => "because autogenerated messages are configured to only be sent to privileged users (RedistributeAutoGeneratedMessages)",
'squelch:attachment'
=> "by RT-Squelch-Replies-To header in the incoming message",
'squelch:transaction'
- => "by squelch settings for this transaction only",
+ => "by notification checkboxes for this transaction",
'squelch:ticket'
- => "by squelch settings for this ticket",
+ => "by notification checkboxes on this ticket's People page",
);
commit 27c318783d10e84ce02e4a04f1aed578d34502e3
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Mar 23 16:47:56 2012 -0400
Remove the trailing newline from the msgid before logging using it
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index f990ece..ea0574e 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -765,6 +765,8 @@ sub RemoveInappropriateRecipients {
# If the transaction has content and has the header RT-Squelch-Replies-To
my $msgid = $self->TemplateObj->MIMEObj->head->get('Message-Id');
+ chomp $msgid;
+
if ( my $attachment = $self->TransactionObj->Attachments->First ) {
if ( $attachment->GetHeader('RT-DetectedAutoGenerated') ) {
commit 68db7cb0588647171b56377f6dde8aacd77b3a0a
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Mar 23 16:48:58 2012 -0400
Refactor RemoveInappropriateRecipients to use a callback interface
This allows other subclasses to add their own particular cases for why
an address should be skipped, by overriding
RemoveInappropriateRecipients to call an additional RecipientFilter.
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index ea0574e..bf856b6 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -824,35 +824,68 @@ sub RemoveInappropriateRecipients {
Email::Address->parse( $address );
}
- # Cycle through the people we're sending to and pull out anyone on the blacklist
- foreach my $type (@EMAIL_RECIPIENT_HEADERS) {
+ $self->RecipientFilter(
+ Callback => sub {
+ return if RT::EmailParser->CullRTAddresses( $_[0] );
+ return "$_[0] appears to point to this RT instance. Skipping";
+ },
+ All => 1,
+ );
+
+ $self->RecipientFilter(
+ Callback => sub {
+ return unless $blacklist{ lc $_[0] };
+ return "$_[0] is blacklisted $squelch_reasons{ $blacklist{ lc $_[0] } }. Skipping";
+ },
+ );
+
+
+ # Cycle through the people we're sending to and pull out anyone that meets any of the callbacks
+ for my $type (@EMAIL_RECIPIENT_HEADERS) {
my @addrs;
- foreach my $addr ( @{ $self->{$type} } ) {
- # Weed out any RT addresses. We really don't want to talk to ourselves!
- # If we get a reply back, that means it's not an RT address
- if ( !RT::EmailParser->CullRTAddresses($addr) ) {
- $RT::Logger->info( $msgid . "$addr appears to point to this RT instance. Skipping" );
- next;
- }
- if ( my $reason = $blacklist{ lc $addr } ) {
- $RT::Logger->info( $msgid . " $addr is blacklisted $squelch_reasons{ $reason }. Skipping" );
- next;
+ ADDRESS:
+ for my $addr ( @{ $self->{$type} } ) {
+ for my $filter ( map {$_->{Callback}} @{$self->{RecipientFilter}} ) {
+ my $skip = $filter->($addr);
+ next unless $skip;
+ $RT::Logger->info( "$msgid $skip" );
+ next ADDRESS;
}
push @addrs, $addr;
}
- foreach my $addr ( @{ $self->{'NoSquelch'}{$type} || [] } ) {
- # never send email to itself
- if ( !RT::EmailParser->CullRTAddresses($addr) ) {
- $RT::Logger->info( $msgid . "$addr appears to point to this RT instance. Skipping" );
- next;
+
+ NOQUELCH_ADDRESS:
+ for my $addr ( @{ $self->{NoSquelch}{$type} } ) {
+ for my $filter ( map {$_->{Callback}} grep {$_->{All}} @{$self->{RecipientFilter}} ) {
+ my $skip = $filter->($addr);
+ next unless $skip;
+ $RT::Logger->info( "$msgid $skip" );
+ next NOSQUELCH_ADDRESS;
}
push @addrs, $addr;
}
+
@{ $self->{$type} } = @addrs;
}
}
+=head2 RecipientFilter Callback => SUB, [All => 1]
+
+Registers a filter to be applied to addresses by
+L<RemoveInappropriateRecipients>. The C<Callback> will be called with
+one address at a time, and should return false if the address should
+receive mail, or a message explaining why it should not be. Passing a
+true value for C<All> will cause the filter to also be applied to
+NoSquelch (one-time Cc and Bcc) recipients as well.
+
+=cut
+
+sub RecipientFilter {
+ my $self = shift;
+ push @{ $self->{RecipientFilter}}, {@_};
+}
+
=head2 SetReturnAddress is_comment => BOOLEAN
Calculate and set From and Reply-To headers based on the is_comment flag.
commit dc0481db8a88fd2dfe0416e948229fbc4f6e8954
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Mar 23 16:50:18 2012 -0400
Use RecipientFilter to provide a log message when NotifyActor kicks in
diff --git a/lib/RT/Action/Notify.pm b/lib/RT/Action/Notify.pm
index f1aef40..c8a8500 100644
--- a/lib/RT/Action/Notify.pm
+++ b/lib/RT/Action/Notify.pm
@@ -131,24 +131,9 @@ sub SetRecipients {
}
}
- my $creatorObj = $self->TransactionObj->CreatorObj;
- my $creator = $creatorObj->EmailAddress() || '';
-
- #Strip the sender out of the To, Cc and AdminCc and set the
- # recipients fields used to build the message by the superclass.
- # unless a flag is set
- my $TransactionCurrentUser = RT::CurrentUser->new;
- $TransactionCurrentUser->LoadByName($creatorObj->Name);
- if (RT->Config->Get('NotifyActor',$TransactionCurrentUser)) {
- @{ $self->{'To'} } = @To;
- @{ $self->{'Cc'} } = @Cc;
- @{ $self->{'Bcc'} } = @Bcc;
- }
- else {
- @{ $self->{'To'} } = grep ( lc $_ ne lc $creator, @To );
- @{ $self->{'Cc'} } = grep ( lc $_ ne lc $creator, @Cc );
- @{ $self->{'Bcc'} } = grep ( lc $_ ne lc $creator, @Bcc );
- }
+ @{ $self->{'To'} } = @To;
+ @{ $self->{'Cc'} } = @Cc;
+ @{ $self->{'Bcc'} } = @Bcc;
@{ $self->{'PseudoTo'} } = @PseudoTo;
if ( $arg =~ /\bOtherRecipients\b/ ) {
@@ -161,6 +146,24 @@ sub SetRecipients {
}
}
+sub RemoveInappropriateRecipients {
+ my $self = shift;
+
+ my $creatorObj = $self->TransactionObj->CreatorObj;
+ my $creator = $creatorObj->EmailAddress() || '';
+ my $TransactionCurrentUser = RT::CurrentUser->new;
+ $TransactionCurrentUser->LoadByName($creatorObj->Name);
+
+ $self->RecipientFilter(
+ Callback => sub {
+ return unless lc $_[0] eq lc $creator;
+ return "not sending to $creator, creator of the transaction, due to NotifyActor setting";
+ },
+ ) unless RT->Config->Get('NotifyActor',$TransactionCurrentUser);
+
+ $self->SUPER::RemoveInappropriateRecipients();
+}
+
RT::Base->_ImportOverlays();
1;
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list