Bug 1169181 - The bug_user_last_visit method returns an empty array for old bugs

r=dkl

3 files changed, 23 insertions, 30 deletions

diff --git a/Bugzilla/API/1_0/Resource/BugUserLastVisit.pm b/Bugzilla/API/1_0/Resource/BugUserLastVisit.pm
index fd25524f9..1d2d4f582 100644
--- a/Bugzilla/API/1_0/Resource/BugUserLastVisit.pm
+++ b/Bugzilla/API/1_0/Resource/BugUserLastVisit.pm
@@ -86,7 +86,7 @@ sub update {
 
         push(
             @results,
-            $self->_bug_user_last_visit_to_hash(
+            _bug_user_last_visit_to_hash(
                 $bug, $last_visit_ts, $params
             ));
     }
@@ -102,31 +102,27 @@ sub get {
 
     $user->login(LOGIN_REQUIRED);
 
+    my @last_visits;
     if ($ids) {
         # Cache permissions for bugs. This highly reduces the number of calls to
         # the DB.  visible_bugs() is only able to handle bug IDs, so we have to
         # skip aliases.
         $user->visible_bugs([grep /^[0-9]$/, @$ids]);
-    }
-
-    my @last_visits = @{ $user->last_visited };
 
-    if ($ids) {
-        # remove bugs that we are not interested in if ids is passed in.
-        my %id_set = map { ($_ => 1) } @$ids;
-        @last_visits = grep { $id_set{ $_->bug_id } } @last_visits;
+        my %last_visit  = map { $_->bug_id => $_->last_visit_ts } @{ $user->last_visited($ids) };
+        @last_visits = map { _bug_user_last_visit_to_hash($_, $last_visit{$_}, $params) } @$ids;
+    }
+    else {
+        @last_visits = map {
+            _bug_user_last_visit_to_hash($_->bug_id, $_->last_visit_ts, $params)
+        } @{ $user->last_visited };
     }
 
-    return [
-        map {
-            $self->_bug_user_last_visit_to_hash($_->bug_id, $_->last_visit_ts,
-                $params)
-        } @last_visits
-    ];
+    return \@last_visits;
 }
 
 sub _bug_user_last_visit_to_hash {
-    my ($self, $bug_id, $last_visit_ts, $params) = @_;
+    my ($bug_id, $last_visit_ts, $params) = @_;
 
     my %result = (id            => as_int($bug_id),
                   last_visit_ts => as_datetime($last_visit_ts));
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index ca9b5aa28..caaa6c51a 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -930,9 +930,10 @@ sub groups {
 }
 
 sub last_visited {
-    my ($self) = @_;
+    my ($self, $ids) = @_;
 
-    return Bugzilla::BugUserLastVisit->match({ user_id => $self->id });
+    return Bugzilla::BugUserLastVisit->match({ user_id => $self->id,
+                                               $ids ? ( bug_id => $ids ) : () });
 }
 
 sub is_involved_in_bug {
diff --git a/Bugzilla/WebService/BugUserLastVisit.pm b/Bugzilla/WebService/BugUserLastVisit.pm
index 19a56ff46..3ee8cac03 100644
--- a/Bugzilla/WebService/BugUserLastVisit.pm
+++ b/Bugzilla/WebService/BugUserLastVisit.pm
@@ -67,27 +67,23 @@ sub get {
 
     $user->login(LOGIN_REQUIRED);
 
+    my @last_visits;
     if ($ids) {
         # Cache permissions for bugs. This highly reduces the number of calls to
         # the DB.  visible_bugs() is only able to handle bug IDs, so we have to
         # skip aliases.
         $user->visible_bugs([grep /^[0-9]$/, @$ids]);
-    }
-
-    my @last_visits = @{ $user->last_visited };
 
-    if ($ids) {
-        # remove bugs that we are not interested in if ids is passed in.
-        my %id_set = map { ($_ => 1) } @$ids;
-        @last_visits = grep { $id_set{ $_->bug_id } } @last_visits;
+        my %last_visit  = map { $_->bug_id => $_->last_visit_ts } @{ $user->last_visited($ids) };
+        @last_visits = map { $self->_bug_user_last_visit_to_hash($_, $last_visit{$_}, $params) } @$ids;
+    }
+    else {
+        @last_visits = map {
+            $self->_bug_user_last_visit_to_hash($_->bug_id, $_->last_visit_ts, $params)
+        } @{ $user->last_visited };
     }
 
-    return [
-        map {
-            $self->_bug_user_last_visit_to_hash($_->bug_id, $_->last_visit_ts,
-                $params)
-        } @last_visits
-    ];
+    return \@last_visits;
 }
 
 sub _bug_user_last_visit_to_hash {