Bug 644281: When the sort order of a buglist is modified, the "Show next bug in my list" user pref still uses the original sort order to decide which bug to display next

r=glob a=LpSolit

2 files changed, 25 insertions, 31 deletions

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 849c1d915..8455e64c1 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -516,26 +516,21 @@ sub save_last_search {
 
     return if !@$bug_ids;
 
+    my $search;
     if ($self->id) {
         on_main_db {
-            my $search;
             if ($list_id) {
-                # Use eval so that people can still use old search links or
-                # links that don't belong to them.
-                $search = eval { Bugzilla::Search::Recent->check(
-                                    { id => $list_id }) };
+                $search = Bugzilla::Search::Recent->check_quietly({ id => $list_id });
             }
 
             if ($search) {
-                # We only update placeholders. (Placeholders are
-                # Saved::Search::Recent objects with empty bug lists.)
-                # Otherwise, we could just keep creating new searches
-                # for the same refreshed list over and over.
-                if (!@{ $search->bug_list }) {
-                    $search->set_list_order($order);
+                if (join(',', @{$search->bug_list}) ne join(',', @$bug_ids)) {
                     $search->set_bug_list($bug_ids);
-                    $search->update();
                 }
+                if (!$search->list_order || $order ne $search->list_order) {
+                    $search->set_list_order($order);
+                }
+                $search->update();
             }
             else {
                 # If we already have an existing search with a totally
@@ -548,11 +543,14 @@ sub save_last_search {
                     user_id => $self->id, bug_list => $list_string });
            
                 if (!scalar(@$existing_search)) {
-                    Bugzilla::Search::Recent->create({
+                    $search = Bugzilla::Search::Recent->create({
                         user_id    => $self->id,
                         bug_list   => $bug_ids,
                         list_order => $order });
                 }
+                else {
+                    $search = $existing_search->[0];
+                }
             }
         };
         delete $self->{recent_searches};
@@ -574,6 +572,7 @@ sub save_last_search {
             $vars->{'toolong'} = 1;
         }
     }
+    return $search;
 }
 
 sub settings {
diff --git a/buglist.cgi b/buglist.cgi
index 85a8ae760..7b513e607 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -164,14 +164,13 @@ my $params;
 # If the user is retrieving the last bug list they looked at, hack the buffer
 # storing the query string so that it looks like a query retrieving those bugs.
 if (my $last_list = $cgi->param('regetlastlist')) {
-    my ($bug_ids, $order);
+    my $bug_ids;
 
     # Logged-out users use the old cookie method for storing the last search.
     if (!$user->id or $last_list eq 'cookie') {
-        $cgi->cookie('BUGLIST') || ThrowUserError("missing_cookie");
-        $order = "reuse last sort" unless $order;
-        $bug_ids = $cgi->cookie('BUGLIST');
+        $bug_ids = $cgi->cookie('BUGLIST') or ThrowUserError("missing_cookie");
         $bug_ids =~ s/[:-]/,/g;
+        $order ||= "reuse last sort";
     }
     # But logged in users store the last X searches in the DB so they can
     # have multiple bug lists available.
@@ -179,10 +178,11 @@ if (my $last_list = $cgi->param('regetlastlist')) {
         my $last_search = Bugzilla::Search::Recent->check(
             { id => $last_list });
         $bug_ids = join(',', @{ $last_search->bug_list });
-        $order   = $last_search->list_order if !$order;
+        $order ||= $last_search->list_order;
     }
     # set up the params for this new query
     $params = new Bugzilla::CGI({ bug_id => $bug_ids, order => $order });
+    $params->param('list_id', $last_list);
 }
 
 # Figure out whether or not the user is doing a fulltext search.  If not,
@@ -972,14 +972,6 @@ if ($format->{'extension'} eq 'ics') {
     }
 }
 
-# The list of query fields in URL query string format, used when creating
-# URLs to the same query results page with different parameters (such as
-# a different sort order or when taking some action on the set of query
-# results).  To get this string, we call the Bugzilla::CGI::canoncalise_query
-# function with a list of elements to be removed from the URL.
-$vars->{'urlquerypart'} = $params->canonicalise_query('order',
-                                                      'cmdtype',
-                                                      'query_based_on');
 $vars->{'order'} = $order;
 $vars->{'caneditbugs'} = 1;
 $vars->{'time_info'} = $time_info;
@@ -1113,17 +1105,20 @@ my $contenttype;
 my $disposition = "inline";
 
 if ($format->{'extension'} eq "html" && !$agent) {
-    if (!$cgi->param('regetlastlist')) {
-        Bugzilla->user->save_last_search(
-            { bugs => \@bugidlist, order => $order, vars => $vars,
-              list_id => scalar $cgi->param('list_id') });
-    }
+    my $list_id = $cgi->param('list_id') || $cgi->param('regetlastlist');
+    my $search = $user->save_last_search(
+        { bugs => \@bugidlist, order => $order, vars => $vars, list_id => $list_id });
+    $cgi->param('list_id', $search->id) if $search;
     $contenttype = "text/html";
 }
 else {
     $contenttype = $format->{'ctype'};
 }
 
+# Set 'urlquerypart' once the buglist ID is known.
+$vars->{'urlquerypart'} = $params->canonicalise_query('order', 'cmdtype',
+                                                      'query_based_on');
+
 if ($format->{'extension'} eq "csv") {
     # We set CSV files to be downloaded, as they are designed for importing
     # into other programs.