[Rt-commit] r4410 - in rt/branches/QUEBEC-EXPERIMENTAL: . html/Elements/CollectionAsTable lib/RT

jesse at bestpractical.com jesse at bestpractical.com
Thu Jan 19 10:15:42 EST 2006


Author: jesse
Date: Thu Jan 19 10:15:41 2006
New Revision: 4410

Modified:
   rt/branches/QUEBEC-EXPERIMENTAL/   (props changed)
   rt/branches/QUEBEC-EXPERIMENTAL/html/Elements/CollectionAsTable/ParseFormat
   rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Action/SendEmail.pm
   rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Template_Overlay.pm
   rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Tickets_Overlay.pm

Log:
 r22717 at truegrounds:  jesse | 2006-01-19 08:59:52 -0500
 * Merged forward from RT 3.4


Modified: rt/branches/QUEBEC-EXPERIMENTAL/html/Elements/CollectionAsTable/ParseFormat
==============================================================================
--- rt/branches/QUEBEC-EXPERIMENTAL/html/Elements/CollectionAsTable/ParseFormat	(original)
+++ rt/branches/QUEBEC-EXPERIMENTAL/html/Elements/CollectionAsTable/ParseFormat	Thu Jan 19 10:15:41 2006
@@ -48,7 +48,7 @@
 </%ARGS>
 
 <%init>
-use Regexp::Common;
+use Regexp::Common qw/delimited/;
 my @Columns;
 
 while ($Format =~ /($RE{delimited}{-delim=>qq{\'"}}|[{}\w.]+)/go) {

Modified: rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Action/SendEmail.pm
==============================================================================
--- rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Action/SendEmail.pm	(original)
+++ rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Action/SendEmail.pm	Thu Jan 19 10:15:41 2006
@@ -253,12 +253,28 @@
 
     if ( $RT::MailCommand eq 'sendmailpipe' ) {
         eval {
-            open( my $mail, "|$RT::SendmailPath $RT::SendmailArguments" ) || die $!;
+            # don't ignore CHLD signal to get proper exit code
+            local $SIG{'CHLD'} = 'DEFAULT';
+
+            my $mail;
+            unless( open $mail, "|$RT::SendmailPath $RT::SendmailArguments" ) {
+                die "Couldn't run $RT::SendmailPath: $!";
+            }
+
+            # if something wrong with $mail->print we will get PIPE signal, handle it
+            local $SIG{'PIPE'} = sub { die "$RT::SendmailPath closed pipe" };
             $MIMEObj->print($mail);
-            close($mail);
+
+            unless ( close $mail ) {
+                die "Close failed: $!" if $!; # system error
+                # sendmail exit statuses mostly errors with data not software
+                # TODO: status parsing: core dump, exit on signal or EX_*
+                $RT::Logger->warning( "$RT::SendmailPath exitted with status $?" );
+            }
         };
         if ($@) {
-            $RT::Logger->crit( $msgid . "Could not send mail. -" . $@ );
+            $RT::Logger->crit( $msgid . "Could not send mail: " . $@ );
+            return 0;
         }
     }
     else {

Modified: rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Template_Overlay.pm
==============================================================================
--- rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Template_Overlay.pm	(original)
+++ rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Template_Overlay.pm	Thu Jan 19 10:15:41 2006
@@ -103,23 +103,11 @@
 
 sub _Set {
     my $self = shift;
-
-    # use super::value or we get acl blocked
-    if ( ( defined $self->SUPER::_Value('Queue') )
-        && ( $self->SUPER::_Value('Queue') == 0 ) )
-    {
-        unless ( $self->CurrentUser->HasRight( Object => $RT::System, Right => 'ModifyTemplate') ) {
-            return ( 0, $self->loc('Permission Denied') );
-        }
-    }
-    else {
-
-        unless ( $self->CurrentUserHasQueueRight('ModifyTemplate') ) {
-            return ( 0, $self->loc('Permission Denied') );
-        }
+    
+    unless ( $self->CurrentUserHasQueueRight('ModifyTemplate') ) {
+        return ( 0, $self->loc('Permission Denied') );
     }
-    return ( $self->SUPER::_Set(@_) );
-
+    return $self->SUPER::_Set( @_ );
 }
 
 # }}}
@@ -147,26 +135,12 @@
 =cut
 
 sub _Value {
-
     my $self  = shift;
-    my $field = shift;
 
-   
-    #If the current user doesn't have ACLs, don't let em at it.  
-    #use super::value or we get acl blocked
-    if ( ( !defined $self->__Value('Queue') )
-        || ( $self->__Value('Queue') == 0 ) )
-    {
-        unless ( $self->CurrentUser->HasRight( Object => $RT::System, Right => 'ShowTemplate') ) {
-            return (undef);
-        }
+    unless ( $self->CurrentUserHasQueueRight('ShowTemplate') ) {
+        return undef;
     }
-    else {
-        unless ( $self->CurrentUserHasQueueRight('ShowTemplate') ) {
-            return (undef);
-        }
-    }
-    return ( $self->__Value($field) );
+    return $self->__Value( @_ );
 
 }
 
@@ -183,18 +157,12 @@
 sub Load {
     my $self       = shift;
     my $identifier = shift;
+    return undef unless $identifier;
 
-    if ( !$identifier ) {
-        return (undef);
-    }
-
-    if ( $identifier !~ /\D/ ) {
-        $self->SUPER::LoadById($identifier);
-    }
-    else {
-        $self->LoadByCol( 'Name', $identifier );
-
+    if ( $identifier =~ /\D/ ) {
+        return $self->LoadByCol( 'Name', $identifier );
     }
+    return $self->LoadById( $identifier );
 }
 
 # }}}
@@ -260,32 +228,32 @@
         Content     => undef,
         Queue       => 0,
         Description => '[no description]',
-        Type => 'Action',    #By default, template are 'Action' templates
-        Name => undef,
+        Type        => 'Action', #By default, template are 'Action' templates
+        Name        => undef,
         @_
     );
 
-    if ( !$args{'Queue'}  ) {
+    unless ( $args{'Queue'} ) {
         unless ( $self->CurrentUser->HasRight(Right =>'ModifyTemplate', Object => $RT::System) ) {
-            return (undef);
+            return ( undef, $self->loc('Permission denied') );
         }
         $args{'Queue'} = 0;
     }
     else {
         my $QueueObj = new RT::Queue( $self->CurrentUser );
-        $QueueObj->Load( $args{'Queue'} ) || return ( 0, $self->loc('Invalid queue') );
+        $QueueObj->Load( $args{'Queue'} ) || return ( undef, $self->loc('Invalid queue') );
     
         unless ( $QueueObj->CurrentUserHasRight('ModifyTemplate') ) {
-            return (undef);
+            return ( undef, $self->loc('Permission denied') );
         }
         $args{'Queue'} = $QueueObj->Id;
     }
 
     my $result = $self->SUPER::Create(
-        Content => $args{'Content'},
-        Queue   =>  $args{'Queue'},
+        Content     => $args{'Content'},
+        Queue       => $args{'Queue'},
         Description => $args{'Description'},
-        Name        => $args{'Name'}
+        Name        => $args{'Name'},
     );
 
     return ($result);
@@ -340,15 +308,16 @@
     my $self = shift;
 
     #We're passing in whatever we were passed. it's destined for _ParseContent
-    my $content = $self->_ParseContent(@_);
+    my ($content, $msg) = $self->_ParseContent(@_);
+    return ( 0, $msg ) unless defined $content;
 
     #Lets build our mime Entity
 
     my $parser = MIME::Parser->new();
 
-        # On some situations TMPDIR is non-writable. sad but true.
-        $parser->output_to_core(1);
-        $parser->tmp_to_core(1);
+    # On some situations TMPDIR is non-writable. sad but true.
+    $parser->output_to_core(1);
+    $parser->tmp_to_core(1);
 
     #If someone includes a message, don't extract it
     $parser->extract_nested_messages(1);
@@ -363,15 +332,13 @@
     ### Should we forgive normally-fatal errors?
     $parser->ignore_errors(1);
     $self->{'MIMEObj'} = eval { $parser->parse_data($content) };
-    my $error = ( $@ || $parser->last_error );
-
-    if ($error) {
-        $RT::Logger->error("$error");
+    if ( my $error = $@ || $parser->last_error ) {
+        $RT::Logger->error( "$error" );
         return ( 0, $error );
     }
 
     # Unfold all headers
-    $self->{'MIMEObj'}->head->unfold();
+    $self->{'MIMEObj'}->head->unfold;
 
     return ( 1, $self->loc("Template parsed") );
 
@@ -400,9 +367,13 @@
     $T::rtname      = $RT::rtname;
     *T::loc         = sub { $T::Ticket->loc(@_) };
 
+    my $content = $self->Content;
+    unless ( defined $content ) {
+        return ( undef, $self->loc("Permissions denied") );
+    }
+
     # We need to untaint the content of the template, since we'll be working
     # with it
-    my $content = $self->Content();
     $content =~ s/^(.*)$/$1/;
     my $template = Text::Template->new(
         TYPE   => 'STRING',
@@ -413,11 +384,11 @@
     my $retval = $template->fill_in( PACKAGE => 'T', BROKEN => sub {
         my (%args) = @_;
         $RT::Logger->error("Template parsing error: $args{error}")
-	    unless $args{error} =~ /^Died at /; # ignore intentional die()
+            unless $args{error} =~ /^Died at /; # ignore intentional die()
         $is_broken++;
-	return undef;
+        return undef;
     } );
-    return undef if $is_broken;
+    return ( undef, $self->loc('Template parsing error') ) if $is_broken;
 
     # MIME::Parser has problems dealing with high-bit utf8 data.
     Encode::_utf8_off($retval);

Modified: rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Tickets_Overlay.pm
==============================================================================
--- rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Tickets_Overlay.pm	(original)
+++ rt/branches/QUEBEC-EXPERIMENTAL/lib/RT/Tickets_Overlay.pm	Thu Jan 19 10:15:41 2006
@@ -273,7 +273,7 @@
         or $op     eq "!=";
 
     my $meta = $FIELD_METADATA{$field};
-    if ( defined $meta->[1] ) {
+    if ( defined $meta->[1] && defined $value && $value !~ /^\d+$/ ) {
         my $class = "RT::" . $meta->[1];
         my $o     = $class->new( $sb->CurrentUser );
         $o->Load($value);
@@ -1467,12 +1467,11 @@
         @_
     );
 
-    #TODO  VALUE should also take queue names and queue objects
-    #TODO FIXME why are we canonicalizing to name, not id, robrt?
-    if ( $args{VALUE} =~ /^\d+$/ ) {
+    #TODO  VALUE should also take queue objects
+    if ( defined $args{'VALUE'} && $args{'VALUE'} !~ /^\d+$/ ) {
         my $queue = new RT::Queue( $self->CurrentUser );
         $queue->Load( $args{'VALUE'} );
-        $args{VALUE} = $queue->Name;
+        $args{'VALUE'} = $queue->Id;
     }
 
     # What if they pass in an Id?  Check for isNum() and convert to
@@ -1482,10 +1481,10 @@
 
     $self->Limit(
         FIELD       => 'Queue',
-        VALUE       => $args{VALUE},
+        VALUE       => $args{'VALUE'},
         OPERATOR    => $args{'OPERATOR'},
         DESCRIPTION => join(
-            ' ', $self->loc('Queue'), $args{'OPERATOR'}, $args{VALUE},
+            ' ', $self->loc('Queue'), $args{'OPERATOR'}, $args{'VALUE'},
         ),
     );
 


More information about the Rt-commit mailing list