Skip to content

Commit

Permalink
Merge pull request #89 from oarepo/miroslavsimek/be-570-incorrectly-g…
Browse files Browse the repository at this point in the history
…enerated-query_filter-for

fix: community roles generate incorrect query_filter
  • Loading branch information
mesemus authored Nov 29, 2024
2 parents 97bac46 + 6b4b296 commit 243e924
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 4 deletions.
32 changes: 29 additions & 3 deletions oarepo_communities/services/permissions/generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand All @@ -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 NotImplementedError()

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):

Expand All @@ -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
Expand All @@ -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

Expand All @@ -206,13 +227,18 @@ 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):

def roles(self, **kwargs):
"""Roles."""
return [r.name for r in current_roles]

def query_filter_field(self):
return "parent.communities.default"


PrimaryCommunityMembers = DefaultCommunityMembers

Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[metadata]
name = oarepo-communities
version = 5.1.0
version = 5.1.1
description =
authors = Ronald Krist <[email protected]>
readme = README.md
Expand Down
19 changes: 19 additions & 0 deletions tests/test_communities/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions tests/test_communities/permissions/conftest.py
Original file line number Diff line number Diff line change
@@ -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
32 changes: 32 additions & 0 deletions tests/test_communities/permissions/test_community_members.py
Original file line number Diff line number Diff line change
@@ -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)
]
}
})
26 changes: 26 additions & 0 deletions tests/test_communities/permissions/test_community_role.py
Original file line number Diff line number Diff line change
@@ -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)
]
}
})
Original file line number Diff line number Diff line change
@@ -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)
]
}
})
25 changes: 25 additions & 0 deletions tests/test_communities/permissions/test_default_community_role.py
Original file line number Diff line number Diff line change
@@ -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)
]
}
})

0 comments on commit 243e924

Please sign in to comment.