PR#872: _grplist_unblock leaves unblocked package in place - koji

koji

#872 _grplist_unblock leaves unblocked package in place

Closed 5 years ago by mikem. Opened 6 years ago by tkopecek.

tkopecek/koji issue871 into master

fixed tests for grplist_unblock

Tomas Kopecek • 5 years ago

b661bf7

_grplist_unblock leaves unblocked package in place

Tomas Kopecek • 5 years ago

6db17b1

hub/kojihub.py

file modified

+6 -6

		`@@ -1692,22 +1692,22 @@`


		`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`

tests/test_hub/test_group_operations.py

file modified

+37 -2

		`@@ -15,6 +15,9 @@`
		`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 @@`
		`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 @@`
		`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 @@`
		`# 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 = {}`

tkopecek commented 6 years ago

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

1 new commit added