Bug 449705: Make buglist.cgi's LookupNamedQuery use Bugzilla::Search::Saved

Bugzilla/Object.pm

@@ -121,8 +121,13 @@ sub check {
     my $check_param = exists $param->{id} ? $param->{id} : $param->{name};
     $check_param = trim($check_param);
     $check_param || ThrowUserError('object_not_specified', { class => $class });
-    my $obj = $class->new($param)
-        || ThrowUserError('object_does_not_exist', {%$param, class => $class});
+    my $obj = $class->new($param);
+    if (!$obj) {
+        # We don't want to override the normal template "user" object if
+        # "user" is one of the params.
+        delete $param->{user};
+        ThrowUserError('object_does_not_exist', { %$param, class => $class });
+    }
     return $obj;
 }
 

Bugzilla/Search/Saved.pm

@@ -32,6 +32,8 @@ use Bugzilla::Search qw(IsValidQueryType);
 use Bugzilla::User;
 use Bugzilla::Util;
 
+use Scalar::Util qw(blessed);
+
 #############
 # Constants #
 #############
@@ -57,6 +59,63 @@ use constant VALIDATORS => {
 
 use constant UPDATE_COLUMNS => qw(name query query_type);
 
+###############
+# Constructor #
+###############
+
+sub new {
+    my $class = shift;
+    my $param = shift;
+    my $dbh = Bugzilla->dbh;
+
+    my $user;
+    if (ref $param) {
+        $user = $param->{user} || Bugzilla->user;
+        my $name = $param->{name};
+        if (!defined $name) {
+            ThrowCodeError('bad_arg',
+                {argument => 'name',
+                 function => "${class}::new"});
+        }
+        my $condition = 'userid = ? AND name = ?';
+        my $user_id = blessed $user ? $user->id : $user;
+        detaint_natural($user_id)
+          || ThrowCodeError('param_must_be_numeric',
+                            {function => $class . '::_init', param => 'user'});
+        my @values = ($user_id, $name);
+        $param = { condition => $condition, values => \@values };
+    }
+
+    unshift @_, $param;
+    my $self = $class->SUPER::new(@_);
+    if ($self) {
+        $self->{user} = $user if blessed $user;
+
+        # Some DBs (read: Oracle) incorrectly mark the query string as UTF-8
+        # when it's coming out of the database, even though it has no UTF-8
+        # characters in it, which prevents Bugzilla::CGI from later reading
+        # it correctly.
+        utf8::downgrade($self->{query}) if utf8::is_utf8($self->{query});
+    }
+    return $self;
+}
+
+sub check {
+    my $class = shift;
+    my $search = $class->SUPER::check(@_);
+    my $user = Bugzilla->user;
+    return $search if $search->user->id == $user->id;
+
+    if (!$search->shared_with_group
+        or !$user->in_group($search->shared_with_group)) 
+    {
+        ThrowUserError('missing_query', { queryname => $search->name, 
+                                          sharer_id => $search->user->id });
+    }
+
+    return $search;
+}
+
 ##############
 # Validators #
 ##############
@@ -210,7 +269,7 @@ sub shared_with_users {
 # Simple Accessors #
 ####################
 
-sub bug_ids_only { return ($_[0]->{'query_type'} == LIST_OF_BUGS) ? 1 : 0; }
+sub type { return $_[0]->{'query_type'}; }
 sub url  { return $_[0]->{'query'}; }
 
 sub user {
@@ -264,7 +323,8 @@ documented below.
 
 =item C<new>
 
-Does not accept a bare C<name> argument. Instead, accepts only an id.
+Takes either an id, or the named parameters C<user> and C<name>.
+C<user> can be either a L<Bugzilla::User> object or a numeric user id.
 
 See also: L<Bugzilla::Object/new>.
 
@@ -297,9 +357,9 @@ Whether or not this search should be displayed in the footer for the
 I<current user> (not the owner of the search, but the person actually
 using Bugzilla right now).
 
-=item C<bug_ids_only>
+=item C<type>
 
-True if the search contains only a list of Bug IDs.
+The numeric id of the type of search this is (from L<Bugzilla::Constants>).
 
 =item C<shared_with_group>
 

Bugzilla/User.pm

@@ -499,6 +499,7 @@ sub bless_groups {
 
 sub in_group {
     my ($self, $group, $product_id) = @_;
+    $group = $group->name if blessed $group;
     if (scalar grep($_->name eq $group, @{ $self->groups })) {
         return 1;
     }

buglist.cgi

@@ -229,64 +229,25 @@ sub DiffDate {
 
 sub LookupNamedQuery {
     my ($name, $sharer_id, $query_type, $throw_error) = @_;
-    my $user = Bugzilla->login(LOGIN_REQUIRED);
-    my $dbh = Bugzilla->dbh;
-    my $owner_id;
     $throw_error = 1 unless defined $throw_error;
 
-    # $name and $sharer_id are safe -- we only use them below in SELECT
-    # placeholders and then in error messages (which are always HTML-filtered).
-    $name || ThrowUserError("query_name_missing");
-    trick_taint($name);
-    if ($sharer_id) {
-        $owner_id = $sharer_id;
-        detaint_natural($owner_id);
-        $owner_id || ThrowUserError('illegal_user_id', {'userid' => $sharer_id});
-    }
-    else {
-        $owner_id = $user->id;
-    }
+    Bugzilla->login(LOGIN_REQUIRED);
 
-    my @args = ($owner_id, $name);
-    my $extra = '';
-    # If $query_type is defined, then we restrict our search.
-    if (defined $query_type) {
-        $extra = ' AND query_type = ? ';
-        detaint_natural($query_type);
-        push(@args, $query_type);
-    }
-    my ($id, $result) = $dbh->selectrow_array("SELECT id, query
-                                                 FROM namedqueries
-                                                WHERE userid = ? AND name = ?
-                                                      $extra",
-                                               undef, @args);
-
-    # Some DBs (read: Oracle) incorrectly mark this string as UTF-8
-    # even though it has no UTF-8 characters in it, which prevents
-    # Bugzilla::CGI from later reading it correctly.
-    utf8::downgrade($result) if utf8::is_utf8($result);
-
-    if (!defined($result)) {
-        return 0 unless $throw_error;
-        ThrowUserError("missing_query", {'queryname' => $name,
-                                         'sharer_id' => $sharer_id});
-    }
+    my $constructor = $throw_error ? 'check' : 'new';
+    my $query = Bugzilla::Search::Saved->$constructor(
+        { user => $sharer_id, name => $name });
 
-    if ($sharer_id) {
-        my $group = $dbh->selectrow_array('SELECT group_id
-                                             FROM namedquery_group_map
-                                            WHERE namedquery_id = ?',
-                                          undef, $id);
-        if (!grep { $_->id == $group } @{ $user->groups }) {
-            ThrowUserError("missing_query", {'queryname' => $name,
-                                             'sharer_id' => $sharer_id});
-        }
+    return $query if (!$query and !$throw_error);
+
+    if (defined $query_type and $query->type != $query_type) {
+        ThrowUserError("missing_query", { queryname => $name,
+                                          sharer_id => $sharer_id });
     }
 
-    $result
-       || ThrowUserError("buglist_parameters_required", {'queryname' => $name});
+    $query->url
+       || ThrowUserError("buglist_parameters_required", { queryname  => $name });
 
-    return wantarray ? ($result, $id) : $result;
+    return wantarray ? ($query->url, $query->id) : $query->url;
 }
 
 # Inserts a Named Query (a "Saved Search") into the database, or

template/en/default/global/code-error.html.tmpl

@@ -340,8 +340,8 @@
 
   [% ELSIF error == "param_must_be_numeric" %]
     [% title = "Invalid Parameter" %]
-    Invalid parameter passed to [% function FILTER html %].
-    It must be numeric.
+    Invalid parameter <code>[% param FILTER html %]</code> passed to 
+    <code>[% function FILTER html %]</code>: It must be numeric.
 
   [% ELSIF error == "param_required" %]
     [% title = "Missing Parameter" %]

template/en/default/global/per-bug-queries.html.tmpl

@@ -54,7 +54,7 @@
     [%# Get existing lists of bugs for this user %]
     [% lists_of_bugs = [] %]
     [% FOREACH q = user.queries %]
-      [% NEXT UNLESS q.bug_ids_only %]
+      [% NEXT UNLESS q.type == constants.LIST_OF_BUGS %]
       [% lists_of_bugs.push(q.name) %]
     [% END %]
     <div class="label"></div>

template/en/default/global/user-error.html.tmpl

@@ -1745,6 +1745,8 @@
     flagtype
   [% ELSIF class == "Bugzilla::Field" %]
     field
+  [% ELSIF class == "Bugzilla::Search::Saved" %]
+    saved search
   [% ELSIF ( matches = class.match('^Bugzilla::Field::Choice::(.+)') ) %]
     [% SET field_name = matches.0 %]
     [% field_descs.$field_name FILTER html %]