Merge lp://staging/~gmb/launchpad/bugzilla-3.4-bug-422848-bug-423046 into lp://staging/launchpad

Proposed by Graham Binns
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~gmb/launchpad/bugzilla-3.4-bug-422848-bug-423046
Merge into: lp://staging/launchpad
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~gmb/launchpad/bugzilla-3.4-bug-422848-bug-423046
Reviewer Review Type Date Requested Status
Gavin Panella (community) code Approve
Review via email: mp+11043@code.staging.launchpad.net

Commit message

Launchpad should now actually be able to import comments from Bugzilla 3.4. Really.

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) wrote :

This branch fixes a Schroedingbug that I discovered in the Bugzilla 3.4 compatibility work. Basically, we should never have been able to sync comments with Bugzilla 3.4 instances because the code didn't DTRT. I've now fixed this (*and* QA'd the fix against the Gnome Bugzilla for the sake of certainty).

Revision history for this message
Gavin Panella (allenap) wrote :

Wow, this stuff is quite painful.

We could consider using Zope schemas (or something else sensible) to help define and validate the data structures we pass between bug trackers. Lists of dicts and dicts within dicts and so on can get quite confusing as to what goes where.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/bugs/externalbugtracker/bugzilla.py'
--- lib/lp/bugs/externalbugtracker/bugzilla.py 2009-08-28 12:54:58 +0000
+++ lib/lp/bugs/externalbugtracker/bugzilla.py 2009-09-02 08:32:27 +0000
@@ -605,14 +605,16 @@
605605
606 # Get only the remote comment IDs and store them in the606 # Get only the remote comment IDs and store them in the
607 # 'comments' field of the bug.607 # 'comments' field of the bug.
608 bug_comments_dict = self.xmlrpc_proxy.Bug.comments({608 return_dict = self.xmlrpc_proxy.Bug.comments({
609 'ids': [actual_bug_id],609 'ids': [actual_bug_id],
610 'include_fields': ['id'],610 'include_fields': ['id'],
611 })611 })
612612
613 # We need to convert bug and comment ids to strings (see bugs613 # We need to convert bug and comment ids to strings (see bugs
614 # 248662 amd 248938).614 # 248662 amd 248938).
615 bug_comments = bug_comments_dict['bugs'][str(actual_bug_id)]615 bug_comments_dict = return_dict['bugs']
616 bug_comments = bug_comments_dict[str(actual_bug_id)]['comments']
617
616 return [str(comment['id']) for comment in bug_comments]618 return [str(comment['id']) for comment in bug_comments]
617619
618 def fetchComments(self, bug_watch, comment_ids):620 def fetchComments(self, bug_watch, comment_ids):
@@ -636,7 +638,10 @@
636 if int(comment['bug_id']) != actual_bug_id:638 if int(comment['bug_id']) != actual_bug_id:
637 del comments[comment_id]639 del comments[comment_id]
638640
639 self._bugs[actual_bug_id]['comments'] = return_dict['comments']641 # Ensure that comment IDs are converted to ints.
642 comments_with_int_ids = dict(
643 (int(id), comments[id]) for id in comments)
644 self._bugs[actual_bug_id]['comments'] = comments_with_int_ids
640645
641 def getPosterForComment(self, bug_watch, comment_id):646 def getPosterForComment(self, bug_watch, comment_id):
642 """See `ISupportsCommentImport`."""647 """See `ISupportsCommentImport`."""
643648
=== modified file 'lib/lp/bugs/tests/bugzilla-api-xmlrpc-transport.txt'
--- lib/lp/bugs/tests/bugzilla-api-xmlrpc-transport.txt 2009-08-26 13:37:03 +0000
+++ lib/lp/bugs/tests/bugzilla-api-xmlrpc-transport.txt 2009-09-02 09:18:14 +0000
@@ -292,7 +292,7 @@
292 ... for key in sorted(bugs_dict):292 ... for key in sorted(bugs_dict):
293 ... print "Bug %s:" % key293 ... print "Bug %s:" % key
294 ... bug_comments = sorted(294 ... bug_comments = sorted(
295 ... bugs_dict[key],295 ... bugs_dict[key]['comments'],
296 ... key=operator.itemgetter(sort_key))296 ... key=operator.itemgetter(sort_key))
297 ...297 ...
298 ... for comment in bug_comments:298 ... for comment in bug_comments:
299299
=== modified file 'lib/lp/bugs/tests/externalbugtracker.py'
--- lib/lp/bugs/tests/externalbugtracker.py 2009-08-27 16:42:55 +0000
+++ lib/lp/bugs/tests/externalbugtracker.py 2009-09-02 08:32:27 +0000
@@ -916,8 +916,17 @@
916 # different when passed to TestBugzillaXMLRPCTransport.comments.916 # different when passed to TestBugzillaXMLRPCTransport.comments.
917 del comments_args['ids']917 del comments_args['ids']
918 comments_args['bug_ids'] = arguments['ids']918 comments_args['bug_ids'] = arguments['ids']
919 [return_dict] = TestBugzillaXMLRPCTransport.comments(919 [returned_dict] = TestBugzillaXMLRPCTransport.comments(
920 self, comments_args)920 self, comments_args)
921
922 # We need to move the comments for each bug in to a
923 # 'comments' dict.
924 bugs_dict = returned_dict['bugs']
925 bug_comments_dict = {}
926 for bug_id, comment_list in bugs_dict.items():
927 bug_comments_dict[bug_id] = {'comments': comment_list}
928
929 return_dict = {'bugs': bug_comments_dict}
921 else:930 else:
922 return_dict = {'bugs': {}}931 return_dict = {'bugs': {}}
923932