Merge branch '5.0/rest2-transaction-pages' into 5.0-trunk

lib/RT/REST2.pm

@@ -1275,6 +1275,11 @@ or previous page, then the URL for that page is returned in the next_page
 and prev_page variables respectively. It is up to you to store the required
 JSON to pass with the following page request.
 
+If current user doesn't have enough rights to see all returned results, then
+returned C<pages> and C<total> will be C<null>, in which case you can still
+loop through results by means of C<next_page> that contains the link to the next
+page of results, until it becomes absent.
+
 =head2 Disabled items
 
 By default, only enabled objects are returned. To include disabled objects

lib/RT/REST2/Resource/Collection.pm

@@ -61,6 +61,7 @@ use Module::Runtime qw( require_module );
 use RT::REST2::Util qw( expand_uid format_datetime error_as_json );
 use POSIX qw( ceil );
 use Encode;
+our $PREVIEW_LIMIT = 200;
 
 has 'collection_class' => (
     is  => 'ro',
@@ -91,6 +92,11 @@ sub setup_paging {
     my $page = $self->request->param('page') || 1;
     if    ( $page !~ /^\d+$/ ) { $page = 1 }
     elsif ( $page == 0 )       { $page = 1 }
+    elsif ( $page > 1 && $self->collection->CurrentUserCanSeeAll ) {
+        if ( my $pages = ceil( $self->collection->CountAll / $per_page ) ) {
+            $page = $pages if $page > $pages;
+        }
+    }
     $self->collection->GotoPage($page - 1);
 }
 
@@ -187,9 +193,12 @@ sub serialize {
         push @results, $result;
     }
 
+    my $total = $collection->CountAll;
+    my $pages = ceil( $total / $collection->RowsPerPage );
+
     my %results = (
         count       => scalar(@results)         + 0,
-        total       => $collection->CurrentUserCanSeeAll ? ( $collection->CountAll + 0 ) : undef,
+        total       => $collection->CurrentUserCanSeeAll ? $total : undef,
         per_page    => $collection->RowsPerPage + 0,
         page        => ($collection->FirstRow / $collection->RowsPerPage) + 1,
         items       => \@results,
@@ -205,21 +214,62 @@ sub serialize {
         }
     }
 
-    $results{pages} = defined $results{total} ? ceil($results{total} / $results{per_page}) : undef;
+    $results{pages} = defined $results{total} ? $pages : undef;
     if ( $results{pages} ) {
         if ($results{page} < $results{pages}) {
-            my $page = $results{page} + 1;
             $uri->query_form( @query_form, page => $results{page} + 1 );
             $results{next_page} = $uri->as_string;
         }
-        if ($results{page} > 1) {
-            # If we're beyond the last page, set prev_page as the last page
-            # available, otherwise, the previous page.
-            $uri->query_form( @query_form, page => ($results{page} > $results{pages} ? $results{pages} : $results{page} - 1) );
+
+        if ( $results{page} > 1 ) {
+            $uri->query_form( @query_form, page => $results{page} - 1 );
             $results{prev_page} = $uri->as_string;
         }
     }
 
+    if ( (not exists $results{next_page}) && (not defined $results{total}) ) {
+        # If total is undef, this collection checks ACLs in code so we can't
+        # use the collection count directly. Try to peek ahead to see if there
+        # are more records available so we can return next_page without giving
+        # away specific counts.
+
+        my $page = $results{page};
+
+        # If current user can't find any records after checking about $PREVIEW_LIMIT
+        # items, assuming no records left.
+        for ( 1 .. ceil( $PREVIEW_LIMIT / $results{per_page} ) ) {
+            $collection->NextPage();
+            $page++;
+            last if $page > $pages;
+
+            while ( my $item = $collection->Next ) {
+
+                # As soon as we get one record, we know it's the next page
+                $uri->query_form( @query_form, page => $page );
+                $results{next_page} = $uri->as_string;
+                last;
+            }
+            last if $results{next_page};
+        }
+
+        $page = $results{page};
+        $collection->GotoPage( $page - 1 );
+        for ( 1 .. ceil( $PREVIEW_LIMIT / $results{per_page} ) ) {
+            $collection->PrevPage();
+            $page--;
+            last if $page < 1;
+
+            while ( my $item = $collection->Next ) {
+
+                # As soon as we get one record, we know it's the prev page
+                $uri->query_form( @query_form, page => $page );
+                $results{prev_page} = $uri->as_string;
+                last;
+            }
+            last if $results{prev_page};
+        }
+    }
+
     return \%results;
 }
 

lib/RT/SearchBuilder.pm

@@ -1187,6 +1187,7 @@ sub DistinctFieldValues {
 
 sub CurrentUserCanSeeAll {
     my $self = shift;
+    return 1 if $self->{_current_user_can_see_all};
     return $self->CurrentUser->HasRight( Right => 'SuperUser', Object => RT->System ) ? 1 : 0;
 }
 

lib/RT/Ticket.pm

@@ -3106,22 +3106,30 @@ sub Transactions {
     if ( $self->CurrentUserHasRight('ShowTicket') ) {
         $transactions->LimitToTicket($self->id);
 
+        my @types;
+
         # if the user may not see comments do not return them
         unless ( $self->CurrentUserHasRight('ShowTicketComments') ) {
+            push @types, 'Comment', 'CommentEmailRecord';
+        }
+
+        unless ( $self->CurrentUserHasRight('ShowOutgoingEmail') ) {
+            push @types, 'EmailRecord';
+        }
+
+        if (@types) {
             $transactions->Limit(
-                SUBCLAUSE => 'acl',
-                FIELD    => 'Type',
-                OPERATOR => '!=',
-                VALUE    => "Comment"
-            );
-            $transactions->Limit(
-                SUBCLAUSE => 'acl',
-                FIELD    => 'Type',
-                OPERATOR => '!=',
-                VALUE    => "CommentEmailRecord",
+                SUBCLAUSE       => 'acl',
+                FIELD           => 'Type',
+                OPERATOR        => 'NOT IN',
+                VALUE           => \@types,
                 ENTRYAGGREGATOR => 'AND'
             );
+        }
 
+        if ( $self->CurrentUserHasRight('SeeCustomField') ) {
+            # We have checked all related rights, current user should be able to see all results
+            $transactions->{_current_user_can_see_all} = 1;
         }
     } else {
         $transactions->Limit(

lib/RT/Transactions.pm

@@ -141,6 +141,7 @@ sub LimitToTicket {
 sub AddRecord {
     my $self = shift;
     my ($record) = @_;
+    return $self->SUPER::AddRecord($record) if $self->{_current_user_can_see_all};
 
     if ( $self->{_is_ticket_only_search} && RT->Config->Get('UseSQLForACLChecks') ) {
         # UseSQLForACLChecks implies ShowTicket only, need to check out extra rights here.
@@ -1260,6 +1261,7 @@ sub CleanSlate {
         $self->SUPER::CleanSlate(@_);
     }
     delete $self->{_is_ticket_only_search};
+    delete $self->{_current_user_can_see_all};
 }
 
 RT::Base->_ImportOverlays();

t/rest2/pagination.t

@@ -138,10 +138,10 @@ my $bravo_id = $bravo->Id;
         my $content = $mech->json_response;
         if ($param eq 'page') {
         if ($value eq '30') {
-            is($content->{count}, 0);
-            is($content->{page}, 30);
-            is(scalar @{$content->{items}}, 0);
-            like($content->{prev_page}, qr[$url\?page=1]);
+            is($content->{count}, 3);
+            is($content->{page}, 1);
+            is(scalar @{$content->{items}}, 3);
+            is($content->{prev_page}, undef);
         } else {
             is($content->{count}, 3);
             is($content->{page}, 1);

t/rest2/tickets.t

@@ -411,6 +411,80 @@ my ($ticket_url, $ticket_id);
     }
 }
 
+# Transactions with paging
+{
+    my $res = $mech->get($ticket_url,
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+
+    my $history_pages_url = $mech->url_for_hypermedia('history');
+
+    # Set low per_page to get at least two pages
+    $history_pages_url .= '?per_page=10';
+    $res = $mech->get($history_pages_url,
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+
+    my $content = $mech->json_response;
+    is($content->{count}, 10);
+    is($content->{page}, 1);
+    is($content->{per_page}, 10);
+    is($content->{prev_page}, undef, 'No prev_page');
+    is($content->{next_page}, $history_pages_url . '&page=2');
+
+    is($content->{total}, undef, 'No total');
+    is(scalar @{$content->{items}}, 10);
+
+    for my $txn (@{ $content->{items} }) {
+        is($txn->{type}, 'transaction');
+        like($txn->{_url}, qr{$rest_base_path/transaction/\d+$});
+    }
+
+    # Get next_page
+    $res = $mech->get($content->{next_page},
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+
+    $content = $mech->json_response;
+    is($content->{count}, 4);
+    is($content->{page}, 2);
+    is($content->{per_page}, 10);
+    is($content->{next_page}, undef, 'No next_page');
+    is($content->{prev_page}, $history_pages_url . '&page=1');
+
+    is($content->{total}, undef, 'No total');
+    is(scalar @{$content->{items}}, 4);
+
+    for my $txn (@{ $content->{items} }) {
+        is($txn->{type}, 'transaction');
+        like($txn->{_url}, qr{$rest_base_path/transaction/\d+$});
+    }
+
+    # Back to prev_page
+    $res = $mech->get($content->{prev_page},
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+
+    $content = $mech->json_response;
+    is($content->{count}, 10);
+    is($content->{page}, 1);
+    is($content->{per_page}, 10);
+    is($content->{prev_page}, undef, 'No prev_page');
+    is($content->{next_page}, $history_pages_url . '&page=2');
+
+    is($content->{total}, undef, 'No total');
+    is(scalar @{$content->{items}}, 10);
+
+    for my $txn (@{ $content->{items} }) {
+        is($txn->{type}, 'transaction');
+        like($txn->{_url}, qr{$rest_base_path/transaction/\d+$});
+    }
+}
+
 # Ticket Reply
 {
     # we know from earlier tests that look at hypermedia without ReplyToTicket