[add-package] Fix group handling

koschei/frontend/views.py

@@ -763,22 +763,22 @@ def add_packages():
             except IndexError:
                 abort(404)
 
+            try:
+                added = data.track_packages(session, collection, names)
+            except data.PackagesDontExist as e:
+                db.rollback()
+                flash_nak(str(e))
+                return render_template("add-packages.html", form=form)
+
             if form.group.data:
-                name, namespace = PackageGroup.parse_name(form.group.data)
+                namespace, name = PackageGroup.parse_name(form.group.data)
                 group = db.query(PackageGroup)\
                           .filter_by(namespace=namespace, name=name)\
                           .first_or_404()
                 if not group.editable:
                     abort(400)
                 data.set_group_content(session, group, names, append=True)
 
-            try:
-                added = data.track_packages(session, collection, names)
-            except data.PackagesDontExist as e:
-                db.rollback()
-                flash_nak(str(e))
-                return render_template("add-packages.html", form=form)
-
             flash_ack("Packages added: {}".format(','.join(p.name for p in added)))
             db.commit()
             return redirect(request.form.get('next') or url_for('frontpage'))

test/frontend_test.py

@@ -43,8 +43,8 @@ def authenticate(fn):
     @wraps(fn)
     def decorated(*args, **kwargs):
         self = args[0]
-        user = self.prepare_user(name='jdoe', admin=False)
-        login(self.client, user)
+        self.user = self.prepare_user(name='jdoe', admin=False)
+        login(self.client, self.user)
         return fn(*args, **kwargs)
     return decorated
 
@@ -53,13 +53,17 @@ def authenticate_admin(fn):
     @wraps(fn)
     def decorated(*args, **kwargs):
         self = args[0]
-        user = self.prepare_user(name='admin', admin=True)
-        login(self.client, user)
+        self.user = self.prepare_user(name='admin', admin=True)
+        login(self.client, self.user)
         return fn(*args, **kwargs)
     return decorated
 
 
 class FrontendTest(DBTest):
+    def __init__(self, *args, **kwargs):
+        super(FrontendTest, self).__init__(*args, **kwargs)
+        self.user = None
+
     def get_session(self):
         return db
 
@@ -162,9 +166,18 @@ def test_cancel_build_pending(self):
 
     @authenticate
     def test_add_package_nonexistent(self):
+        group = self.prepare_group(
+            name='foo',
+            content=['bar'],
+            owners=[self.user.name],
+        )
         reply = self.client.post(
             'add-packages',
-            data=dict(packages='SimplyHTML', collection=self.collection.name),
+            data=dict(
+                packages='SimplyHTML',
+                collection=self.collection.name,
+                group=group.full_name,
+            ),
             follow_redirects=True,
         )
         self.assertEqual(200, reply.status_code)
@@ -184,6 +197,61 @@ def test_add_package(self):
         self.assertIn('Packages added: xpp3', reply.data.decode('utf-8'))
         self.assertTrue(pkg.tracked)
 
+    def _test_add_package_group(self, namespace):
+        pkg = self.prepare_packages('xpp3')[0]
+        group = self.prepare_group(
+            name='foo',
+            namespace=namespace,
+            content=['bar'],
+            owners=[self.user.name],
+        )
+        pkg.tracked = False
+        self.db.commit()
+        reply = self.client.post(
+            'add-packages',
+            data=dict(
+                packages='xpp3',
+                collection=self.collection.name,
+                group=group.full_name,
+            ),
+            follow_redirects=True,
+        )
+        self.assertEqual(200, reply.status_code)
+        self.assertIn('Packages added: xpp3', reply.data.decode('utf-8'))
+        self.assertTrue(pkg.tracked)
+        self.assertEqual([group], pkg.groups)
+
+    @authenticate
+    def test_add_package_user_group(self):
+        self._test_add_package_group(None)
+
+    @authenticate
+    def test_add_package_global_group(self):
+        self._test_add_package_group('user')
+
+    @authenticate
+    def test_add_package_grup_no_permission(self):
+        pkg = self.prepare_packages('xpp3')[0]
+        group = self.prepare_group(
+            name='foo',
+            content=['bar'],
+            owners=['someone_else'],
+        )
+        pkg.tracked = False
+        self.db.commit()
+        reply = self.client.post(
+            'add-packages',
+            data=dict(
+                packages='xpp3',
+                collection=self.collection.name,
+                group=group.full_name,
+            ),
+            follow_redirects=True,
+        )
+        self.assertEqual(400, reply.status_code)
+        self.assertFalse(pkg.tracked)
+        self.assertEqual(0, len(pkg.groups))
+
     def test_create_group_unauth(self):
         reply = self.client.post(
             'add-group',

test/model_test.py

@@ -20,11 +20,23 @@
 from sqlalchemy import literal_column
 from datetime import datetime, timedelta
 
-from koschei.models import Package, Collection, Build, ResourceConsumptionStats, ScalarStats, KojiTask
+from koschei.models import (
+    Package, Collection, Build, ResourceConsumptionStats, ScalarStats, KojiTask,
+    PackageGroup,
+)
 from test.common import DBTest
 
 
 class GroupTest(DBTest):
+    def test_group_name_format(self):
+        group1 = self.prepare_group('foo', content=['foo'])
+        group2 = self.prepare_group('bar', namespace='ns', content=['foo'])
+        self.assertEqual('foo', group1.full_name)
+        self.assertEqual('ns/bar', group2.full_name)
+
+    def test_group_name_parse(self):
+        self.assertEqual((None, 'foo'), PackageGroup.parse_name('foo'))
+        self.assertEqual(('ns', 'bar'), PackageGroup.parse_name('ns/bar'))
 
     def test_group_cardinality(self):
         group = self.prepare_group('xyzzy', content=['foo', 'bar', 'baz'])