Skip to content

Commit

Permalink
Services: Communities: Add feature to require community based on config
Browse files Browse the repository at this point in the history
  • Loading branch information
sakshamarora1 committed Sep 16, 2024
1 parent 0c834f3 commit 385c788
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 4 deletions.
6 changes: 6 additions & 0 deletions invenio_rdm_records/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ def always_valid(identifier):
RDM_ALLOW_RESTRICTED_RECORDS = True
"""Allow users to set restricted/private records."""

#
# Record communities
#
RDM_RECORD_ALWAYS_IN_COMMUNITY = True
"""Enforces at least one community per record on remove community function."""

#
# Search configuration
#
Expand Down
1 change: 1 addition & 0 deletions invenio_rdm_records/resources/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ class RDMRecordResourceConfig(RecordResourceConfig, ConfiguratorMixin):
)

error_handlers = {
**ErrorHandlersMixin.error_handlers,
DeserializerError: create_error_handler(
lambda exc: HTTPJSONException(
code=400,
Expand Down
16 changes: 14 additions & 2 deletions invenio_rdm_records/services/communities/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from invenio_communities.proxies import current_communities
from invenio_drafts_resources.services.records.uow import (
ParentRecordCommitOp,
RecordCommitOp,
)
from invenio_i18n import lazy_gettext as _
from invenio_notifications.services.uow import NotificationOp
Expand All @@ -23,7 +22,10 @@
Service,
ServiceSchemaWrapper,
)
from invenio_records_resources.services.errors import PermissionDeniedError
from invenio_records_resources.services.errors import (
PermissionDeniedError,
CannotRemoveCommunityError,
)
from invenio_records_resources.services.uow import (
IndexRefreshOp,
RecordIndexOp,
Expand Down Expand Up @@ -205,6 +207,16 @@ def _remove(self, identity, community_id, record):
if community_id not in record.parent.communities.ids:
raise RecordCommunityMissing(record.id, community_id)

# If config is True and there is only 1 communities left to remove
# Then, check for permissions to remove last community
if (
current_app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"]
and len(record.parent.communities.ids) == 1
):
if not self.check_permission(
identity, "remove_community", record=record, community_id=community_id
):
raise CannotRemoveCommunityError()
# check permission here, per community: curator cannot remove another community
self.require_permission(
identity, "remove_community", record=record, community_id=community_id
Expand Down
11 changes: 11 additions & 0 deletions invenio_rdm_records/services/generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,3 +414,14 @@ def needs(self, request=None, **kwargs):
return [AccessRequestTokenNeed(request["payload"]["token"])]

return []


class IfOneCommunity(ConditionalGenerator):
"""Conditional generator for records always in communities case."""

def _condition(self, record=None, **kwargs):
"""Check if the record is associated with zero or one community."""
if record is None:
return True
rec_communities = record.parent.communities.ids
return len(rec_communities) == 1
27 changes: 25 additions & 2 deletions invenio_rdm_records/services/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
AccessGrant,
CommunityInclusionReviewers,
GuestAccessRequestToken,
IfOneCommunity,
IfCreate,
IfDeleted,
IfExternalDOIRecord,
Expand Down Expand Up @@ -199,7 +200,17 @@ class RDMRecordPermissionPolicy(RecordPermissionPolicy):
),
]
# Allow publishing a new record or changes to an existing record.
can_publish = can_review
can_publish = [
IfConfig(
"RDM_RECORD_ALWAYS_IN_COMMUNITY",
then_=[
IfOneCommunity(
then_=can_review, else_=[Administration(), SystemProcess()]
)
],
else_=can_review,
)
]
# Allow lifting a record or draft.
can_lift_embargo = can_manage

Expand All @@ -209,11 +220,23 @@ class RDMRecordPermissionPolicy(RecordPermissionPolicy):
# Who can add record to a community
can_add_community = can_manage
# Who can remove a community from a record
can_remove_community = [
can_remove_community_ = [
RecordOwners(),
CommunityCurators(),
SystemProcess(),
]
can_remove_community = [
IfConfig(
"RDM_RECORD_ALWAYS_IN_COMMUNITY",
then_=[
IfOneCommunity(
then_=[Administration(), SystemProcess()],
else_=can_remove_community_,
)
],
else_=can_remove_community_,
)
]
# Who can remove records from a community
can_remove_record = [CommunityCurators()]
# Who can add records to a community in bulk
Expand Down
177 changes: 177 additions & 0 deletions tests/resources/test_resources_communities.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

"""Tests record's communities resources."""

from contextlib import contextmanager
from copy import deepcopy

import pytest
Expand Down Expand Up @@ -889,3 +890,179 @@ def test_add_record_to_restricted_community_submission_open_member(
assert not response.json.get("errors")
processed = response.json["processed"]
assert len(processed) == 1

# Assure Records community exists tests
# -------------------------------------
from invenio_records_resources.services.errors import CommunityNotSelectedError, CannotRemoveCommunityError


@contextmanager
def ensure_record_community_exists_config(app):
"""
Context manager to ensure record community exists config
is set to True during a specific code block.
Parameters:
app: app fixture
Usage:
with ensure_record_community_exists_config(app):
# code block that requires the flag to be set
"""
try:
app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"] = True
yield
finally:
app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"] = False

def test_restricted_record_creation(
app, record_community, uploader, curator, community_owner, test_user, superuser
):
"""Verify CommunityNotSelectedError is raised when direct publish a record"""
# You can directly publish a record when the config is disabled
rec = record_community.create_record()
assert rec.id
with ensure_record_community_exists_config(app):
# You can't directly publish
users = [
curator,
test_user,
uploader,
community_owner,
]
for user in users:
with pytest.raises(CommunityNotSelectedError):
record_community.create_record(uploader=user)

# Super user can!
super_user_rec = record_community.create_record(uploader=superuser)
assert super_user_rec.id


def test_remove_last_existing_non_existing_community(
app, client, uploader, record_community, headers, community
):
"""Test removal of an existing and non-existing community from the record,
while ensuring at least one community exists."""
data = {
"communities": [
{"id": "wrong-id"},
{"id": community.id},
{"id": "wrong-id2"},
]
}

client = uploader.login(client)
record = record_community.create_record()
with ensure_record_community_exists_config(app):
with pytest.raises(CannotRemoveCommunityError):
response = client.delete(
f"/records/{record.pid.pid_value}/communities",
headers=headers,
json=data,
)
assert response.status_code == 400
# Should get 3 errors: Can't remove community, 2 bad IDs
assert len(response.json["errors"]) == 3
record_saved = client.get(f"/records/{record.pid.pid_value}", headers=headers)
assert record_saved.json["parent"]["communities"]


def test_remove_last_community_api_error_handling(
record_community,
community,
uploader,
headers,
curator,
client,
app,
):
"""Testing error message when trying to remove last community."""
record = record_community.create_record()
data = {"communities": [{"id": community.id}]}
for user in [uploader, curator]:
client = user.login(client)
response = client.get(
f"/communities/{community.id}/records",
headers=headers,
json=data,
)
assert (
len(response.json["hits"]["hits"][0]["parent"]["communities"]["ids"]) == 1
)
with ensure_record_community_exists_config(app):
response = client.delete(
f"/records/{record.pid.pid_value}/communities",
headers=headers,
json=data,
)
assert response.is_json
assert response.status_code == 400

record_saved = client.get(
f"/records/{record.pid.pid_value}", headers=headers
)
assert record_saved.json["parent"]["communities"]

client = user.logout(client)
# check communities number
response = client.get(
f"/communities/{community.id}/records",
headers=headers,
json=data,
)
assert (
len(response.json["hits"]["hits"][0]["parent"]["communities"]["ids"])
== 1
)


def test_remove_record_last_community_with_multiple_communities(
closed_review_community,
open_review_community,
record_community,
community2,
uploader,
headers,
client,
app,
db,
):
"""Testing correct removal of multiple communities"""
client = uploader.login(client)

record = record_community.create_record()
comm = [
community2,
open_review_community,
closed_review_community,
] # one more in the rec fixture so it's 4
for com in comm:
_add_to_community(db, record, com)
assert len(record.parent.communities.ids) == 4

with ensure_record_community_exists_config(app):
data = {"communities": [{"id": x} for x in record.parent.communities.ids]}

response = client.delete(
f"/records/{record.pid.pid_value}/communities",
headers=headers,
json=data,
)
# You get res 200 with error msg if all communities you are deleting
assert response.status_code == 200
assert "error" in str(response.data)

rec_com_left = client.get(f"/records/{record.pid.pid_value}", headers=headers)
assert len(rec_com_left.json["parent"]["communities"]["ids"]) == 1

# You get res 400 with error msg if you Delete the last one only.
response = client.delete(
f"/records/{record.pid.pid_value}/communities",
headers=headers,
json={"communities": [{"id": str(record.parent.communities.ids[0])}]},
)
assert response.status_code == 400
assert "error" in str(response.data)

record_saved = client.get(f"/records/{record.pid.pid_value}", headers=headers)
# check that only one community ID is associated with the record
assert len(record_saved.json["parent"]["communities"]["ids"]) == 1

0 comments on commit 385c788

Please sign in to comment.