[Rt-devel] [PATCH] Improve RT::Interface::Email::Filter::SpamAssassin

[Petter Reinholdtsen]

I had a look at the Filter::SpamAssassin mail plugin, and have a
question and some suggested improvements.

When reading the code, I'm unable to understand how $item can contain
a reference to the message.  The argument to GetCurrentUser() is a
hash of arguments, not a reference to the message.  Can anyone explain
how the current code work?

The current code wipe out the CurrentUser variable, and must be called
first in the chain of plugins.  There is no good reason why it need to
do this, so I propose to change it to keep the CurrentUser value.

Here is a patch to change the argument handling, to pass
$args{'Message'} to spamassasin, instead of the strange $item value.
The patch also make sure CurrentUser and AuthLevel is passed through
when no change is wanted.

If the patch is correct, please include it in a future version of RT.

--- lib/RT/Interface/Email/Filter/SpamAssassin.pm       2004-04-30 00:26:36.000000000 +0200
+++ local/lib/RT/Interface/Email/Filter/SpamAssassinPere.pm     2004-08-26 10:46:34.000000000 +0200
@@ -27,15 +27,20 @@
 my $spamtest = Mail::SpamAssassin->new();

 sub GetCurrentUser {
+    my %args = ( Message     => undef,
+                 CurrentUser => undef,
+                 AuthLevel   => undef,
+                 @_ );
     my $item = shift;
-    my $status = $spamtest->check ($item);
-    return (undef, 0) unless $status->is_spam();
+    my $status = $spamtest->check ($args{'Message'});
+    return ($args{'CurrentUser'}, $args{'AuthLevel'})
+       unless $status->is_spam();
     eval { $status->rewrite_mail() };
     if ($status->get_hits > $status->get_required_hits()*1.5) {
         # Spammy indeed
-        return (undef, -1);
+        return ($args{'CurrentUser'}, -1);
     }
-    return (undef, 0);
+    return ($args{'CurrentUser'}, $args{'AuthLevel'});
 }

 =head1 NAME