[Rt-commit] rt branch, 4.4/recently-viewed-bogus-tickets, created. rt-4.4.4-15-gdd95c46b9
Michel Rodriguez
michel at bestpractical.com
Fri Mar 6 11:30:04 EST 2020
The branch, 4.4/recently-viewed-bogus-tickets has been created
at dd95c46b91697608e1ee9c8789deaeb6d74790d4 (commit)
- Log -----------------------------------------------------------------
commit 840738e0259de886a7c7f56451952c6191c172a8
Author: michel <michel at bestpractical.com>
Date: Thu Apr 11 16:00:29 2019 +0200
Fix RecentlyViewedTickets to deal with shredded/merged tickets.
If a ticket that was recently visited is:
shredded : it is now removed from the list
merged : the list is deduplicated if the parent was already included
The cached RecentlyViewedTickets attribute is then updated
RecentlyViewedTickets now returns a list of hashes { id => , subject => }
so that the code in the template doesn't have to deal with this
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 6c4bc08d8..8ef4ceb73 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -2179,16 +2179,52 @@ sub ToggleBookmark {
=head2 RecentlyViewedTickets TICKET
-Returns a list of two-element (ticket id, timestamp) array references ordered by recently viewed first.
+Returns a list of hashrefs { id => <ticket_id>, subject => <ticket_subject> } )
+ordered by recently viewed first.
+
+If a ticket cannot be loaded (eg because it has been shredded) or is duplicated
+(eg because 2 tickets on the list have been merged), it is not returned and the
+user's RecentlyViewedTickets list is updated
=cut
sub RecentlyViewedTickets {
my $self = shift;
- my $content = $self->FirstAttribute('RecentlyViewedTickets');
- return $content ? @{$content->Content} : ();
+ my $recentlyViewedTickets = $self->FirstAttribute( 'RecentlyViewedTickets' );
+ my $content = $recentlyViewedTickets ? $recentlyViewedTickets->Content : undef;
+ my @storedList = $content ? @$content : (); # [ [ <id>, <timestamp>], ...]
+ my @toDisplay = (); # [ { id => <id>, title => <title> }, ... ]
+ if ( @storedList ) {
+ my @currentList; # same as @storedList, without deleted tickets
+ my $updateStoredList; # set to 1 if a ticket does not load
+ my %seen; # used to check that we don't have duplicates (in case of merges)
+
+ foreach my $ticketRef ( @storedList ) {
+ my ($ticketId, $timestamp) = @$ticketRef;
+ my $ticket = RT::Ticket->new( $self->CurrentUser );
+ $ticket->Load( $ticketId );
+ my $realId= $ticket->Id; # can be undef or changed in case of merge
+ if ( $ticket->Id && ! $seen{$realId} ) {
+ push @toDisplay, { id => $ticket->Id, subject => $ticket->Subject };
+ push @currentList, [ $ticketId, $timestamp ];
+ $seen{$realId}++;
+ } else {
+ # non existent ticket, do not add to @currentList, list needs to be updated
+ $updateStoredList = 1;
+ }
+ }
+
+ if ( $updateStoredList ) {
+ $self->SetAttribute(
+ Name => 'RecentlyViewedTickets',
+ Content => \@currentList,
+ );
+ }
+ }
+ return @toDisplay;
}
+
=head2 AddRecentlyViewedTicket TICKET
Takes an RT::Ticket object and adds it to the current user's RecentlyViewedTickets
diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index 755c8d219..7a75b39d7 100644
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -604,20 +604,15 @@ my $build_main_nav = sub {
$tickets->child( new => title => loc('New Search'), path => "/Search/Build.html?NewQuery=1" );
my $recents = $tickets->child( recent => title => loc('Recently Viewed'));
- for ($session{CurrentUser}->RecentlyViewedTickets) {
- my ($ticketId, $timestamp) = @$_;
- my $ticket = RT::Ticket->new($session{CurrentUser});
- $ticket->Load($ticketId);
- if ($ticket->Id) {
- my $title = $ticket->Subject || loc("(No subject)");
- if (length $title > 50) {
- $title = substr($title, 0, 47);
- $title =~ s/\s+$//;
- $title .= "...";
- }
- $title = "#$ticketId: " . $title;
- $recents->child( "$ticketId" => title => $title, path => "/Ticket/Display.html?id=" . $ticket->Id );
+ for my $ticket ( $session{CurrentUser}->RecentlyViewedTickets ) {
+ my $title = $ticket->{subject} || loc( "(No subject)" );
+ if ( length $title > 50 ) {
+ $title = substr($title, 0, 47);
+ $title =~ s/\s+$//;
+ $title .= "...";
}
+ $title = "#$ticket->{id}: " . $title;
+ $recents->child( "$ticket->{id}" => title => $title, path => "/Ticket/Display.html?id=" . $ticket->{id} );
}
$search->child( articles => title => loc('Articles'), path => "/Articles/Article/Search.html" )
commit 05e156db45945a07a6c5d02297308e96207ef44a
Author: michel <michel at bestpractical.com>
Date: Fri Mar 6 16:38:57 2020 +0100
Fix bug that kept 11 tickets in the "recently visited" list instead of 10.
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 8ef4ceb73..6ace0071f 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -2248,18 +2248,15 @@ sub AddRecentlyViewedTicket {
}
my @tickets;
- my $i = 0;
for (@recentTickets) {
my ($ticketId, $timestamp) = @$_;
#Skip the ticket if it exists in recents already
if ($ticketId != $ticket->Id) {
push @tickets, $_;
- if ($i >= $maxCount - 1) {
- last;
- }
+ # -1 is for the new ticket that will be added
+ last if @tickets >= $maxCount - 1;
}
- $i++;
}
#Add the new ticket
commit dd95c46b91697608e1ee9c8789deaeb6d74790d4
Author: michel <michel at bestpractical.com>
Date: Wed Feb 26 11:29:49 2020 +0100
Test recently viewed tickets that are merged or shredded.
diff --git a/t/api/user_recently_visited.t b/t/api/user_recently_visited.t
new file mode 100644
index 000000000..00db90b47
--- /dev/null
+++ b/t/api/user_recently_visited.t
@@ -0,0 +1,100 @@
+use strict;
+use warnings;
+
+# test the RecentlyViewedTickets method
+
+use RT::Test::Shredder tests => undef;
+my $test = "RT::Test::Shredder";
+
+{
+ my $user = RT::Test->load_or_create_user( Name => 'tester' );
+ ok( $user && $user->Id, 'created user' );
+
+ # need ShowTicket to visit, ModifyTicket to merge
+ ok( RT::Test->add_rights(
+ { Principal => $user,
+ Right => [qw( SeeQueue CreateTicket ShowTicket ModifyTicket )]
+ }
+ ),
+ 'granting rights'
+ );
+
+ is( visited_nb( $user ), 0, 'init: empty RecentlyViewedTickets' );
+
+ my %ticket;
+ foreach my $ticket_code ( 'a' .. 't' ) {
+ my $new_ticket = RT::Test->create_ticket(
+ Subject => $ticket_code,
+ Queue => 'General'
+ );
+ my $id = $new_ticket->id;
+ ok( $new_ticket, "ticket '$ticket_code' created" );
+ $ticket{$ticket_code}
+ = { ticket => $new_ticket, id => $id, subject => $ticket_code };
+ $new_ticket->ApplyTransactionBatch
+ ; # needed for the tests with shredder to work
+ }
+
+ is( visited_nb( $user ), 0,
+ 'tickets created: empty RecentlyViewedTickets'
+ );
+
+ # using reverse on the list so it's easier to read later (last visited is first in RecentlyViewedTicket)
+ foreach my $viewed ( reverse qw( c q p b e d c b a ) ) {
+ $user->AddRecentlyViewedTicket( $ticket{$viewed}->{ticket} );
+ }
+
+ is( visited( $user ), 'c,q,p,b,e,d,a',
+ 'visited tickets after inital visits'
+ );
+
+ my $shredder = $test->shredder_new();
+
+ $shredder->PutObjects( Objects => $ticket{m}->{ticket} );
+ $shredder->WipeoutAll;
+ is( visited( $user ), 'c,q,p,b,e,d,a',
+ 'visited tickets after shredding an unvisited ticket'
+ );
+
+ $shredder->PutObjects( Objects => $ticket{p}->{ticket} );
+ $shredder->PutObjects( Objects => $ticket{d}->{ticket} );
+ $shredder->WipeoutAll;
+ is( visited( $user ), 'c,q,b,e,a',
+ 'visited tickets after shredding 2 visited tickets'
+ );
+
+ my ( $ok, $msg ) = $ticket{b}->{ticket}->MergeInto( $ticket{j}->{id} );
+ if ( !$ok ) { die "error merging: $msg\n"; }
+ is( visited( $user ), 'c,q,j,e,a',
+ 'visited tickets after merging into a ticket that was NOT on the list'
+ );
+
+ $ticket{c}->{ticket}->MergeInto( $ticket{a}->{id} );
+ is( visited( $user ), 'a,q,j,e',
+ 'visited tickets after merging into a ticket that was on the list'
+ );
+
+ $ticket{e}->{ticket}->MergeInto( $ticket{j}->{id} );
+ is( visited( $user ), 'a,q,j',
+ 'visited tickets after merging into a ticket that was on the list (merged)'
+ );
+
+ foreach my $viewed ( reverse qw( r j a h k a q a k i s t f g d ) ) {
+ $user->AddRecentlyViewedTicket( $ticket{$viewed}->{ticket} );
+ }
+
+ is( visited( $user ), 'r,j,a,h,k,q,i,s,t,f',
+ 'visited more than 10 tickets'
+ );
+
+}
+
+done_testing();
+
+sub visited_nb {
+ return scalar shift->RecentlyViewedTickets;
+}
+
+sub visited {
+ return join ',', map { $_->{subject} } shift->RecentlyViewedTickets;
+}
-----------------------------------------------------------------------
More information about the rt-commit
mailing list