[Rt-commit] rt branch, 4.4/recently-viewed-bogus-tickets, created. rt-4.4.4-14-g967526f84
Michel Rodriguez
michel at bestpractical.com
Wed Feb 26 06:26:19 EST 2020
The branch, 4.4/recently-viewed-bogus-tickets has been created
at 967526f8403a22d5331d1f5589c7615d761fff06 (commit)
- Log -----------------------------------------------------------------
commit 6c69944e20fd0e45328c3fd2e9ee0989b28442e9
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
Fixes: I#213199
cosm
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 6c4bc08d8..b2724ffa8 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 );
+ $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 967526f8403a22d5331d1f5589c7615d761fff06
Author: michel <michel at bestpractical.com>
Date: Wed Feb 26 11:29:49 2020 +0100
Tests for recently-viewed-bogus-tickets.
diff --git a/t/api/user_recently_visited.t b/t/api/user_recently_visited.t
new file mode 100644
index 000000000..e72f60cb0
--- /dev/null
+++ b/t/api/user_recently_visited.t
@@ -0,0 +1,77 @@
+use strict;
+use warnings;
+
+# test the RecentlyViewedTickets method
+
+use Test::Deep;
+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( $user->_visited_nb, 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( $user->_visited_nb, 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( $user->_visited, '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( $user->_visited, '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( $user->_visited, '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( $user->_visited, '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( $user->_visited, 'a,q,j,e', 'visited tickets after merging into a ticket that was on the list');
+
+ $ticket{e}->{ticket}->MergeInto( $ticket{j}->{id} );
+ is( $user->_visited, '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( $user->_visited, 'r,j,a,h,k,q,i,s,t,f,g', 'visited more than 11 tickets');
+
+}
+
+done_testing();
+
+# monkey patching not strictly necessary, but makes for a nicer syntax in the tests
+package RT::User;
+
+sub _visited_nb {
+ return scalar shift->RecentlyViewedTickets;
+}
+
+sub _visited {
+ return join ',', map { $_->{subject} } shift->RecentlyViewedTickets;
+}
-----------------------------------------------------------------------
More information about the rt-commit
mailing list