[Rt-commit] rt branch, 4.4/attach-from-transactions, created. rt-4.0.1-409-gf54f1b6
Alex Vandiver
alexmv at bestpractical.com
Mon Jul 22 23:30:38 EDT 2013
The branch, 4.4/attach-from-transactions has been created
at f54f1b6e56cb0dbbd347d037282c1e13c00e99f9 (commit)
- Log -----------------------------------------------------------------
commit 0f43ef5248c702fb14dae95dea1918039f1ac997
Author: Thomas Sibley <trs at bestpractical.com>
Date: Wed Aug 10 18:05:34 2011 -0400
Add the ability to include existing attachments when responding to a transaction
The ticket update page (and any other place which uses
/Ticket/Elements/AddAttachments) will offer up the named attachments
from the transaction being quoted (if any). Each selected attachment
causes an RT-Attach: id header to be added to the update.
The RT-Attach headers are inspected to actually add the attachments to
outgoing mail in RT::Action::SendEmail, as long as the creator of the
transaction with the RT-Attach headers can see the requested
attachments. If the outgoing template has a false RT-Attach-Message
value, then RT-Attach headers are skipped just like attachments directly
on the transaction.
Any RT-Attach headers in the template are merged with the ones from the
transaction. This makes it possible, if permissions are correct, to use
a template-level RT-Attach header to attach arbitrary content.
This feature is accessible via the Perl API by passing in the
AttachExisting option (as an arrayref) to RT::Ticket->Correspond and
RT::Ticket->Comment, or manually setting RT-Attach headers on
transaction content.
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index 553b736..fac2481 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -58,6 +58,7 @@ use base qw(RT::Action);
use RT::EmailParser;
use RT::Interface::Email;
use Email::Address;
+use List::MoreUtils qw(uniq);
our @EMAIL_RECIPIENT_HEADERS = qw(To Cc Bcc);
@@ -394,6 +395,51 @@ sub AddAttachments {
}
$self->AddAttachment($attach);
}
+
+ # attach any attachments requested by the transaction or template
+ # that aren't part of the transaction itself
+ $self->AddAttachmentsFromHeaders;
+}
+
+=head2 AddAttachmentsFromHeaders
+
+Add attachments requested by the transaction or template that aren't part of
+the transaction itself.
+
+This inspects C<RT-Attach> headers, which are expected to contain an
+L<RT::Attachment> ID that the transaction's creator can See.
+
+L<RT::Ticket->_RecordNote> accepts an C<AttachExisting> argument which sets
+C<RT-Attach> headers appropriately on Comment/Correspond.
+
+=cut
+
+sub AddAttachmentsFromHeaders {
+ my $self = shift;
+ my $orig = $self->TransactionObj->Attachments->First;
+ my $email = $self->TemplateObj->MIMEObj;
+
+ # Add the RT-Attach headers from the transaction to the email
+ if ($orig and $orig->GetHeader('RT-Attach')) {
+ for my $id ($orig->ContentAsMIME(Children => 0)->head->get_all('RT-Attach')) {
+ $email->head->add('RT-Attach' => $id);
+ }
+ }
+
+ # Take all RT-Attach headers and add the attachments to the outgoing mail
+ my $seen_attachment = 0;
+ for my $id (uniq $email->head->get_all('RT-Attach')) {
+ my $attach = RT::Attachment->new( $self->TransactionObj->CreatorObj );
+ $attach->Load($id);
+ next unless $attach->Id
+ and $attach->TransactionObj->CurrentUserCanSee;
+
+ if ( !$seen_attachment ) {
+ $email->make_multipart( 'mixed', Force => 1 );
+ $seen_attachment = 1;
+ }
+ $self->AddAttachment($attach, $email);
+ }
}
=head2 AddAttachment $attachment
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index cb22847..41af8ce 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -1521,7 +1521,8 @@ sub ProcessUpdateMessage {
Sign => ( $args{ARGSRef}->{'Sign'} ? 1 : 0 ),
Encrypt => ( $args{ARGSRef}->{'Encrypt'} ? 1 : 0 ),
MIMEObj => $Message,
- TimeTaken => $args{ARGSRef}->{'UpdateTimeWorked'}
+ TimeTaken => $args{ARGSRef}->{'UpdateTimeWorked'},
+ AttachExisting => $args{ARGSRef}->{'AttachExisting'},
);
_ProcessUpdateMessageRecipients(
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 8a3e78c..f1d0e19 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -2183,6 +2183,7 @@ sub _RecordNote {
TimeTaken => 0,
CommitScrips => 1,
SquelchMailTo => undef,
+ AttachExisting => [],
@_
);
@@ -2199,6 +2200,17 @@ sub _RecordNote {
# convert text parts into utf-8
RT::I18N::SetMIMEEntityToUTF8( $args{'MIMEObj'} );
+ # Set the magic RT headers which include existing attachments on this note
+ if ($args{'AttachExisting'}) {
+ $args{'AttachExisting'} = [$args{'AttachExisting'}]
+ if not ref $args{'AttachExisting'} eq 'ARRAY';
+
+ for my $attach (@{$args{'AttachExisting'}}) {
+ next if $attach =~ /\D/;
+ $args{'MIMEObj'}->head->add( 'RT-Attach' => $attach );
+ }
+ }
+
# If we've been passed in CcMessageTo and BccMessageTo fields,
# add them to the mime object for passing on to the transaction handler
# The "NotifyOtherRecipients" scripAction will look for RT-Send-Cc: and
diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index ba97a83..937730c 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -59,3 +59,32 @@
<tr><td class="label"><&|/l&>Attach</&>:</td><td><input name="Attach" type="file" /><input type="submit" class="button" name="AddMoreAttach" value="<&|/l&>Add More Files</&>" /><input type="hidden" class="hidden" name="UpdateAttach" value="1" />
</td></tr>
+% if (@quoted_attachments) {
+<tr>
+ <td class="label"><&|/l&>Include attachments</&>:</td>
+ <td>
+% for my $attach (@quoted_attachments) {
+ <label>
+ <input type="checkbox" class="checkbox" name="AttachExisting" value="<% $attach->Id %>" \
+ <% (grep { $attach->Id == $_ } @AttachExisting) ? 'checked' : '' %> />
+ <% $attach->Filename %>
+ </label>
+% }
+ </td>
+</tr>
+% }
+<%init>
+my @quoted_attachments;
+if ($QuoteTransaction) {
+ my $txn = RT::Transaction->new( $session{'CurrentUser'} );
+ $txn->Load($QuoteTransaction);
+ if ($txn->Id and $txn->CurrentUserCanSee) {
+ @quoted_attachments = grep { defined $_->Filename and length $_->Filename }
+ @{$txn->Attachments->ItemsArrayRef};
+ }
+}
+</%init>
+<%args>
+ at AttachExisting => ()
+$QuoteTransaction => ''
+</%args>
commit 7b0cb858d1848cfa2ce53745fc377279a1ba9124
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Aug 11 12:40:51 2011 -0400
Simplify the copying of RT-Attach headers from the txn into the mail
By adding an RT::Attachment->GetAllHeaders method comparable to
MIME::Head->get_all.
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index fac2481..f162fab 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -416,12 +416,11 @@ C<RT-Attach> headers appropriately on Comment/Correspond.
sub AddAttachmentsFromHeaders {
my $self = shift;
- my $orig = $self->TransactionObj->Attachments->First;
my $email = $self->TemplateObj->MIMEObj;
# Add the RT-Attach headers from the transaction to the email
- if ($orig and $orig->GetHeader('RT-Attach')) {
- for my $id ($orig->ContentAsMIME(Children => 0)->head->get_all('RT-Attach')) {
+ if (my $attachment = $self->TransactionObj->Attachments->First) {
+ for my $id ($attachment->GetAllHeaders('RT-Attach')) {
$email->head->add('RT-Attach' => $id);
}
}
diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index 1feb632..392d8f3 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -580,8 +580,8 @@ sub EncodedHeaders {
=head2 GetHeader $TAG
-Returns the value of the header Tag as a string. This bypasses the weeding out
-done in Headers() above.
+Returns the value of the B<first> header Tag as a string. This bypasses the
+weeding out done in Headers() above.
=cut
@@ -599,6 +599,24 @@ sub GetHeader {
return undef;
}
+=head2 GetAllHeaders $TAG
+
+Returns a list of all values for the the given header tag, in the order they
+appear.
+
+=cut
+
+sub GetAllHeaders {
+ my $self = shift;
+ my $tag = shift;
+ my @values = ();
+ foreach my $line ($self->_SplitHeaders) {
+ next unless $line =~ /^\Q$tag\E:\s+(.*)$/si;
+ push @values, $1;
+ }
+ return @values;
+}
+
=head2 DelHeader $TAG
Delete a field from the attachment's headers.
commit af0c0adbb7bd577a8e2fec89eb04e51f9dc136d3
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Aug 11 12:42:11 2011 -0400
Each include attachment checkbox and filename gets its own line
diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index 937730c..a8f2878 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -61,14 +61,14 @@
% if (@quoted_attachments) {
<tr>
- <td class="label"><&|/l&>Include attachments</&>:</td>
+ <td class="label" valign="top"><&|/l&>Include attachments</&>:</td>
<td>
% for my $attach (@quoted_attachments) {
<label>
<input type="checkbox" class="checkbox" name="AttachExisting" value="<% $attach->Id %>" \
<% (grep { $attach->Id == $_ } @AttachExisting) ? 'checked' : '' %> />
<% $attach->Filename %>
- </label>
+ </label><br />
% }
</td>
</tr>
commit ea61a555cc95a4423d8a94f1aa32751f88704a71
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Aug 11 13:17:25 2011 -0400
Display RT-Attach headers in history by linking the filename
It is possible recipients of the transaction saw the attachment via
email but can't see it via the web. This should be improved in the
future, but it's an acceptable edge case at the moment since we avoid
storing the attachment twice.
diff --git a/share/html/Ticket/Elements/ShowMessageHeaders b/share/html/Ticket/Elements/ShowMessageHeaders
index 943b567..5220f9a 100755
--- a/share/html/Ticket/Elements/ShowMessageHeaders
+++ b/share/html/Ticket/Elements/ShowMessageHeaders
@@ -78,6 +78,17 @@ unless ( $display_headers{'_all'} ) {
my $ticket = $Message->TransactionObj->TicketObj;
foreach my $f (@headers) {
$m->comp('/Elements/MakeClicky', content => \$f->{'Value'}, ticket => $ticket, %ARGS);
+ if ($f->{'Tag'} eq 'RT-Attach') {
+ # Blat in the filename and linkify
+ my $att = RT::Attachment->new( $session{'CurrentUser'} );
+ $att->Load($f->{'Value'});
+ next unless $att->Id and $att->TransactionObj->CurrentUserCanSee;
+
+ $f->{'Value'} = sprintf '<a href="%s/Ticket/Attachment/%d/%d/%s">%s</a>',
+ RT->Config->Get('WebPath'), $att->TransactionObj->Id, $att->Id,
+ $m->interp->apply_escapes($att->Filename, qw(u h)),
+ $m->interp->apply_escapes($att->Filename, 'h');
+ }
}
if ( $Localize ) {
diff --git a/share/html/Ticket/Elements/ShowTransactionAttachments b/share/html/Ticket/Elements/ShowTransactionAttachments
index ccf35fd..3569003 100644
--- a/share/html/Ticket/Elements/ShowTransactionAttachments
+++ b/share/html/Ticket/Elements/ShowTransactionAttachments
@@ -118,7 +118,7 @@ if ( $Transaction->Type =~ /EmailRecord$/ ) {
# If the transaction has anything attached to it at all
elsif (!$ShowHeaders) {
- @DisplayHeaders = qw(To From RT-Send-Cc Cc Bcc Date Subject);
+ @DisplayHeaders = qw(To From RT-Send-Cc Cc Bcc RT-Attach Date Subject);
push @DisplayHeaders, 'RT-Send-Bcc' if RT->Config->Get('ShowBccHeader');
}
commit d8877810cc4900fda296319e52333ff39f7e295e
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Aug 11 18:25:30 2011 -0400
Consistently order the existing attachment checkboxes by name
Uses the same order as /Ticket/Elements/ShowAttachments.
diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index a8f2878..7bf1081 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -79,7 +79,8 @@ if ($QuoteTransaction) {
my $txn = RT::Transaction->new( $session{'CurrentUser'} );
$txn->Load($QuoteTransaction);
if ($txn->Id and $txn->CurrentUserCanSee) {
- @quoted_attachments = grep { defined $_->Filename and length $_->Filename }
+ @quoted_attachments = sort { lc($a->Filename) cmp lc($b->Filename) }
+ grep { defined $_->Filename and length $_->Filename }
@{$txn->Attachments->ItemsArrayRef};
}
}
commit 4ad6fdfcb1cb90ab9a62b46223a916156e9fa729
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Aug 11 18:26:20 2011 -0400
Tests for the new RT-Attach functionality
diff --git a/t/data/image.png b/t/data/image.png
new file mode 100644
index 0000000..8a87374
Binary files /dev/null and b/t/data/image.png differ
diff --git a/t/data/owls.jpg b/t/data/owls.jpg
new file mode 100644
index 0000000..33c6558
Binary files /dev/null and b/t/data/owls.jpg differ
diff --git a/t/web/attach-from-txn.t b/t/web/attach-from-txn.t
new file mode 100644
index 0000000..eef1a46
--- /dev/null
+++ b/t/web/attach-from-txn.t
@@ -0,0 +1,125 @@
+use strict;
+
+use RT::Test tests => 46;
+
+my $LogoName = 'image.png';
+my $ImageName = 'owls.jpg';
+my $LogoFile = RT::Test::get_relocatable_file($LogoName, '..', 'data');
+my $ImageFile = RT::Test::get_relocatable_file($ImageName, '..', 'data');
+
+# reply to ticket = nothing
+# reply to correspond = getting the right list
+# maintaining the checked ones
+# storing the header
+# getting attached to mail
+# showing up in the web history
+
+my ($baseurl, $m) = RT::Test->started_ok;
+ok $m->login, 'logged in';
+
+my $queue = RT::Queue->new(RT->Nobody);
+my $qid = $queue->Load('General');
+ok( $qid, "Loaded General queue" );
+
+# Create ticket
+$m->form_name('CreateTicketInQueue');
+$m->field('Queue', $qid);
+$m->field('Requestors', 'owls at localhost');
+$m->submit;
+is($m->status, 200, "request successful");
+$m->content_contains("Create a new ticket", 'ticket create page');
+
+$m->form_name('TicketCreate');
+$m->field('Subject', 'Attachments test');
+$m->field('Content', 'Some content');
+$m->submit;
+is($m->status, 200, "request successful");
+
+$m->content_contains('Attachments test', 'we have subject on the page');
+$m->content_contains('Some content', 'and content');
+
+# Reply with uploaded attachments
+$m->follow_link_ok({text => 'Reply'}, "reply to the ticket");
+$m->content_lacks('AttachExisting');
+$m->form_name('TicketUpdate');
+$m->field('Attach', $LogoFile);
+$m->click('AddMoreAttach');
+is($m->status, 200, "request successful");
+
+$m->form_name('TicketUpdate');
+$m->field('Attach', $ImageFile);
+$m->field('UpdateContent', 'Message');
+$m->click('SubmitTicket');
+is($m->status, 200, "request successful");
+
+$m->content_contains("Download $LogoName", 'page has file name');
+$m->content_contains("Download $ImageName", 'page has file name');
+
+# clear mail catcher
+RT::Test->fetch_caught_mails;
+
+# Reply to first correspondence, including an attachment
+$m->follow_link_ok({text => 'Reply', n => 3}, "reply to the reply");
+$m->content_contains('AttachExisting');
+$m->content_contains($LogoName);
+$m->content_contains($ImageName);
+# check stuff
+$m->form_name('TicketUpdate');
+$m->current_form->find_input('AttachExisting', 'checkbox', 2)->check; # owls.jpg
+$m->click('AddMoreAttach');
+is($m->status, 200, "request successful");
+
+# ensure it's still checked
+$m->form_name('TicketUpdate');
+ok $m->current_form->find_input('AttachExisting', 'checkbox', 2)->value, 'still checked';
+$m->field('UpdateContent', 'Here are some attachments');
+$m->click('SubmitTicket');
+is($m->status, 200, "request successful");
+
+# yep, we got it and processed the header!
+$m->content_contains('Here are some attachments');
+$m->content_like(qr/RT-Attach:.+?\Q$ImageName\E/s, 'found rt attach header');
+
+# outgoing looks good
+$m->follow_link_ok({text => 'Show', n => 3}, "found show link");
+$m->content_like(qr/RT-Attach: \d+/, "found RT-Attach header");
+$m->content_like(qr/RT-Attachment: \d+\/\d+\/\d+/, "found RT-Attachment header");
+$m->content_lacks($ImageName);
+$m->back;
+
+# check that it got into mail
+my @mails = RT::Test->fetch_caught_mails;
+is scalar @mails, 1, "got one outgoing email";
+my $mail = shift @mails;
+like $mail, qr/To: owls\@localhost/, 'got To';
+like $mail, qr/RT-Attach: \d+/, "found attachment we expected";
+like $mail, qr/RT-Attachment: \d+\/\d+\/\d+/, "found RT-Attachment header";
+like $mail, qr/filename=.?\Q$ImageName\E.?/, "found filename";
+
+# add header to template, make a normal reply, and see that it worked
+my $link = $m->find_link(text_regex => qr/\Q$LogoName\E/, url_regex => qr/Attachment/);
+ok $link;
+my ($id) = $link->url =~ /Attachment\/\d+\/(\d+)/;
+ok $id;
+my $template = RT::Template->new( RT->SystemUser );
+$template->LoadGlobalTemplate('Correspondence');
+ok $template->Id;
+$template->SetContent( "RT-Attach: $id\n" . $template->Content );
+like $template->Content, qr/RT-Attach/, "updated template";
+
+# reply...
+$m->follow_link_ok({text => 'Reply'}, "reply to the ticket");
+$m->form_name('TicketUpdate');
+$m->field('UpdateContent', 'who gives a hoot');
+$m->click('SubmitTicket');
+is($m->status, 200, "request successful");
+$m->content_contains('who gives a hoot');
+
+# then see if we got the right mail
+my @mails = RT::Test->fetch_caught_mails;
+is scalar @mails, 1, "got one outgoing email";
+my $mail = shift @mails;
+like $mail, qr/To: owls\@localhost/, 'got To';
+like $mail, qr/RT-Attach: $id/, "found attachment we expected";
+like $mail, qr/RT-Attachment: \d+\/\d+\/$id/, "found RT-Attachment header";
+like $mail, qr/filename=.?\Q$LogoName\E.?/, "found filename";
commit 9fe31d11eec302d5f663cf1436342419175af753
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Aug 11 18:47:48 2011 -0400
Test that users who can't see an attachment can't attach it
diff --git a/t/web/attach-from-txn.t b/t/web/attach-from-txn.t
index eef1a46..5f7bc12 100644
--- a/t/web/attach-from-txn.t
+++ b/t/web/attach-from-txn.t
@@ -1,6 +1,7 @@
use strict;
+use warnings;
-use RT::Test tests => 46;
+use RT::Test tests => 54;
my $LogoName = 'image.png';
my $ImageName = 'owls.jpg';
@@ -99,13 +100,14 @@ like $mail, qr/filename=.?\Q$ImageName\E.?/, "found filename";
# add header to template, make a normal reply, and see that it worked
my $link = $m->find_link(text_regex => qr/\Q$LogoName\E/, url_regex => qr/Attachment/);
ok $link;
-my ($id) = $link->url =~ /Attachment\/\d+\/(\d+)/;
-ok $id;
+my ($LogoId) = $link->url =~ /Attachment\/\d+\/(\d+)/;
+ok $LogoId;
my $template = RT::Template->new( RT->SystemUser );
$template->LoadGlobalTemplate('Correspondence');
ok $template->Id;
-$template->SetContent( "RT-Attach: $id\n" . $template->Content );
-like $template->Content, qr/RT-Attach/, "updated template";
+my $old_template = $template->Content;
+$template->SetContent( "RT-Attach: $LogoId\n" . $template->Content );
+like $template->Content, qr/RT-Attach:/, "updated template";
# reply...
$m->follow_link_ok({text => 'Reply'}, "reply to the ticket");
@@ -120,6 +122,31 @@ my @mails = RT::Test->fetch_caught_mails;
is scalar @mails, 1, "got one outgoing email";
my $mail = shift @mails;
like $mail, qr/To: owls\@localhost/, 'got To';
-like $mail, qr/RT-Attach: $id/, "found attachment we expected";
-like $mail, qr/RT-Attachment: \d+\/\d+\/$id/, "found RT-Attachment header";
+like $mail, qr/RT-Attach: $LogoId/, "found attachment we expected";
+like $mail, qr/RT-Attachment: \d+\/\d+\/$LogoId/, "found RT-Attachment header";
like $mail, qr/filename=.?\Q$LogoName\E.?/, "found filename";
+
+# create a new, privileged user who can't see this ticket but can reply
+$template->SetContent($old_template);
+unlike $template->Content, qr/RT-Attach:/, "no header in template anymore";
+
+my $peter = RT::Test->load_or_create_user(
+ Name => 'peter',
+ EmailAddress => 'peter at localhost',
+);
+ok( RT::Test->add_rights({ Principal => 'Everyone', Right => [qw(ReplyToTicket)] }), 'add ReplyToTicket rights');
+
+my $ticket = RT::Ticket->new($peter);
+$ticket->Load(1);
+ok $ticket->Id, "loaded ticket";
+
+my ($ok, $msg, $txn) = $ticket->Correspond( AttachExisting => $LogoId, Content => 'Hi' );
+ok $ok, $msg;
+
+# check mail that went out doesn't contain the logo
+my @mails = RT::Test->fetch_caught_mails;
+is scalar @mails, 1, "got one outgoing email";
+my $mail = shift @mails;
+like $mail, qr/RT-Attach: $LogoId/, "found header we expected";
+unlike $mail, qr/RT-Attachment: \d+\/\d+\/$LogoId/, "lacks RT-Attachment header";
+unlike $mail, qr/filename=.?\Q$LogoName\E.?/, "lacks filename";
commit 2844b2af88e8739fdb642ebb082e15d3bd87f4fb
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Aug 11 19:21:44 2011 -0400
Avoid some "masks earlier declaration" warnings
diff --git a/t/web/attach-from-txn.t b/t/web/attach-from-txn.t
index 5f7bc12..9491326 100644
--- a/t/web/attach-from-txn.t
+++ b/t/web/attach-from-txn.t
@@ -118,9 +118,9 @@ is($m->status, 200, "request successful");
$m->content_contains('who gives a hoot');
# then see if we got the right mail
-my @mails = RT::Test->fetch_caught_mails;
+ at mails = RT::Test->fetch_caught_mails;
is scalar @mails, 1, "got one outgoing email";
-my $mail = shift @mails;
+$mail = shift @mails;
like $mail, qr/To: owls\@localhost/, 'got To';
like $mail, qr/RT-Attach: $LogoId/, "found attachment we expected";
like $mail, qr/RT-Attachment: \d+\/\d+\/$LogoId/, "found RT-Attachment header";
@@ -144,9 +144,9 @@ my ($ok, $msg, $txn) = $ticket->Correspond( AttachExisting => $LogoId, Content =
ok $ok, $msg;
# check mail that went out doesn't contain the logo
-my @mails = RT::Test->fetch_caught_mails;
+ at mails = RT::Test->fetch_caught_mails;
is scalar @mails, 1, "got one outgoing email";
-my $mail = shift @mails;
+$mail = shift @mails;
like $mail, qr/RT-Attach: $LogoId/, "found header we expected";
unlike $mail, qr/RT-Attachment: \d+\/\d+\/$LogoId/, "lacks RT-Attachment header";
unlike $mail, qr/filename=.?\Q$LogoName\E.?/, "lacks filename";
commit d164d13c060afa8a392e14dbd3c2e93e7067657e
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Sep 19 13:43:41 2011 -0400
Headers often come with leading spaces, which make SQLite choke on comparisons
This was noticed because of test failures, but does affect functionality
on SQLite. Pg and MySQL can Do The Right Thing since they know they're
dealing with numeric types.
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index f162fab..3c32419 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -428,6 +428,8 @@ sub AddAttachmentsFromHeaders {
# Take all RT-Attach headers and add the attachments to the outgoing mail
my $seen_attachment = 0;
for my $id (uniq $email->head->get_all('RT-Attach')) {
+ $id =~ s/(?:^\s*|\s*$)//g;
+
my $attach = RT::Attachment->new( $self->TransactionObj->CreatorObj );
$attach->Load($id);
next unless $attach->Id
diff --git a/share/html/Ticket/Elements/ShowMessageHeaders b/share/html/Ticket/Elements/ShowMessageHeaders
index 5220f9a..4d3baa5 100755
--- a/share/html/Ticket/Elements/ShowMessageHeaders
+++ b/share/html/Ticket/Elements/ShowMessageHeaders
@@ -79,6 +79,8 @@ my $ticket = $Message->TransactionObj->TicketObj;
foreach my $f (@headers) {
$m->comp('/Elements/MakeClicky', content => \$f->{'Value'}, ticket => $ticket, %ARGS);
if ($f->{'Tag'} eq 'RT-Attach') {
+ $f->{'Value'} =~ s/(?:^\s*|\s*$)//g;
+
# Blat in the filename and linkify
my $att = RT::Attachment->new( $session{'CurrentUser'} );
$att->Load($f->{'Value'});
commit ae16e9720470778211f6b6802413a8417506104b
Author: Thomas Sibley <trs at bestpractical.com>
Date: Tue Oct 18 13:25:06 2011 -0400
Link to the download page for existing attachments
This lets people quickly verify the attachments they're including.
diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index 7bf1081..aeec6a2 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -67,7 +67,12 @@
<label>
<input type="checkbox" class="checkbox" name="AttachExisting" value="<% $attach->Id %>" \
<% (grep { $attach->Id == $_ } @AttachExisting) ? 'checked' : '' %> />
- <% $attach->Filename %>
+<%perl>
+my $url = sprintf '%s/Ticket/Attachment/%d/%d/%s',
+ RT->Config->Get('WebPath'), $attach->TransactionObj->Id, $attach->Id,
+ $m->interp->apply_escapes($attach->Filename, 'u');
+</%perl>
+ <a href="<% $url %>" target="_blank"><% $attach->Filename %></a>
</label><br />
% }
</td>
commit dbcab6000ccbac84d41ae7173a1735c9dcb2aaa0
Author: Thomas Sibley <trs at bestpractical.com>
Date: Wed Oct 19 13:24:14 2011 -0400
Move the link out of the label so clicking doesn't both open the attachment and select it for inclusion
I'm not thrilled by the (View) solution, but I'm at a loss for something
better at the moment. The filename _should_ be the checkbox label, and
clicking it should tick the checkbox, but not necessarily open the
attachment.
diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index aeec6a2..04a4c40 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -67,13 +67,15 @@
<label>
<input type="checkbox" class="checkbox" name="AttachExisting" value="<% $attach->Id %>" \
<% (grep { $attach->Id == $_ } @AttachExisting) ? 'checked' : '' %> />
+ <% $attach->Filename %>
+ </label>
<%perl>
my $url = sprintf '%s/Ticket/Attachment/%d/%d/%s',
RT->Config->Get('WebPath'), $attach->TransactionObj->Id, $attach->Id,
$m->interp->apply_escapes($attach->Filename, 'u');
</%perl>
- <a href="<% $url %>" target="_blank"><% $attach->Filename %></a>
- </label><br />
+ (<a href="<% $url %>" target="_blank"><&|/l&>View</&></a>)
+ <br />
% }
</td>
</tr>
commit 923d73c8660a66f5919fd1c953b448536115e3aa
Author: Thomas Sibley <trs at bestpractical.com>
Date: Fri Dec 9 10:46:55 2011 -0500
Don't present transaction attachments when we don't have an existing ticket
QuoteTransaction may be used on Ticket/Create.html, but we don't
currently handle including attachments from a transaction on create,
only on update.
diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index 04a4c40..8b93519 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -82,7 +82,7 @@ my $url = sprintf '%s/Ticket/Attachment/%d/%d/%s',
% }
<%init>
my @quoted_attachments;
-if ($QuoteTransaction) {
+if ($QuoteTransaction and $TicketObj and $TicketObj->id) {
my $txn = RT::Transaction->new( $session{'CurrentUser'} );
$txn->Load($QuoteTransaction);
if ($txn->Id and $txn->CurrentUserCanSee) {
@@ -95,4 +95,5 @@ if ($QuoteTransaction) {
<%args>
@AttachExisting => ()
$QuoteTransaction => ''
+$TicketObj => undef
</%args>
commit 52dcce0024e9a336666390bc51b856439c303ac8
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Feb 8 16:05:47 2012 +0800
email could be multipart already before calling AddAttachmentsFromHeaders
the bug's behavior is: when people select attachments and upload some too at
the same time, the uploaded attachments will leak to EmailRecord txn, which
results in duplicates for uploaded ones.
this is because in RT::Action::SendEmail::RecordOutgoingMailTransaction, we
filter top parts of email by checking their "RT-Attachment" headers, so we
can't make multipart again if the email is already multipart.
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index 3c32419..665e9a1 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -426,7 +426,6 @@ sub AddAttachmentsFromHeaders {
}
# Take all RT-Attach headers and add the attachments to the outgoing mail
- my $seen_attachment = 0;
for my $id (uniq $email->head->get_all('RT-Attach')) {
$id =~ s/(?:^\s*|\s*$)//g;
@@ -435,9 +434,8 @@ sub AddAttachmentsFromHeaders {
next unless $attach->Id
and $attach->TransactionObj->CurrentUserCanSee;
- if ( !$seen_attachment ) {
+ if ( !$email->is_multipart ) {
$email->make_multipart( 'mixed', Force => 1 );
- $seen_attachment = 1;
}
$self->AddAttachment($attach, $email);
}
commit f54f1b6e56cb0dbbd347d037282c1e13c00e99f9
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Feb 8 21:17:53 2012 +0800
test attachments selection with uploaded ones in the mean time
diff --git a/t/web/attach-from-txn.t b/t/web/attach-from-txn.t
index 9491326..b0de370 100644
--- a/t/web/attach-from-txn.t
+++ b/t/web/attach-from-txn.t
@@ -1,7 +1,7 @@
use strict;
use warnings;
-use RT::Test tests => 54;
+use RT::Test tests => 70;
my $LogoName = 'image.png';
my $ImageName = 'owls.jpg';
@@ -97,6 +97,38 @@ like $mail, qr/RT-Attach: \d+/, "found attachment we expected";
like $mail, qr/RT-Attachment: \d+\/\d+\/\d+/, "found RT-Attachment header";
like $mail, qr/filename=.?\Q$ImageName\E.?/, "found filename";
+# Reply to first correspondence, including an attachment with an uploaded one
+$m->follow_link_ok({text => 'Reply', n => 3}, "reply to the reply");
+$m->form_name('TicketUpdate');
+$m->current_form->find_input('AttachExisting', 'checkbox', 2)->check; # owls.jpg
+$m->field( 'UpdateContent', 'attachments from both list and upload' );
+$m->field('Attach', $LogoFile);
+$m->click('SubmitTicket');
+is($m->status, 200, "request successful");
+
+# yep, we got it and processed the header!
+$m->content_contains('attachments from both list and upload');
+$m->content_like(qr/(RT-Attach:.+?\Q$ImageName\E).*\1/s, 'found rt attach header');
+$m->content_like(qr/Subject:.+?\Q$LogoName\E/s, 'found rt attach header');
+
+# outgoing looks good
+$m->follow_link_ok({text => 'Show', n => 4}, "found show link");
+$m->content_like(qr/RT-Attach: \d+/, "found RT-Attach header");
+$m->content_like(qr/RT-Attachment: \d+\/\d+\/\d+/, "found RT-Attachment header");
+$m->content_lacks($ImageName);
+$m->content_lacks($LogoName);
+$m->back;
+
+# check that it got into mail
+ at mails = RT::Test->fetch_caught_mails;
+is scalar @mails, 1, "got one outgoing email";
+$mail = shift @mails;
+like $mail, qr/To: owls\@localhost/, 'got To';
+like $mail, qr/RT-Attach: \d+/, "found attachment we expected";
+like $mail, qr/RT-Attachment: \d+\/\d+\/\d+/, "found RT-Attachment header";
+like $mail, qr/filename=.?\Q$ImageName\E.?/, "found selected filename";
+like $mail, qr/filename=.?\Q$LogoName\E.?/, "found uploaded filename";
+
# add header to template, make a normal reply, and see that it worked
my $link = $m->find_link(text_regex => qr/\Q$LogoName\E/, url_regex => qr/Attachment/);
ok $link;
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list