[copr] master: Don't delete coprs, rather mark them as deleted and don't show them (c539b68)

bkabrda at fedoraproject.org bkabrda at fedoraproject.org
Wed Apr 3 09:55:44 UTC 2013


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

On branch  : master

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

commit c539b68d736fcbcc3a2186ee809adbf9c9ab6004
Author: Bohuslav Kabrda <bkabrda at redhat.com>
Date:   Wed Apr 3 11:55:30 2013 +0200

    Don't delete coprs, rather mark them as deleted and don't show them


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

 coprs_frontend/coprs/logic/coprs_logic.py          |   23 ++++++++++++++-----
 coprs_frontend/coprs/models.py                     |    5 +++-
 .../coprs/templates/admin/legal-flag.html          |   21 +++++++++++++-----
 .../test_views/test_coprs_ns/test_coprs_general.py |   22 ++++++++++++++++++-
 4 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 0066b1d..c304fdd 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -20,7 +20,8 @@ class CoprsLogic(object):
                            join(models.Copr.owner).\
                            options(db.contains_eager(models.Copr.owner)).\
                            filter(models.Copr.name == coprname).\
-                           filter(models.User.openid_name == models.User.openidize_name(username))
+                           filter(models.User.openid_name == models.User.openidize_name(username)).\
+                           filter(models.Copr.deleted==False)
 
         if with_builds:
             query = query.outerjoin(models.Copr.builds).\
@@ -40,6 +41,7 @@ class CoprsLogic(object):
         user_relation = kwargs.get('user_relation', None)
         username = kwargs.get('username', None)
         with_mock_chroots = kwargs.get('with_mock_chroots', None)
+        incl_deleted = kwargs.get('incl_deleted', None)
         ids = kwargs.get('ids', None)
 
         query = db.session.query(models.Copr).\
@@ -47,6 +49,9 @@ class CoprsLogic(object):
                            options(db.contains_eager(models.Copr.owner)).\
                            order_by(models.Copr.id.desc())
 
+        if not incl_deleted:
+            query = query.filter(models.Copr.deleted==False)
+
         if isinstance(ids, list): # can be an empty list
             query = query.filter(models.Copr.id.in_(ids))
 
@@ -69,7 +74,9 @@ class CoprsLogic(object):
 
     @classmethod
     def get_multiple_fulltext(cls, user, search_string):
-        query = models.Copr.query.join(models.User).whooshee_search(search_string)
+        query = models.Copr.query.join(models.User).\
+                                  filter(models.Copr.deleted==False).\
+                                  whooshee_search(search_string)
         return query
 
     @classmethod
@@ -88,7 +95,7 @@ class CoprsLogic(object):
         return copr
 
     @classmethod
-    def new(cls, user, copr, check_for_duplicates = True):
+    def new(cls, user, copr, check_for_duplicates=True):
         if check_for_duplicates and cls.exists_for_user(user, copr.name).all():
             raise exceptions.DuplicateException(
                 'Copr: "{0}" already exists'.format(copr.name))
@@ -109,7 +116,9 @@ class CoprsLogic(object):
         else: # we're renaming
             # if we fire a models.Copr.query, it will use the modified copr in session
             # -> workaround this by just getting the name
-            old_copr_name = db.session.query(models.Copr.name).filter(models.Copr.id==copr.id).first()[0]
+            old_copr_name = db.session.query(models.Copr.name).filter(models.Copr.id==copr.id).\
+                                                               filter(models.Copr.deleted==False).\
+                                                               first()[0]
             action = models.Action(action_type=helpers.ActionTypeEnum('rename'),
                                    object_type='copr',
                                    object_id=copr.id,
@@ -131,16 +140,18 @@ class CoprsLogic(object):
                                old_value='{0}/{1}'.format(copr.owner.name, copr.name),
                                new_value='',
                                created_on=int(time.time()))
+        copr.deleted = True
 
         db.session.add(action)
-        db.session.delete(copr)
 
         return copr
 
     @classmethod
-    def exists_for_user(cls, user, coprname):
+    def exists_for_user(cls, user, coprname, incl_deleted=False):
         existing = models.Copr.query.filter(models.Copr.name==coprname).\
                                      filter(models.Copr.owner_id==user.id)
+        if not incl_deleted:
+            existing = existing.filter(models.Copr.deleted==False)
 
         return existing
 
diff --git a/coprs_frontend/coprs/models.py b/coprs_frontend/coprs/models.py
index d6d5aee..f31de29 100644
--- a/coprs_frontend/coprs/models.py
+++ b/coprs_frontend/coprs/models.py
@@ -122,7 +122,9 @@ class User(db.Model, Serializer):
     @property
     def coprs_count(self):
         """Get number of coprs for this user."""
-        return Copr.query.filter_by(owner=self).count()
+        return Copr.query.filter_by(owner=self).\
+                          filter_by(deleted=False).\
+                          count()
 
 
 class Copr(db.Model, Serializer):
@@ -140,6 +142,7 @@ class Copr(db.Model, Serializer):
     instructions = db.Column(db.Text)
     # duplicate information, but speeds up a lot and makes queries simpler
     build_count = db.Column(db.Integer, default = 0)
+    deleted = db.Column(db.Boolean, default=False)
 
     # relations
     owner_id = db.Column(db.Integer, db.ForeignKey('user.id'))
diff --git a/coprs_frontend/coprs/templates/admin/legal-flag.html b/coprs_frontend/coprs/templates/admin/legal-flag.html
index 6dc0e2a..75f61c1 100644
--- a/coprs_frontend/coprs/templates/admin/legal-flag.html
+++ b/coprs_frontend/coprs/templates/admin/legal-flag.html
@@ -5,18 +5,27 @@
 {% block admin_body %}
   {% for flag in legal_flags %}
     <div class="legal-flag">
-      <div>
+	    <div>
+        {% if not flag.copr.deleted %}
         <a href={{ url_for('coprs_ns.copr_detail', username=flag.copr.owner.name, coprname=flag.copr.name) }}>
-          {{ flag.copr.owner.name }}/{{ flag.copr.name }}
-        </a>
-        | <a href="">Freeze</a>
-	| <a href="{{ url_for('coprs_ns.copr_delete', username=flag.copr.owner.name, coprname=flag.copr.name) }}">Delete</a>
+        {% endif %}
+        {{ flag.copr.owner.name }}/{{ flag.copr.name }}
+        {% if not flag.copr.deleted %}
+          </a>
+        {% endif %}
+	|
+        {% if flag.copr.deleted %}
+          Already deleted
+        {% else %}
+          <a href="">Freeze</a>
+          | <a href="{{ url_for('coprs_ns.copr_delete', username=flag.copr.owner.name, coprname=flag.copr.name) }}">Delete</a>
+        {% endif %}
 	<form method="post" action="{{ url_for('admin_ns.legal_flag_resolve', flag_id=flag.id) }}">
             <input type="submit" value="Resolve">
           </form>
       </div>
       <div class="message">
-        {{ flag.message }}
+        {{ flag.raise_message }}
       </div>
     </div>
   {% else %}
diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
index 6fa8e19..380d59d 100644
--- a/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
+++ b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
@@ -132,6 +132,25 @@ class TestCoprNew(CoprsTestCase):
             assert copr.build_count == 1
             assert 'Initial packages were successfully submitted' in r.data
 
+    def test_copr_new_is_allowed_even_if_deleted_has_same_name(self, f_users, f_coprs, f_mock_chroots, f_db):
+        with self.tc as c:
+            with c.session_transaction() as s:
+                s['openid'] = self.u1.openid_name
+
+            self.db.session.add(self.c1)
+            self.c1.deleted = True
+            self.c1.owner = self.u1
+            self.db.session.commit()
+
+            self.db.session.add(self.c1)
+            r = c.post('/coprs/new/', data = {'name': self.c1.name, 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects = True)
+            self.db.session.add_all([self.c1, self.u1])
+            assert len(self.models.Copr.query.filter(self.models.Copr.name==self.c1.name).\
+                                              filter(self.models.Copr.owner==self.u1).\
+                                              all()) == 2
+            assert self.success_string in r.data
+
+
 class TestCoprDetail(CoprsTestCase):
     def test_copr_detail_not_found(self):
         r = self.tc.get('/coprs/detail/foo/bar/')
@@ -407,8 +426,9 @@ class TestCoprDelete(CoprsTestCase):
                        data = {'verify': 'yes'},
                        follow_redirects = True)
             assert 'Copr was deleted successfully' in r.data
+            self.db.session.add(self.c1)
             assert self.models.Action.query.first().id == self.c1.id
-            assert not self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first()
+            assert self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first().deleted
 
     def test_copr_delete_does_not_delete_if_verify_filled_wrongly(self, f_users, f_coprs, f_db):
         with self.tc as c:



More information about the copr-devel mailing list