Closed Bug 398281 Opened 17 years ago Closed 16 years ago

Basic search for bugs via WebService (Bug.search)

Categories

(Bugzilla :: WebService, enhancement, P1)

Product:
Bugzilla
Component:
WebService
Version:
3.1.3
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 3.4

People

(Reporter: nick.breau, Assigned: mkanat)

References

Details

(Whiteboard: [roadmap: 4.0])

Attachments

(3 files, 12 obsolete files)

perl script for demonstration of criteria and composite order
3.03 KB, text/plain
Details
Code for implementing complicated advanced search via WebService
29.00 KB, patch
LpSolit
: review-
Details | Diff | Splinter Review
v4
11.76 KB, patch
dkl
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.6) Gecko/20061201 Firefox/2.0.0.6 (Ubuntu-feisty)
Build Identifier: 

Currently the xml-rpc does not implement any methods that can be used to fetch a list of bugs according to various criteria, for example, fetch a list of bugs assigned to user x.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
I would love to have it implemented for 3.2. Max, if you read this. ;)
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
First we have to re-write Search.pm to take normal arguments instead of a CGI object, and return just a list of bug ids or Bug objects.
Depends on: 278032, 398308
Summary: Need functionality to fetch a list of bugs assigned to a user, product, etc... → Search for bugs via WebService
For what it's worth, this would add a significant delay to 3.2, and so I don't think it's going to make it.
Target Milestone: --- → Bugzilla 4.0
If we call QuickSearch.pm, this would save us some pain. And this would give us a chance to have it for 3.2.
I'm not going to make the crazy syntax of QuickSearch into a permanent API for Bugzilla.
Well, I expect to use QS features from the webservice, among others. So we could start with it. So you could either pass {quicksearch => $some_string} or a full hash with some syntax to define. But I don't why we should forbird the 'quicksearch' hash key.
Hi Guys,

The full functionality of the search isn't expected by any means, but it would be nice to have some xml-rpc method calls where a list of bugs could somehow be fetched. As far as I can tell currently the only bugs that could be listed in a 3rd party frontend using the xml-rpc would be the bugs the user creates in that current session, using the id fetched from the bug's create method call.

Nick.

(In reply to comment #7)
> As far as I can tell currently the only bugs that could be listed in a
> 3rd party frontend using the xml-rpc would be the bugs the user creates in that
> current session, using the id fetched from the bug's create method call.

  No, you can use get_bugs to get any list of bug ids.
(In reply to comment #6)
> Well, I expect to use QS features from the webservice

  For what use case? Who would actually need that?
It accepts a list of bug id's as a parameter.

When a user first logs in, how would you display a list of bugs, ie is there any way to fetch bug id's aside from the bug_id returned when creating a bug ?
(In reply to comment #10)
> When a user first logs in, how would you display a list of bugs, ie is there
> any way to fetch bug id's aside from the bug_id returned when creating a bug ?

  No, not at the moment.
I have developed two pages of perl code to implement a basic search function. I am calling it Bugs.find(), it takes a hash ref as input and requires a mandatory booleancharts key-value pair, an optional limit key-value pair and an optional order key-value pair.

The idea behind it is to maximize re-use of existing code and features that are familiar to WebService API programmer and yet simple enough to be used.

The booleancharts will emulate the standard predicate convention we use in the Boolean Charts of Bugzilla::Search. 

The limit and the order may be used to implement navigatioin functionality, page fetch etc. later.

The following is an example on how I would set up a simple query:

Query Text: 
<Get me all the customer related bugs that is modified in the last 12 hours only>

Here is the code sample
-----------------------

my $soapresult = 
Bug.find({
  booleancharts => [
    [
       [ 
          ["cf_cust_issue", "equals", 1],
       ], #row 1
       [
          ["delta_ts", "greaterthan", "+12h"],
       ]
       # -> ("cf_cust_issue" = 1 AND delta_ts > '2007-12-05 04:10:00')
    ], #chart 1
    # ... chart 2 here etc.
  ], #charts
 limit => 50, # could be huge, so I am pulling out the top 50
 order => 'bugs.bug_id DESC', # reverse, generally most recent
 });

-----------------------

The booleanchart fields are identical to the what Search accepts internally, so there will not be any logics added that may introduce bugs into the WebService API.
 
Charts are added as AND predicates,
Columns are added as OR predicates,
Rows are added as AND predicates and so on...

Using of internal tokens is based on the anticipation of Internationalization issue, although this can be circumvented by 'eval'ing what embedded in the search-*.html.tmpl. But for the first cut, the internal literals is already language independent. 

The returned result will be identical to the Bugs.get_bugs(), in fact the Bugs.find() issue a call to Bugs.get_bugs() to create all the Bugzilla::Bug objects.

I diff(ed) the Search.pm between 3.0.2 (which is the one that I am working on right now) verses Search.pm of 3.1.2, the deltas are changes made in the email parsing of the funcdefs function table and should not affect anything external. So the Bugs.find() shouldn't not affected by the changes as well.

I have run test on the WebService API, for with the above constraint, and it is running. 

Our company likes to contribute this work back into the existing Bugzilla build, so please review the API parameter and let me know of any comment question.

Constructive criticism are always welcome, I am also a newbie developer in Bugzilla, please provide any 'education' on how to contribute the code back to the project as needed.

- Clement

Let's call it Bugs.search. The API looks OK, but you will need to translate the names of certain fields into things that are more sane. For example, instead of delta_ts on input, it should be called last_modified.

For how to contribute code, see:

http://wiki.mozilla.org/Bugzilla:Developers
Assignee: webservice → cchan
Also, call the parameter "criteria" instead of "booleancharts".

And to really approve the API, I'd have to see the code.
Max,

I would change the method to search, and booleancharts to criteria.

I kind of like the idea of using the column name, because their are already exposed on the URL when query.cgi send a HTTP GET to buglist.cgi. That would also help application developers verify the criteria composition easily. 

In regards to field name, as an alternative to satisfy both the bugzilla long term support and the WebService API applications, how about if I create a translation hash map that will support the existing static fields from the table fielddefs and document the map values?.

If the field name cannot be found, it will become a pass through value. For example, other bugzilla sites can create their own custom field that won't be included in the translation hash map. But the pass through logic will encompass these custom fields.












In addition, to the search method, I also added the get_duplicate argument parameter to trace back to the source of the duplicate bug if the status and resolution both became 'RESOLVED' and 'DUPLICATE'. This will allow the tracking of disjointed problem with going into the oblivion. 
@Max, Bugzilla/WebService/Bug.pm is pasted here for review and approval.

# -*- 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.
#
# Contributor(s): Marc Schumann <wurblzap@gmail.com>
#                 Max Kanat-Alexander <mkanat@bugzilla.org>
#                 Clement Ka Chan <cchan@mvista.com>

package Bugzilla::WebService::Bug;

use strict;
use base qw(Bugzilla::WebService);
import SOAP::Data qw(type);

use Bugzilla::Constants;
use Bugzilla::Error;
use Bugzilla::Field;
use Bugzilla::WebService::Constants;
use Bugzilla::Util qw(detaint_natural);
use Bugzilla::Bug;
use Bugzilla::BugMail;
use Bugzilla::Constants;
use Bugzilla::CGI;
use Bugzilla::Search;

#############
# Constants #
#############

# This maps the names of internal Bugzilla bug fields to things that would
# make sense to somebody who's not intimately familiar with the inner workings
# of Bugzilla. (These are the field names that the WebService uses.)
use constant FIELD_MAP => {
    status      => 'bug_status',
    severity    => 'bug_severity',
    description => 'comment',
    summary     => 'short_desc',
    platform    => 'rep_platform',
};

use constant GLOBAL_SELECT_FIELDS => qw(
    bug_severity
    bug_status
    op_sys
    priority
    rep_platform
    resolution
);

use constant PRODUCT_SPECIFIC_FIELDS => qw(version target_milestone component);

use constant CRITERIA_MAP => {
    'bug_num'			 => 'bug_id',
    'BNUM'			 => 'bug_id', 			# short form
    'summary'			 => 'short_desc',
    'SUMM'			 => 'short_desc', 		# short form
    'product'			 => 'product',
    'PROD'			 => 'product', 			# short form
    'version'			 => 'version',
    'VERS'			 => 'version', 			# short form
    'Platform'			 => 'rep_platform',
    'PLAT'			 => 'rep_platform', 		# short form
    'url'			 => 'bug_file_loc',
    'URL'			 => 'bug_file_loc', 		# short form
    'os_version'		 => 'op_sys',
    'OS_V'		  	 => 'op_sys', 			# short form
    'status'			 => 'bug_status',
    'STAT'			 => 'bug_status', 		# short form
    'status_whiteboard'		 => 'status_whiteboard',
    'WBRD'			 => 'status_whiteboard', 	# short form
    'keywords'			 => 'keywords',
    'KEYW'			 => 'keywords', 		# short form
    'resolution'		 => 'resolution',
    'RESO'		 	 => 'resolution', 		# short form
    'severity'			 => 'bug_severity',
    'SEVE'			 => 'bug_severity', 		# short form
    'priority'			 => 'priority',
    'PRIO'			 => 'priority', 		# short form
    'component'			 => 'component',
    'COMP'			 => 'component', 		# short form
    'assigned_to'		 => 'assigned_to',
    'ASGN'			 => 'assigned_to', 		# short form
    'reported_by'		 => 'reporter',
    'REPR'		    	 => 'reporter', 		# short form
    'votes'			 => 'votes',
    'VOTE'			 => 'votes', 			# short form
    'qa_contact'		 => 'qa_contact',
    'QA'		         => 'qa_contact', 		# short form
    'cc'			 => 'cc',
    'CC'			 => 'cc', 			# short form
    'depends_on'		 => 'dependson',
    'DEPS'		  	 => 'dependson', 		# short form
    'blocks'			 => 'blocked',
    'BLKS'			 => 'blocked', 			# short form
    'target_milestone'		 => 'target_milestone',
    'TRGT'		 	 => 'target_milestone', 	# short form
    'last_changed'		 => 'delta_ts',
    'LCHG'			 => 'delta_ts', 		# short form
    'days_changed'		 => 'days_elapsed',
    'DAYS'		         => 'days_elapsed', 		# short form
    'comment'			 => 'longdesca',
    'CMMN'			 => 'longdesca', 		# short form
    'alias'			 => 'alias',
    'ALIA'			 => 'alias', 			# short form
    'ever_confirmed'		 => 'everconfirmed',
    'CNFM'		 	 => 'everconfirmed', 		# short form
    'reporter_accessible' 	 => 'reporter_accessible',
    'RACC' 	 		 => 'reporter_accessible', 	# short form
    'cc_accessible'		 => 'cclist_accessible',
    'CACC'		 	 => 'cclist_accessible', 	# short form
    'group'			 => 'bug_groupa',
    'GRP'			 => 'bug_groupa', 		# short form
    'estimated_hours'		 => 'estimated_time',
    'EST'			 => 'estimated_time', 		# short form
    'remaining_hours'		 => 'remaining_time',
    'RMNG'			 => 'remaining_time', 		# short form
    'hours_worked'		 => 'work_time',
    'HRS'			 => 'work_time', 		# short form
    'percentage_complete'	 => 'percentage_complete',
    'PERC'			 => 'percentage_complete', 	# short form
    'content'			 => 'content',
    'CNTN'			 => 'content', 			# short form
    'classification'		 => 'classification',
    'CLSS'			 => 'classification', 		# short form
    'creation_date'		 => 'creation_ts',
    'CREA'			 => 'creation_ts', 		# short form
    'deadline'			 => 'deadline',
    'DDLN'			 => 'deadline', 		# short form
    'commenter'			 => 'commenter',
    'CMTR'			 => 'commenter', 		# short form
};

###########
# Methods #
###########
sub _xl {
    my ($name) = @_;
    $name =~ s/^\s*//g;  # remove leading and trailing spaces
    $name =~ s/\s*$//g;
    return FIELD_MAP->{$name} if (FIELD_MAP->{$name} ne '');
    return CRITERIA_MAP->{$name} if (CRITERIA_MAP->{$name} ne '');
    # custom field not defined in static map will pass through w/o translation
    return $name;
}

sub _get_boolean_chart {
    my ($root_aref) = @_;
    my $buffer = {};
    my $chart_idx = 0;
    foreach my $chart_aref (@$root_aref) {
        my $row_idx = 0;
        foreach my $row_aref (@$chart_aref) {
            my $col_idx = 0;
            foreach my $col_aref (@$row_aref) {
                $buffer->{"field$chart_idx-$row_idx-$col_idx"} =  _xl($col_aref->[0]);
                $buffer->{"type$chart_idx-$row_idx-$col_idx"}  =  $col_aref->[1];
                $buffer->{"value$chart_idx-$row_idx-$col_idx"} =  $col_aref->[2];
                $col_idx++;
            }
            $row_idx++;
        }
        $chart_idx++;
    }
    return $buffer;
}

sub search {
    my ($self, $params) = @_;
    my @orderstrings = ($params->{order} ne '') ? ($params->{order}) : ();

    ThrowCodeError('param_required', {param => 'criteria'}) 
      unless (defined($params->{'criteria'}) && $params->{'criteria'} ne ''); 

    my $href = _get_boolean_chart($params->{'criteria'});
    my $srch_params = new Bugzilla::CGI;
    while (my ($k,$v) = each %$href) {
        $srch_params->param($k,$v);
    }
    $srch_params->param('query_format', 'advanced');

    my $search;
    eval { $search = new Bugzilla::Search(
			  'fields' => ['bugs.bug_id'],
			  'params' => $srch_params,
			  'order' => \@orderstrings); }; 
    if($@) { ThrowCodeError('param_required', {function => 'Bugs.search', param => "$@"});}

    my $dbh   = Bugzilla->dbh;
    my $query = $search->getSQL();
    $query .= " " . $dbh->sql_limit($params->{'limit'}) if ($params->{'limit'} ne '');

    my $buglist_sth;
    eval {
	$buglist_sth = $dbh->prepare($query);
	$buglist_sth->execute(); };
    if($@) { ThrowCodeError('param_required', {function => 'Bugs.search', param => "$@"}); }

    my $ids_aref = $buglist_sth->fetchall_arrayref();
    my @ids = map { $_->[0] } @$ids_aref;
    my $href = {ids =>\@ids};
    $href->{get_duplicates} = $params->{get_duplicates} if ($params->{get_duplicates} ne '');
    return $self->get_bugs($href);
}

sub _get_duplicates {
    my ($return_aref) = @_;
    my @ids = grep { $_->{'internals'}->{'bug_status'} =~ /RESOLVED/  
                     && $_->{'internals'}->{'resolution'} =~/DUPLICATE/ } @$return_aref;
    return unless (@ids > 0);

    my $id_list = join(',', map { $_->{'internals'}->{'bug_id'} } @ids );
    my ($dbh, $sth);

    $dbh = Bugzilla->dbh;
    $sth = $dbh->prepare("SELECT dupe, dupe_of FROM duplicates WHERE dupe IN ($id_list)");
    $sth->execute;
    my $dupe_of_href = $sth->fetchall_arrayref;

    my %dupe_of = map {$_->[0] => $_->[1]} @$dupe_of_href;

    foreach my $item (@$return_aref) {
	if ($dupe_of{$item->{'internals'}->{'bug_id'}} ne '') {
	    $item->{'duplicate_of'} = type('int')->value(0 + $dupe_of{$item->{'internals'}->{'bug_id'}});
	}
    }
}

sub get_bugs {
    my ($self, $params) = @_;
    my $ids = $params->{ids};
    defined $ids || ThrowCodeError('param_required', { param => 'ids' });

    my @return;
    foreach my $bug_id (@$ids) {
        ValidateBugID($bug_id);
        my $bug = new Bugzilla::Bug($bug_id);

        # Timetracking fields are deleted if the user doesn't belong to
        # the corresponding group.
        unless (Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'})) {
            delete $bug->{'estimated_time'};
            delete $bug->{'remaining_time'};
            delete $bug->{'deadline'};
        }
        # This is done in this fashion in order to produce a stable API.
        # The internals of Bugzilla::Bug are not stable enough to just
        # return them directly.
        my $creation_ts = $self->datetime_format($bug->creation_ts);
        my $delta_ts    = $self->datetime_format($bug->delta_ts);
        my %item;
        $item{'creation_time'}    = type('dateTime')->value($creation_ts);
        $item{'last_change_time'} = type('dateTime')->value($delta_ts);
        $item{'internals'}        = $bug;
        $item{'id'}               = type('int')->value($bug->bug_id);
        $item{'summary'}          = type('string')->value($bug->short_desc);
        $item{'resolution'}       = type('string')->value($bug->resolution);
        $item{'status'}           = type('string')->value($bug->bug_status);

        if (Bugzilla->params->{'usebugaliases'}) {
            $item{'alias'} = type('string')->value($bug->alias);
        }
        else {
            # For API reasons, we always want the value to appear, we just
            # don't want it to have a value if aliases are turned off.
            $item{'alias'} = undef;
        }

        push(@return, \%item);
    }
    _get_duplicates(\@return) if (exists $params->{get_duplicates} && ($params->{get_duplicates} > 0));
    return { bugs => \@return };
}

sub create {
    my ($self, $params) = @_;

    Bugzilla->login(LOGIN_REQUIRED);

    my %field_values;
    foreach my $field (keys %$params) {
        my $field_name = FIELD_MAP->{$field} || $field;
        $field_values{$field_name} = $params->{$field}; 
    }

    # Make sure all the required fields are in the hash.
    foreach my $field (Bugzilla::Bug::REQUIRED_CREATE_FIELDS) {
        $field_values{$field} = undef unless exists $field_values{$field};
    }

    # WebService users can't set the creation date of a bug.
    delete $field_values{'creation_ts'};

    my $bug = Bugzilla::Bug->create(\%field_values);

    Bugzilla::BugMail::Send($bug->bug_id, { changer => $bug->reporter->login });

    return { id => type('int')->value($bug->bug_id) };
}

sub legal_values {
    my ($self, $params) = @_;
    my $field = FIELD_MAP->{$params->{field}} || $params->{field};

    my @custom_select =
        Bugzilla->get_fields({ type => FIELD_TYPE_SINGLE_SELECT });
    # We only want field names.
    @custom_select = map {$_->name} @custom_select;

    my $values;
    if (grep($_ eq $field, GLOBAL_SELECT_FIELDS, @custom_select)) {
        $values = get_legal_field_values($field);
    }
    elsif (grep($_ eq $field, PRODUCT_SPECIFIC_FIELDS)) {
        my $id = $params->{product_id};
        defined $id || ThrowCodeError('param_required',
            { function => 'Bug.legal_values', param => 'product_id' });
        grep($_->id eq $id, @{Bugzilla->user->get_accessible_products})
            || ThrowUserError('product_access_denied', { product => $id });

        my $product = new Bugzilla::Product($id);
        my @objects;
        if ($field eq 'version') {
            @objects = @{$product->versions};
        }
        elsif ($field eq 'target_milestone') {
            @objects = @{$product->milestones};
        }
        elsif ($field eq 'component') {
            @objects = @{$product->components};
        }

        $values = [map { $_->name } @objects];
    }
    else {
        ThrowCodeError('invalid_field_name', { field => $params->{field} });
    }

    my @result;
    foreach my $val (@$values) {
        push(@result, type('string')->value($val));
    }

    return { values => \@result };
}

1;

__END__

=head1 NAME

Bugzilla::Webservice::Bug - The API for creating, changing, and getting the
details of bugs.

=head1 DESCRIPTION

This part of the Bugzilla API allows you to file a new bug in Bugzilla,
or get information about bugs that have already been filed.

=head1 METHODS

See L<Bugzilla::WebService> for a description of B<STABLE>, B<UNSTABLE>,
and B<EXPERIMENTAL>.

=head2 Utility Functions

=over

=item C<legal_values> B<EXPERIMENTAL>

=over

=item B<Description>

Tells you what values are allowed for a particular field.

=item B<Params>

=over

=item C<field> - The name of the field you want information about.
This should be the same as the name you would use in L</create>, below.

=item C<product_id> - If you're picking a product-specific field, you have
to specify the id of the product you want the values for.

=back

=item B<Returns> 

C<values> - An array of strings: the legal values for this field.
The values will be sorted as they normally would be in Bugzilla.

=item B<Errors>

=over

=item 106 (Invalid Product)

You were required to specify a product, and either you didn't, or you
specified an invalid product (or a product that you can't access).

=item 108 (Invalid Field Name)

You specified a field that doesn't exist or isn't a drop-down field.

=back

=back


=back

=head2 Bug Creation and Modification

=over

=item C<get_bugs> B<EXPERIMENTAL>

=over

=item B<Description>

Gets information about particular bugs in the database.

=item B<Params>

=over

=item C<ids>

An array of numbers and strings.

If an element in the array is entirely numeric, it represents a bug_id
from the Bugzilla database to fetch. If it contains any non-numeric 
characters, it is considered to be a bug alias instead, and the bug with 
that alias will be loaded. 

Note that it's possible for aliases to be disabled in Bugzilla, in which
case you will be told that you have specified an invalid bug_id if you
try to specify an alias. (It will be error 100.)

=back

=item B<Returns>

A hash containing a single element, C<bugs>. This is an array of hashes. 
Each hash contains the following items:

=over

=item id

C<int> The numeric bug_id of this bug.

=item alias

C<string> The alias of this bug. If there is no alias or aliases are 
disabled in this Bugzilla, this will be an empty string.

=item summary

C<string> The summary of this bug.

=item creation_time

C<dateTime> When the bug was created.

=item last_change_time

C<dateTime> When the bug was last changed.

=item status

C<string> bug status

=item resolution

C<string> resolution

=item duplicate_of

C<string> bug id for which the current bug is a duplicate of.

=item internals B<UNSTABLE>

A hash. The internals of a L<Bugzilla::Bug> object. This is extremely
unstable, and you should only rely on this if you absolutely have to. The
structure of the hash may even change between point releases of Bugzilla.

=back

=item B<Errors>

=over

=item 100 (Invalid Bug Alias)

If you specified an alias and either: (a) the Bugzilla you're querying
doesn't support aliases or (b) there is no bug with that alias.

=item 101 (Invalid Bug ID)

The bug_id you specified doesn't exist in the database.

=item 102 (Access Denied)

You do not have access to the bug_id you specified.

=back

=back



=item C<create> B<EXPERIMENTAL>

=over

=item B<Description>

This allows you to create a new bug in Bugzilla. If you specify any
invalid fields, they will be ignored. If you specify any fields you
are not allowed to set, they will just be set to their defaults or ignored.

You cannot currently set all the items here that you can set on enter_bug.cgi.

The WebService interface may allow you to set things other than those listed
here, but realize that anything undocumented is B<UNSTABLE> and will very
likely change in the future.

=item B<Params>

Some params must be set, or an error will be thrown. These params are
marked B<Required>. 

Some parameters can have defaults set in Bugzilla, by the administrator.
If these parameters have defaults set, you can omit them. These parameters
are marked B<Defaulted>.

Clients that want to be able to interact uniformly with multiple
Bugzillas should always set both the params marked B<Required> and those 
marked B<Defaulted>, because some Bugzillas may not have defaults set
for B<Defaulted> parameters, and then this method will throw an error
if you don't specify them.

The descriptions of the parameters below are what they mean when Bugzilla is
being used to track software bugs. They may have other meanings in some
installations.

=over

=item C<product> (string) B<Required> - The name of the product the bug
is being filed against.

=item C<component> (string) B<Required> - The name of a component in the
product above.

=item C<summary> (string) B<Required> - A brief description of the bug being
filed.

=item C<version> (string) B<Required> - A version of the product above;
the version the bug was found in.

=item C<description> (string) B<Defaulted> - The initial description for 
this bug. Some Bugzilla installations require this to not be blank.

=item C<op_sys> (string) B<Defaulted> - The operating system the bug was
discovered on.

=item C<platform> (string) B<Defaulted> - What type of hardware the bug was
experienced on.

=item C<priority> (string) B<Defaulted> - What order the bug will be fixed
in by the developer, compared to the developer's other bugs.

=item C<severity> (string) B<Defaulted> - How severe the bug is.

=item C<alias> (string) - A brief alias for the bug that can be used 
instead of a bug number when accessing this bug. Must be unique in
all of this Bugzilla.

=item C<assigned_to> (username) - A user to assign this bug to, if you
don't want it to be assigned to the component owner.

=item C<cc> (array) - An array of usernames to CC on this bug.

=item C<qa_contact> (username) - If this installation has QA Contacts
enabled, you can set the QA Contact here if you don't want to use
the component's default QA Contact.

=item C<status> (string) - The status that this bug should start out as.
Note that only certain statuses can be set on bug creation.

=item C<target_milestone> (string) - A valid target milestone for this
product.

=back

In addition to the above parameters, if your installation has any custom
fields, you can set them just by passing in the name of the field and
its value as a string.

=item B<Returns>

A hash with one element, C<id>. This is the id of the newly-filed bug.

=item B<Errors>

=over

=item 103 (Invalid Alias)

The alias you specified is invalid for some reason. See the error message
for more details.

=item 104 (Invalid Field)

One of the drop-down fields has an invalid value. The error message will
have more detail.

=item 105 (Invalid Component)

Either you didn't specify a component, or the component you specified was
invalid.

=item 106 (Invalid Product)

Either you didn't specify a product, this product doesn't exist, or
you don't have permission to enter bugs in this product.

=item 107 (Invalid Summary)

You didn't specify a summary for the bug.

=item 504 (Invalid User)

Either the QA Contact, Assignee, or CC lists have some invalid user
in them. The error message will have more details.

=back

=back

=item C<search> B<EXPERIMENTAL>

=over

=item B<Description>

This function allows you to construct a query similar to the Boolean 
Charts on the Advance Search tab and the obtain the search result 
in the return hash value. The Boolean Charts internally are arranged 
in tables, rows and then columns. Each column contains a single 
boolean expression with three fields, the data field, the boolean 
operator and the operand. (See the Boolean Chart section in 
the Advance Search tab for all the supported operator type features). 
Each field value will be accepted as plain text. A mandatory criteria 
parameter is required, and parameters like limit and order are optional. 
If the get_duplicates key is set, the corresponding duplicate of bug id
will be retrieved in the duplicate_of field of the returned bug object.
Composite OR expression can be contructed by arranging multiple columns
in the same rows. And Composite AND expression can be contructed by
stacking them as rows with in a chart, and so on with multiple charts.
The limit and the order options allow you to contruct page size, 
navigator etc.

=item B<Params>

Some params must be set, or an error will be thrown. These params are
marked B<Required>. 

The descriptions of the parameters below are what they mean when Bugzilla is
being used to track software bugs. They may have other meanings in some
installations.

=over

=item C<criteria> (string) B<Required> - The critera in the form of an
array reference, it is comprised of at least one boolean chart, with 
one row and one column. Each column is an array reference pointer to
three fields, the query field, the boolean operator type and the value 
to be used in the boolean expression. The following illustrate how the
criteria is being passed to the search WebService API

 $soapresult = $proxy->call('Bugs.search', 
  {criteria => [
     [
       [
         [col 1], [col2]
       ], #row 1 
       [
         [col 1]
       ], #row 2 
     ],  #chart 1
     [
       [
         [col 1], [col2]
       ], #row 1 
       [
         [col 1]
       ], #row 2 
     ],  #chart 2
   ], 
  limit => ...,
  order => ...,
  get_duplicates => ...,
 })
  
=item C<limit> (integer) B<Optional> - The number of row to be retrieved
per call, this should curtail the straining of the server resource if 
the result set is huge.

=item C<order> (string) B<Optional> - The sort columns followed by sort order
token ASC - as ascending, and DESC as descending. You may put column name
if you know exactly the column name of the internal data schema of the 
repository of bugs, otherwise see the mapping symbol below.

=item C<get_duplicates> (string) B<Optional> - A flag when specified will
as the WebService API to pull up it's duplicate if it's status became RESOLVED,
and it's resolution became DUPLICATE. The {duplicate_of} field of the returned
bug will contain the duplicate bug's id.

=item C<criteria map> (string) B<Defaulted> - In order to support the future
grow of Bugzilla database schema and properly shield the WebService API 
Application from the impact of future changes. A translation CRITERIA_MAP
is created. Using the values from the CRITERIA_MAP will ask the API to perform
translation automatically before performing the querying. 

The following is the static criteria field values (long form, short form):

    'bug_num'             'BNUM'
    'summary'	          'SUMM'
    'product'             'PROD'
    'version'             'VERS'
    'Platform'	          'PLAT'
    'url'                 'URL'
    'os_version'          'OS_V'
    'status'              'STAT'
    'status_whiteboard'   'WBRD'
    'keywords'            'KEYW'	
    'resolution'          'RESO'	
    'severity'            'SEVE'
    'priority'            'PRIO'
    'component'           'COMP'
    'assigned_to'         'ASGN'
    'reported_by'         'REPR'
    'votes'               'VOTE'
    'qa_contact'          'QA'
    'cc'                  'CC'
    'depends_on'          'DEPS'
    'blocks'              'BLKS'
    'target_milestone'    'TRGT'
    'last_changed'        'LCHG'
    'days_changed'        'DAYS'
    'comment'             'CMMN'
    'alias'               'ALIA'
    'ever_confirmed'      'CNFM'
    'reporter_accessible' 'RACC'
    'cc_accessible'       'CACC'
    'group'               'GRP'
    'estimated_hours'     'EST'
    'remaining_hours'     'RMNG'
    'hours_worked'        'HRS'
    'percentage_complete' 'PERC'
    'content'             'CNTN'
    'classification'	  'CLSS'
    'creation_date'       'CREA'
    'deadline'            'DDLN'
    'commenter'           'CMTR'

Anything that doesn't fit into this list will bypass the mapping checks as pass
through value. If the spelling mistake is made, the Search module or 
the MySQL will throw an exception. This will allow custom fields to be included
in the search criteria.

The following is a list of supported boolean operators and it's English
representation (you may also extract them from the page source of 
the Advanced Search tab).

    'equals'         => 'is equal to'
    'notequals'      => 'is not equal to'
    'anyexact'       => 'is equal to any of the strings'
    'substring'      => 'contains the string'
    'casesubstring'  => 'contains the string (exact case)'

    'notsubstring'   => 'does not contain the string'
    'anywordssubstr' => 'contains any of the strings'
    'allwordssubstr' => 'contains all of the strings'
    'nowordssubstr'  => 'contains none of the strings'
    'regexp'         => 'contains regexp'
    'notregexp'      => 'does not contain regexp'

    'lessthan'       => 'is less than'
    'greaterthan'    => 'is greater than'
    'anywords'       => 'contains any of the words'
    'allwords'       => 'contains all of the words'
    'nowords'        => 'contains none of the words'
    'changedbefore'  => 'changed before'

    'changedafter'   => 'changed after'
    'changedfrom'    => 'changed from'
    'changedto'      => 'changed to'
    'changedby'      => 'changed by'
    'matches'        => 'matches'

Please see Bugzilla Search Guide for all the values of
the boolean expression.

=item B<Boolean Expression inside as the column in the criteria chart>

Sample of a a query 

C<Query Text> Get all the customer related and show stopper bugs that has been 
modified in the last 12 hours.

criteria =>
[
  [ 
    ["last_changed", "greaterthan", "-12h"], #row 1
    ["cf_customer_issue", "equals", "1"], ["PRIO", "equals", "P1"] #row 2
  ], #chart 1
   #... more chart if needed
], # critera chart

The resulting filter to be applied should be 
 (last_changed > -12h) AND 
   (cf_customer_issue = 1 OR priorty = P1)

Since the columns are used as OR, and rows are used as AND. Negation
and re-forumulation using set algebra may be needed to create the 
desirable composite boolean expression.

=back

=item B<Returns>

Please see C<get_bugs> for the return values, errors and exceptions.


=back

Arghh.... do not paste patches as comments. We have attachments exactly for that purpose.
oops, you can tell I am a newbie, part of the learning curve.
Attachment #292070 - Attachment is obsolete: true
Comment on attachment 292075 [details]
Bugzilla::WebService::Bug::search fixed bug in missing fieldname translation for the order string

The bug did not address the issue of duplicate, but I needed the feature to make an external app works so I added the get_duplicates flag in get_bugs() to support it. Please see the pod doc.
Attachment #292075 - Flags: review?(mkanat)
Comment on attachment 292075 [details]
Bugzilla::WebService::Bug::search fixed bug in missing fieldname translation for the order string

Please re-attach this as a "cvs diff -u".
Attachment #292075 - Flags: review?(mkanat) → review-
This is the source and I did checkout from:

export CVSROOT=:pserver:anonymous@cvs-mirror.mozilla.org:/cvsroot
$ cvs login
(Logging in to anonymous@cvs-mirror.mozilla.org)
CVS password: anonymous
cvs checkout -P -d bugzilla Bugzilla
Attachment #292075 - Attachment is obsolete: true
Attachment #292101 - Flags: review?(mkanat)
BTW, There is a bug in the logic of order processing, it didn't process any composite columns sorting order. For instance, if I want to sort column A in descending order, and B in ascending. I would need the parameter

Bugs.search({
  order => 'A desc, B'
  ...
});

The current patch take one sorting column only and cannot process more than one sort column. I will submit another patch to fix this problem.
Attachment #292101 - Attachment is obsolete: true
Attachment #292101 - Flags: review?(mkanat)

        Attachment #291960 -
        Attachment is obsolete: true

        Attachment #292427 -
        Flags: review?(mkanat)

    
    

    
  
Comment on attachment 292428 [details]
perl script for demonstration of criteria and composite order

Please don't mind; application/x-perl is fine, but it should be browser-viewable, so I'm setting the MIME type of your attachment to text/plain.

        Attachment #292428 -
        Attachment mime type: application/x-perl → text/plain

    
    

    
  
Not at all, now I know that the real purpose behind the drop down box.

    
    

    
  
Please use: cvs diff -puN when creating patches. Yours is hardly readable without more context. Oh, and could we use shortcuts similar to those defined in QuickSearch.pm? Else we have again another syntax to learn to run queries.

    
    

    
  


  
The search() API adds a symbolic column translation layer for the search columns for shielding applications from the impact of modification of database schema, and the rest externalizes the interface of the Boolean Chart. 

The patch file is created again using cvs diff -puN as suggested by Frédéric. It also contains minor extension to the current get_bugs() API to include bug `status`, bug `resolution` and `duplicate_of` bugs list. 

Lots of documentation in POD is added on showing how boolean chart is arranged in rows and columns. Please see attached perl script referenced by 292428 on how to create a search result using Boolean Chart. The operators and operands are exactly identical to what the Advanced Search tab uses and can be extracted using the browser's view page source command.

        Attachment #292427 -
        Attachment is obsolete: true

        Attachment #295117 -
        Flags: review?(mkanat)

        Attachment #292427 -
        Flags: review?(mkanat)

    
    

    
  
Frédéric,

This is to address the question on comment #31. The current search() API patch performs more complex lower level search particularly for custom defined fields. 

From the webpage, it seems like quicksearch is the equivalence of the [Find] button. If that is the case, it is easy to add quicksearch into another method Bugs.find(). I can add another patch along with the search() patch quickly because it is functionally similar. 

- Clement

    
    

    
  
(In reply to comment #33)
> From the webpage, it seems like quicksearch is the equivalence of the [Find]
> button.

Yes, [Find] calls quicksearch.
Status: NEW → ASSIGNED

    
    

    
  
(In reply to comment #34)
> (In reply to comment #33)
> > From the webpage, it seems like quicksearch is the equivalence of the [Find]
> > button.
> 
> Yes, [Find] calls quicksearch.
> 
Does it only support substring search?



    
    

    
  


  
patch.diff is created using cvs diff -puN for the search() method (Boolean Chart API), get_bugs() method upgrade to support status/resolution and get_duplicates; with the addition of the find() method that implements the Find button of the bugzilla search bar in the form of a WebService API.

        Attachment #295117 -
        Attachment is obsolete: true

        Attachment #295168 -
        Flags: review?(mkanat)

        Attachment #295117 -
        Flags: review?(mkanat)

    
    

    
  
(In reply to comment #35)
> (In reply to comment #34)
> > (In reply to comment #33)
> > > From the webpage, it seems like quicksearch is the equivalence of the [Find]
> > > button.
> > 
> > Yes, [Find] calls quicksearch.
> > 
> Does it only support substring search?
> 
Frédéric,
I verified quicksearch() does a few things in quicksearch.pm, would you verify the patch.diff? I ran two simply tests on the API, one with bug ids like "1 2 3", and the other with a search string 'driver'.

- Clement 



    
    

    
  
Comment on attachment 295168 [details] [diff] [review]
WebService API for Boolean Chart Search(), or Find(), and upgrade for get_bugs() to support status/resolution and get_duplicates

Okay, first off, your patch does too many things:

* Gets duplicates
* Adds stats/resolution to get_bugs
* Creates find()
* Invents some strange abbreviation syntax

Please attach a patch that does only *one* thing--adding "find". You don't even have to add QuickSearch support. And call it "search", because that's what we call it almost everywhere else in Bugzilla.

Then I'll do a review of that.

The documentation looks pretty good, though, on a brief glance--thank you for typing up so much of it.

        Attachment #295168 -
        Flags: review?(mkanat) → review-

    
    

    
  
(In reply to comment #38)
> (From update of attachment 295168 [details] [diff] [review])
> Okay, first off, your patch does too many things:
> 
> * Gets duplicates
> * Adds stats/resolution to get_bugs
> * Creates find()
> * Invents some strange abbreviation syntax
> 
> Please attach a patch that does only *one* thing--adding "find". You don't even
> have to add QuickSearch support. And call it "search", because that's what we
> call it almost everywhere else in Bugzilla.
Max,

Are you referring to keeping all the fixes in a separate attachment files? and keep the series of patches in the proper sequence? 

- Clement 

> 
> Then I'll do a review of that.
> 
> The documentation looks pretty good, though, on a brief glance--thank you for
> typing up so much of it.
> 




    
    

    
  
> The documentation looks pretty good, though, on a brief glance--thank you for
> typing up so much of it.

You are welcome :-) I have no choice because the API is being reviewed and referenced by Salesforces and Pervasive Software. 

    
    

    
  
Ah, okay. Well, whatever API we work out here will be B<UNSTABLE>. I'm really not sure I like the idea of making Search.pm's API our actual API.

In fact, I want a much simpler API, just "field => $value" for each field name.

    
    

    
  
(In reply to comment #41)
> Ah, okay. Well, whatever API we work out here will be B<UNSTABLE>. I'm really
> not sure I like the idea of making Search.pm's API our actual API.
Unless you are thinking about revamping the business logic of the search functionality, I don't see why not. Because it guarantees that the result set of the Web Service API will be identical to the Web Front End.
> 
> In fact, I want a much simpler API, just "field => $value" for each field name.
> 
Are you referring to the search criteria? For a search criteria, you need to specify the field name, the boolean operator and the value to be compared against. For example, "bug_status = NEW" can be  represented by 

{field => "bug_status",
 type  => "=",
 value => "NEW"}


    
    

    
  
(In reply to comment #42)
> Unless you are thinking about revamping the business logic of the search
> functionality, I don't see why not. Because it guarantees that the result set
> of the Web Service API will be identical to the Web Front End.

  Because the Search.pm API is too complex.

> For example, "bug_status = NEW" can be  represented by 
> 
> {field => "bug_status",
>  type  => "=",
>  value => "NEW"}

  Right, instead of that I want "bug_status => 'NEW'". Very simple that way, eh?

    
    

    
  
(In reply to comment #43)
>   Because the Search.pm API is too complex.
Yes, I see :-)

> 
>   Right, instead of that I want "bug_status => 'NEW'". Very simple that way,
> eh?
> 
Let me give it some thought, it seems like a simple way of doing things, but will require some out of the box thinking.




    
    

    
  
Perhaps you could do it via Bugzilla::Object::match, which Bugzilla::Bug inherits.

    
    

    
  
(In reply to comment #43)
>   Right, instead of that I want "bug_status => 'NEW'". Very simple that way,

We shouldn't restrict the API in any way, though. If we want such a simple API, then we a need a powerful one as well, which can build all queries you can build with boolean charts, including AND and OR constructs and what have you.

    
    

    
  
(In reply to comment #46)
> We shouldn't restrict the API in any way, though. If we want such a simple API,
> then we a need a powerful one as well, which can build all queries you can
> build with boolean charts, including AND and OR constructs and what have you.

  Yes, absolutely. That comes after this simple API.

    
    

    
  
(In reply to comment #45)
> Perhaps you could do it via Bugzilla::Object::match, which Bugzilla::Bug
> inherits.
> 
What would you like the API to be? For example, Bugs.match('bug_status' => 'NEW'), etc. 


    
    

    
  
No, I didn't want you to call the function Bugs.match. There already exists a function inside of Bugzilla itself in Bugzilla::Object called match(). You can use this to implement the simple syntax for Bugs.search(). However, you'd have to do security checking yourself, I suppose. Maybe we could add a Bugzilla::Bug::match_secure function that uses match and then does the security checks itself.

    
    

    
  
(In reply to comment #49)
> No, I didn't want you to call the function Bugs.match. There already exists a
> function inside of Bugzilla itself in Bugzilla::Object called match(). You can
> use this to implement the simple syntax for Bugs.search(). However, you'd have
> to do security checking yourself, I suppose. Maybe we could add a
> Bugzilla::Bug::match_secure function that uses match and then does the security
> checks itself.
> 
The security check is quite complex, I tried $user->can_see_bug() on the first attempt, and I used what Search.pm is using in the Bugzilla::Bug::match_secure() method on the second attempt. 

It seems like the can_see_bug() is faster for a small result set even it is executed one precomiled SQL per ValidateBugID() call. Wonder I should do some timing check on this one?

If I use the SQL strict out of the Search.pm, then chances are it will be a maintainence nightmare. Becuase anytime Search.pm is upgraded, the match_secure() method has to be synchronized again.

But in terms of API, it can be called this way
$proxy->call('Bugs.search', {bug_status =>'NEW', priority=>'P1', # etc...
                            });

The hashref input simply turns into a serial AND expression for the WHERE clause. This will satisfy the narrowing of a search. 

May be I can keep the Boolean Chart as the 'Bugs.advance_search' API later, because the current Bugs.search implementation can not be used to pull up 'NEW', 'ASSIGNED'  and 'OPEN' at once with OR operators.




    
    

    
  
(In reply to comment #50)
> The security check is quite complex, I tried $user->can_see_bug() on the first
> attempt, and I used what Search.pm is using in the
> Bugzilla::Bug::match_secure() method on the second attempt. 

  Maybe we need $user->can_see_bugs().

> The hashref input simply turns into a serial AND expression for the WHERE
> clause. 

  Of course, because that's how Bugzilla::Object->match works. You *are* using Bugzilla::Object->match, like I said to, aren't you?

> the current Bugs.search implementation can not be used to pull up
> 'NEW', 'ASSIGNED'  and 'OPEN' at once with OR operators.

  Untrue. match() supports things like status => ['NEW', 'ASSIGNED', 'OPEN'].

    
    

    
  
(In reply to comment #51)
> (In reply to comment #50)
> > The security check is quite complex, I tried $user->can_see_bug() on the first
> > attempt, and I used what Search.pm is using in the
> > Bugzilla::Bug::match_secure() method on the second attempt. 
> 
>   Maybe we need $user->can_see_bugs().
Great ideas, only from those who is very intimate with the code ;) This should consolidate everything in one SQL.
> 
> > The hashref input simply turns into a serial AND expression for the WHERE
> > clause. 
> 
>   Of course, because that's how Bugzilla::Object->match works. You *are* using
> Bugzilla::Object->match, like I said to, aren't you?
Yeah, as you suggested.
> 
> > the current Bugs.search implementation can not be used to pull up
> > 'NEW', 'ASSIGNED'  and 'OPEN' at once with OR operators.
> 
>   Untrue. match() supports things like status => ['NEW', 'ASSIGNED', 'OPEN'].
Are you sure? may be my CVS setting didn't point to the proper repository, here is my cvs setting:

 export CVSROOT=:pserver:anonymous@cvs-mirror.mozilla.org:/cvsroot

The following is the section of code that performs the $where clause generation,

  171       return [$class->get_all] if !$criteria;
  172
  173       my (@terms, @values);
  174       foreach my $field (keys %$criteria) {
  175           my $value = $criteria->{$field};
  176           if ($value eq NOT_NULL) {
  177               push(@terms, "$field IS NOT NULL");
  178           }
  179           elsif ($value eq IS_NULL) {
  180               push(@terms, "$field IS NULL");
  181           }
  182           else {
  183               push(@terms, "$field = ?");
  184               push(@values, $value);
  185           }
  186       }
  187
  188       my $where = join(' AND ', @terms);
  189       my $ids   = $dbh->selectcol_arrayref(
  190           "SELECT $id FROM $table WHERE $where", undef, @values)
  191           || [];

If this is not so, would like give to the link to the cvs repository of the current build?
Othewise, May be I can add the detection for OR case on line 175...
> 



    
    

    
  
Oh, strange that match() doesn't have IN support yet. The patch is attached to some bug, somewhere...

    
    

    
  
Bug 372795 was the bug I was thinking of.

    
    

    
  
(In reply to comment #54)
> Bug 372795 was the bug I was thinking of.
> 
Ahh, very good the IN() clause should satisfy the OR expression in the patch of 372795.

In my app, I need to track chronological information; for example, bugs in the last 12 hours, or between a certain date:hr:min period. Since timestamp can never be matched exactly, it will need the >,>=,<, <= to set the upper and lower time boundary. 

How will the Bugzilla::Object::match() be upgraded to support datetime field without the indication of any boundary check? (Please note that we only have key value pair to show column name and column value, the default comparison operator is always 'equal' in the current code base.)



    
    

    
  
(In reply to comment #55)
> How will the Bugzilla::Object::match() be upgraded to support datetime field
> without the indication of any boundary check?

  It won't. We'll have to wait for a more advanced search functionality.
Summary: Search for bugs via WebService → Basic search for bugs via WebService

    
    

    
  


  
HI ,
In our redhat bugzilla version 2.18 we have xmlrpc module Query.pm that has function runQuery() to perform simple and advanced searches via xmlrpc interface. attached is a tar ball that has the xmlrpc module Query.pm and Bugzilla module Bugzilla::Search and example to test it. please give  us your opinion about implementing something similar to this in upstream bugzilla 3.2 , as this function is the most used function in our redhat bugzilla xmlrpc api and it is very critical to us.

Thanks,
Noura

    
    

    
  


  
This is an implementation of the basic simple search method for the WebService API. The input is a hash ref. To query for bugs that is NEW, ASSIGNED, REOPENED and having a priority of P5. It can be done using the method in the following way:

$proxy->call( 'Bug.search', ({bug_status => ['NEW','ASSIGNED','REOPENED'],
                             priority => 'P5'});

The key of the hash element must be a valid field of the bugs table.

        Attachment #301628 -
        Flags: review?(mkanat)

    
    

    
  
Comment on attachment 301628 [details] [diff] [review]
patch for implementing the simple Bug.search() WebService API

>+++ bugzilla/Bugzilla/Object.pm	2008-02-05 21:46:17.000000000 -0800

>+            push(@terms, "$field IN ($qmarks)");

Don't use IN (), but sql_in() instead. Else Oracle will fail.

    
    

    
  
(In reply to comment #59)
> (From update of attachment 301628 [details] [diff] [review])
> >+++ bugzilla/Bugzilla/Object.pm	2008-02-05 21:46:17.000000000 -0800
> 
> >+            push(@terms, "$field IN ($qmarks)");
> 
> Don't use IN (), but sql_in() instead. Else Oracle will fail.
> 
I checked the Oracle Press Oracle 8 PL/SQL reference and googled for 'Oracle SQL IN operator', the result seems to show that the IN () statement is standard SQL compliant. 

Would you verify what version of Oracle is causing the failure, and what is the error code generated from the Oracle Client?

Thanks,

- Clement 


    
    

    
  
(In reply to comment #60)
> I checked the Oracle Press Oracle 8 PL/SQL reference and googled for 'Oracle
> SQL IN operator', the result seems to show that the IN () statement is standard
> SQL compliant. 

I know IN() exists in Oracle. But the problem is that you cannot have more than 1000 arguments in it, see bug 408172. So you have to use this Bugzilla method instead.

    
    

    
  


  
Attaching patch to add ability to query for bugs in the Bugzilla database using WebService API. This is to replace functionality of bugzilla.runQuery() in the current Red Hat 2.18 code base. We need the full ability to mimic the same queries that can be performed on query.cgi since we need to be able to query for things like flag values, etc.

Differences from 2.18 bugzilla.runQuery()

1. Requires login using stored cookies
2. Now called Bug.search($params_hash)
3. Now adds the ability to pass in stored query name as 'savedsearch'.
4. Now adds the ability to perform quicksearch using similar keywords 
as the Bugzilla home page search field. Pass is as 'quicksearch' key. For more info see https://bugzilla.mozilla.org/page.cgi?id=quicksearchhack.html
5. The DefineColumn() definitions were moved out of buglist.cgi and into
Bugzilla/Search.pm to allow both buglist.cgi and Bug.search() to access the
column information.

TODO: Alot of code still being copied from buglist.cgi to Bugzilla/WebService/Bug.pm. Need to move alot of stuff out of buglist.cgi to allow for simpler interface to the same data. DefineColumn() was a good start but more to go.

Thoughts?
Dave

        Attachment #300203 -
        Attachment is obsolete: true

        Attachment #306578 -
        Flags: review?

    
    

    
  
Comment on attachment 301628 [details] [diff] [review]
patch for implementing the simple Bug.search() WebService API

>+++ bugzilla/Bugzilla/User.pm	2008-02-05 21:46:06.000000000 -0800

>+# can_see_bugs() is basically a batch version of can_see_bug()

I don't like this code being duplicated, especially when talking about security checks. We should have a single method which accept a list of bug IDs. Maybe refactor the code a bit if two separate methods is still needed.

        Attachment #301628 -
        Flags: review?(mkanat) → review-

    
    

    
  
Comment on attachment 306578 [details] [diff] [review]
Code for implementing complicated advanced search via WebService

Having code being duplicated in both buglist.cgi and B::W::B is bad. I think we have to fix blockers first, especially bug 398308. This means keeping all the CGI code in buglist.cgi, and have Search.pm only take a hash of arguments so that we don't need these hacks when using WebService. This will be a big tasks for Bugzilla 4.0.

        Attachment #306578 -
        Flags: review? → review-

    
  
Priority: -- → P1
Whiteboard: [roadmap: 4.0]
Version: unspecified → 3.1.3

    
    

    
  

      
      
      
      

        Attached patch
          
          
        v2 - WebService::Bug search  (obsolete)
        — 
          — Splinter Review
      

    


  
This is another initial implementation of a simple API using Bug.search. It utilizes Bugzilla::Bug->match combined with an existing patch for (bug 440656) that implements Bugzilla::User->visible_bugs function.

Currently, it allows for searching on bug_id, bug_status, priority, bug_severity, and assigned_to. This can be easily extended, but I just wanted to get some feedback prior to taking it any further.

        Attachment #329923 -
        Flags: review?(wurblzap)

        Attachment #329923 -
        Flags: review?(mkanat)

    
    

    
  
Comment on attachment 329923 [details] [diff] [review]
v2 - WebService::Bug search 

Can I get a patch with just your changes in it (don't include the visible_bugs changes). I think visible_bugs exists upstream now, so this should be easier for you.

        Attachment #329923 -
        Flags: review?(wurblzap)

        Attachment #329923 -
        Flags: review?(mkanat)

        Attachment #329923 -
        Flags: review-

    
    

    
  
Comment on attachment 329923 [details] [diff] [review]
v2 - WebService::Bug search 

  Also, might as well give quick comments while I'm here.

>Index: Bugzilla/WebService/Bug.pm
>+sub search {
>+    $search_params->{bug_status} = ($params->{bug_status}) if defined $params->{bug_status};
>+    $search_params->{bug_id} = ($params->{bug_id}) if defined $params->{bug_id};
>+    $search_params->{priority} = ($params->{priority}) if defined $params->{priority};
>+    $search_params->{bug_severity} = ($params->{severity}) if defined $params->{severity};

  That's all unnecessary. At least, it shouldn't all be on one line.

>+        my $user_class = 'Bugzilla::User';

  You can just use the string, you don't need to put it in a variable.

  Also, we could use MATCH_JOIN for this...which doesn't exist yet, but I had a patch that did it.

  I don't think we need to create objects just for this.

>+            push( @user_ids, $user_obj->{userid} );

  You should never be accessing the hash fields of objects, you should always be using their accessors.

>-        # Timetracking fields are deleted if the user doesn't belong to
>-        # the corresponding group.
>-        unless (Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'})) {

  Don't we have a $user->is_timetracker? I thought I added that.

  Oh, I see, you are. Is this the same thing we did in Bug.get?

>+    # Map a handful of fields for each bug found (this needs to expand)

  Don't add new fields here at the same time as we're adding a new function.

  Maybe we should just make this code a part of Bug.get. I think that would make sense as a good standard API for all our objects, too...I don't see why not.

>+           'status:s'       => \$bug_status,
>+           'severity:s'     => \$severity,
>+           'priority:s'     => \$priority,
>+           'owner:s'        => \$owner,

  Hmm. I'm wary of adding a zillion new parameters to this as we add new fields you can search on.

    
  
Summary: Basic search for bugs via WebService → Basic search for bugs via WebService (Bug.search)

    
    

    
  
Okay, I'm going to take this over and do it myself (since there's been no updates in a while).
Assignee: cchan → mkanat

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Work In Progress (mkanat) (obsolete)
        — 
          — Splinter Review
      

    


  
Okay, here's what I have so far. This pretty much works, it just needs a few adjustments to be totally secure and actually match its POD.

        Attachment #295168 -
        Attachment is obsolete: true

        Attachment #301628 -
        Attachment is obsolete: true

        Attachment #329923 -
        Attachment is obsolete: true

    
    

    
  
Now that we're just doing a basic search that doesn't use Search.pm, we don't need the dependencies that are listed here.
No longer depends on: 278032, 398308

    
  
Depends on: 471942

    
  
Depends on: 471943

    
    

    
  

      
      
      
      

        Attached patch
          
          
        v3 (obsolete)
        — 
          — Splinter Review
      

    


  
Okay, here we go! This requires the patches from both of the blockers.

        Attachment #354910 -
        Attachment is obsolete: true

        Attachment #355200 -
        Flags: review?(dkl)

    
    

    
  
Comment on attachment 355200 [details] [diff] [review]
v3

This patch looks good to me codewise and also works as expected through the different tests I ran.

One thing for future is that we may want to add the ability to filter which fields that are returned from _bug_to_hash() by checking (include|exclude)_fields params passed in but that could be a separate bug. We do similar with comments() now.

Also it would be nice if _bug_to_hash() also added in the actual product/component/version/etc. text to the internals hash that is returned to make it more useful to the clients but that is being addressed in a different bug as I recall.

Dave

        Attachment #355200 -
        Flags: review?(dkl) → review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        v4
        — 
          — Splinter Review
      

    


  
I had forgotten to delete timetracking parameters on the way in for users who aren't in the timetrackinggroup. Even though the parameters are undocumented, people could still have searched for bugs using them, since match() would have accepted them.

The patch had also bitrotted, so it's fixed now.

        Attachment #355200 -
        Attachment is obsolete: true

        Attachment #357892 -
        Flags: review?(dkl)

        Attachment #357892 -
        Flags: review?(dkl) → review-

    
    

    
  
Comment on attachment 357892 [details] [diff] [review]
v4

Searching by component name is not working properly. Same for product. We need to convert to component_id/product_id before
calling Bugzilla::Bug->match().

DBD::mysql::db selectall_arrayref failed: Unknown column 'component' in 'where clause' [for Statement "SELECT alias,assigned_to,bug_file_loc,bug_id,bug_severity,bug_status,cclist_accessible,component_id,delta_ts,estimated_time,everconfirmed,op_sys,priority,product_id,qa_contact,remaining_time,rep_platform,reporter_accessible,resolution,short_desc,status_whiteboard,target_milestone,version,reporter    AS reporter_id,DATE_FORMAT(creation_ts, '%Y.%m.%d %H:%i') AS creation_ts,DATE_FORMAT(deadline, '%Y-%m-%d') AS deadline FROM bugs WHERE bug_status = ? AND rep_platform = ? AND component = ? AND version = ? AND product = ? AND short_desc = ?  ORDER BY bug_id"] at /var/www/html/bugzilla/Bugzilla/Object.pm line 201

Also assigned_to, reporter, and qa_contact only works if you pass in the userid instead of the login_name. So those need
to be converted as well. These go against the documentation.

Dave

    
    

    
  
Comment on attachment 357892 [details] [diff] [review]
v4

That's what the patch on the dependency is for, which you have to apply first.

        Attachment #357892 -
        Flags: review- → review?(dkl)

        Attachment #357892 -
        Flags: review?(dkl) → review+

    
    

    
  
Comment on attachment 357892 [details] [diff] [review]
v4

Doh, (me smacks head). Forgot to also apply the other. Once I did that all was well and tests worked fine. Just a note for ourselves, this is cool for simple searches but we will still need a full fledged search in the future as we do alot of boolean searches via xmlrpc now. But this is a good start. r=dkl

    
    

    
  
Great, thanks! :-)
Flags: approval+
Target Milestone: Bugzilla 4.0 → Bugzilla 3.4

    
    

    
  
Checking in Bugzilla/WebService/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Bug.pm,v  <--  Bug.pm
new revision: 1.25; previous revision: 1.24
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED

    
  
Keywords: relnote

    
    

    
  
Anybody interested in Advanced Search (using the Boolean Charts) should follow bug 475754.

    
  
Flags: testcase?

    
    

    
  
Added to the release notes for Bugzilla 3.4 in bug 494037.
Keywords: relnote

    
  
Blocks: 515191
No longer blocks: 515191

    
  
Blocks: 515191

    
  
Flags: testcase? → testcase+

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: