Fix editing multi-build updates when title failed to update

Sometimes an update title fails to update, and the title was used
in multi-build updates to determine whether a particular build was
in the update being edited.
This patch makes it actually look at whether the build is in the
current update by comparing.

Signed-off-by: Patrick Uiterwijk <[email protected]>

bodhi/server/validators.py

@@ -258,13 +258,11 @@ def validate_builds(request, **kwargs):
                                    'Cannot edit stable updates')
 
         for nvr in request.validated.get('builds', []):
-            # If the build is new
-            if nvr not in edited:
-                # Ensure it doesn't already exist
-                build = request.db.query(Build).filter_by(nvr=nvr).first()
-                if build and build.update is not None:
-                    request.errors.add('body', 'builds',
-                                       "Update for {} already exists".format(nvr))
+            # Ensure it doesn't already exist in another update
+            build = request.db.query(Build).filter_by(nvr=nvr).first()
+            if build and build.update is not None and up.title != build.update.title:
+                request.errors.add('body', 'builds',
+                                   "Update for {} already exists".format(nvr))
 
         return
 

bodhi/tests/server/services/test_updates.py

@@ -2630,6 +2630,60 @@ def test_edit_testing_update_with_new_builds(self, publish, *args):
         self.assertEquals(len(publish.call_args_list), 3)
         publish.assert_called_with(topic='update.edit', msg=ANY)
 
+    @mock.patch(**mock_valid_requirements)
+    @mock.patch('bodhi.server.notifications.publish')
+    def test_edit_testing_update_with_failed_title_change(self, publish, *args):
+        """Test that editing a multi-build update is still possible if title updating failed.
+
+        At some point, the text of the title was used to determine whether a build was
+        in the current update.
+        """
+        nvrs = 'a-1.0-1.fc17,b-1.0-1.fc17,c-1.0-1.fc17'
+        args = self.get_update(nvrs)
+        r = self.app.post_json('/updates/', args)
+        publish.assert_called_with(topic='update.request.testing', msg=ANY)
+
+        # Mark it as testing
+        upd = Update.get(r.json_body['alias'])
+        newtitle = upd.title.replace('a-1.0', 'a-1.5')
+        upd.title = newtitle
+        upd.status = UpdateStatus.testing
+        upd.request = None
+        self.db.commit()
+
+        args['edited'] = newtitle
+        # Kept a and c the same, bumped b
+        args['builds'] = ['a-1.0-1.fc17', 'b-2.0-1.fc17', 'c-1.0-1.fc17']
+        r = self.app.post_json('/updates/', args)
+        up = r.json_body
+        self.assertEquals(up['title'], u'a-1.0-1.fc17 b-2.0-1.fc17 c-1.0-1.fc17')
+        self.assertEquals(up['status'], u'pending')
+        self.assertEquals(up['request'], u'testing')
+        self.assertEquals(up['comments'][-1]['text'],
+                          u'This update has been submitted for testing by guest. ')
+        comment = textwrap.dedent("""
+        guest edited this update.
+
+        New build(s):
+
+        - b-2.0-1.fc17
+
+        Removed build(s):
+
+        - b-1.0-1.fc17
+
+        Karma has been reset.
+        """).strip()
+        self.assertMultiLineEqual(up['comments'][-2]['text'], comment)
+        self.assertEquals(up['comments'][-3]['text'],
+                          u'This update has been submitted for testing by guest. ')
+        self.assertEquals(len(up['builds']), 3)
+        self.assertEquals(up['builds'][0]['nvr'], u'a-1.0-1.fc17')
+        self.assertEquals(up['builds'][1]['nvr'], u'c-1.0-1.fc17')
+        self.assertEquals(up['builds'][2]['nvr'], u'b-2.0-1.fc17')
+        self.assertEquals(len(publish.call_args_list), 3)
+        publish.assert_called_with(topic='update.edit', msg=ANY)
+
     @mock.patch(**mock_valid_requirements)
     @mock.patch('bodhi.server.notifications.publish')
     def test_edit_testing_update_with_new_builds_with_stable_request(self, publish, *args):