Bug 427882

Summary: XMLRPC function runQuery()
Product: [Community] Bugzilla Reporter: Noura El hawary <nelhawar>
Component: WebServiceAssignee: David Lawrence <dkl>
Status: CLOSED NEXTRELEASE QA Contact:
Severity: medium Docs Contact:
Priority: high    
Version: 3.2   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-03-11 03:58:48 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 406071, 406131, 427053    
Attachments:
Description Flags
Patch to add Bug.search capability to WebServices API
none
Patch to add Bug.search capability to WebServices API (v2)
nelhawar: review-, dkl: review? (kbaker)
Patch to add Bug.search capability to WebServices API (v3) nelhawar: review+

Description Noura El hawary 2008-01-08 01:10:12 UTC
write xmlrpc function runQuery() that is to be contributed to the upstream.

currently in rh_bugzilla_2.18 we have this function in
Bugzilla::RPC::Query::runQuery()

This function is not available in the upstream. It is the most used xmlrpc
function in redhat bugzilla.

Comment 1 Kevin Baker 2008-01-08 14:46:23 UTC
See upstream bug 'Search for bugs via WebService'

https://bugzilla.mozilla.org/show_bug.cgi?id=398281

Comment 2 David Lawrence 2008-01-08 15:57:29 UTC
(In reply to comment #1)
> See upstream bug 'Search for bugs via WebService'
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=398281

Initially it seemed liked for 3.2 they were only going to include a very simple
search API based on quicksearch which would not allow us to do complex searches
using boolean charts that we need. So we may still need to implement
bug.run_query() or other just in case we need the full functionality.


Comment 3 Noura El hawary 2008-01-11 07:44:05 UTC
I guess they are planning to build on the extra searches with the boolean charts
after they have the simple API implemented ,, so I guess we can build on their
current simple API and submit a patch? I talked to mkanat and he said the right
procedure would be to file a bug for each new function that we want to
contribute to the upstream and get it reviewed by them.

Comment 4 Kevin Baker 2008-01-13 17:36:48 UTC
Noura,

did you mean to update 'Orig Estimate' or 'Hours Worked' with that 2 hours?

Comment 5 Noura El hawary 2008-01-15 07:18:14 UTC
(In reply to comment #4)
> Noura,
> 
> did you mean to update 'Orig Estimate' or 'Hours Worked' with that 2 hours?

Kevin,, I meant the hours worked, but to update hours worked I guess I have to
first put an Orig Estimate so I am not sure how long would that be, or how would
i calculate it, I guess it will be based on the LOC so what is the formula for that?

Thanks,
Noura

Comment 6 Kevin Baker 2008-01-15 16:33:49 UTC
> Kevin,, I meant the hours worked, but to update hours worked I guess I have 
to
> first put an Orig Estimate so I am not sure how long would that be, or how 
would
> i calculate it, I guess it will be based on the LOC so what is the formula 
for that?

Just calculate the LOC the same way we did it before. Via function points or 
direcly by LOC if you already have an implementation. Put the LOC in the devel 
whiteboard. 

For now I have entered 150 hours which I based on 500 LOC (I made 500 up if 
you are wondering). I'm using the Cocomo calculation 

 months = 2.4 * ((LOC/1000)**1.05)
 hours  = months * 132

I'll put up scripts on the wiki that will do these calculations.

Comment 7 Noura El hawary 2008-01-30 01:32:33 UTC
sent our code for runQuery to the upstream to see what they think about it and
if we can implement something similar to it in bugzilla 3.2

Comment 8 David Lawrence 2008-02-29 20:50:54 UTC
Created attachment 296412 [details]
Patch to add Bug.search capability to WebServices API

Attaching patch to add ability to query for bugs in the Bugzilla database over
XMLRPC. This is to replace functionality of bugzilla.runQuery() in our current
2.18 code base.

Differences from 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,
value pair. For more info see https://bugzilla.redhat.com/quicksearch.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 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.

Please look over what I have so far.

Thanks
Dave

Comment 9 Noura El hawary 2008-03-05 14:35:20 UTC
Hi Dave ,,

I had a look at your patch , and tested it. It looks good and worked fine for me
when using search criteria and when using quicksearch. however for the saved
search it doesn't work so if i have a search called "maysavedseach" and i call
like the following:

$call = $rpc->call('Bug.search', {savedsearch => "mysavedsearch"}); it doesn
work and it gives this error:

You may not search, or create saved searches, without any search terms.

looking at the code I think that the savedsearch query gets completely wiped out
in this line in Bugzilla/WebService/Bug.pm
my $saved_search_data = Bugzilla->cgi->clean_search_url($saved_search->url);

it is either this or maybe i am misunderstanding how it should work!!
-------------

Also for the quicksearch it works fine for everything except for when you pass a
single bug id as the following:

$call = $rpc->call('Bug.search', {quicksearch => "1"});

if you pass more than one bug id it words good but for single bug ids it fails i
am guessing caze the behaviour is that it redirects to show_bug.cgi , It is not
really a bit issue I guess.

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

and one more thing is the pod for the function search it is missing 
=back 
right at the end of the pod to validate.

other than that I think it is looking good.

Noura

Comment 10 Noura El hawary 2008-03-05 14:39:23 UTC
also Dave in Bugzilla/WebService/Constants.pm and in the POD , can you please
change the error code :
buglist_parameters_required => 506, 

to be 
buglist_parameters_required => 507,

As i have already committed a patch that uses 506. 

Thanks,
Noura

Comment 11 David Lawrence 2008-03-05 20:42:44 UTC
Created attachment 296930 [details]
Patch to add Bug.search capability to WebServices API (v2)

Thanks for the review Noura. I have addressed the following:

1. You need to be logged in for the savedsearch to work which was the original
reason you were not able to get it to work. I have reworked it to throw an
error if a savedsearch value is passed in and user is not logged in. Also throw
error now when they are logged in but pass a savedsearch name that doesn't
exist. I changed the fault codes to not conflict with yours also.

2. I fixed quicksearch() from redirecting by passing a $noredirect value in for
single bug ids so that should be good now.

3. I fixed the POD doc errors as well as added the new error messages.

Please take a look
Thanks Dave

Comment 12 Kevin Baker 2008-03-06 21:08:00 UTC
Question, will the arguments to Bug.search be the same as they were for 
runQuery?

A nice feature of our runQuery is that you can take a search url from the web 
browser and use those same arguments in the xmlrpc call. I'd hope that this 
was still the case. OR, if not that some transformation was possible. like 
click a link to produce the args need to build a WebService query.

do I make sense? :)

Comment 13 David Lawrence 2008-03-06 22:08:22 UTC
(In reply to comment #12)
> Question, will the arguments to Bug.search be the same as they were for 
> runQuery?
> 
> A nice feature of our runQuery is that you can take a search url from the web 
> browser and use those same arguments in the xmlrpc call. I'd hope that this 
> was still the case. OR, if not that some transformation was possible. like 
> click a link to produce the args need to build a WebService query.
> 
> do I make sense? :)

yes and yes :)

The Bug.search() method will take the same form variable names as the query.cgi
UI. So should be compatible with bugzilla.runQuery. The main difference is the
ability to run a saved search or pass in simple quicksearch keyworkds.

Dave


Comment 14 Noura El hawary 2008-03-10 07:42:02 UTC
Comment on attachment 296930 [details]
Patch to add Bug.search capability to WebServices API (v2)

Hi Dave,,

I have tested the patch in bugdev,, here is what i found:


in Bugzilla/Search.pm:


>+        # REDHAT EXTENSION START 406151
>+        devel_whiteboard    => { name => "bugs.devel_whiteboard",      title => "Devel Whiteboard"    },
>+        qa_whiteboard       => { name => "bugs.qa_whiteboard",         title => "QA Whiteboard"       },
>+        internal_whiteboard => { name => "bugs.internal_whiteboard",   title => "Internal Whiteboard" },
>+        fixed_in            => { name => "bugs.fixed_in",              title => "Fixed In"            },
>+        # REDHAT EXTENSION END 406151
>+

I think we should change the column names to have cf_ as how they are called in
bugs table so it will be:

	# REDHAT EXTENSION START 406151
	devel_whiteboard    => { name => "bugs.cf_devel_whiteboard",	  title
=> "Devel Whiteboard"	 },
	qa_whiteboard	    => { name => "bugs.cf_qa_whiteboard",	  title
=> "QA Whiteboard"	 },
	internal_whiteboard => { name => "bugs.cf_internal_whiteboard",   title
=> "Internal Whiteboard" },
	fixed_in	    => { name => "bugs.cf_fixed_in",		  title
=> "Fixed In"		 },
	cust_facing	    => { name => "bugs.cf_cust_facing", 	  title
=> "Cust Facing"	 },
	# REDHAT EXTENSION END 406151


Note: I added cust_facing 

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

also shall we append type to those field names so we would have at line 402 in
Bugzilla/Search.pm the following:

    foreach my $f ("short_desc", "long_desc", "bug_file_loc",
"status_whiteboard",
		   "cf_devel_whiteboard",
"cf_qa_whiteboard","cf_internal_whiteboard", "cf_fixed_in", "cf_cust_facing") {
	if (defined $params->param($f)) {
	    my $s = trim($params->param($f));
	    if ($s ne "") {
		my $n = $f;
		my $q = $dbh->quote($s);
		trick_taint($q);
		my $type = $params->param($f . "_type");
		push(@specialchart, [$f, $type, $s]);
	    }
	}
    }

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

in query.cgi I guess we would have to add those custom fields to the function
PrefillForm as the following  line 130:

		      "status_whiteboard_type", "cf_devel_whiteboard",
		      "cf_devel_whiteboard_type" , "cf_qa_whiteboard",
		      "cf_qa_whiteboard_type", "cf_internal_whiteboard",
		      "cf_internal_whiteboard_type", "cf_fixed_in",
"cf_fixed_in_type",
		      "cf_cust_facing","cf_cust_facing_type", "bug_id",

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


>+        # Clean out any empty variables in the stored query.
>+        # Should only affect pre-3.x queries as new queries are cleaned
>+        # before they go into the database
>+        my $saved_search_data = Bugzilla->cgi->clean_search_url($saved_search->url);
>+        if ($saved_search_data ne "") {
>+            $search_params = Bugzilla::CGI->new($saved_search_data);
>+        }
>+    }
>+


savedsearch still doesn't want to work for me , I tested in my localhost
installation and bugdev and still same error, it only worked for me when i
skiped cleaning the url by replacing all the above lines with:

 $search_params = Bugzilla::CGI->new($saved_search->url);


Everything else seems to work fine for me.

Noura

Comment 15 David Lawrence 2008-03-10 18:23:18 UTC
Created attachment 297485 [details]
Patch to add Bug.search capability to WebServices API (v3)

Thanks Noura.

1. I have removed the references to the {devel,qa,internal}_whiteboard fields
as well as fixed_id and cust_facing in Bugzilla/Search.pm. Since these have
become custom fields, they are no longer needed to be explicitly declared in
the columns hash.

2. I have also removed the clean_search_url() function from Bug.search() and
will just load the full string into Bugzilla::CGI. This should have no negative
side effect as CGI.pm should ignore the empty fields anyway or leave them
empty.

See if this works better for you.

Thanks
Dave

Comment 16 Noura El hawary 2008-03-11 01:38:19 UTC
Comment on attachment 297485 [details]
Patch to add Bug.search capability to WebServices API (v3)

Hi Dave,,

It is working perfectly now. it just took me sometime to realize that now the
cf_ fields will be in the boolean chart and not as how status_whiteboard is. so
now all works good.

I think it can be committed to cvs.

Noura

Comment 17 David Lawrence 2008-03-11 03:58:48 UTC
Checked into CVS. Please update your trees.

Comment 18 Noura El hawary 2008-04-10 12:54:33 UTC
Comment on attachment 296412 [details]
Patch to add Bug.search capability to WebServices API

clear old review flags for obsolete patch