[copr] master: Fix removing multiple mock chroots (5722893)

bkabrda at fedoraproject.org bkabrda at fedoraproject.org
Thu Jan 31 12:06:29 UTC 2013


Repository : http://git.fedorahosted.org/cgit/copr.git

On branch  : master

>---------------------------------------------------------------

commit 5722893d0eced4b7f5f31958f72c3b6f08e68a85
Author: Bohuslav Kabrda <bkabrda at redhat.com>
Date:   Thu Jan 31 13:04:08 2013 +0100

    Fix removing multiple mock chroots


>---------------------------------------------------------------

 coprs_frontend/coprs/logic/coprs_logic.py          |    8 ++++++-
 .../tests/test_views/test_coprs_ns/test_general.py |   21 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 54ae9fe..f8a6f24 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -156,10 +156,16 @@ class CoprChrootsLogic(object):
         for mock_chroot in new_chroots:
             if mock_chroot not in current_chroots:
                 db.session.add(models.CoprChroot(copr=copr, mock_chroot=mock_chroot))
+
         # delete no more present
+        to_remove = []
         for mock_chroot in current_chroots:
             if mock_chroot not in new_chroots:
-                copr.mock_chroots.remove(mock_chroot)
+                # can't delete here, it would change current_chroots and break iteration
+                to_remove.append(mock_chroot)
+
+        for mc in to_remove:
+            copr.mock_chroots.remove(mc)
 
 class MockChrootsLogic(object):
     @classmethod
diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py
index 2d96e84..9f3fad6 100644
--- a/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py
+++ b/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py
@@ -255,6 +255,27 @@ class TestCoprUpdate(CoprsTestCase):
             assert self.mc3.chroot_name in mock_chroots_names
             assert self.mc1.chroot_name not in mock_chroots_names
 
+    def test_update_deletes_multiple_chroots(self, f_users, f_coprs, f_copr_permissions, f_mock_chroots):
+        # https://fedorahosted.org/copr/ticket/42
+        with self.tc as c:
+            with c.session_transaction() as s:
+                s['openid'] = self.u2.openid_name
+
+            self.db.session.add_all([self.u2, self.c2, self.mc1])
+            # add one more mock_chroot, so that we can remove two
+            cc = self.models.CoprChroot()
+            cc.mock_chroot = self.mc1
+            self.c2.copr_chroots.append(cc)
+
+            r = c.post('/coprs/detail/{0}/{1}/update/'.format(self.u2.name, self.c2.name),
+                       data = {'name': self.c2.name, self.mc1.chroot_name: 'y', 'id': self.c2.id},
+                       follow_redirects = True)
+            assert 'Copr was updated successfully' in r.data
+            self.db.session.add_all([self.c2, self.mc1])
+            mock_chroots = self.models.MockChroot.query.join(self.models.CoprChroot).\
+                                                        filter(self.models.CoprChroot.copr_id==\
+                                                               self.c2.id).all()
+            assert len(mock_chroots) == 1
 
 class TestCoprApplyForPermissions(CoprsTestCase):
     def test_apply(self, f_users, f_coprs):



More information about the copr-devel mailing list