Red Hat Bugzilla – Bug 427883
XMLRPC functions getBug() and getBugSimple() to be replaced by Bug.get()
Last modified: 2013-06-24 00:18:24 EDT
This is one of the highly used functions in rh_bugzilla_2_18 , in the upstream they have partially similar function to it called get_bugs. the function will be reworked to be compatible with the upstream. in rh_bugzilla_2_18 the function is at: Bugzilla::RPC::Bug::getBug() in the upstream partially similar one is: Bugzilla::WebService::Bug::getBug()
You mean that the Red Hat Bugzilla::RPC::Bug::getBug will be replaced with the upstream Bugzilla::WebService::Bug::get_bugs right?
Hi Kevin, Yes I think we can replave getBug with get_bugs , only we will have to give the bug id in an array as we can give one id or a list ,, also with get_bugs in the upstream this is an example of its output: 'summary' => 'this is atest bug', 'internals' => { 'priority' => 'P3', 'bug_id' => '2', '_multi_selects' => [], 'bug_file_loc' => '', 'cclist_accessible' => '1', 'rep_platform' => 'All', 'product_id' => '1', 'creation_ts' => '2008.01.08 16:19', 'assigned_to' => '2', 'everconfirmed' => '1', 'qa_contact' => '', 'short_desc' => 'assa', 'status_whiteboard' => '', 'bug_severity' => 'normal', 'bug_status' => 'NEW', 'delta_ts' => '2008-01-08 17:25:54', 'version' => 'unspecified', 'reporter_id' => '1', 'component_id' => '1', 'reporter_accessible' => '1', 'resolution' => '', 'target_milestone' => '---', 'alias' => '', 'op_sys' => 'All' }, 'creation_time' => '20080108T16:19:00', 'last_change_time' => '20080108T17:25:54', 'id' => '2', 'alias' => '' from the upstream API POD the internal section of the return is UNSTABLE so we can change and add the other things that we return from getBugSimple and getBug like the keywords for example. Noura
I think we need to discuss if we are going to replace both getBug() and getBugSimple() with the upstream function get_bugs() , basically our getBug() returns more comprehensive information than our getBugSimple(), but getBug() returns the bug details in User View format that is exactly similar to what the user see in show_bug.cgi only that it is textual. My suggestion would be to use the upstream get_bugs() to replace both functions and add to the return list all the extra info that we need like keywords, groups etc, unless the User View returned by getBug() is required by some people in redhat.
Filed bug upstream to rename Bug.get_bugs() to Bug.get() since the word bug twice is redundant and not part of a consistent method naming scheme. https://bugzilla.mozilla.org/show_bug.cgi?id=412850 Dave
LOC estimation: This function exist upstream as Bugzilla::WebService::Bug::get() ,, few modification will be done to it to enable it to return all the fields that are needed by the caller like groups, comments, keywords etc. I have submitted a patch for that upstream, added it to the external bugzilla links. LOC to update the function will be about: 20 LOC LOC for xmlrpc test case: 200 LOC total LOC = 220
*** Bug 427891 has been marked as a duplicate of this bug. ***
Created attachment 297050 [details] Bugzilla::WebService::Bug::get() Hey Dave, I re-thought about the Bug.get() function and I think the right behaviour would be: 1- if the user doesn't specify the fields they want in return then they get the stable bug fields returned + the internals hash , the internals hash should have all the bug information that they can get just as how our 2.18 getBug returns. 2- if they specify the fields they want then they get the stable bug fields returned + the fields they specified. I have made a comparison with what getBug returned and what Bug.get returns and added all the extra fields in getBug to the internals hash in Bug.get Please review the patch and let me know what you think. Thanks, Noura
Created attachment 297432 [details] Bugzilla::WebService::Bug::get() As Agreed with Dave ,, this is now how Bug.get() works: 1- if the user only passes bug ids, aliases then they get the stable bug info plus the internals hash with the basic bug info as how they are returned by upstream code. 2- id the user specifies the fields they want then they get the stable bug info plus the internals hash the contains the fields they specified. 3- if the user passes the params all set to 1 , then they get the stable bug info + internals hash with all the bug info that they can get. Note: for all the returned info I have converted ids to names to be more user friendly, as in the upstream code they mostly retun ids for product. component and users. Please review and let me know what you think. Thanks, Noura
Comment on attachment 297432 [details] Bugzilla::WebService::Bug::get() >+# REDHAT EXTENSION START 406151 >+# adding redhat custom fields to the mapping starting with cf_* > use constant FIELD_MAP => { >- status => 'bug_status', >- severity => 'bug_severity', >- description => 'comment', >- summary => 'short_desc', >- platform => 'rep_platform', >+ status => 'bug_status', >+ severity => 'bug_severity', >+ description => 'comment', >+ summary => 'short_desc', >+ platform => 'rep_platform', >+ comments => 'longdescs', >+ url => 'bug_file_loc', >+ milestone_url => 'url', >+ qa_whiteboard => 'cf_qa_whiteboard', >+ devel_whiteboard => 'cf_devel_whiteboard', >+ internal_whiteboard => 'cf_internal_whiteboard', >+ fixed_in => 'cf_fixed_in', >+ cust_facing => 'cf_cust_facing', > }; >+# REDHAT EXTENSION END 406151 Eventually we will need to add cf_issuetracker as well when the issuetracker support finally lands. >+# now the function can be called with optional hashkey fields as the following: >+# $call = $rpc->call( 'Bug.get', { ids => [1,3] , fields => ['status', 'groups', 'keywords', 'comments']} ); >+# the unstable internal section will be replaced with the passed in list of fields. > sub get { > my ($self, $params) = @_; > my $ids = $params->{ids}; Old argument. I would still prefer if we can pass in a single id as well as a list. my @ids = ref($params->{ids}) ? @{$params->{ids} : ($params->{ids}); foreach my $bug_id (@ids) { >@@ -76,6 +90,7 @@ sub get { > foreach my $bug_id (@$ids) { > ValidateBugID($bug_id); > my $bug = new Bugzilla::Bug($bug_id); >+ my $b = new Bugzilla::Bug($bug_id); > > # Timetracking fields are deleted if the user doesn't belong to > # the corresponding group. >@@ -92,10 +107,54 @@ sub get { > 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); >+ my $internals; >+ >+ # if the user has specified a list of fields that they want >+ # to get for the bug ids they are passing then return that list >+ # instead of the internals section, otherwise if no fields >+ # are specified then return the internals section >+ if ($params->{fields}) { >+ $item{internals} = {}; >+ $internals = $item{internals}; >+ foreach my $field ( @{ $params->{fields} } ) { >+ my $field_name = FIELD_MAP->{$field} || $field; >+ $internals->{$field_name} = $bug->$field_name; >+ } >+ } >+ elsif ($params->{all}) { >+ $item{internals} = $bug; >+ $internals = $bug; I am not entirely sure what is happening here. You are placing a copy of the $bug object into $item{internals} and then you placing another copy of the $bug object into $internals. But I do not see anywhere where you are assigning $item{internals} to $internals before you push the %item hash onto @return. >+ $internals->{cc} = $b->cc; >+ $internals->{dependson} = $b->dependson; >+ $internals->{blocked} = $b->blocked; >+ $internals->{longdescs} = $b->longdescs; >+ $internals->{keywords} = $b->keywords; >+ $internals->{groups} = $b->groups; >+ $internals->{attachments} = $b->attachments; >+ $internals->{flag_types} = $b->flag_types; >+ $internals->{dupe_id} = $b->dup_id; >+ $internals->{actual_time} = $b->actual_time; >+ $internals->{milestone_url} = $b->milestoneurl; >+ } >+ else { >+ $item{internals} = $bug; >+ $internals = $bug; Same here >+ } >+ >+ # convert ids to names >+ $internals->{qa_contact} = type('string')->value($b->qa_contact->login) >+ if ($internals->{qa_contact} && $b->qa_contact); >+ $internals->{assigned_to} = type('string')->value($b->assigned_to->login) >+ if $internals->{assigned_to}; >+ $internals->{reporter} = type('string')->value($b->reporter->login) >+ if $internals->{reporter_id}; >+ $internals->{product} = type('string')->value($b->product) if $internals->{product_id}; >+ $internals->{component} = type('string')->value($b->component) if $internals->{component_id}; >+ Should you be assigning $internals to $item{internal} here so that the %item hash will have all of the requested information? $item{internals} = $internals; When I run a test script against this patch on bugdev.devel.redhat.com/bugzilla, I always get the basic information in the $bug object back for the 'internals' key. It doesn't matter if I pass in all => 1 or specify specific fields, I get the same data every time. Dave
Created attachment 297556 [details] Bugzilla::WebService::Bug::get() Thanks for the review Dave.. Here we go another patch with your suggestions applied. Noura
This feature is included now in Milestone 2. Any bugs found with this feature should be file as a new report and set to block the 3.2 final release tracker.
Comment on attachment 297556 [details] Bugzilla::WebService::Bug::get() Clearing review flag since this is has been postponed.