[Rt-commit] rt branch, 5.0-trunk, updated. rt-5.0.0alpha1-213-gd0b0fdfdd6
? sunnavy
sunnavy at bestpractical.com
Fri May 1 19:34:37 EDT 2020
The branch, 5.0-trunk has been updated
via d0b0fdfdd648b4b8d2235d79f32378db75dbe247 (commit)
via cd8dbde9764028533e41b72bd866d991ab627e22 (commit)
via 9e90e551faafa1a01044ed1b7d444f960c88cf13 (commit)
from 66b6e91580eaac161523754713801c6fc6e405ea (commit)
Summary of changes:
share/html/Elements/ShowTransactionAttachments | 29 ++++++------
share/static/css/elevator-light/history.css | 9 ++--
t/web/attach-from-txn.t | 4 +-
t/web/attachments.t | 63 +++++++++++++-------------
t/web/csrf.t | 2 +-
t/web/ticket_txn_content.t | 4 +-
6 files changed, 54 insertions(+), 57 deletions(-)
- Log -----------------------------------------------------------------
commit 9e90e551faafa1a01044ed1b7d444f960c88cf13
Author: Blaine Motsinger <blaine at bestpractical.com>
Date: Fri Apr 24 18:21:16 2020 -0500
Display the attachment name in ticket history
If a ticket has a lot of attachments it can be cumbersome to hover
over the download link to find the file you want. This commit makes
the filename available on the page and moves the filetype and size
into the tooltip instead.
This commit additionally corrects an alignment issue where the
downloadattachment div overlapped the above element's border if
there were many attachments in one transaction.
diff --git a/share/html/Elements/ShowTransactionAttachments b/share/html/Elements/ShowTransactionAttachments
index 53283d2fa2..610d17bca5 100644
--- a/share/html/Elements/ShowTransactionAttachments
+++ b/share/html/Elements/ShowTransactionAttachments
@@ -64,22 +64,22 @@ foreach my $message ( @{ $Attachments->{ $Parent || 0 } || [] } ) {
</%PERL>
<div class="downloadattachment">
% if (my $url = RT->System->ExternalStorageURLFor($message)) {
-<a href="<% $url %>">
+<a href="<% $url %>"
% } else {
-<a href="<% $AttachmentPath %>/<% $Transaction->Id %>/<% $message->Id %>/<% $name | un %>" target="_blank">
+<a href="<% $AttachmentPath %>/<% $Transaction->Id %>/<% $message->Id %>/<% $name | un %>" target="_blank"
% }
-% # the defined name denotes a downloadable attachment, else just text content in the transaction.
-% my ( $download_alt, $fa_classes );
-% if ( length $name ) {
-% $download_alt = loc( 'Download [_1]', $name );
-% $fa_classes = 'fas fa-paperclip';
-% }
-% else {
-% $download_alt = loc( 'View source' );
-% $fa_classes = 'far fa-file';
+% if ( length $name ) { # download link with filename
+% my $download_alt = loc( 'Download [_1] [_2]', $message->ContentType, $message->FriendlyContentLength );
+alt="<% $download_alt %>" data-toggle="tooltip" data-placement="bottom" data-original-title="<% $download_alt %>">
+ <span class="fas fa-paperclip fa-2x"></span>
+ <span class="downloadfilename"><% $name %></span>
+</a>
% }
-<span class="<% $fa_classes %> fa-2x" alt="<% $download_alt %>" data-toggle="tooltip" data-placement="bottom" data-original-title="<% $download_alt %>"></span></a>
-
+% else { # view source and view source headers, without filename or size
+% my $view_source_alt = loc( 'View source' );
+>
+ <span class="far fa-file fa-2x" alt="<% $view_source_alt %>" data-toggle="tooltip" data-placement="bottom" data-original-title="<% $view_source_alt %>"></span>
+</a>
% if ( $DownloadableHeaders && ! length $name && $message->ContentType =~ /text/ ) {
% my $download_with_headers_alt = loc('View source with headers');
<a href="<% $AttachmentPath %>/WithHeaders/<% $message->Id %>" target="_blank">
@@ -89,10 +89,9 @@ foreach my $message ( @{ $Attachments->{ $Parent || 0 } || [] } ) {
</span>
</a>
% }
-
+% }
% $m->callback(CallbackName => 'AfterDownloadLinks', ARGSRef => \%ARGS, Object => $Object, Transaction => $Transaction, Attachment => $message);
<br />
-<span class="downloadcontenttype"><% $message->ContentType %> <% $message->FriendlyContentLength %></span>
</div>
% }
%# If there is sub-messages, open a dedicated div
diff --git a/share/static/css/elevator-light/history.css b/share/static/css/elevator-light/history.css
index 794e46f189..91b7b7e96b 100644
--- a/share/static/css/elevator-light/history.css
+++ b/share/static/css/elevator-light/history.css
@@ -165,14 +165,11 @@ border: none;
vertical-align: top;
}
-.transaction div.downloadattachment .downloadcontenttype{
- color: #ccc;
- display: block;
- margin-top: 0.25rem;
-padding-right:0.25em;
+.transaction div.downloadattachment .downloadfilename {
+ font-size: 0.9rem;
+ padding-left: 0.1em;
}
-
.transaction .message-header-key {
width: 7em;
font-weight: bold;
commit cd8dbde9764028533e41b72bd866d991ab627e22
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat May 2 07:20:24 2020 +0800
Update attachment link checking code in tests as we updated link DOM
diff --git a/t/web/attach-from-txn.t b/t/web/attach-from-txn.t
index a19e0e250e..502f5c9cb7 100644
--- a/t/web/attach-from-txn.t
+++ b/t/web/attach-from-txn.t
@@ -53,8 +53,8 @@ $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');
+ok( $m->find_link( text => $LogoName, url_regex => qr{Attachment/} ), 'page has file link');
+ok( $m->find_link( text => $ImageName, url_regex => qr{Attachment/} ), 'page has file link');
# clear mail catcher
RT::Test->fetch_caught_mails;
diff --git a/t/web/attachments.t b/t/web/attachments.t
index 4224e42b02..b6c1b06db3 100644
--- a/t/web/attachments.t
+++ b/t/web/attachments.t
@@ -39,7 +39,7 @@ diag "with one attachment";
$m->content_contains('Attachments test', 'we have subject on the page');
$m->content_contains('Some content', 'and content');
- $m->content_contains('Download bpslogo.png', 'page has file name');
+ ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
diag "with two attachments";
@@ -61,8 +61,8 @@ diag "with two attachments";
$m->content_contains('Attachments test', 'we have subject on the page');
$m->content_contains('Some content', 'and content');
- $m->content_contains('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
SKIP: {
@@ -88,8 +88,8 @@ diag "with one attachment, but delete one along the way";
$m->content_contains('Attachments test', 'we have subject on the page');
$m->content_contains('Some content', 'and content');
- $m->content_lacks('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( !$m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page lacks the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
diag "with one attachment, but delete one along the way";
@@ -120,8 +120,8 @@ diag "with one attachment, but delete one along the way";
$m->content_contains('Attachments test', 'we have subject on the page');
$m->content_contains('Some content', 'and content');
- $m->content_lacks('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( !$m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page lacks the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
}
@@ -143,7 +143,7 @@ diag "with one attachment";
$m->click('SubmitTicket');
is($m->status, 200, "request successful");
- $m->content_contains('Download bpslogo.png', 'page has file name');
+ ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
diag "with two attachments";
@@ -167,8 +167,8 @@ diag "with two attachments";
$m->click('SubmitTicket');
is($m->status, 200, "request successful");
- $m->content_contains('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
SKIP: {
@@ -195,8 +195,8 @@ diag "with one attachment, delete one along the way";
$m->click('SubmitTicket');
is($m->status, 200, "request successful");
- $m->content_lacks('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( !$m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page lacks the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
}
@@ -218,7 +218,7 @@ diag "with one attachment";
is($m->status, 200, "request successful");
$m->goto_ticket( $ticket->id );
- $m->content_contains('Download bpslogo.png', 'page has file name');
+ ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
diag "with two attachments";
@@ -243,8 +243,8 @@ diag "with two attachments";
is($m->status, 200, "request successful");
$m->goto_ticket( $ticket->id );
- $m->content_contains('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
SKIP: {
@@ -272,8 +272,8 @@ diag "with one attachment, delete one along the way";
is($m->status, 200, "request successful");
$m->goto_ticket( $ticket->id );
- $m->content_lacks('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( !$m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page lacks the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
}
@@ -301,8 +301,8 @@ diag "one attachment";
foreach my $ticket ( @tickets ) {
$m->goto_ticket( $ticket->id );
- $m->content_lacks('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( !$m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page lacks the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
}
@@ -334,8 +334,8 @@ diag "two attachments";
foreach my $ticket ( @tickets ) {
$m->goto_ticket( $ticket->id );
- $m->content_contains('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
}
@@ -370,8 +370,8 @@ diag "one attachment, delete one along the way";
foreach my $ticket ( @tickets ) {
$m->goto_ticket( $ticket->id );
- $m->content_lacks('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( !$m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page lacks the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
}
}
@@ -389,7 +389,7 @@ diag "create with attachment";
$m->click('SubmitTicket');
is($m->status, 200, "request successful");
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
diag "update with attachment";
@@ -408,7 +408,7 @@ diag "update with attachment";
$m->click('SubmitTicket');
is($m->status, 200, "request successful");
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
}
diag "mobile ui";
@@ -463,8 +463,8 @@ diag "check content type and content";
$m->content_contains('Attachments test', 'we have subject on the page');
$m->content_contains('Some content', 'and content');
- $m->content_contains('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download mobile.css', 'page has file name');
+ ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
+ ok( $m->find_link( text => 'mobile.css', url_regex => qr{Attachment/} ), 'page has the file link' );
$m->follow_link_ok( { url_regex => qr/Attachment\/\d+\/\d+\/bpslogo\.png/ } );
is($m->response->header('Content-Type'), 'image/png', 'Content-Type of png lacks charset' );
@@ -509,13 +509,14 @@ diag "update and create";
$m->click('SubmitTicket');
is($m->status, 200, "request successful");
- $m->content_lacks('Download bpslogo.png', 'page has file name');
- $m->content_contains('Download favicon.png', 'page has file name');
+ ok( !$m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page lacks the file link' );
+ ok( $m->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page has the file link' );
$m2->form_name('TicketUpdate');
$m2->click('SubmitTicket');
- $m2->content_contains('Download bpslogo.png', 'page has file name');
- $m2->content_lacks('Download favicon.png', 'page has no file name');
+ ok( $m2->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
+ ok( !$m2->find_link( text => 'favicon.png', url_regex => qr{Attachment/} ), 'page lacks the file link' );
+
}
done_testing;
diff --git a/t/web/csrf.t b/t/web/csrf.t
index 42a22b6436..748558003e 100644
--- a/t/web/csrf.t
+++ b/t/web/csrf.t
@@ -193,7 +193,7 @@ $m->click('SubmitTicket');
$m->content_contains("Possible cross-site request forgery");
$m->content_contains("If you really intended to visit <tt>$baseurl/Ticket/Create.html</tt>");
$m->follow_link(text_regex => qr{resume your request});
-$m->content_contains('Download bpslogo.png', 'page has file name');
+ok( $m->find_link( text => 'bpslogo.png', url_regex => qr{Attachment/} ), 'page has the file link' );
$m->follow_link_ok( { url_regex => qr/Attachment\/\d+\/\d+\/bpslogo\.png/ } );
is($m->content, $logo_contents, "Binary content matches");
diff --git a/t/web/ticket_txn_content.t b/t/web/ticket_txn_content.t
index e4f70a2635..60240c63fb 100644
--- a/t/web/ticket_txn_content.t
+++ b/t/web/ticket_txn_content.t
@@ -54,7 +54,7 @@ for my $type ( 'text/plain', 'text/html' ) {
$m->content_contains('with plain attachment',
'we have subject on the page' );
$m->content_contains('this is main content', 'main content' );
- $m->content_contains("Download $plain_name", 'download plain file link' );
+ ok( $m->find_link( text => $plain_name, url_regex => qr{Attachment/} ), 'download plain file link' );
# Check for Message-IDs
follow_parent_with_headers_link($m, url_regex => qr/Attachment\/WithHeaders\//, n => 1);
@@ -89,7 +89,7 @@ for my $type ( 'text/plain', 'text/html' ) {
is( $m->status, 200, "request successful" );
$m->content_contains("this is main reply content", 'main reply content' );
- $m->content_contains("Download $html_name", 'download html file link' );
+ ok( $m->find_link( text => $html_name, url_regex => qr{Attachment/} ), 'download html file link' );
# Check for Message-IDs
follow_parent_with_headers_link($m, url_regex => qr/Attachment\/WithHeaders\//, n => 2);
commit d0b0fdfdd648b4b8d2235d79f32378db75dbe247
Merge: 66b6e91580 cd8dbde976
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat May 2 07:24:23 2020 +0800
Merge branch '5.0/display-attachment-name-in-history' into 5.0-trunk
-----------------------------------------------------------------------
More information about the rt-commit
mailing list