[Rt-commit] rt branch, 4.4/txn-content-skip-mime-attachments, created. rt-4.4.4-34-g2505933f07

? sunnavy sunnavy at bestpractical.com
Fri Aug 14 15:13:37 EDT 2020


The branch, 4.4/txn-content-skip-mime-attachments has been created
        at  2505933f070b68f0bc59feb4e060f59c05359eb6 (commit)

- Log -----------------------------------------------------------------
commit 5dc91ca1477e4d8e10ab2406adb80835deff4dd7
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Jun 26 03:30:30 2018 +0800

    Exclude MIME attachments when questing a transaction's content
    
    It doesn't make sense to use MIME attachments' content here, what we
    really want is the main body of the MIME.

diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index 66a0e4e6d1..45bd479073 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -561,6 +561,18 @@ sub IsMessageContentType {
     return $self->ContentType =~ m{^\s*message/}i ? 1 : 0;
 }
 
+=head2 IsAttachmentContentDisposition
+
+Returns a boolean indicating if the Content-Disposition of this attachment
+is a MIME attachment.
+
+=cut
+
+sub IsAttachmentContentDisposition {
+    my $self = shift;
+    return ( $self->GetHeader('Content-Disposition') // '' ) =~ m{^\s*attachment\b}i ? 1 : 0;
+}
+
 =head2 Addresses
 
 Returns a hashref of all addresses related to this attachment.
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index b027ef6648..f60e9e548d 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -341,6 +341,9 @@ part. If $args{'Type'} is missing, it defaults to the value of
 C<$RT::Transaction::PreferredContentType>, if that's missing too, 
 defaults to textual.
 
+All the MIME attachments are excluded here, because it doesn't make much sense
+to use them as transaction content.
+
 =cut
 
 sub Content {
@@ -556,8 +559,10 @@ sub ContentObj {
     # displayable".  For now, this maintains backcompat
     my $all_parts = $self->Attachments;
     while ( my $part = $all_parts->Next ) {
-        next unless _IsDisplayableTextualContentType($part->ContentType)
-        && $part->Content;
+        next unless _IsDisplayableTextualContentType( $part->ContentType )
+          && !$part->Filename
+          && !$part->IsAttachmentContentDisposition
+          && $part->Content;
         return $part;
     }
 
@@ -572,6 +577,10 @@ sub _FindPreferredContentObj {
     # If we don't have any content, return undef now.
     return undef unless $Attachment;
 
+    # If it's a MIME attachment, return undef.
+    return undef if $Attachment->Filename;
+    return undef if $Attachment->IsAttachmentContentDisposition;
+
     # If it's a textual part, just return the body.
     if ( _IsDisplayableTextualContentType($Attachment->ContentType) ) {
         return ($Attachment);

commit 2505933f070b68f0bc59feb4e060f59c05359eb6
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Jun 26 03:35:11 2018 +0800

    Add tests for transaction content

diff --git a/t/api/transaction.t b/t/api/transaction.t
index 22c3cfe72f..149b7bada3 100644
--- a/t/api/transaction.t
+++ b/t/api/transaction.t
@@ -47,6 +47,108 @@ use_ok ('RT::Transaction');
                     "Caught URI warning";
 
     is( $brief, 'Reference to ticket 42 deleted', "Got string description: $brief");
+
+}
+
+diag 'Test Content';
+{
+    require MIME::Entity;
+
+    my $plain_file = File::Spec->catfile( RT::Test->temp_directory, 'attachment.txt' );
+    open my $plain_fh, '>', $plain_file or die $!;
+    print $plain_fh 'this is attachment';
+    close $plain_fh;
+
+    my @mime;
+
+    my $mime = MIME::Entity->build( Data => [ 'main body' ] );
+    push @mime, { object => $mime, expected => 'main body', description => 'no attachment' };
+
+    $mime = MIME::Entity->build( Type => 'multipart/mixed' );
+    $mime->attach(
+        Type => 'text/plain',
+        Data => [ 'main body' ],
+    );
+    $mime->attach(
+        Path => $plain_file,
+        Type => 'text/plain',
+    );
+    push @mime, { object => $mime, expected => 'main body', description => 'has an attachment' };
+
+    $mime = MIME::Entity->build( Type => 'multipart/mixed' );
+    $mime->attach(
+        Path => $plain_file,
+        Type => 'text/plain',
+    );
+    $mime->attach(
+        Type => 'text/plain',
+        Data => [ 'main body' ],
+    );
+    push @mime, { object => $mime, expected => 'main body', description => 'has an attachment as the first part' };
+
+    $mime = MIME::Entity->build( Type => 'multipart/mixed' );
+    $mime->attach(
+        Path => $plain_file,
+        Type => 'text/plain',
+    );
+    push @mime,
+      { object => $mime, expected => 'This transaction appears to have no content', description => 'has an attachment but no main part' };
+
+    my $parser = MIME::Parser->new();
+    $parser->output_to_core(1);
+    $mime = $parser->parse_data( <<EOF );
+Content-Type: multipart/mixed; boundary="=-=-="
+
+--=-=-=
+Content-Type: message/rfc822
+Content-Disposition: inline
+
+Content-Type: text/plain
+Subject: test
+
+main body
+--=-=-=
+EOF
+
+    push @mime, { object => $mime, expected => "main body", description => 'has an rfc822 message' };
+
+    $mime = $parser->parse_data( <<EOF );
+Content-Type: multipart/mixed; boundary="=-=-="
+
+--=-=-=
+Content-Type: message/rfc822
+Content-Disposition: attachment
+
+Content-Type: text/plain
+Subject: test
+
+inner body of rfc822
+
+--=-=-=
+Content-Type: text/plain
+Subject: test
+
+main body
+--=-=-=
+
+EOF
+
+    push @mime,
+      { object => $mime, expected => 'main body', description => 'has an attachment of rfc822 message and main part' };
+
+    for my $mime ( @mime ) {
+        my $ticket = RT::Ticket->new( RT->SystemUser );
+        my ( $id, $txn_id ) = $ticket->Create(
+            Queue   => 'General',
+            Subject => 'Testing content',
+            MIMEObj => $mime->{object},
+        );
+        ok( $id,     'Created ticket' );
+        ok( $txn_id, 'Created transaction' );
+        my $txn = RT::Transaction->new( RT->SystemUser );
+        $txn->Load( $txn_id );
+        is( $txn->Content, $mime->{expected}, "Got expected content for MIME: $mime->{description}" );
+    }
 }
 
 done_testing;
diff --git a/t/web/ticket_txn_content.t b/t/web/ticket_txn_content.t
index 096d78e314..773f3afe87 100644
--- a/t/web/ticket_txn_content.t
+++ b/t/web/ticket_txn_content.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use RT::Test tests => 63;
+use RT::Test;
 my $plain_file = File::Spec->catfile( RT::Test->temp_directory, 'attachment.txt' );
 open my $plain_fh, '>', $plain_file or die $!;
 print $plain_fh "this is plain content";
@@ -112,3 +112,42 @@ for my $type ( 'text/plain', 'text/html' ) {
     $m->content_contains("this is main reply content", 'email contains main reply content');
     $m->back;
 }
+
+$m->goto_create_ticket( $qid );
+$m->submit_form_ok(
+    {
+        form_name => 'TicketCreate',
+        fields    => {
+            Subject => 'with main body',
+            Content => 'this is main body',
+            Attach  => $plain_file,
+        }
+    },
+    'submit TicketCreate form'
+);
+$m->text_like( qr/Ticket \d+ created in queue/, 'ticket is created' );
+$m->content_contains( "Download $plain_name", 'download plain file link' );
+$m->follow_link_ok( { text => 'Reply', url_regex => qr/QuoteTransaction=/ }, 'reply the create transaction' );
+my $form    = $m->form_name( 'TicketUpdate' );
+my $content = $form->find_input( 'UpdateContent' );
+like( $content->value, qr/this is main body/, 'has transaction content' );
+
+$m->goto_create_ticket( $qid );
+$m->submit_form_ok(
+    {
+        form_name => 'TicketCreate',
+        fields    => {
+            Subject => 'without main body',
+            Attach  => $plain_file,
+        }
+    },
+    'submit TicketCreate form'
+);
+$m->text_like( qr/Ticket \d+ created in queue/, 'ticket is created' );
+$m->content_contains( "Download $plain_name", 'download plain file link' );
+$m->follow_link_ok( { text => 'Reply', url_regex => qr/QuoteTransaction=/ }, 'reply the create transaction' );
+$form    = $m->form_name( 'TicketUpdate' );
+$content = $form->find_input( 'UpdateContent' );
+like( $content->value, qr/This transaction appears to have no content/, 'no transaction content' );
+
+done_testing;

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


More information about the rt-commit mailing list