From e21dfae1526814152603122f2ac4c9d36a7d814d Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Fri, 13 Feb 2009 13:52:19 +1100 Subject: [PATCH] [views] Check count() for duplicate bundle patches, rather than try/except If the exception is raised, the transaction will still be in a bad state, due to the foreign key constraint. Signed-off-by: Jeremy Kerr --- apps/patchwork/tests/bundles.py | 43 +++++++++++++++++++++++++++++++++ apps/patchwork/utils.py | 10 +++++--- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/apps/patchwork/tests/bundles.py b/apps/patchwork/tests/bundles.py index d9c0bed..2f2f122 100644 --- a/apps/patchwork/tests/bundles.py +++ b/apps/patchwork/tests/bundles.py @@ -240,6 +240,49 @@ class BundleAddFromListTest(BundleTestBase): for i in [0, 1] ] self.failUnless(bps[0].order < bps[1].order) + def testAddDuplicate(self): + self.bundle.append_patch(self.patches[0]) + count = self.bundle.patches.count() + patch = self.patches[0] + + params = {'form': 'patchlistform', + 'action': 'Add', + 'project': defaults.project.id, + 'bundle_id': self.bundle.id, + 'patch_id:%d' % patch.id: 'checked'} + + response = self.client.post( + '/project/%s/list/' % defaults.project.linkname, + params) + + self.assertContains(response, 'Patch '%s' already in bundle' \ + % patch.name, count = 1, status_code = 200) + + self.assertEquals(count, self.bundle.patches.count()) + + def testAddNewAndDuplicate(self): + self.bundle.append_patch(self.patches[0]) + count = self.bundle.patches.count() + patch = self.patches[0] + + params = {'form': 'patchlistform', + 'action': 'Add', + 'project': defaults.project.id, + 'bundle_id': self.bundle.id, + 'patch_id:%d' % patch.id: 'checked', + 'patch_id:%d' % self.patches[1].id: 'checked'} + + response = self.client.post( + '/project/%s/list/' % defaults.project.linkname, + params) + + self.assertContains(response, 'Patch '%s' already in bundle' \ + % patch.name, count = 1, status_code = 200) + self.assertContains(response, 'Patch '%s' added to bundle' \ + % self.patches[1].name, count = 1, + status_code = 200) + self.assertEquals(count + 1, self.bundle.patches.count()) + class BundleAddFromPatchTest(BundleTestBase): def testAddToEmptyBundle(self): patch = self.patches[0] diff --git a/apps/patchwork/utils.py b/apps/patchwork/utils.py index 3737e89..797951c 100644 --- a/apps/patchwork/utils.py +++ b/apps/patchwork/utils.py @@ -117,13 +117,15 @@ def set_bundle(user, project, action, data, patches, context): for patch in patches: if action == 'create' or action == 'add': - try: + bundlepatch_count = BundlePatch.objects.filter(bundle = bundle, + patch = patch).count() + if bundlepatch_count == 0: bundle.append_patch(patch) context.add_message("Patch '%s' added to bundle %s" % \ (patch.name, bundle.name)) - except Exception, ex: - context.add_message("Couldn't add patch '%s' to bundle: %s" % \ - (patch.name, ex.message)) + else: + context.add_message("Patch '%s' already in bundle %s" % \ + (patch.name, bundle.name)) elif action == 'remove': try: -- 2.39.2