Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Services: Communities: Add feature to require community based on config #1813

Merged
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 (
sakshamarora1 marked this conversation as resolved.
Show resolved Hide resolved
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."""
sakshamarora1 marked this conversation as resolved.
Show resolved Hide resolved
if record is None:
return True
rec_communities = record.parent.communities.ids
return len(rec_communities) == 1
kpsherva marked this conversation as resolved.
Show resolved Hide resolved
sakshamarora1 marked this conversation as resolved.
Show resolved Hide resolved
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 = [
sakshamarora1 marked this conversation as resolved.
Show resolved Hide resolved
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_,
ntarocco marked this conversation as resolved.
Show resolved Hide resolved
)
],
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
sakshamarora1 marked this conversation as resolved.
Show resolved Hide resolved


@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
Loading