[Rt-commit] rt branch, 4.4/recently-viewed-bogus-tickets, created. rt-4.4.4-14-gf06ee2882
Michel Rodriguez
michel at bestpractical.com
Fri Apr 12 10:43:32 EDT 2019
The branch, 4.4/recently-viewed-bogus-tickets has been created
at f06ee2882efd363213d9f9dd6746a3c28e8883d5 (commit)
- Log -----------------------------------------------------------------
commit 8f74b487703ff2e7884c05001904bd21bed9255e
Author: michel <michel at bestpractical.com>
Date: Thu Apr 11 16:00:29 2019 +0200
fixes 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
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 6c4bc08d8..76c9de808 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -2179,16 +2179,53 @@ 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..e5eef4fbe 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" )
diff --git a/t/ticket/recently_visited.t b/t/ticket/recently_visited.t
new file mode 100644
index 000000000..f7fb53cd5
--- /dev/null
+++ b/t/ticket/recently_visited.t
@@ -0,0 +1,69 @@
+use strict;
+use warnings;
+
+# test the RecentlyViewedTickets method
+
+use Test::Deep;
+use RT::Test::Shredder tests => 8;
+my $test = "RT::Test::Shredder";
+
+$test->create_savepoint('clean');
+
+use RT::Ticket;
+
+{ my @tickets=(undef); # the empty element is there so that $ticket[$i]->Id is $i
+ # (arrays start at 0, ids at 1);
+
+
+ my $user = RT->SystemUser;
+ is( $user->_visited_nb, 0, 'init: empty RecentlyViewedTickets');
+
+ foreach my $ticket_nb (1..20) {
+ my $ticket = RT::Ticket->new( $user );
+ my ($id) = $ticket->Create( Subject => "test #$ticket_nb", Queue => 1 );
+ push @tickets, $ticket;
+ $ticket->ApplyTransactionBatch;
+ }
+
+ is( $user->_visited_nb, 0, 'tickets created: empty RecentlyViewedTickets');
+
+ foreach my $viewed ( 1, 2, 3, 4, 5, 2, 16, 17, 3) {
+ $user->AddRecentlyViewedTicket( $tickets[$viewed]);
+ }
+
+ is( $user->_visited, '3,17,16,2,5,4,1', 'visited tickets after inital visits');
+
+ my $shredder = $test->shredder_new();
+
+ $shredder->PutObjects( Objects => $tickets[13] );
+ $shredder->WipeoutAll;
+ is( $user->_visited, '3,17,16,2,5,4,1', 'visited tickets after shredding an unvisited ticket');
+
+ $shredder->PutObjects( Objects => $tickets[16]);
+ $shredder->PutObjects( Objects => $tickets[4] );
+ $shredder->WipeoutAll;
+ is( $user->_visited, '3,17,2,5,1', 'visited tickets after shredding 2 visited tickets');
+
+ $tickets[2]->MergeInto( 10);
+ is( $user->_visited, '3,17,10,5,1', 'visited tickets after merging into a ticket that was NOT on the list');
+
+ $tickets[5]->MergeInto( 10);
+ is( $user->_visited, '3,17,10,1', 'visited tickets after merging into a ticket that was on the list');
+
+ foreach my $viewed ( 12, 14, 18, 10, 3, 8, 11, 3, 17, 3, 11, 9) {
+ $user->AddRecentlyViewedTicket( $tickets[$viewed]);
+ }
+ is( $user->_visited, '9,11,3,17,8,10,18,14,12,1', 'visited more than 10 tickets');
+}
+
+package RT::User;
+
+sub _visited_nb
+ { return scalar shift->RecentlyViewedTickets; }
+
+sub _visited
+ { return join ',', map { $_->{id} } shift->RecentlyViewedTickets; }
+
+
+
+
commit f06ee2882efd363213d9f9dd6746a3c28e8883d5
Author: michel <michel at bestpractical.com>
Date: Fri Apr 12 16:42:31 2019 +0200
Fixes 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/t/ticket/recently_visited.t b/t/ticket/recently_visited.t
deleted file mode 100644
index f7fb53cd5..000000000
--- a/t/ticket/recently_visited.t
+++ /dev/null
@@ -1,69 +0,0 @@
-use strict;
-use warnings;
-
-# test the RecentlyViewedTickets method
-
-use Test::Deep;
-use RT::Test::Shredder tests => 8;
-my $test = "RT::Test::Shredder";
-
-$test->create_savepoint('clean');
-
-use RT::Ticket;
-
-{ my @tickets=(undef); # the empty element is there so that $ticket[$i]->Id is $i
- # (arrays start at 0, ids at 1);
-
-
- my $user = RT->SystemUser;
- is( $user->_visited_nb, 0, 'init: empty RecentlyViewedTickets');
-
- foreach my $ticket_nb (1..20) {
- my $ticket = RT::Ticket->new( $user );
- my ($id) = $ticket->Create( Subject => "test #$ticket_nb", Queue => 1 );
- push @tickets, $ticket;
- $ticket->ApplyTransactionBatch;
- }
-
- is( $user->_visited_nb, 0, 'tickets created: empty RecentlyViewedTickets');
-
- foreach my $viewed ( 1, 2, 3, 4, 5, 2, 16, 17, 3) {
- $user->AddRecentlyViewedTicket( $tickets[$viewed]);
- }
-
- is( $user->_visited, '3,17,16,2,5,4,1', 'visited tickets after inital visits');
-
- my $shredder = $test->shredder_new();
-
- $shredder->PutObjects( Objects => $tickets[13] );
- $shredder->WipeoutAll;
- is( $user->_visited, '3,17,16,2,5,4,1', 'visited tickets after shredding an unvisited ticket');
-
- $shredder->PutObjects( Objects => $tickets[16]);
- $shredder->PutObjects( Objects => $tickets[4] );
- $shredder->WipeoutAll;
- is( $user->_visited, '3,17,2,5,1', 'visited tickets after shredding 2 visited tickets');
-
- $tickets[2]->MergeInto( 10);
- is( $user->_visited, '3,17,10,5,1', 'visited tickets after merging into a ticket that was NOT on the list');
-
- $tickets[5]->MergeInto( 10);
- is( $user->_visited, '3,17,10,1', 'visited tickets after merging into a ticket that was on the list');
-
- foreach my $viewed ( 12, 14, 18, 10, 3, 8, 11, 3, 17, 3, 11, 9) {
- $user->AddRecentlyViewedTicket( $tickets[$viewed]);
- }
- is( $user->_visited, '9,11,3,17,8,10,18,14,12,1', 'visited more than 10 tickets');
-}
-
-package RT::User;
-
-sub _visited_nb
- { return scalar shift->RecentlyViewedTickets; }
-
-sub _visited
- { return join ',', map { $_->{id} } shift->RecentlyViewedTickets; }
-
-
-
-
-----------------------------------------------------------------------
More information about the rt-commit
mailing list