From 6db17b10ca42ee5702e55182847a1b74b84103eb Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Jul 12 2018 10:05:26 +0000
Subject: [PATCH 1/2] _grplist_unblock leaves unblocked package in place


Previousle calling unblock caused package to be completely removed from
listing and needed to be followed by pkg add call. This behaviour is
counterintuitive and is changed to only remove block.

Fixes: https://pagure.io/koji/issue/871

---

diff --git a/hub/kojihub.py b/hub/kojihub.py
index 22dd484..09a1354 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -1692,22 +1692,22 @@ def grplist_unblock(taginfo, grpinfo):
 
 
 def _grplist_unblock(taginfo, grpinfo):
-    """grplist_unblock without permssion check"""
+    """grplist_unblock without permission check"""
     tag = lookup_tag(taginfo, strict=True)
     group = lookup_group(grpinfo, strict=True)
     tag_id = tag['id']
     grp_id = group['id']
     table = 'group_config'
-    clauses = ('group_id=%(grp_id)s', 'tag_id=%(tag_id)s')
+    clauses = ('group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'active=TRUE')
     query = QueryProcessor(columns=['blocked'], tables=[table],
-                           clauses=('active = TRUE',)+clauses,
+                           clauses=clauses,
                            values=locals(), opts={'rowlock':True})
     blocked = query.singleValue(strict=False)
     if not blocked:
         raise koji.GenericError("group %s is NOT blocked in tag %s" % (group['name'], tag['name']))
-    update = UpdateProcessor(table, values=locals(), clauses=clauses)
-    update.make_revoke()
-    update.execute()
+
+    # just set block to false, leave other data untouched
+    _grplist_add(taginfo, grpinfo, block=False, force=True)
 
 
 # tag-group-pkg operations

From b661bf7947eb232d4405e4839c119b7c6b90fa12 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Jul 12 2018 10:14:20 +0000
Subject: [PATCH 2/2] fixed tests for grplist_unblock


---

diff --git a/tests/test_hub/test_group_operations.py b/tests/test_hub/test_group_operations.py
index aee0095..899d5db 100644
--- a/tests/test_hub/test_group_operations.py
+++ b/tests/test_hub/test_group_operations.py
@@ -15,6 +15,9 @@ class TestGrouplist(unittest.TestCase):
     def getQuery(self, *args, **kwargs):
         query = QP(*args, **kwargs)
         query.execute = mock.MagicMock()
+        if self.qp_rv:
+            query.singleValue = mock.MagicMock()
+            query.singleValue.return_value = self.qp_rv
         self.queries.append(query)
         return query
 
@@ -44,6 +47,7 @@ class TestGrouplist(unittest.TestCase):
         self.QueryProcessor = mock.patch('kojihub.QueryProcessor',
                 side_effect=self.getQuery).start()
         self.queries = []
+        self.qp_rv = None
         self.InsertProcessor = mock.patch('kojihub.InsertProcessor',
                 side_effect=self.getInsert).start()
         self.inserts = []
@@ -178,7 +182,8 @@ class TestGrouplist(unittest.TestCase):
         self.assertEqual(update.data, {'revoke_event': 42, 'revoker_id': 24})
         self.assertEqual(update.rawdata, {'active': 'NULL'})
 
-    def test_grplist_unblock(self):
+    @mock.patch('kojihub._grp_pkg_add')
+    def test_grplist_unblock_unblocked(self, _grp_pkg_add):
         # identical with test_grplist_add except blocked=True
         tag = 'tag'
         group = 'group'
@@ -199,11 +204,41 @@ class TestGrouplist(unittest.TestCase):
         # db
         self.assertEqual(len(self.queries), 1)
         query = self.queries[0]
-        q = "SELECT blocked FROM group_config WHERE (active = TRUE) AND (group_id=%(grp_id)s) AND (tag_id=%(tag_id)s) FOR UPDATE"
+        q = "SELECT blocked FROM group_config WHERE (group_id=%(grp_id)s) AND (tag_id=%(tag_id)s) AND (active=TRUE) FOR UPDATE"
         actual = ' '.join(str(query).split())
         self.assertEqual(actual, q)
         self.assertEqual(len(self.updates), 0)
         self.assertEqual(len(self.inserts), 0)
+        _grp_pkg_add.assert_not_called()
+
+    @mock.patch('kojihub._grplist_add')
+    def test_grplist_unblock(self, _grplist_add):
+        # identical with test_grplist_add except blocked=True
+        tag = 'tag'
+        group = 'group'
+        self.lookup_tag.return_value = {'name': 'tag', 'id': 'tag_id'}
+        self.lookup_group.return_value = {'name': 'group', 'id': 'group_id'}
+        #self.context.event_id = 42
+        #self.context.session.user_id = 24
+        self.qp_rv = True
+
+        # will fail for non-blocked group
+        kojihub.grplist_unblock(tag, group)
+
+        # what was called
+        self.context.session.assertPerm.assert_called_once_with('admin')
+        self.lookup_tag.assert_called_once_with(tag, strict=True)
+        self.lookup_group.assert_called_once_with(group, strict=True)
+
+        # db
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        q = "SELECT blocked FROM group_config WHERE (group_id=%(grp_id)s) AND (tag_id=%(tag_id)s) AND (active=TRUE) FOR UPDATE"
+        actual = ' '.join(str(query).split())
+        self.assertEqual(actual, q)
+        self.assertEqual(len(self.updates), 0)
+        self.assertEqual(len(self.inserts), 0)
+        _grplist_add.assert_called_once_with(tag, group, block=False, force=True)
 
     def test_readTagGroups_empty(self):
         self.get_tag_groups.return_value = {}