[Rt-commit] rt branch, 4.4/txn-content-skip-mime-attachments, created. rt-4.4.4-34-g7ac5b6859
? sunnavy
sunnavy at bestpractical.com
Thu May 2 12:41:04 EDT 2019
The branch, 4.4/txn-content-skip-mime-attachments has been created
at 7ac5b6859a7a095283bbfa9c4a7e604642c9b157 (commit)
- Log -----------------------------------------------------------------
commit d6d4629df54f884d0e4dd8dd1e9444eeb4117dca
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/Transaction.pm b/lib/RT/Transaction.pm
index b027ef664..3b9aac788 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->GetHeader('Content-Disposition') // '' ) !~ /^\s*attachment\b/i
+ && $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->GetHeader('Content-Disposition') // '' ) =~ /^\s*attachment\b/i;
+
# If it's a textual part, just return the body.
if ( _IsDisplayableTextualContentType($Attachment->ContentType) ) {
return ($Attachment);
commit 7ac5b6859a7a095283bbfa9c4a7e604642c9b157
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 22c3cfe72..149b7bada 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 096d78e31..773f3afe8 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