Skip to content

Commit

Permalink
service: prevent creating a request if invalid restrictions
Browse files Browse the repository at this point in the history
* Community selection modal checks if the community is
  restricted and disables it if the record is public
* closes inveniosoftware/invenio-app-rdm#2384
  • Loading branch information
jrcastro2 committed Sep 12, 2023
1 parent b9cfe75 commit 5e0ff7f
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class CommunityHeaderComponent extends Component {
showCommunitySelectionButton,
disableCommunitySelectionButton,
showCommunityHeader,
record,
} = this.props;
const { modalOpen } = this.state;

Expand Down Expand Up @@ -73,6 +74,7 @@ class CommunityHeaderComponent extends Component {
modalOpen={modalOpen}
chosenCommunity={community}
displaySelected
record={record}
trigger={
<Button
className="community-header-button"
Expand Down Expand Up @@ -119,6 +121,7 @@ CommunityHeaderComponent.propTypes = {
showCommunitySelectionButton: PropTypes.bool.isRequired,
showCommunityHeader: PropTypes.bool.isRequired,
changeSelectedCommunity: PropTypes.func.isRequired,
record: PropTypes.object.isRequired,
};

CommunityHeaderComponent.defaultProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import React, { useContext } from "react";
import { Button, Icon, Label } from "semantic-ui-react";
import { CommunityCompactItem } from "@js/invenio_communities/community";
import { CommunityContext } from "./CommunityContext";
import { InvenioPopup } from "react-invenio-forms";

export const CommunityListItem = ({ result }) => {
export const CommunityListItem = ({ result, record }) => {
const {
setLocalCommunity,
getChosenCommunity,
Expand All @@ -23,17 +24,35 @@ export const CommunityListItem = ({ result }) => {
const { metadata } = result;
const itemSelected = getChosenCommunity()?.id === result.id;
const userMembership = userCommunitiesMemberships[result["id"]];
const invalidPermissionLevel =
record.access.record === "public" && result.access.visibility === "restricted";

const actions = (
<Button
content={
displaySelected && itemSelected ? i18next.t("Selected") : i18next.t("Select")
}
size="tiny"
positive={displaySelected && itemSelected}
onClick={() => setLocalCommunity(result)}
aria-label={i18next.t("Select {{title}}", { title: metadata.title })}
/>
<>
{invalidPermissionLevel && (
<InvenioPopup
popupId="community-inclusion-info-popup"
size="small"
trigger={
<Icon className="mb-5" color="grey" name="question circle outline" />
}
ariaLabel={i18next.t("Community inclusion information")}
content={i18next.t(
"Submission to this community is only allowed if the record is restricted."
)}
/>
)}
<Button
content={
displaySelected && itemSelected ? i18next.t("Selected") : i18next.t("Select")
}
size="tiny"
positive={displaySelected && itemSelected}
onClick={() => setLocalCommunity(result)}
disabled={invalidPermissionLevel}
aria-label={i18next.t("Select {{title}}", { title: metadata.title })}
/>
</>
);

const extraLabels = userMembership && (
Expand All @@ -55,4 +74,5 @@ export const CommunityListItem = ({ result }) => {

CommunityListItem.propTypes = {
result: PropTypes.object.isRequired,
record: PropTypes.object.isRequired,
};
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export class CommunitySelectionModalComponent extends Component {
modalOpen,
apiConfigs,
handleClose,
record,
} = this.props;

return (
Expand Down Expand Up @@ -93,7 +94,7 @@ export class CommunitySelectionModalComponent extends Component {
</Header>
</Modal.Header>

<CommunitySelectionSearch apiConfigs={apiConfigs} />
<CommunitySelectionSearch apiConfigs={apiConfigs} record={record} />

{extraContentComponents && (
<Modal.Content>{extraContentComponents}</Modal.Content>
Expand All @@ -120,6 +121,7 @@ CommunitySelectionModalComponent.propTypes = {
modalOpen: PropTypes.bool,
apiConfigs: PropTypes.object,
handleClose: PropTypes.func.isRequired,
record: PropTypes.object.isRequired,
};

CommunitySelectionModalComponent.defaultProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { i18next } from "@translations/invenio_rdm_records/i18next";
import React, { Component } from "react";
import { OverridableContext } from "react-overridable";
import { OverridableContext, parametrize } from "react-overridable";
import {
EmptyResults,
Error,
Expand Down Expand Up @@ -47,10 +47,13 @@ export class CommunitySelectionSearch extends Component {
} = this.state;
const {
apiConfigs: { allCommunities, myCommunities },
record,
} = this.props;
const searchApi = new InvenioSearchApi(selectedsearchApi);
const overriddenComponents = {
[`${selectedAppId}.ResultsList.item`]: CommunityListItem,
[`${selectedAppId}.ResultsList.item`]: parametrize(CommunityListItem, {
record: record,
}),
};
return (
<OverridableContext.Provider value={overriddenComponents}>
Expand Down Expand Up @@ -163,6 +166,7 @@ CommunitySelectionSearch.propTypes = {
searchApi: PropTypes.object.isRequired,
}),
}),
record: PropTypes.object.isRequired,
};

CommunitySelectionSearch.defaultProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class SubmitReviewOrPublishComponent extends Component {
showChangeCommunityButton,
showDirectPublishButton,
showSubmitForReviewButton,
record,
...ui
} = this.props;
const { modalOpen } = this.state;
Expand All @@ -52,6 +53,7 @@ class SubmitReviewOrPublishComponent extends Component {
onModalChange={(value) => this.setState({ modalOpen: value })}
modalOpen={modalOpen}
displaySelected
record={record}
chosenCommunity={community}
trigger={
<Button content={i18next.t("Change community")} fluid className="mb-10" />
Expand All @@ -77,6 +79,7 @@ SubmitReviewOrPublishComponent.propTypes = {
showChangeCommunityButton: PropTypes.bool.isRequired,
showDirectPublishButton: PropTypes.bool.isRequired,
showSubmitForReviewButton: PropTypes.bool.isRequired,
record: PropTypes.object.isRequired,
};

SubmitReviewOrPublishComponent.defaultProps = {
Expand Down
4 changes: 1 addition & 3 deletions invenio_rdm_records/requests/community_inclusion.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from invenio_i18n import lazy_gettext as _
from invenio_records_resources.services.uow import RecordIndexOp
from invenio_requests.customizations import RequestType, actions
from invenio_requests.errors import CannotExecuteActionError

from ..proxies import current_rdm_records_service as service
from ..services.errors import InvalidAccessRestrictions
Expand Down Expand Up @@ -53,8 +52,7 @@ def execute(self, identity, uow):
assert not record.parent.review

if not is_access_restriction_valid(record, community):
description = InvalidAccessRestrictions.description
raise CannotExecuteActionError(description)
raise InvalidAccessRestrictions()

# set the community to `default` if it is the first
default = not record.parent.communities
Expand Down
6 changes: 6 additions & 0 deletions invenio_rdm_records/requests/community_submission.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
from invenio_requests.customizations import actions

from ..proxies import current_rdm_records_service as service
from ..services.errors import InvalidAccessRestrictions
from .base import ReviewRequest
from .community_inclusion import is_access_restriction_valid


#
Expand Down Expand Up @@ -42,6 +44,10 @@ def execute(self, identity, uow):
community = self.request.receiver.resolve()
service._validate_draft(identity, draft)

# validate record and community access
if not is_access_restriction_valid(draft, community):
raise InvalidAccessRestrictions()

# Unset review from record (still accessible from request)
# The curator (receiver) should still have access, via the community
# The creator (uploader) should also still have access, because
Expand Down
39 changes: 25 additions & 14 deletions tests/resources/test_resources_communities.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
)
from invenio_rdm_records.records.api import RDMDraft, RDMRecord
from invenio_rdm_records.requests.community_inclusion import CommunityInclusion
from invenio_rdm_records.services.errors import InvalidAccessRestrictions


def _add_to_community(db, record, community):
Expand Down Expand Up @@ -313,12 +314,13 @@ def test_restrict_community_before_accepting_inclusion(
assert response.status_code == 200

# accept request should fail
response = client.post(
f"/requests/{request_id}/actions/accept",
headers=headers,
json={},
)
assert response.status_code == 400
# The error handlers for this action are defined in invenio-app-rdm, therefore we catch the exception here
with pytest.raises(InvalidAccessRestrictions):
client.post(
f"/requests/{request_id}/actions/accept",
headers=headers,
json={},
)


def test_create_new_version_after_inclusion_request(
Expand Down Expand Up @@ -417,14 +419,15 @@ def test_create_new_version_after_inclusion_request(
assert hit["metadata"]["title"] == second_version_record["metadata"]["title"]


def test_include_public_record_in_restricted_community(
def test_accept_public_record_in_restricted_community(
client,
uploader,
record_community,
headers,
restricted_community,
community_owner,
):
"""Test include public record in restricted community."""
"""Test accept public record in restricted community."""
client = uploader.login(client)

data = {
Expand All @@ -438,12 +441,20 @@ def test_include_public_record_in_restricted_community(
headers=headers,
json=data,
)
assert response.status_code == 400
assert response.json["errors"]
assert (
"cannot be included in a restricted community"
in response.json["errors"][0]["message"]
)
assert response.status_code == 200
assert response.json["processed"]
assert len(response.json["processed"]) == 1
request_id = response.json["processed"][0]["request_id"]
client = uploader.logout(client)
client = community_owner.login(client)

# The error handlers for this action are defined in invenio-app-rdm, therefore we catch the exception here
with pytest.raises(InvalidAccessRestrictions):
client.post(
f"/requests/{request_id}/actions/accept",
headers=headers,
json={},
)


def test_include_community_already_in(
Expand Down
14 changes: 8 additions & 6 deletions tests/services/test_service_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,14 +377,16 @@ def test_create_review_after_draft(running_app, community, service, minimal_reco
assert draft["status"] == DraftStatus.review_to_draft_statuses[req["status"]]


def test_submit_public_record_review_to_restricted_community(
running_app, public_draft_review_restricted, service
def test_accept_public_record_review_to_restricted_community(
running_app, public_draft_review_restricted, service, requests_service
):
"""Test creation of review after draft was created."""
# Create draft
"""Test invalid record/community restrictions on accept."""
request = service.review.submit(
running_app.superuser_identity, public_draft_review_restricted.id
)
with pytest.raises(InvalidAccessRestrictions):
service.review.submit(
running_app.superuser_identity, public_draft_review_restricted.id
requests_service.execute_action(
running_app.superuser_identity, request.id, "accept", {}
)


Expand Down

0 comments on commit 5e0ff7f

Please sign in to comment.