mozreview: Match against old-style Bugzilla attachment URLs (bug 1232924); r?mdoglio draft
authorMark Cote <mcote@mozilla.com>
Wed, 16 Dec 2015 11:13:15 -0500
changeset 6387 8659d1139122d9b11b6904c2ca697360b60ec6b5
parent 6329 c9f5ffc714329f6eb04d58a842e56ba677641757
push id464
push usermcote@mozilla.com
push dateWed, 16 Dec 2015 16:18:17 +0000
reviewersmdoglio
bugs1232924
mozreview: Match against old-style Bugzilla attachment URLs (bug 1232924); r?mdoglio We switched the RB URL stored in Bugzilla attachments to be the diff URL instead of the reviews URL, but we didn't take that into account when looking for old-style Bugzilla attachments to set review flags or obsolete them.
pylib/mozreview/mozreview/bugzilla/client.py
--- a/pylib/mozreview/mozreview/bugzilla/client.py
+++ b/pylib/mozreview/mozreview/bugzilla/client.py
@@ -177,17 +177,19 @@ class Bugzilla(object):
         reviewers = reviewers.copy()
         params = self._auth_params({})
         flags = []
         rb_attachment = None
         attachments = self.get_rb_attachments(bug_id)
 
         # Find the associated attachment, then go through the review flags.
         for a in attachments:
-            if a['data'] != url:
+
+            # Make sure we check for old-style URLs as well.
+            if not self._rb_attach_url_matches(a['data'], url):
                 continue
 
             rb_attachment = a
 
             for f in a.get('flags', []):
                 if f['name'] not in ['review', 'feedback']:
                     # We only care about review and feedback flags.
                     continue
@@ -265,20 +267,25 @@ class Bugzilla(object):
             'include_fields': ['id', 'data', 'content_type', 'is_obsolete',
                                'flags']
         })
 
         return [a for a
                 in self.proxy.Bug.attachments(params)['bugs'][str(bug_id)]
                 if a['content_type'] == 'text/x-review-board-request']
 
+    def _rb_attach_url_matches(self, attach_url, rb_url):
+        # Make sure we check for old-style URLs as well.
+        return (attach_url == rb_url or
+                '%sdiff/#index_header' % attach_url == rb_url)
+
     def _get_review_request_attachment(self, bug_id, rb_url):
         """Obtain a Bugzilla attachment for a review request."""
         for a in self.get_rb_attachments(bug_id):
-            if a.get('data') == rb_url:
+            if self._rb_attach_url_matches(a.get('data'), rb_url):
                 return a
 
         return None
 
     @xmlrpc_to_bugzilla_errors
     def r_plus_attachment(self, bug_id, reviewer, rb_url, comment=None):
         """Set a review flag to "+".
 
@@ -371,17 +378,18 @@ class Bugzilla(object):
         want to leave any lingering references in Bugzilla.
         """
         params = self._auth_params({
             'ids': [],
             'is_obsolete': True,
         })
 
         for a in self.get_rb_attachments(bug_id):
-            if a.get('data') == rb_url and not a.get('is_obsolete'):
+            if (self._rb_attach_url_matches(a.get('data'), rb_url) and
+                not a.get('is_obsolete')):
                 params['ids'].append(a['id'])
 
         if params['ids']:
             self.proxy.Bug.update_attachment(params)
 
     @xmlrpc_to_bugzilla_errors
     def valid_api_key(self, username, api_key):
         try: