[Rt-commit] rt branch, 4.4/attachments-list, created. rt-4.4.0-53-g884669b

Shawn Moore shawn at bestpractical.com
Fri May 6 11:54:58 EDT 2016


The branch, 4.4/attachments-list has been created
        at  884669be5e4a984d2e60aad9744f44fe60a1caaf (commit)

- Log -----------------------------------------------------------------
commit dd89c593aa4cc5f819c4ce13b9d014fd03d8c47a
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Feb 17 04:11:54 2016 +0000

    Add a parameter to hide the titlebox in ShowAttachments

diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index f32ac9c..bfa03d7 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -49,7 +49,8 @@
 <&| /Widgets/TitleBox, title => loc('Attachments'), 
         title_class=> 'inverse',  
         class => 'ticket-info-attachments',
-        color => "#336699" &>
+        color => "#336699",
+        hide_chrome => $HideTitleBox &>
 
 % foreach my $key (sort { lc($a) cmp lc($b) } keys %documents) {
 
@@ -95,5 +96,6 @@ while ( my $attach = $Attachments->Next() ) {
 $Ticket => undef
 $Attachments => undef
 $DisplayPath => $session{'CurrentUser'}->Privileged ? 'Ticket' : 'SelfService'
+$HideTitleBox => 0
 </%ARGS>
 
diff --git a/share/html/Widgets/TitleBox b/share/html/Widgets/TitleBox
index 822170c..7c65082 100644
--- a/share/html/Widgets/TitleBox
+++ b/share/html/Widgets/TitleBox
@@ -46,11 +46,16 @@
 %#
 %# END BPS TAGGED BLOCK }}}
 <div class="<% $class %>">
+% if ($hide_chrome) {
+  <% $content | n %>
+% } else {
   <& TitleBoxStart, %ARGS &><% $content | n %><& TitleBoxEnd &>
+% }
 </div>
 <%ARGS>
 $class => ''
 $hide_empty => 0
+$hide_chrome => 0
 </%ARGS>
 <%INIT>
 my $content = $m->content;

commit c4aed50cc67615ece9c0865afeb6687b9f77e10f
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Feb 17 04:29:15 2016 +0000

    Use the existing template and stylings of attachment display for reuse
    
        The previous display was a bit harder to parse.
    
    Fixes: I#31709

diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index 3a63c59..34798e2 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -163,25 +163,16 @@ jQuery( function() {
         </div>
     </td>
 </tr>
-% if (@quoted_attachments) {
+% if ($TicketObj && $TicketObj->id) {
 <tr>
   <td class="label" valign="top"><&|/l&>Include attachments</&>:</td>
   <td id="reuse-attachments">
-%     for my $attach (@quoted_attachments) {
-    <label>
-      <input type="checkbox" class="checkbox" name="AttachExisting" value="<% $attach->Id %>" \
-             <% (grep { $attach->Id == $_ } @AttachExisting) ? 'checked' : '' %> />
-      <% $attach->Filename %>
-      <% loc("[_1] ([_2]) by [_3]", $attach->CreatedAsString, $attach->FriendlyContentLength, $m->scomp('/Elements/ShowUser', User => $attach->CreatorObj)) |n %>
-    </label>
-<%perl>
-my $url = RT->System->ExternalStorageURLFor($attach) || 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"><&|/l&>View</&></a>)
-      <br />
-%     }
+    <& /Ticket/Elements/ShowAttachments,
+      Ticket       => $TicketObj,
+      Selectable   => 1,
+      HideTitleBox => 1,
+      Checked      => \@AttachExisting,
+    &>
   </td>
 </tr>
 % }
@@ -197,11 +188,4 @@ my $attachments;
 if ( exists $session{'Attachments'}{ $Token } && keys %{ $session{'Attachments'}{ $Token } } ) {
     $attachments = $session{'Attachments'}{ $Token };
 }
-
-my @quoted_attachments;
-if ($TicketObj and $TicketObj->id) {
-    @quoted_attachments = sort { lc($a->Filename) cmp lc($b->Filename) }
-                          grep { defined $_->Filename and length $_->Filename }
-                            @{$TicketObj->Attachments->ItemsArrayRef};
-}
 </%INIT>
diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index bfa03d7..2a49c91 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -55,10 +55,17 @@
 % foreach my $key (sort { lc($a) cmp lc($b) } keys %documents) {
 
 <%$key%><br />
-<ul>
+<ul <% $Selectable ? 'class="selectable"' : '' |n %> >
 % foreach my $rev (@{$documents{$key}}) {
 % if ($rev->ContentLength) {
 <li><font size="-2">
+
+% if ($Selectable) {
+    <label>
+    <input type="checkbox" class="checkbox" name="AttachExisting" value="<% $rev->Id %>" \
+             <% $is_checked{$rev->Id} ? 'checked' : '' %> />
+% }
+
 % if (my $url = RT->System->ExternalStorageURLFor($rev)) {
 <a href="<%$url%>">
 % } else {
@@ -67,6 +74,11 @@
 % my $desc = loc("[_1] ([_2]) by [_3]", $rev->CreatedAsString, $rev->FriendlyContentLength, $m->scomp('/Elements/ShowUser', User => $rev->CreatorObj));
 <% $desc |n%>
 </a>
+
+% if ($Selectable) {
+    </label>
+% }
+
 </font></li>
 % }
 % }
@@ -91,11 +103,14 @@ while ( my $attach = $Attachments->Next() ) {
    unshift( @{ $documents{ $attach->Filename } }, $attach );
 }
 
+my %is_checked = map { $_ => 1 } @Checked;
 </%INIT>
 <%ARGS>
 $Ticket => undef
 $Attachments => undef
 $DisplayPath => $session{'CurrentUser'}->Privileged ? 'Ticket' : 'SelfService'
 $HideTitleBox => 0
+$Selectable => 0
+ at Checked => ()
 </%ARGS>
 
diff --git a/share/static/css/base/forms.css b/share/static/css/base/forms.css
index 9f57331..a03be9b 100644
--- a/share/static/css/base/forms.css
+++ b/share/static/css/base/forms.css
@@ -251,6 +251,19 @@ form div.submit div.buttons div.next {
     display: none;
 }
 
+ul.selectable {
+    list-style-type: none;
+}
+
+ul.selectable input[type=checkbox],
+ul.selectable a {
+    vertical-align: bottom;
+}
+
+#reuse-attachments {
+    padding-top: 0.25em;
+}
+
 /* query builder */
 
 #formatbuttons {

commit 9de4ca7675f8398d7ef78992d1817f91b8d050b1
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Feb 17 04:33:32 2016 +0000

    Replace explicit <font> tag with CSS

diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index 2a49c91..d9432b7 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -58,7 +58,7 @@
 <ul <% $Selectable ? 'class="selectable"' : '' |n %> >
 % foreach my $rev (@{$documents{$key}}) {
 % if ($rev->ContentLength) {
-<li><font size="-2">
+<li>
 
 % if ($Selectable) {
     <label>
@@ -79,7 +79,7 @@
     </label>
 % }
 
-</font></li>
+</li>
 % }
 % }
 </ul>
diff --git a/share/static/css/base/forms.css b/share/static/css/base/forms.css
index a03be9b..2584ee0 100644
--- a/share/static/css/base/forms.css
+++ b/share/static/css/base/forms.css
@@ -264,6 +264,10 @@ ul.selectable a {
     padding-top: 0.25em;
 }
 
+.ticket-info-attachments ul li {
+    font-size: .8em;
+}
+
 /* query builder */
 
 #formatbuttons {

commit 1a25f83b94ccd8fdf1c1be34c6b6da2ec2e0ca5f
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Feb 17 04:41:53 2016 +0000

    Filter out empty attachment names in SQL rather than Perl
    
        Cloning is necessary when ShowHistory is set to immediate,
        as the Attachments object is shared across this template and
        the transaction log; without ->Clone, all messages are filtered
        away.

diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index d9432b7..b6a9d85 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -95,11 +95,20 @@
 # then we need to find one
 $Attachments ||= $Ticket->Attachments;
 
-# XXX PERF: why doesn't this Limit on Filename to avoid fetching *all* the
-# attachments?
+# Avoid applying limits to this collection that may be used elsewhere
+# (e.g. transaction display)
+$Attachments = $Attachments->Clone;
+
+# Remember, each message in a transaction is an attachment; we only
+# want named attachments (real files)
+$Attachments->Limit(
+    FIELD    => 'Filename',
+    OPERATOR => '!=',
+    VALUE    => '',
+);
+
 my %documents;
 while ( my $attach = $Attachments->Next() ) {
-    next unless defined $attach->Filename && length $attach->Filename;
    unshift( @{ $documents{ $attach->Filename } }, $attach );
 }
 

commit 8b35ca835122e4d5335cf0d8ff31896b759eaa78
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Feb 17 04:57:44 2016 +0000

    Add a way to display just the last N attachments

diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index b6a9d85..3ff586e 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -108,7 +108,20 @@ $Attachments->Limit(
 );
 
 my %documents;
+
+# if we're limiting the number of attachments, show only the most recent
+# if we're not limiting the number, order doesn't much matter
+if (defined($Count)) {
+    $Attachments->OrderByCols(
+        { FIELD => 'Created', ORDER => 'DESC' },
+        { FIELD => 'id', ORDER => 'DESC' },
+    );
+}
+
 while ( my $attach = $Attachments->Next() ) {
+   if (defined($Count) && --$Count < 0) {
+       last;
+   }
    unshift( @{ $documents{ $attach->Filename } }, $attach );
 }
 
@@ -120,6 +133,7 @@ $Attachments => undef
 $DisplayPath => $session{'CurrentUser'}->Privileged ? 'Ticket' : 'SelfService'
 $HideTitleBox => 0
 $Selectable => 0
+$Count => undef
 @Checked => ()
 </%ARGS>
 

commit 651cb49bf06abd129fb0922d41dad9120110f666
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Feb 17 05:08:24 2016 +0000

    Display only latest 5 attachments, with an AJAX "Show all" link
    
    Fixes: I#31710

diff --git a/share/html/Helpers/TicketAttachments b/share/html/Helpers/TicketAttachments
new file mode 100644
index 0000000..2292a45
--- /dev/null
+++ b/share/html/Helpers/TicketAttachments
@@ -0,0 +1,63 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2016 Best Practical Solutions, LLC
+%#                                          <sales at bestpractical.com>
+%#
+%# (Except where explicitly superseded by other copyright notices)
+%#
+%#
+%# LICENSE:
+%#
+%# This work is made available to you under the terms of Version 2 of
+%# the GNU General Public License. A copy of that license should have
+%# been provided with this software, but in any event can be snarfed
+%# from www.gnu.org.
+%#
+%# This work is distributed in the hope that it will be useful, but
+%# WITHOUT ANY WARRANTY; without even the implied warranty of
+%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+%# General Public License for more details.
+%#
+%# You should have received a copy of the GNU General Public License
+%# along with this program; if not, write to the Free Software
+%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+%# 02110-1301 or visit their web page on the internet at
+%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+%#
+%#
+%# CONTRIBUTION SUBMISSION POLICY:
+%#
+%# (The following paragraph is not intended to limit the rights granted
+%# to you to modify and distribute this software under the terms of
+%# the GNU General Public License and is only of importance to you if
+%# you choose to contribute your changes and enhancements to the
+%# community by submitting them to Best Practical Solutions, LLC.)
+%#
+%# By intentionally submitting any modifications, corrections or
+%# derivatives to this work, or any other work intended for use with
+%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+%# you are the copyright holder for those contributions and you grant
+%# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+%# royalty-free, perpetual, license to use, copy, create derivative
+%# works based on those contributions, and sublicense and distribute
+%# those contributions and any derivatives thereof.
+%#
+%# END BPS TAGGED BLOCK }}}
+<%ARGS>
+$id
+$Selectable => undef
+$Count => undef
+</%ARGS>
+<%INIT>
+my $TicketObj = RT::Ticket->new($session{'CurrentUser'});
+$TicketObj->Load($id);
+</%INIT>
+<& /Ticket/Elements/ShowAttachments,
+  Ticket       => $TicketObj,
+  HideTitleBox => 1,
+  Selectable   => $Selectable,
+  Count        => $Count,
+&>
+% $m->abort();
diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index 34798e2..fb4ec23 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -172,6 +172,7 @@ jQuery( function() {
       Selectable   => 1,
       HideTitleBox => 1,
       Checked      => \@AttachExisting,
+      Count        => 5,
     &>
   </td>
 </tr>
diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index 3ff586e..f878381 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -52,6 +52,8 @@
         color => "#336699",
         hide_chrome => $HideTitleBox &>
 
+<div class="attachment-list">
+
 % foreach my $key (sort { lc($a) cmp lc($b) } keys %documents) {
 
 <%$key%><br />
@@ -85,6 +87,21 @@
 </ul>
 
 % }
+
+% if ($show_more) {
+<span class="show-more-link">
+% my %params = %ARGS;
+% delete $params{Ticket};
+% delete $params{Attachments};
+% delete $params{Count};
+
+% my $query = $m->comp('/Elements/QueryString', %params, id => $Ticket->id );
+% my $url   = RT->Config->Get('WebPath')."/Helpers/TicketAttachments?$query";
+<a href="#" onclick="jQuery(this).parent().text(<% loc('Loading...') | n,j%>).closest('.attachment-list').load(<% $url |n,j %>); return false;" ><% loc('Show all attachments') %></a>
+</span>
+% }
+
+</div>
 </&>
 
 % }
@@ -107,6 +124,7 @@ $Attachments->Limit(
     VALUE    => '',
 );
 
+my $show_more = 0;
 my %documents;
 
 # if we're limiting the number of attachments, show only the most recent
@@ -119,7 +137,9 @@ if (defined($Count)) {
 }
 
 while ( my $attach = $Attachments->Next() ) {
+   # display "show more" only when there will be more attachments
    if (defined($Count) && --$Count < 0) {
+       $show_more = 1;
        last;
    }
    unshift( @{ $documents{ $attach->Filename } }, $attach );
diff --git a/share/html/Ticket/Elements/ShowSummary b/share/html/Ticket/Elements/ShowSummary
index f891631..4183dab 100644
--- a/share/html/Ticket/Elements/ShowSummary
+++ b/share/html/Ticket/Elements/ShowSummary
@@ -64,7 +64,7 @@
         class => 'ticket-info-people',
     &><& /Ticket/Elements/ShowPeople, Ticket => $Ticket &></&>
 % $m->callback( %ARGS, CallbackName => 'AfterPeople' );
-    <& /Ticket/Elements/ShowAttachments, Ticket => $Ticket, Attachments => $Attachments &>
+    <& /Ticket/Elements/ShowAttachments, Ticket => $Ticket, Attachments => $Attachments, Count => 5 &>
 % $m->callback( %ARGS, CallbackName => 'AfterAttachments' );
     <& /Ticket/Elements/ShowRequestor, Ticket => $Ticket &>
 % $m->callback( %ARGS, CallbackName => 'LeftColumn' );

commit c453a8a0d43cb2c61069bcf4df1639492c54125f
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Feb 17 05:25:32 2016 +0000

    Preserve selected attachments when loading the rest in via ajax

diff --git a/share/html/Helpers/TicketAttachments b/share/html/Helpers/TicketAttachments
index 2292a45..c4de7e9 100644
--- a/share/html/Helpers/TicketAttachments
+++ b/share/html/Helpers/TicketAttachments
@@ -49,6 +49,7 @@
 $id
 $Selectable => undef
 $Count => undef
+ at AttachExisting => ()
 </%ARGS>
 <%INIT>
 my $TicketObj = RT::Ticket->new($session{'CurrentUser'});
@@ -59,5 +60,6 @@ $TicketObj->Load($id);
   HideTitleBox => 1,
   Selectable   => $Selectable,
   Count        => $Count,
+  Checked      => \@AttachExisting,
 &>
 % $m->abort();
diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index f878381..5c8022f 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -94,10 +94,24 @@
 % delete $params{Ticket};
 % delete $params{Attachments};
 % delete $params{Count};
-
 % my $query = $m->comp('/Elements/QueryString', %params, id => $Ticket->id );
 % my $url   = RT->Config->Get('WebPath')."/Helpers/TicketAttachments?$query";
-<a href="#" onclick="jQuery(this).parent().text(<% loc('Loading...') | n,j%>).closest('.attachment-list').load(<% $url |n,j %>); return false;" ><% loc('Show all attachments') %></a>
+
+<script type="text/javascript">
+    function showAllAttachments(node) {
+        var container = node.closest('.attachment-list');
+        var params = node.closest('form').find('input[name=AttachExisting]').serialize();
+
+        node.parent().text(<% loc('Loading...') | n,j%>);
+
+        var url = <% $url |n,j %>;
+        if (params) url += '&' + params;
+        container.load(url);
+    }
+</script>
+
+<a href="#" onclick="showAllAttachments(jQuery(this)); return false;" ><% loc('Show all attachments') %></a>
+
 </span>
 % }
 

commit ec1f85b090b741227ae21bd9e692be0800a699af
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Feb 17 05:28:37 2016 +0000

    Always pull out attachments in newest-first order
    
        Without this change, the order of attachments with the
        same name would reverse based on whether you provided a $Count
        or not.

diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index 5c8022f..ac663a9 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -141,14 +141,11 @@ $Attachments->Limit(
 my $show_more = 0;
 my %documents;
 
-# if we're limiting the number of attachments, show only the most recent
-# if we're not limiting the number, order doesn't much matter
-if (defined($Count)) {
-    $Attachments->OrderByCols(
-        { FIELD => 'Created', ORDER => 'DESC' },
-        { FIELD => 'id', ORDER => 'DESC' },
-    );
-}
+# show newest first
+$Attachments->OrderByCols(
+    { FIELD => 'Created', ORDER => 'DESC' },
+    { FIELD => 'id',      ORDER => 'DESC' },
+);
 
 while ( my $attach = $Attachments->Next() ) {
    # display "show more" only when there will be more attachments
@@ -156,7 +153,7 @@ while ( my $attach = $Attachments->Next() ) {
        $show_more = 1;
        last;
    }
-   unshift( @{ $documents{ $attach->Filename } }, $attach );
+   push @{ $documents{ $attach->Filename } }, $attach;
 }
 
 my %is_checked = map { $_ => 1 } @Checked;

commit 884669be5e4a984d2e60aad9744f44fe60a1caaf
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri May 6 11:26:57 2016 -0400

    Only show "include attachments" if we have attachments

diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index fb4ec23..f61bb6a 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -163,7 +163,7 @@ jQuery( function() {
         </div>
     </td>
 </tr>
-% if ($TicketObj && $TicketObj->id) {
+% if ($HasExisting) {
 <tr>
   <td class="label" valign="top"><&|/l&>Include attachments</&>:</td>
   <td id="reuse-attachments">
@@ -189,4 +189,16 @@ my $attachments;
 if ( exists $session{'Attachments'}{ $Token } && keys %{ $session{'Attachments'}{ $Token } } ) {
     $attachments = $session{'Attachments'}{ $Token };
 }
+
+my $HasExisting = 0;
+
+if ($TicketObj && $TicketObj->id) {
+    my $Existing = $TicketObj->Attachments;
+    $Existing->Limit(
+        FIELD    => 'Filename',
+        OPERATOR => '!=',
+        VALUE    => '',
+    );
+    $HasExisting = 1 if $Existing->Count;
+}
 </%INIT>

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


More information about the rt-commit mailing list