From 065d39786e854a9199c35d06511a405d09f9c377 Mon Sep 17 00:00:00 2001 From: Mirek Simek Date: Fri, 29 Nov 2024 14:41:51 +0100 Subject: [PATCH 1/2] fix: community roles generate incorrect query_filter --- .../services/permissions/generators.py | 26 +++++++++++++++ setup.cfg | 2 +- tests/test_communities/conftest.py | 19 +++++++++++ .../test_communities/permissions/conftest.py | 31 ++++++++++++++++++ .../permissions/test_community_members.py | 32 +++++++++++++++++++ .../permissions/test_community_role.py | 26 +++++++++++++++ .../test_default_community_members.py | 28 ++++++++++++++++ .../test_default_community_role.py | 25 +++++++++++++++ 8 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 tests/test_communities/permissions/conftest.py create mode 100644 tests/test_communities/permissions/test_community_members.py create mode 100644 tests/test_communities/permissions/test_community_role.py create mode 100644 tests/test_communities/permissions/test_default_community_members.py create mode 100644 tests/test_communities/permissions/test_default_community_role.py diff --git a/oarepo_communities/services/permissions/generators.py b/oarepo_communities/services/permissions/generators.py index 16be710..b3cd455 100644 --- a/oarepo_communities/services/permissions/generators.py +++ b/oarepo_communities/services/permissions/generators.py @@ -154,6 +154,21 @@ def needs(self, record=None, data=None, **kwargs): _needs.add(CommunityRoleNeed(c, role)) return _needs + @abc.abstractmethod + def query_filter_field(self): + """Field for query filter. + + returns parent.communities.ids or parent.communities.default + """ + raise NotImplemented() + + def query_filter(self, identity=None, **kwargs): + """Filter for current identity.""" + community_ids = self.communities(identity) + if not community_ids: + return dsl.Q("match_none") + return dsl.Q("terms", **{self.query_filter_field(): community_ids}) + class CommunityRole(CommunityRoleMixin, OARepoCommunityRoles): @@ -164,6 +179,9 @@ def __init__(self, role): def roles(self, **kwargs): return [self._role] + def query_filter_field(self): + return "parent.communities.ids" + class DefaultCommunityRole( DefaultCommunityRoleMixin, RecipientGeneratorMixin, OARepoCommunityRoles @@ -180,6 +198,9 @@ def reference_receivers(self, **kwargs): community_id = self._get_record_communities(**kwargs)[0] return [{"community_role": f"{community_id}:{self._role}"}] + def query_filter_field(self): + return "parent.communities.default" + PrimaryCommunityRole = DefaultCommunityRole @@ -206,6 +227,8 @@ def roles(self, **kwargs): """Roles.""" return [r.name for r in current_roles] + def query_filter_field(self): + return "parent.communities.ids" class DefaultCommunityMembers(DefaultCommunityRoleMixin, OARepoCommunityRoles): @@ -213,6 +236,9 @@ def roles(self, **kwargs): """Roles.""" return [r.name for r in current_roles] + def query_filter_field(self): + return "parent.communities.default" + PrimaryCommunityMembers = DefaultCommunityMembers diff --git a/setup.cfg b/setup.cfg index 1baa07b..6179a86 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = oarepo-communities -version = 5.1.0 +version = 5.1.1 description = authors = Ronald Krist readme = README.md diff --git a/tests/test_communities/conftest.py b/tests/test_communities/conftest.py index 9663198..df57991 100644 --- a/tests/test_communities/conftest.py +++ b/tests/test_communities/conftest.py @@ -487,6 +487,25 @@ def community(app, minimal_community, community_owner): ) + +@pytest.fixture() +def communities(app, minimal_community, community_owner): + return { + "aaa": _community_get_or_create( + community_owner.identity, { + **minimal_community, + 'slug': "aaa", + }, workflow="default" + ), + "bbb": _community_get_or_create( + community_owner.identity, { + **minimal_community, + 'slug': "bbb", + }, workflow="default" + ), + } + + @pytest.fixture(autouse=True) def init_cf(app, db, cache): from oarepo_runtime.services.custom_fields.mappings import prepare_cf_indices diff --git a/tests/test_communities/permissions/conftest.py b/tests/test_communities/permissions/conftest.py new file mode 100644 index 0000000..271d33d --- /dev/null +++ b/tests/test_communities/permissions/conftest.py @@ -0,0 +1,31 @@ +import json + +import pytest + + +@pytest.fixture +def sample_record_with_community_data(communities): + community_aaa = str(communities['aaa'].id) + community_bbb = str(communities['bbb'].id) + return { + 'parent': { + 'communities': { + 'ids': { + community_aaa, community_bbb + }, + 'default': community_aaa + } + } + } + + +@pytest.fixture +def as_comparable_dict(): + def _as_comparable(d): + if isinstance(d, dict): + return {k: _as_comparable(v) for k, v in sorted(d.items())} + if isinstance(d, (list, tuple)): + return set(_as_comparable(v) for v in d) + return d + + return _as_comparable \ No newline at end of file diff --git a/tests/test_communities/permissions/test_community_members.py b/tests/test_communities/permissions/test_community_members.py new file mode 100644 index 0000000..45d365f --- /dev/null +++ b/tests/test_communities/permissions/test_community_members.py @@ -0,0 +1,32 @@ +from invenio_communities.generators import CommunityRoleNeed + +from oarepo_communities.services.permissions.generators import CommunityMembers + + +def test_community_members_needs(app, db, sample_record_with_community_data, communities): + members = CommunityMembers() + assert set(members.needs(data=sample_record_with_community_data)) == { + CommunityRoleNeed(str(communities['aaa'].id), 'owner'), + CommunityRoleNeed(str(communities['bbb'].id), 'owner'), + CommunityRoleNeed(str(communities['aaa'].id), 'manager'), + CommunityRoleNeed(str(communities['bbb'].id), 'manager'), + CommunityRoleNeed(str(communities['aaa'].id), 'curator'), + CommunityRoleNeed(str(communities['bbb'].id), 'curator'), + CommunityRoleNeed(str(communities['aaa'].id), 'reader'), + CommunityRoleNeed(str(communities['bbb'].id), 'reader'), + } + +def test_community_members_excludes(app, db, sample_record_with_community_data, communities): + members = CommunityMembers() + assert not set(members.excludes(data=sample_record_with_community_data)) + +def test_community_members_query_filter(app, db, sample_record_with_community_data, communities, community_owner, as_comparable_dict): + members = CommunityMembers() + assert as_comparable_dict(members.query_filter(identity=community_owner.identity).to_dict()) == as_comparable_dict({ + 'terms': { + 'parent.communities.ids': [ + str(communities['aaa'].id), + str(communities['bbb'].id) + ] + } + }) \ No newline at end of file diff --git a/tests/test_communities/permissions/test_community_role.py b/tests/test_communities/permissions/test_community_role.py new file mode 100644 index 0000000..c4327bc --- /dev/null +++ b/tests/test_communities/permissions/test_community_role.py @@ -0,0 +1,26 @@ +from invenio_communities.generators import CommunityRoleNeed + +from oarepo_communities.services.permissions.generators import CommunityRole + + +def test_community_role_needs(app, db, sample_record_with_community_data, communities): + role = CommunityRole("owner") + assert set(role.needs(data=sample_record_with_community_data)) == { + CommunityRoleNeed(str(communities['aaa'].id), 'owner'), + CommunityRoleNeed(str(communities['bbb'].id), 'owner'), + } + +def test_community_role_excludes(app, db, sample_record_with_community_data, communities): + role = CommunityRole("owner") + assert not set(role.excludes(data=sample_record_with_community_data)) + +def test_community_role_query_filter(app, db, sample_record_with_community_data, communities, community_owner, as_comparable_dict): + role = CommunityRole("owner") + assert as_comparable_dict(role.query_filter(identity=community_owner.identity).to_dict()) == as_comparable_dict({ + 'terms': { + 'parent.communities.ids': [ + str(communities['aaa'].id), + str(communities['bbb'].id) + ] + } + }) \ No newline at end of file diff --git a/tests/test_communities/permissions/test_default_community_members.py b/tests/test_communities/permissions/test_default_community_members.py new file mode 100644 index 0000000..754a9e6 --- /dev/null +++ b/tests/test_communities/permissions/test_default_community_members.py @@ -0,0 +1,28 @@ +from invenio_communities.generators import CommunityRoleNeed + +from oarepo_communities.services.permissions.generators import DefaultCommunityMembers + + +def test_community_members_needs(app, db, sample_record_with_community_data, communities): + members = DefaultCommunityMembers() + assert set(members.needs(data=sample_record_with_community_data)) == { + CommunityRoleNeed(str(communities['aaa'].id), 'owner'), + CommunityRoleNeed(str(communities['aaa'].id), 'manager'), + CommunityRoleNeed(str(communities['aaa'].id), 'curator'), + CommunityRoleNeed(str(communities['aaa'].id), 'reader'), + } + +def test_community_members_excludes(app, db, sample_record_with_community_data, communities): + members = DefaultCommunityMembers() + assert not set(members.excludes(data=sample_record_with_community_data)) + +def test_community_members_query_filter(app, db, sample_record_with_community_data, communities, community_owner, as_comparable_dict): + members = DefaultCommunityMembers() + assert as_comparable_dict(members.query_filter(identity=community_owner.identity).to_dict()) == as_comparable_dict({ + 'terms': { + 'parent.communities.default': [ + str(communities['aaa'].id), + str(communities['bbb'].id) + ] + } + }) \ No newline at end of file diff --git a/tests/test_communities/permissions/test_default_community_role.py b/tests/test_communities/permissions/test_default_community_role.py new file mode 100644 index 0000000..e700c44 --- /dev/null +++ b/tests/test_communities/permissions/test_default_community_role.py @@ -0,0 +1,25 @@ +from invenio_communities.generators import CommunityRoleNeed + +from oarepo_communities.services.permissions.generators import CommunityRole, DefaultCommunityRole + + +def test_default_community_role_needs(app, db, sample_record_with_community_data, communities): + role = DefaultCommunityRole("owner") + assert set(role.needs(data=sample_record_with_community_data)) == { + CommunityRoleNeed(str(communities['aaa'].id), 'owner'), + } + +def test_default_community_role_excludes(app, db, sample_record_with_community_data, communities): + role = DefaultCommunityRole("owner") + assert not set(role.excludes(data=sample_record_with_community_data)) + +def test_default_community_role_query_filter(app, db, sample_record_with_community_data, communities, community_owner, as_comparable_dict): + role = DefaultCommunityRole("owner") + assert as_comparable_dict(role.query_filter(identity=community_owner.identity).to_dict()) == as_comparable_dict({ + 'terms': { + 'parent.communities.default': [ + str(communities['aaa'].id), + str(communities['bbb'].id) + ] + } + }) \ No newline at end of file From 6b4b2960b9d918d1a8c029eea75e512deeef712c Mon Sep 17 00:00:00 2001 From: Mirek Simek Date: Fri, 29 Nov 2024 14:44:08 +0100 Subject: [PATCH 2/2] Better exception --- oarepo_communities/services/permissions/generators.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/oarepo_communities/services/permissions/generators.py b/oarepo_communities/services/permissions/generators.py index b3cd455..4220653 100644 --- a/oarepo_communities/services/permissions/generators.py +++ b/oarepo_communities/services/permissions/generators.py @@ -131,15 +131,15 @@ def communities(self, identity): @abc.abstractmethod def _get_record_communities(self, record=None, **kwargs): - raise NotImplemented() + raise NotImplementedError() @abc.abstractmethod def _get_data_communities(self, data=None, **kwargs): - raise NotImplemented() + raise NotImplementedError() @abc.abstractmethod def roles(self, **kwargs): - raise NotImplemented() + raise NotImplementedError() def needs(self, record=None, data=None, **kwargs): """Set of Needs granting permission.""" @@ -160,7 +160,7 @@ def query_filter_field(self): returns parent.communities.ids or parent.communities.default """ - raise NotImplemented() + raise NotImplementedError() def query_filter(self, identity=None, **kwargs): """Filter for current identity."""