Description: Red Hat, Inc. uses a tracking system called Issue Tracker for dealing with customer issues with our supported products. Bugzilla is also an issue tracker but is not normally used for support tickets and issues. The two systems have API code which allows updates to be transferred between the two systems. Function Requirements: Mostly in Bugzilla/IssueTrackerList.pm but has calls to the module from several cgi's.
LOC Estimation: Bugzilla/IssueTrackerList.pm: 448 data/params: 7 process_bug.cgi: 41 XMLRPC tests needed: 3 methods * 20 lines of code avg = 60 Module API tests needed: 24 methods * 20 lines of code avg: 480 template lines of code: 80 Bugzilla/Bug.pm: 20 Bugzilla/Search.pm: 10 Bugzilla/BugMail.pm: 15 long_list.cgi: 2 globals.pl: 12 defparams.pl: 46 colchange.cgi: 4 CGI.pl: 2 Bugzilla/DB/Schema.pm: 10 Total LOC: 1237
It must keep in sync Status, Priority and Severity if it doesn't already.
(In reply to comment #2) > It must keep in sync Status, Priority and Severity if it doesn't already. see bug 406461 for tracking this issue
Created attachment 295481 [details] Issue tracker integration (v1) I have completed a patch to add IssueTracker support to rh_bugzilla_3. I have done it as a Bugzilla extension which keeps the code (for the most part) separate from the main code base. I have also updated IssueTrackerList.pm to send any bug_severity and priority changes along with the normal change set to IssueTracker. IssueTracker will need to have support on its end to recognize when a severity or priority change has occurred and update its report. Also IssueTracker will need to use the appropriate update{Severity,Priority} XMLRPC call when a change occurs on it's side as well.
Worked 12 hours
Created attachment 295565 [details] Issue tracker integration (v2) Some fixes as well as added in missing support for 1. Adding of Issue Tracker ids when creating a new bug 2. Sending bug changes to issuetracker. Please review. Let me know if you have any questions as extension support is new ground for all of us :) Dave
Created attachment 295587 [details] Issue tracker integration (v3) All previous changes plus added ability to change issuetracker params using the editparams.cgi UI.
Q1) rh_bugzilla_3/Bugzilla/Template/Plugin/Hook.pm Did you add these changes or were they upstream? My concern is that you are altering their $template var which may have unforseen consequences. Q2) post_bug.cgi You added a hook 'get_bug_fields' which adds the issuetracker field to the @bug_fields array but we have also an extension here 406151 which is adding other custom fields. Can we merge these two? Also, I see that custom fields is adding fields to the @bug_fields array. Which makes me wonder if we shouldn't be using them? Q3) Those hooks you added to the pm files, are they likely to be accepted upstream? I guess they will push back and say we should be using custom fields.
(In reply to comment #8) > Q1) rh_bugzilla_3/Bugzilla/Template/Plugin/Hook.pm > > Did you add these changes or were they upstream? My concern is that you are > altering their $template var which may have unforseen consequences. > Bug 318205 – the path to the hook is incorrect when called from inside a block https://bugzilla.mozilla.org/show_bug.cgi?id=318205 > Q2) post_bug.cgi > > You added a hook 'get_bug_fields' which adds the issuetracker field to the > @bug_fields array but we have also an extension here 406151 which is adding > other custom fields. Can we merge these two? > > Also, I see that custom fields is adding fields to the @bug_fields array. > Which makes me wonder if we shouldn't be using them? > I thought about using custom fields for this but it would take similar amount of work as we would still need to add hooks to various places to validate the issue ids that are entered, send changes to issuetracker when bugs are changed that have issue ids attached, and filter the issuetracker diffs for bugs activity view and email notifications. Also there will need to be an import from the standard bugzilla->issuetracker mapping tables to the custom fields tables when we move the database. That is not that hard but with the Hooks we are able to leave the database schema as is. > Q3) Those hooks you added to the pm files, are they likely to be accepted > upstream? I guess they will push back and say we should be using custom > fields. I am going to see what I can do. I tried to keep it as generic as possible and put it in places where we could add new functionality easily in the future. So they may take some of them. http://www.bugzilla.org/docs/3.0/html/cust-hooks.html " If there is no hook at the appropriate place within the Bugzilla source file or template you want to extend, file a bug requesting one, specifying: the source or template file for which you are requesting a hook; where in the file you would like the hook to be placed (line number/position for latest version of the file in CVS or description of location); the purpose of the hook; a link to information about your extension, if any."
Hi Dave, sounds reasonable on all points. ACK
Created attachment 295999 [details] Issue tracker integration (v4) Same patch as v3 except created new code file extensions/issuetracker/code/quote_urls.pl and added Hook::process('quote_urls') in Bugzilla::Template::quoteUrls. quote_urls.pl handles the linkification(sp) of the Issue Tracker links in bug comments. Please review. Dave
Created attachment 297255 [details] Issue tracker integration using custom fields (v1) I have reimplemented issue tracker id support using custom fields instead of having separate redhat only mapping tables. I had to extend custom field support to allow for a freetext field for multiple integer values. I will try to get this enhancement submitted upstream as I feel this could be useful for others. Pros: 1. Allows us to use the standard backend code for adding/removing/changing values in the fields instead of using complete separate custom code. 2. Can add other fields similar in the future as needed. Cons: 1. Looses support for attaching a score to each issue tracker ID although Lisa says it may not be needed anymore anyway. Currently I am just validating ids that are added against issuetracker to make sure they are valid and not attached to another bug already. I have not yet implemented the actually sending of changes to issuetracker yet. Please take a look. Dave
Bugzilla/DB/Schema.pm - should this also have a FK to bugs.bug_id? I think this should be pushed upstream asap. I'm sure it will change under review. This functionality would be generally useful, think of the blocks & depends on fields.
(In reply to comment #13) > Bugzilla/DB/Schema.pm - should this also have a FK to bugs.bug_id? Will add. > I think this should be pushed upstream asap. I'm sure it will change under > review. This functionality would be generally useful, think of the blocks & > depends on fields. Exactly. Already filed: https://bugzilla.mozilla.org/show_bug.cgi?id=287334 I will create a sterilized patch and submit upstream Dave
Created attachment 298010 [details] Issue tracker integration using custom fields (v2) Added webservice.pl for bugzilla.addIT, bugzilla.deleteIT, and bugzilla.issueExists. Mostly the same as v1 everywhere else. Please take a look. Will install on bugdev.devel.redhat.com/bugzilla for function testing. Dave
Created attachment 298220 [details] Issue tracker integration using custom fields (v3) Fixed some issues with sendChanges() to Issue Tracker. This patch seems to give similar functionality now to 2.18 finally. Thanks Dave
Bugzilla/DB/Schema.pm - should there by a CASCADE clause there too? tfu? Bugzilla/DB.pm - I'm a little curious about this one. Let's talk on the phone. Bugzilla/Field.pm - probably wiser to add a FILE_TYPE_LAST so you don't have to rewrite those detaint_natural($type) && $type < FIELD_TYPE_LAST) But yeh, it's the upstream code so we're already messing on the wrong side of the pond. Bugzilla/Bug.pm _check_freetext_multi_integer - this is my take. note the sort using <=> to correctly sort for numbers. sub _check_freetext_multi_integer { my ($invocant, $value, $field) = @_; return [] if !$value; my %value; foreach my $v (split(/[,\s]+/, $value)) { my $orig = trim($v); detaint_natural($v) || ThrowUserError('field_invalid_value', { value => $orig, field => $field, type => 'integer' }); $value{$v} = 1; # removes duplicates } my @values = sort { $a <=> $b } keys %value; # REDHAT EXTENSION START 406111 Bugzilla::Hook::process('check_freetext_multi_integer', { current => $invocant->$field, field => $field, values => \@values }); # REDHAT EXTENSION END 406111 return \@values; }
(In reply to comment #17) > Bugzilla/DB/Schema.pm > - should there by a CASCADE clause there too? tfu? Good idea. Added. > Bugzilla/DB.pm > - I'm a little curious about this one. Let's talk on the phone. We talked :) > Bugzilla/Field.pm > - probably wiser to add a FILE_TYPE_LAST so you don't have to rewrite those > > detaint_natural($type) && $type < FIELD_TYPE_LAST) > But yeh, it's the upstream code so we're already messing on the wrong > side of the pond. > Good idea. Added. > > Bugzilla/Bug.pm _check_freetext_multi_integer > - this is my take. note the sort using <=> to correctly sort for numbers. > > sub _check_freetext_multi_integer { > my ($invocant, $value, $field) = @_; > > return [] if !$value; > > my %value; > foreach my $v (split(/[,\s]+/, $value)) { > my $orig = trim($v); > detaint_natural($v) > || ThrowUserError('field_invalid_value', > { value => $orig, field => $field, type => 'integer' }); > $value{$v} = 1; # removes duplicates > } > > my @values = sort { $a <=> $b } keys %value; > > # REDHAT EXTENSION START 406111 > Bugzilla::Hook::process('check_freetext_multi_integer', > { current => $invocant->$field, field => $field, values => > \@values }); > # REDHAT EXTENSION END 406111 > > return \@values; > } Added as well.