From fe7cd591631aacb5c8191583e481487025043401 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Mon, 11 Jun 2018 19:59:07 +0200 Subject: [PATCH] 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 --- bodhi/server/validators.py | 12 ++--- bodhi/tests/server/services/test_updates.py | 54 +++++++++++++++++++++ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/bodhi/server/validators.py b/bodhi/server/validators.py index a59d5b0766..f5b89bf44a 100644 --- a/bodhi/server/validators.py +++ b/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 diff --git a/bodhi/tests/server/services/test_updates.py b/bodhi/tests/server/services/test_updates.py index 538a4c5685..25f13d7d20 100644 --- a/bodhi/tests/server/services/test_updates.py +++ b/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):