Bug 24896: Make the First/Last/Prev/Next navigation on bugs work with

multiple buglists at once
r=glob, a=mkanat

7 files changed, 346 insertions, 10 deletions

diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
index 00f23c393..224782152 100644
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -204,6 +204,10 @@ sub clean_search_url {
         $self->delete('order');
     }
 
+    # list_id is added in buglist.cgi after calling clean_search_url,
+    # and doesn't need to be saved in saved searches.
+    $self->delete('list_id'); 
+
     # And now finally, if query_format is our only parameter, that
     # really means we have no parameters, so we should delete query_format.
     if ($self->param('query_format') && scalar($self->param()) == 1) {
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 875a50133..d4e8782c6 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -84,6 +84,8 @@ use File::Basename;
     QUERY_LIST
     LIST_OF_BUGS
 
+    SAVE_NUM_SEARCHES
+
     COMMENT_COLS
     MAX_COMMENT_LENGTH
 
@@ -284,6 +286,9 @@ use constant DEFAULT_MILESTONE => '---';
 use constant QUERY_LIST => 0;
 use constant LIST_OF_BUGS => 1;
 
+# How many of the user's most recent searches to save.
+use constant SAVE_NUM_SEARCHES => 10;
+
 # The column length for displayed (and wrapped) bug comments.
 use constant COMMENT_COLS => 80;
 # Used in _check_comment(). Gives the max length allowed for a comment.
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 97e59efbd..2f74912c2 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -842,6 +842,18 @@ use constant ABSTRACT_SCHEMA => {
         ],
     },
 
+    profile_search => {
+        FIELDS => [
+            id         => {TYPE => 'INTSERIAL', NOTNULL => 1, PRIMARYKEY => 1},
+            user_id    => {TYPE => 'INT3', NOTNULL => 1},
+            bug_list   => {TYPE => 'MEDIUMTEXT', NOTNULL => 1},
+            list_order => {TYPE => 'MEDIUMTEXT'},
+        ],
+        INDEXES => [
+            profile_search_user_id => [qw(user_id)],
+        ],
+    },
+
     profiles_activity => {
         FIELDS => [
             userid        => {TYPE => 'INT3', NOTNULL => 1,
diff --git a/Bugzilla/Search/Recent.pm b/Bugzilla/Search/Recent.pm
new file mode 100644
index 000000000..14d5ebef4
--- /dev/null
+++ b/Bugzilla/Search/Recent.pm
@@ -0,0 +1,147 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Bug Tracking System.
+#
+# The Initial Developer of the Original Code is Everything Solved, Inc.
+# Portions created by the Initial Developer are Copyright (C) 2010 the
+# Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#   Max Kanat-Alexander <mkanat@bugzilla.org>
+
+package Bugzilla::Search::Recent;
+use strict;
+use base qw(Bugzilla::Object);
+
+use Bugzilla::Constants;
+use Bugzilla::Error;
+use Bugzilla::Util;
+
+#############
+# Constants #
+#############
+
+use constant DB_TABLE => 'profile_search';
+use constant LIST_ORDER => 'id';
+
+use constant REQUIRED_CREATE_FIELDS => ();
+
+use constant DB_COLUMNS => qw(
+    id
+    user_id
+    bug_list
+    list_order
+);
+
+use constant VALIDATORS => {
+    user_id    => \&_check_user_id,
+    bug_list   => \&_check_bug_list,
+    list_order => \&_check_list_order,
+};
+
+use constant UPDATE_COLUMNS => qw(bug_list list_order);
+
+###################
+# DB Manipulation #
+###################
+
+sub create {
+    my $class = shift;
+    my $dbh = Bugzilla->dbh;
+    $dbh->bz_start_transaction();
+    my $search = $class->SUPER::create(@_);   
+
+    # Enforce there only being SAVE_NUM_SEARCHES per user.
+    my ($num_searches, $min_id) = $dbh->selectrow_array(
+        'SELECT COUNT(*), MIN(id) FROM profile_search WHERE user_id = ?',
+        undef, $search->user_id);
+    if ($num_searches > SAVE_NUM_SEARCHES) {
+        $dbh->do('DELETE FROM profile_search WHERE id = ?', undef, $min_id);
+    }
+    $dbh->bz_commit_transaction();
+    return $search;
+}
+
+sub create_placeholder {
+    my $class = shift;
+    return $class->create({ user_id  => Bugzilla->user->id,
+                            bug_list => '' });
+}
+
+###############
+# Constructor #
+###############
+
+sub check {
+    my $class = shift;
+    my $search = $class->SUPER::check(@_);
+    my $user = Bugzilla->user;
+    if ($search->user_id != $user->id) {
+        ThrowUserError('object_not_found', { id => $search->id });
+    }
+    return $search;
+}
+
+####################
+# Simple Accessors #
+####################
+
+sub bug_list   { return [split(',', $_[0]->{'bug_list'})]; }
+sub list_order { return $_[0]->{'list_order'}; }
+sub user_id    { return $_[0]->{'user_id'}; }
+
+############
+# Mutators #
+############
+
+sub set_bug_list   { $_[0]->set('bug_list',   $_[1]); }
+sub set_list_order { $_[0]->set('list_order', $_[1]); }
+
+##############
+# Validators #
+##############
+
+sub _check_user_id {
+    my ($invocant, $id) = @_;
+    require Bugzilla::User;
+    return Bugzilla::User->check({ id => $id })->id;
+}
+
+sub _check_bug_list {
+    my ($invocant, $list) = @_;
+
+    my @bug_ids = ref($list) ? @$list : split(',', $list);
+    detaint_natural($_) foreach @bug_ids;
+    return join(',', @bug_ids);
+}
+
+sub _check_list_order { defined $_[1] ? trim($_[1]) : '' }
+
+1;
+
+__END__
+
+=head1 NAME
+
+Bugzilla::Search::Recent - A search recently run by a logged-in user.
+
+=head1 SYNOPSIS
+
+ use Bugzilla::Search::Recent;
+
+
+=head1 DESCRIPTION
+
+This is an implementation of L<Bugzilla::Object>, and so has all the
+same methods available as L<Bugzilla::Object>, in addition to what is
+documented below.
diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index 274ed8847..00d7b6d28 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -764,15 +764,6 @@ sub create {
             # Whether or not keywords are enabled, in this Bugzilla.
             'use_keywords' => sub { return Bugzilla::Keyword->any_exist; },
 
-            'last_bug_list' => sub {
-                my @bug_list;
-                my $cgi = Bugzilla->cgi;
-                if ($cgi->cookie("BUGLIST")) {
-                    @bug_list = split(/:/, $cgi->cookie("BUGLIST"));
-                }
-                return \@bug_list;
-            },
-
             'feature_enabled' => sub { return Bugzilla->feature(@_); },
 
             # field_descs can be somewhat slow to generate, so we generate
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 936cf36e4..815b435fd 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -44,6 +44,7 @@ package Bugzilla::User;
 use Bugzilla::Error;
 use Bugzilla::Util;
 use Bugzilla::Constants;
+use Bugzilla::Search::Recent;
 use Bugzilla::User::Setting;
 use Bugzilla::Product;
 use Bugzilla::Classification;
@@ -51,8 +52,11 @@ use Bugzilla::Field;
 use Bugzilla::Group;
 
 use DateTime::TimeZone;
+use List::Util qw(max);
 use Scalar::Util qw(blessed);
 use Storable qw(dclone);
+use URI;
+use URI::QueryParam;
 
 use base qw(Bugzilla::Object Exporter);
 @Bugzilla::User::EXPORT = qw(is_available_username
@@ -361,6 +365,149 @@ sub queries_available {
     return $self->{queries_available};
 }
 
+##########################
+# Saved Recent Bug Lists #
+##########################
+
+sub recent_searches {
+    my $self = shift;
+    $self->{recent_searches} ||= 
+        Bugzilla::Search::Recent->match({ user_id => $self->id });
+    return $self->{recent_searches};
+}
+
+sub recent_search_containing {
+    my ($self, $bug_id) = @_;
+    my $searches = $self->recent_searches;
+
+    foreach my $search (@$searches) {
+        return $search if grep($_ == $bug_id, @{ $search->bug_list });
+    }
+
+    return undef;
+}
+
+sub recent_search_for {
+    my ($self, $bug) = @_;
+    my $params = Bugzilla->input_params;
+    my $cgi = Bugzilla->cgi;
+
+    if ($self->id) {
+        # First see if there's a list_id parameter in the query string.
+        my $list_id = $params->{list_id};
+        if (!$list_id) {
+            # If not, check for "list_id" in the query string of the referer.
+            my $referer = $cgi->referer;
+            if ($referer) {
+                my $uri = URI->new($referer);
+                if ($uri->path =~ /buglist\.cgi$/) {
+                    $list_id = $uri->query_param('list_id')
+                               || $uri->query_param('regetlastlist');
+                }
+            }
+        }
+
+        if ($list_id) {
+            # If we got a bad list_id (either some other user's or an expired
+            # one) don't crash, just don't return that list.
+            my $search = 
+                eval { Bugzilla::Search::Recent->check({ id => $list_id }) };
+            return $search if $search;
+        }
+
+        # If there's no list_id, see if the current bug's id is contained
+        # in any of the user's saved lists.
+        my $search = $self->recent_search_containing($bug->id);
+        return $search if $search;
+    }
+
+    # Finally (or always, if we're logged out), if there's a BUGLIST cookie
+    # and the selected bug is in the list, then return the cookie as a fake
+    # Search::Recent object.
+    if (my $list = $cgi->cookie('BUGLIST')) {
+        my @bug_ids = split(':', $list);
+        if (grep { $_ == $bug->id } @bug_ids) {
+            return { id => 'cookie', bug_list => \@bug_ids };
+        }
+    }
+
+    return undef;
+}
+
+sub save_last_search {
+    my ($self, $params) = @_;
+    my ($bug_ids, $order, $vars, $list_id) = 
+        @$params{qw(bugs order vars list_id)};
+
+    my $cgi = Bugzilla->cgi;
+    if ($order) {
+        $cgi->send_cookie(-name => 'LASTORDER',
+                          -value => $order,
+                          -expires => 'Fri, 01-Jan-2038 00:00:00 GMT');
+    }
+
+    return if !@$bug_ids;
+
+    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 }) };
+            }
+
+            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);
+                    $search->set_bug_list($bug_ids);
+                    $search->update();
+                }
+            }
+            else {
+                # If we already have an existing search with a totally
+                # identical bug list, then don't create a new one. This
+                # prevents people from writing over their whole 
+                # recent-search list by just refreshing a saved search
+                # (which doesn't have list_id in the header) over and over.
+                my $list_string = join(',', @$bug_ids);
+                my $existing_search = Bugzilla::Search::Recent->match({
+                    user_id => $self->id, bug_list => $list_string });
+           
+                if (!scalar(@$existing_search)) {
+                    Bugzilla::Search::Recent->create({
+                        user_id    => $self->id,
+                        bug_list   => $bug_ids,
+                        list_order => $order });
+                }
+            }
+        };
+        delete $self->{recent_searches};
+    }
+    # Logged-out users use a cookie to store a single last search. We don't
+    # override that cookie with the logged-in user's latest search, because
+    # if they did one search while logged out and another while logged in,
+    # they may still want to navigate through the search they made while
+    # logged out.
+    else {
+        my $bug_list = join(":", @$bug_ids);
+        if (length($bug_list) < 4000) {
+            $cgi->send_cookie(-name => 'BUGLIST',
+                              -value => $bug_list,
+                              -expires => 'Fri, 01-Jan-2038 00:00:00 GMT');
+        }
+        else {
+            $cgi->remove_cookie('BUGLIST');
+            $vars->{'toolong'} = 1;
+        }
+    }
+}
+
 sub settings {
     my ($self) = @_;
 
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 03826c143..840225fbe 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -37,7 +37,7 @@ use base qw(Exporter);
                              css_class_quote html_light_quote url_decode
                              i_am_cgi correct_urlbase remote_ip
                              do_ssl_redirect_if_required use_attachbase
-                             diff_arrays
+                             diff_arrays on_main_db
                              trim wrap_hard wrap_comment find_wrap_point
                              format_time format_time_decimal validate_date
                              validate_time datetime_from
@@ -597,6 +597,14 @@ sub clean_text {
     return trim($dtext);
 }
 
+sub on_main_db (&) {
+    my $code = shift;
+    my $original_dbh = Bugzilla->dbh;
+    Bugzilla->request_cache->{dbh} = Bugzilla->dbh_main;
+    $code->();
+    Bugzilla->request_cache->{dbh} = $original_dbh;
+}
+
 sub get_text {
     my ($name, $vars) = @_;
     my $template = Bugzilla->template_inner;
@@ -690,6 +698,11 @@ Bugzilla::Util - Generic utility functions for bugzilla
   validate_email_syntax($email);
   validate_date($date);
 
+  # DB-related functions
+  on_main_db {
+     ... code here ...
+  };
+
 =head1 DESCRIPTION
 
 This package contains various utility functions which do not belong anywhere
@@ -994,3 +1007,20 @@ Make sure the date has the correct format and returns 1 if
 the check is successful, else returns 0.
 
 =back
+
+=head2 Database
+
+=over
+
+=item C<on_main_db>
+
+Runs a block of code always on the main DB. Useful for when you're inside
+a subroutine and need to do some writes to the database, but don't know
+if Bugzilla is currently using the shadowdb or not. Used like:
+
+ on_main_db {
+     my $dbh = Bugzilla->dbh;
+     $dbh->do("INSERT ...");
+ }
+
+back