changeset 6617:0bae66824ac5

tests: clarify that default parameters are for form - direct model access requires different types _get_repo_group_create_params parent_group_id is thus set to the form value '-1' instead of None. It worked before anyway because the model failed to find the repo '-1' and thus got pretty much the same as None.
author Mads Kiilerich <mads@kiilerich.com>
date Mon, 08 May 2017 05:25:41 +0200
parents 6849b3fad164
children cc1b5e0e01e8
files kallithea/lib/celerylib/tasks.py kallithea/model/repo.py kallithea/model/repo_group.py kallithea/tests/fixture.py kallithea/tests/models/test_repo_groups.py
diffstat 5 files changed, 47 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/lib/celerylib/tasks.py	Tue Mar 21 22:44:21 2017 +0100
+++ b/kallithea/lib/celerylib/tasks.py	Mon May 08 05:25:41 2017 +0200
@@ -452,7 +452,6 @@
                       fork_of.repo_name, '')
         DBS.commit()
 
-        update_after_clone = form_data['update_after_clone'] # FIXME - unused!
         source_repo_path = os.path.join(base_path, fork_of.repo_name)
 
         # now create this repo on Filesystem
--- a/kallithea/model/repo.py	Tue Mar 21 22:44:21 2017 +0100
+++ b/kallithea/model/repo.py	Mon May 08 05:25:41 2017 +0200
@@ -297,6 +297,7 @@
                 cur_repo.owner = User.get_by_username(kwargs['owner'])
 
             if 'repo_group' in kwargs:
+                assert kwargs['repo_group'] != u'-1', kwargs # RepoForm should have converted to None
                 cur_repo.group = RepoGroup.get(kwargs['repo_group'])
                 cur_repo.repo_name = cur_repo.get_new_name(cur_repo.just_name)
             log.debug('Updating repo %s with params:%s', cur_repo, kwargs)
--- a/kallithea/model/repo_group.py	Tue Mar 21 22:44:21 2017 +0100
+++ b/kallithea/model/repo_group.py	Mon May 08 05:25:41 2017 +0200
@@ -273,19 +273,24 @@
 
         return updates
 
-    def update(self, repo_group, form_data):
-
+    def update(self, repo_group, kwargs):
         try:
             repo_group = RepoGroup.guess_instance(repo_group)
             old_path = repo_group.full_path
 
             # change properties
-            repo_group.group_description = form_data['group_description']
-            repo_group.parent_group_id = form_data['parent_group_id']
-            repo_group.enable_locking = form_data['enable_locking']
+            if 'group_description' in kwargs:
+                repo_group.group_description = kwargs['group_description']
+            if 'parent_group_id' in kwargs:
+                repo_group.parent_group_id = kwargs['parent_group_id']
+            if 'enable_locking' in kwargs:
+                repo_group.enable_locking = kwargs['enable_locking']
 
-            repo_group.parent_group = RepoGroup.get(form_data['parent_group_id'])
-            repo_group.group_name = repo_group.get_new_name(form_data['group_name'])
+            if 'parent_group_id' in kwargs:
+                assert kwargs['parent_group_id'] != u'-1', kwargs # RepoGroupForm should have converted to None
+                repo_group.parent_group = RepoGroup.get(kwargs['parent_group_id'])
+            if 'group_name' in kwargs:
+                repo_group.group_name = repo_group.get_new_name(kwargs['group_name'])
             new_path = repo_group.full_path
             Session().add(repo_group)
 
--- a/kallithea/tests/fixture.py	Tue Mar 21 22:44:21 2017 +0100
+++ b/kallithea/tests/fixture.py	Mon May 08 05:25:41 2017 +0200
@@ -77,6 +77,7 @@
         return context()
 
     def _get_repo_create_params(self, **custom):
+        """Return form values to be validated through RepoForm"""
         defs = dict(
             repo_name=None,
             repo_type='hg',
@@ -98,11 +99,12 @@
 
         return defs
 
-    def _get_group_create_params(self, **custom):
+    def _get_repo_group_create_params(self, **custom):
+        """Return form values to be validated through RepoGroupForm"""
         defs = dict(
             group_name=None,
             group_description=u'DESC',
-            parent_group_id=None,
+            parent_group_id=u'-1',
             perms_updates=[],
             perms_new=[],
             enable_locking=False,
@@ -139,17 +141,18 @@
 
         return defs
 
-    def create_repo(self, name, **kwargs):
+    def create_repo(self, name, repo_group=None, **kwargs):
         if 'skip_if_exists' in kwargs:
             del kwargs['skip_if_exists']
             r = Repository.get_by_repo_name(name)
             if r:
                 return r
 
-        if isinstance(kwargs.get('repo_group'), RepoGroup):
-            kwargs['repo_group'] = kwargs['repo_group'].group_id
+        if isinstance(repo_group, RepoGroup):
+            repo_group = repo_group.group_id
 
         form_data = self._get_repo_create_params(repo_name=name, **kwargs)
+        form_data['repo_group'] = repo_group # patch form dict so it can be used directly by model
         cur_user = kwargs.get('cur_user', TEST_USER_ADMIN_LOGIN)
         RepoModel().create(form_data, cur_user)
         Session().commit()
@@ -163,9 +166,7 @@
                                             fork_parent_id=repo_to_fork,
                                             repo_type=repo_to_fork.repo_type,
                                             **kwargs)
-        form_data['update_after_clone'] = False
-
-        #TODO: fix it !!
+        # patch form dict so it can be used directly by model
         form_data['description'] = form_data['repo_description']
         form_data['private'] = form_data['repo_private']
         form_data['landing_rev'] = form_data['repo_landing_rev']
@@ -182,18 +183,19 @@
         RepoModel().delete(repo_name, **kwargs)
         Session().commit()
 
-    def create_repo_group(self, name, **kwargs):
+    def create_repo_group(self, name, parent_group_id=None, **kwargs):
         if 'skip_if_exists' in kwargs:
             del kwargs['skip_if_exists']
             gr = RepoGroup.get_by_group_name(group_name=name)
             if gr:
                 return gr
-        form_data = self._get_group_create_params(group_name=name, **kwargs)
-        owner = kwargs.get('cur_user', TEST_USER_ADMIN_LOGIN)
+        form_data = self._get_repo_group_create_params(group_name=name, **kwargs)
         gr = RepoGroupModel().create(
             group_name=form_data['group_name'],
             group_description=form_data['group_name'],
-            owner=owner, parent=form_data['parent_group_id'])
+            parent=parent_group_id,
+            owner=kwargs.get('cur_user', TEST_USER_ADMIN_LOGIN),
+            )
         Session().commit()
         gr = RepoGroup.get_by_group_name(gr.group_name)
         return gr
--- a/kallithea/tests/models/test_repo_groups.py	Tue Mar 21 22:44:21 2017 +0100
+++ b/kallithea/tests/models/test_repo_groups.py	Mon May 08 05:25:41 2017 +0200
@@ -14,23 +14,23 @@
 fixture = Fixture()
 
 
-def _update_group(id_, group_name, desc=u'desc', parent_id=None):
-    form_data = fixture._get_group_create_params(group_name=group_name,
-                                                 group_desc=desc,
-                                                 parent_group_id=parent_id)
-    gr = RepoGroupModel().update(id_, form_data)
-    return gr
+def _update_repo_group(id_, group_name, desc=u'desc', parent_id=None):
+    form_data = dict(
+        group_name=group_name,
+        group_description=desc,
+        parent_group_id=parent_id,
+        )
+    return RepoGroupModel().update(id_, form_data)
 
 
 def _update_repo(name, **kwargs):
-    form_data = fixture._get_repo_create_params(**kwargs)
     if not 'repo_name' in kwargs:
-        form_data['repo_name'] = name
+        kwargs['repo_name'] = name
     if not 'perms_new' in kwargs:
-        form_data['perms_new'] = []
+        kwargs['perms_new'] = []
     if not 'perms_updates' in kwargs:
-        form_data['perms_updates'] = []
-    r = RepoModel().update(name, **form_data)
+        kwargs['perms_updates'] = []
+    r = RepoModel().update(name, **kwargs)
     return r
 
 
@@ -97,7 +97,7 @@
     def test_rename_single_group(self):
         sg1 = fixture.create_repo_group(u'initial')
 
-        new_sg1 = _update_group(sg1.group_id, u'after')
+        new_sg1 = _update_repo_group(sg1.group_id, u'after')
         assert self.__check_path('after')
         assert RepoGroup.get_by_group_name(u'initial') == None
 
@@ -105,15 +105,15 @@
 
         sg1 = fixture.create_repo_group(u'initial', parent_group_id=self.g1.group_id)
 
-        new_sg1 = _update_group(sg1.group_id, u'after', parent_id=self.g1.group_id)
+        new_sg1 = _update_repo_group(sg1.group_id, u'after', parent_id=self.g1.group_id)
         assert self.__check_path('test1', 'after')
         assert RepoGroup.get_by_group_name(u'test1/initial') == None
 
-        new_sg1 = _update_group(sg1.group_id, u'after', parent_id=self.g3.group_id)
+        new_sg1 = _update_repo_group(sg1.group_id, u'after', parent_id=self.g3.group_id)
         assert self.__check_path('test3', 'after')
         assert RepoGroup.get_by_group_name(u'test3/initial') == None
 
-        new_sg1 = _update_group(sg1.group_id, u'hello')
+        new_sg1 = _update_repo_group(sg1.group_id, u'hello')
         assert self.__check_path('hello')
 
         assert RepoGroup.get_by_group_name(u'hello') == new_sg1
@@ -131,7 +131,7 @@
         Session().commit()
         assert r.repo_name == 'g1/john'
 
-        _update_group(g1.group_id, u'g1', parent_id=g2.group_id)
+        _update_repo_group(g1.group_id, u'g1', parent_id=g2.group_id)
         assert self.__check_path('g2', 'g1')
 
         # test repo
@@ -145,7 +145,7 @@
         assert g2.full_path == 't11/t22'
         assert self.__check_path('t11', 't22')
 
-        g2 = _update_group(g2.group_id, u'g22', parent_id=None)
+        g2 = _update_repo_group(g2.group_id, u'g22', parent_id=None)
         Session().commit()
 
         assert g2.group_name == 'g22'
@@ -162,7 +162,7 @@
         r = fixture.create_repo(u'L1/L2/L3/L3_REPO', repo_group=g3.group_id)
 
         ##rename L1 all groups should be now changed
-        _update_group(g1.group_id, u'L1_NEW')
+        _update_repo_group(g1.group_id, u'L1_NEW')
         Session().commit()
         assert g1.full_path == 'L1_NEW'
         assert g2.full_path == 'L1_NEW/L2'
@@ -177,7 +177,7 @@
 
         r = fixture.create_repo(u'R1/R2/R3/R3_REPO', repo_group=g3.group_id)
         ##rename L1 all groups should be now changed
-        _update_group(g1.group_id, u'R1', parent_id=g4.group_id)
+        _update_repo_group(g1.group_id, u'R1', parent_id=g4.group_id)
         Session().commit()
         assert g1.full_path == 'R1_NEW/R1'
         assert g2.full_path == 'R1_NEW/R1/R2'
@@ -193,7 +193,7 @@
         r = fixture.create_repo(u'X1/X2/X3/X3_REPO', repo_group=g3.group_id)
 
         ##rename L1 all groups should be now changed
-        _update_group(g1.group_id, u'X1_PRIM', parent_id=g4.group_id)
+        _update_repo_group(g1.group_id, u'X1_PRIM', parent_id=g4.group_id)
         Session().commit()
         assert g1.full_path == 'X1_NEW/X1_PRIM'
         assert g2.full_path == 'X1_NEW/X1_PRIM/X2'