diff --git a/.gitignore b/.gitignore index efa7d011..2bff0bd0 100644 --- a/.gitignore +++ b/.gitignore @@ -84,4 +84,8 @@ node_modules # local tmp folder tmp -scripts \ No newline at end of file +scripts + +# nvm, python +.nvmrc +.python-version \ No newline at end of file diff --git a/.invenio b/.invenio index c98ac2e4..83124296 100644 --- a/.invenio +++ b/.invenio @@ -13,7 +13,7 @@ description = CDS RDM InvenioRDM Instance author_name = CERN author_email = info@cds-rdm.com year = 2022 -python_version = 3.9 +python_version = 3.14 database = postgresql search = opensearch2 file_storage = local diff --git a/assets/js/components/record_details/CommitteeApproval.js b/assets/js/components/record_details/CommitteeApproval.js index 354a45a4..638a11cd 100644 --- a/assets/js/components/record_details/CommitteeApproval.js +++ b/assets/js/components/record_details/CommitteeApproval.js @@ -82,9 +82,11 @@ export class CommitteeApprovalManageSection extends Component {

- {pubRn ? i18next.t("EP-approved as ") : i18next.t("EP-approved record")} + {pubRn?.length + ? i18next.t("EP-approved as ") + : i18next.t("EP-approved record")} - {pubRn && {pubRn}} + {pubRn?.length > 0 && {pubRn.join(", ")}} {canViewReviewedVersion && draftRecordId && ( <> @@ -124,7 +126,8 @@ export class CommitteeApprovalManageSection extends Component { const isAccepted = openRequest?.status === "accepted"; // approvedReportNumber is always populated by the backend (scans the full // parent if the current version doesn't carry the CF itself). - const canResubmit = canSubmit && !isPending && !approvedReportNumber && !isAccepted; + const canResubmit = + canSubmit && !isPending && !approvedReportNumber?.length && !isAccepted; // canCreatePublicFlag comes from the backend and already encodes version-order // eligibility (only versions >= the approved version may create a public record). const canCreatePublic = canCreatePublicFlag && !publicRecordId; @@ -139,7 +142,7 @@ export class CommitteeApprovalManageSection extends Component { const step1Active = !step1Completed && !isPending; // Step 2 — EP Board review - const step2Completed = !!approvedReportNumber; + const step2Completed = !!approvedReportNumber?.length; const step2Active = isPending; const step2Disabled = !isPending && !step2Completed; @@ -231,13 +234,13 @@ export class CommitteeApprovalManageSection extends Component { requestLink ? ( {i18next.t("Approved as {{rn}}", { - rn: approvedReportNumber, + rn: approvedReportNumber.join(", "), })} ) : ( i18next.t("Approved as {{rn}}", { - rn: approvedReportNumber, + rn: approvedReportNumber.join(", "), }) ) ) : ( diff --git a/assets/js/components/record_details/RecordVersionItem.js b/assets/js/components/record_details/RecordVersionItem.js index 73b8eafa..27a98065 100644 --- a/assets/js/components/record_details/RecordVersionItem.js +++ b/assets/js/components/record_details/RecordVersionItem.js @@ -25,7 +25,7 @@ export const RecordVersionItemContent = ({ item, activeVersion, doi }) => { const approvedReportNumber = ea.reportnumber; // approved_internal_version: recid of the version that was submitted and approved. const isApprovedVersion = - !!approvedReportNumber && ea.approved_internal_version === item.id; + !!approvedReportNumber?.length && ea.approved_internal_version === item.id; // source_public_version: recid of the internal version used to create the public record. const publicRecordId = ea.approved_public_version; @@ -55,7 +55,7 @@ export const RecordVersionItemContent = ({ item, activeVersion, doi }) => { {" "} - {approvedReportNumber} + {approvedReportNumber.join(", ")} )} @@ -71,7 +71,7 @@ export const RecordVersionItemContent = ({ item, activeVersion, doi }) => { className="text-muted-darken" > - {approvedReportNumber} + {approvedReportNumber.join(", ")} )} diff --git a/invenio.cfg b/invenio.cfg index 74e9bfcf..5d1fa84e 100644 --- a/invenio.cfg +++ b/invenio.cfg @@ -692,7 +692,7 @@ RDM_RECORDS_IDENTIFIERS_SCHEMES = { "validator": always_valid, "datacite": "CDS"}, "apprn": {"label": _("Approval Report Number"), - "validator": schemes.is_approval_report_number, + "validator": always_valid, "datacite": "CDS"}, "aleph": {"label": _("Aleph number"), "validator": schemes.is_aleph, diff --git a/site/cds_rdm/components.py b/site/cds_rdm/components.py index f9c04c04..26719e13 100644 --- a/site/cds_rdm/components.py +++ b/site/cds_rdm/components.py @@ -9,8 +9,7 @@ """CDS RDM service components.""" from flask import current_app -from flask_principal import ActionNeed -from invenio_access import Permission +from invenio_access.permissions import system_user_id from invenio_communities.proxies import current_communities from invenio_drafts_resources.services.records.components import ServiceComponent from invenio_i18n import gettext as _ @@ -132,22 +131,19 @@ def publish(self, identity, draft=None, record=None, **kwargs): class CommitteeApprovalComponent(ServiceComponent): """Guard and sync committee approval identifiers. - 1. Blocks non-privileged users from adding/modifying/deleting ``apprn`` - scheme identifiers — these are system-managed only. - 2. Blocks non-privileged users from adding a ``cdsrn`` identifier whose - value matches any configured committee approval report-number pattern - (e.g. CERN-EP-*). - 3. Regenerates the ``apprn`` metadata identifier from parent committee_approval - on every save — only the public approved record carries it (detected by - ``source_internal_version`` on the parent). + 1. Blocks everyone except the system process from adding/modifying/deleting + ``apprn`` scheme identifiers — including admins via the UI. + 2. Regenerates the ``apprn`` metadata identifier from parent committee_approval + only when ``source_internal_version`` is set on the parent. This covers + two cases: + - Public approved copy in the two-record flow (views.py sets + ``source_internal_version`` to the internal record's recid). + - Single-record migration case (migrate_cdsrn_to_apprn.py sets + ``source_internal_version`` to the record's own recid). + The internal record in the normal flow never has ``source_internal_version`` + on its parent, so it never carries the apprn identifier. """ - def _is_privileged(self, identity): - """Return True if the identity is system or has superuser access.""" - return identity.id == "system" or Permission( - ActionNeed("superuser-access") - ).allows(identity) - def _committee_approval_prefixes(self): """Return the set of fixed prefixes from all configured committee communities. @@ -162,9 +158,9 @@ def _committee_approval_prefixes(self): prefixes.add(prefix) return prefixes - def _validate_identifier_changes(self, identity, data, record): - """Raise ValidationError if the user is modifying protected identifiers.""" - if self._is_privileged(identity): + def _validate_identifier_changes(self, identity, data, record, errors): + """Raise ValidationError if a non-system identity modifies apprn.""" + if identity.id == system_user_id: return incoming_identifiers = (data.get("metadata") or {}).get("identifiers", []) @@ -179,7 +175,7 @@ def _validate_identifier_changes(self, identity, data, record): } if incoming_apprn != stored_apprn: error_msg = _( - "The 'apprn' identifier is system-managed and cannot be " + "The approval report number is system-managed and cannot be " "added, modified, or removed manually." ) @@ -216,7 +212,7 @@ def _validate_identifier_changes(self, identity, data, record): "field": f"metadata.identifiers.{index}.identifier", "messages": [ _( - f"The value '{val}' matches an EP approval " + f"The value '{val}' matches an approval " "report number pattern and cannot be used as " "a CDS report number." ) @@ -226,36 +222,59 @@ def _validate_identifier_changes(self, identity, data, record): if errors: raise ValidationErrorWithMessageAsList(errors) + def _should_sync_apprn(self, record, committee_approval): + """Return True if apprn should be synced with parent committee_approval.""" + reportnumber = committee_approval.get("reportnumber") + source_internal = committee_approval.get("source_internal_version") + if reportnumber and source_internal: + return True + return False + def _regenerate_apprn_identifier(self, record, data): """Keep apprn in metadata.identifiers in sync with parent committee_approval. - The apprn identifier is only added when ``source_internal_version`` is present - on the parent — that key is set exclusively on the public approved record's - parent by the ``publish_public_record`` view. + committee_approval.reportnumber is the source of truth (a list of strings). + The metadata apprn entries are derived from it exactly — no other apprn + entries are preserved. """ - ea = ( + committee_approval = ( (record.parent.get("permission_flags") if record.parent else None) or {} ).get("committee_approval") or {} - reportnumber = ea.get("reportnumber") - source_internal = ea.get("source_internal_version") - identifiers = [ - i - for i in (data.get("metadata") or {}).get("identifiers", []) - if i.get("scheme") != "apprn" - ] - if reportnumber and source_internal: - identifiers = [ - {"scheme": "apprn", "identifier": reportnumber} - ] + identifiers - data.setdefault("metadata", {})["identifiers"] = identifiers + + existing_identifiers = (data.get("metadata") or {}).get("identifiers", []) + app_rn = [i for i in existing_identifiers if i.get("scheme") == "apprn"] + not_apprn = [i for i in existing_identifiers if i.get("scheme") != "apprn"] + + if self._should_sync_apprn(record, committee_approval): + reportnumbers = committee_approval.get("reportnumber") or [] + apprn = [{"scheme": "apprn", "identifier": rn} for rn in reportnumbers] + new_identifiers = apprn + not_apprn + else: + # check if there are any remaining apprn identifiers and raise a validation error if they exist + if app_rn: + errors = [ + { + "field": "metadata.identifiers", + "messages": [ + _( + "The approval report number is system-managed and cannot be " + "added, modified, or removed manually.", + ) + ], + } + ] + raise ValidationErrorWithMessageAsList(errors) + new_identifiers = not_apprn + + data.setdefault("metadata", {})["identifiers"] = new_identifiers def create(self, identity, data=None, record=None, errors=None, **kwargs): """Validate apprn identifier on draft creation.""" - self._validate_identifier_changes(identity, data, record) + self._validate_identifier_changes(identity, data, record, errors) def update_draft(self, identity, data=None, record=None, errors=None, **kwargs): """Validate and regenerate apprn identifier on draft update.""" - self._validate_identifier_changes(identity, data, record) + self._validate_identifier_changes(identity, data, record, errors) self._regenerate_apprn_identifier(record, data) def publish(self, identity, draft=None, record=None, **kwargs): diff --git a/site/cds_rdm/requests/committee_approval.py b/site/cds_rdm/requests/committee_approval.py index e667d394..dbe994c6 100644 --- a/site/cds_rdm/requests/committee_approval.py +++ b/site/cds_rdm/requests/committee_approval.py @@ -243,7 +243,7 @@ def execute(self, identity: Identity, uow: UnitOfWork) -> None: # Write committee_approval into permission_flags — single source of truth. pf = topic.parent.get("permission_flags") or {} pf["committee_approval"] = { - "reportnumber": report_number, + "reportnumber": [report_number], "datetime": datetime.now(timezone.utc).isoformat(), "approved_internal_version": topic["id"], } diff --git a/site/cds_rdm/requests/committee_approval_state.py b/site/cds_rdm/requests/committee_approval_state.py index c54595ad..db6fc409 100644 --- a/site/cds_rdm/requests/committee_approval_state.py +++ b/site/cds_rdm/requests/committee_approval_state.py @@ -154,7 +154,7 @@ def get_committee_approval_state(record_ui, record=None): - community_enrolled: bool - is_public_approved_record: bool - open_request: dict or None — {id, status, links} - - approved_report_number: str or None + - approved_report_number: list or None - approval_date: str or None - committee_approval: dict — raw parent committee_approval (for frontend version badges) - draft_record_id: str or None diff --git a/site/cds_rdm/requests/views.py b/site/cds_rdm/requests/views.py index 12f14db3..bca54717 100644 --- a/site/cds_rdm/requests/views.py +++ b/site/cds_rdm/requests/views.py @@ -244,7 +244,7 @@ def publish_public_record(pid_value): ) if cern_scientific_community_id: try: - current_record_communities_service.add( + _, errors = current_record_communities_service.add( system_identity, new_record.data["id"], data={ @@ -257,7 +257,7 @@ def publish_public_record(pid_value): "content": ( f"This inclusion request was automatically " f"generated when publishing the EP-approved " - f"public record for {report_number}. The " + f"public record for {', '.join(report_number)}. The " f"document has been reviewed and approved by " f"the EP Publication Committee." ) @@ -283,6 +283,7 @@ def publish_public_record(pid_value): pf["committee_approval"] = { **ea, "approved_public_version": new_record_id, + "source_public_version": src_id, } src_rec_obj.parent["permission_flags"] = pf src_rec_obj.parent.commit() diff --git a/site/cds_rdm/templates/semantic-ui/cds_rdm/records/detail.html b/site/cds_rdm/templates/semantic-ui/cds_rdm/records/detail.html index ce00eb79..8f664f25 100644 --- a/site/cds_rdm/templates/semantic-ui/cds_rdm/records/detail.html +++ b/site/cds_rdm/templates/semantic-ui/cds_rdm/records/detail.html @@ -10,17 +10,16 @@ {%- set clc_sync_entry = get_clc_sync_entry(record_ui) %} {%- set additional_permissions = evaluate_permissions(record, ['manage_clc_sync']) %} -{# Find the apprn identifier on the public EP-approved record. #} -{%- set apprn_identifier = namespace(value=None) %} -{%- for ident in record_ui.get("metadata", {}).get("identifiers", []) %} - {%- if ident.get("scheme") == "apprn" %} - {%- set apprn_identifier.value = ident.get("identifier") %} - {%- endif %} -{%- endfor %} +{# Collect all apprn identifiers on the public EP-approved record. #} +{%- set apprn_identifier = record_ui.get("metadata", {}).get("identifiers", []) + | selectattr("scheme", "equalto", "apprn") + | map(attribute="identifier") + | default(None) %} {%- block record_header -%} {{ super() }} -{%- if apprn_identifier.value %} +{%- if apprn_identifier %} +{%- for identifier in apprn_identifier %} +{% endfor %} {%- endif %} {%- endblock record_header -%} diff --git a/site/tests/test_committee_approval.py b/site/tests/test_committee_approval.py index b7c25176..422459c4 100644 --- a/site/tests/test_committee_approval.py +++ b/site/tests/test_committee_approval.py @@ -10,10 +10,14 @@ from datetime import date import pytest +from flask_principal import RoleNeed from invenio_access.permissions import system_identity +from invenio_communities.generators import CommunityRoleNeed from invenio_db import db from invenio_pidstore.models import PersistentIdentifier, PIDStatus from invenio_rdm_records.proxies import current_rdm_records +from invenio_rdm_records.records.api import RDMRecord +from invenio_rdm_records.services.errors import ValidationErrorWithMessageAsList from invenio_records_resources.services.errors import ( PermissionDeniedError, RecordPermissionDeniedError, @@ -22,14 +26,21 @@ current_request_type_registry, current_requests_service, ) +from invenio_users_resources.records.api import UserAggregate from marshmallow import ValidationError +from cds_rdm.generators import ( + COMMITTEE_APPROVAL_GRANT_PERMISSION, + committee_approval_grant_origin, +) from cds_rdm.requests.committee_approval import ( APPRN_PID_TYPE, CommitteeApprovalAcceptAction, ) from cds_rdm.schemes import is_approval_report_number +from .conftest import _publish_record_in_community + # --------------------------------------------------------------------------- # Constants # --------------------------------------------------------------------------- @@ -78,7 +89,6 @@ def ep_referee(UserFixture, ep_referee_group, app, db): The RoleNeed is injected directly — going through the full group membership flow is out of scope for these tests. """ - from invenio_users_resources.records.api import UserAggregate u = UserFixture( email="ep-referee@inveniosoftware.org", @@ -94,8 +104,6 @@ def ep_referee(UserFixture, ep_referee_group, app, db): u.create(app, db) UserAggregate.index.refresh() - from flask_principal import RoleNeed - u.identity.provides.add(RoleNeed(EP_GROUP_NAME)) return u @@ -107,9 +115,6 @@ def community_manager(UserFixture, committee_enrolled_community, app, db): The CommunityRoleNeed is injected directly into the identity because user membership goes through invite→accept (out of scope here). """ - from invenio_communities.generators import CommunityRoleNeed - from invenio_users_resources.records.api import UserAggregate - u = UserFixture( email="ep-manager@inveniosoftware.org", password="ep-manager", @@ -146,7 +151,11 @@ def test_group_email_recipient_generator_adds_cern_email(): notification = Notification( type="committee-approval-request.submit", - context={"request": {"receiver": {"id": "ep-referee-group", "name": "ep-referee-group"}}}, + context={ + "request": { + "receiver": {"id": "ep-referee-group", "name": "ep-referee-group"} + } + }, ) gen = GroupEmailRecipientGenerator("request.receiver") result = gen(notification, {}) @@ -331,7 +340,6 @@ def test_committee_approval_second_request_increments_sequence( ) # Second record in the same enrolled community. - from .conftest import _publish_record_in_community service = current_rdm_records.records_service record2 = _publish_record_in_community( @@ -455,69 +463,138 @@ def test_committee_approval_submit_raises_for_non_enrolled_community( # --------------------------------------------------------------------------- -def test_apprn_identifier_derived_from_parent( +def test_apprn_migrated_single_record_flow( minimal_restricted_record, uploader, app, db ): - """CommitteeApprovalComponent derives apprn from parent committee_approval. + """Migration case: apprn persists on a single record across edit/publish cycles. - Committee approval state lives on the parent record (not the version CF). - The apprn identifier is only added to records where the parent carries - ``source_internal_version`` — that marks the public approved copy. - The internal draft and all its versions do NOT carry the apprn identifier. + migrate_cdsrn_to_apprn.py sets source_internal_version = own recid on the + parent, which is the sentinel CommitteeApprovalComponent uses to decide + whether to keep the apprn identifier. Without it the identifier would be + stripped on the next publish. """ - from invenio_pidstore.models import PersistentIdentifier - from invenio_rdm_records.records.api import RDMRecord - service = current_rdm_records.records_service + report_number = f"CERN-EP-{YEAR}-001" draft = service.create(uploader.identity, minimal_restricted_record) record = service.publish(uploader.identity, id_=draft.id) - report_number = f"CERN-EP-{YEAR}-001" - - # Simulate accept: write committee_approval into permission_flags (no source_internal_version). + # Simulate what migrate_cdsrn_to_apprn.py writes on the parent. + # source_internal_version = own recid because there is no separate public copy. pid_obj = PersistentIdentifier.get("recid", record.id) rec_obj = RDMRecord.get_record(pid_obj.object_uuid) pf = rec_obj.parent.get("permission_flags") or {} pf["committee_approval"] = { - "reportnumber": report_number, - "approved_internal_version": record.id, + "reportnumber": [report_number], + "source_internal_version": record.id, } rec_obj.parent["permission_flags"] = pf rec_obj.parent.commit() db.session.commit() - # Update and re-publish: apprn should NOT be added (no source_internal_version). + # First edit+publish: apprn must appear. sys_draft = service.edit(system_identity, id_=record.id) record = service.publish(system_identity, id_=sys_draft.id) - apprn_ids = [ i for i in record.data.get("metadata", {}).get("identifiers", []) if i.get("scheme") == "apprn" ] - assert len(apprn_ids) == 0, "Internal draft must NOT carry the apprn identifier" + assert len(apprn_ids) == 1 and apprn_ids[0]["identifier"] == report_number - # Simulate public record: set source_internal_version in permission_flags. - pf = rec_obj.parent.get("permission_flags") or {} + # Second edit+publish: apprn must persist across further metadata edits. + sys_draft2 = service.edit(system_identity, id_=record.id) + record2 = service.publish(system_identity, id_=sys_draft2.id) + apprn_ids2 = [ + i + for i in record2.data.get("metadata", {}).get("identifiers", []) + if i.get("scheme") == "apprn" + ] + assert len(apprn_ids2) == 1 and apprn_ids2[0]["identifier"] == report_number + + +def test_apprn_two_record_flow(minimal_restricted_record, uploader, app, db): + """Two-record flow: apprn lives only on the public copy, never on the internal record. + + After committee approval the internal parent has approved_internal_version but + NOT source_internal_version, so CommitteeApprovalComponent._should_sync_apprn + returns False and apprn is never added — whether or not a public copy exists yet. + + The public copy gets source_internal_version from views.py, so it carries apprn. + """ + service = current_rdm_records.records_service + report_number = f"CERN-EP-{YEAR}-002" + + # Create and publish the internal record. + draft = service.create(uploader.identity, minimal_restricted_record) + internal = service.publish(uploader.identity, id_=draft.id) + + internal_pid = PersistentIdentifier.get("recid", internal.id) + internal_rec_obj = RDMRecord.get_record(internal_pid.object_uuid) + + # After committee approval: approved_internal_version set, no source_internal_version. + pf = internal_rec_obj.parent.get("permission_flags") or {} pf["committee_approval"] = { - "reportnumber": report_number, - "source_internal_version": record.id, + "reportnumber": [report_number], + "approved_internal_version": internal.id, } - rec_obj.parent["permission_flags"] = pf - rec_obj.parent.commit() + internal_rec_obj.parent["permission_flags"] = pf + internal_rec_obj.parent.commit() db.session.commit() - # Update draft again: apprn SHOULD now appear (source_internal_version present). - sys_draft2 = service.edit(system_identity, id_=record.id) - record2 = service.publish(system_identity, id_=sys_draft2.id) - + # Edit+publish internal before public record exists: apprn must NOT appear. + sys_draft = service.edit(system_identity, id_=internal.id) + internal_v2 = service.publish(system_identity, id_=sys_draft.id) apprn_ids = [ i - for i in record2.data.get("metadata", {}).get("identifiers", []) + for i in internal_v2.data.get("metadata", {}).get("identifiers", []) if i.get("scheme") == "apprn" ] - assert len(apprn_ids) == 1 and apprn_ids[0]["identifier"] == report_number + assert apprn_ids == [] + + # After public record is created views.py writes approved_public_version back. + # Internal record still must not carry apprn. + pf["committee_approval"] = { + "reportnumber": [report_number], + "approved_internal_version": internal.id, + "approved_public_version": "9999999", + } + internal_rec_obj.parent["permission_flags"] = pf + internal_rec_obj.parent.commit() + db.session.commit() + + sys_draft2 = service.edit(system_identity, id_=internal.id) + internal_v3 = service.publish(system_identity, id_=sys_draft2.id) + apprn_ids2 = [ + i + for i in internal_v3.data.get("metadata", {}).get("identifiers", []) + if i.get("scheme") == "apprn" + ] + assert apprn_ids2 == [] + + # Create the public copy with source_internal_version set (as views.py does). + pub_draft = service.create(uploader.identity, minimal_restricted_record) + public = service.publish(uploader.identity, id_=pub_draft.id) + + public_pid = PersistentIdentifier.get("recid", public.id) + public_rec_obj = RDMRecord.get_record(public_pid.object_uuid) + pf2 = public_rec_obj.parent.get("permission_flags") or {} + pf2["committee_approval"] = { + "reportnumber": [report_number], + "source_internal_version": internal.id, + } + public_rec_obj.parent["permission_flags"] = pf2 + public_rec_obj.parent.commit() + db.session.commit() + + sys_draft3 = service.edit(system_identity, id_=public.id) + public_v2 = service.publish(system_identity, id_=sys_draft3.id) + apprn_ids3 = [ + i + for i in public_v2.data.get("metadata", {}).get("identifiers", []) + if i.get("scheme") == "apprn" + ] + assert len(apprn_ids3) == 1 and apprn_ids3[0]["identifier"] == report_number # --------------------------------------------------------------------------- @@ -550,7 +627,6 @@ def test_committee_approval_submit_permissions( # Simulate the uploader being a community manager by injecting the need # directly into the identity. members.add is groups-only; user membership # goes through invite→accept which is out of scope for this permission test. - from invenio_communities.generators import CommunityRoleNeed community_id = str(committee_enrolled_community.id) uploader.identity.provides.add(CommunityRoleNeed(community_id, "manager")) @@ -580,18 +656,11 @@ def test_referee_grant_added_on_submit_removed_on_decline( db, ): """Submit adds a committee-review grant; decline removes it.""" - from invenio_pidstore.models import PersistentIdentifier - from invenio_rdm_records.records.api import RDMRecord - - from cds_rdm.generators import ( - COMMITTEE_APPROVAL_GRANT_ORIGIN_PREFIX, - COMMITTEE_APPROVAL_GRANT_PERMISSION, - ) request_type = current_request_type_registry.lookup("committee-approval") - expected_origin = ( - f"{COMMITTEE_APPROVAL_GRANT_ORIGIN_PREFIX}{record_in_enrolled_community.id}_1" + expected_origin = committee_approval_grant_origin( + record_in_enrolled_community.id, 1 ) request = current_requests_service.create( @@ -604,8 +673,7 @@ def test_referee_grant_added_on_submit_removed_on_decline( # Grant must be present after submit. pid_obj = PersistentIdentifier.get("recid", record_in_enrolled_community.id) - record_uuid = pid_obj.object_uuid - rec = RDMRecord.get_record(record_uuid) + rec = RDMRecord.get_record(pid_obj.object_uuid) grants = [ g for g in rec.parent.access.grants @@ -623,7 +691,7 @@ def test_referee_grant_added_on_submit_removed_on_decline( ) # Grant must be removed after decline. - rec = RDMRecord.get_record(record_uuid) + rec = RDMRecord.get_record(pid_obj.object_uuid) remaining = [ g for g in rec.parent.access.grants @@ -642,18 +710,11 @@ def test_referee_grant_retained_after_accept( db, ): """Accept keeps the grant so referees retain permanent access to the approved version.""" - from invenio_pidstore.models import PersistentIdentifier - from invenio_rdm_records.records.api import RDMRecord - - from cds_rdm.generators import ( - COMMITTEE_APPROVAL_GRANT_ORIGIN_PREFIX, - COMMITTEE_APPROVAL_GRANT_PERMISSION, - ) request_type = current_request_type_registry.lookup("committee-approval") - expected_origin = ( - f"{COMMITTEE_APPROVAL_GRANT_ORIGIN_PREFIX}{record_in_enrolled_community.id}_1" + expected_origin = committee_approval_grant_origin( + record_in_enrolled_community.id, 1 ) request = current_requests_service.create( @@ -671,8 +732,7 @@ def test_referee_grant_retained_after_accept( ) pid_obj = PersistentIdentifier.get("recid", record_in_enrolled_community.id) - record_uuid = pid_obj.object_uuid - rec = RDMRecord.get_record(record_uuid) + rec = RDMRecord.get_record(pid_obj.object_uuid) grants = [ g for g in rec.parent.access.grants @@ -690,8 +750,7 @@ def test_referee_grant_scoped_to_submitted_version( app, db, ): - """Referee can read the submitted version and new versions created afterwards, but not any versions created before submission.""" - from invenio_rdm_records.proxies import current_rdm_records + """Referee can read the submitted version but not a new version created afterwards.""" request_type = current_request_type_registry.lookup("committee-approval") service = current_rdm_records.records_service @@ -715,16 +774,105 @@ def test_referee_grant_scoped_to_submitted_version( data={"payload": {"content": "

.

", "format": "html"}}, ) - # Referee should not be able to read v1 (created before review submission). + # Referee CAN read the submitted version (v2). + service.read(identity=ep_referee.identity, id_=v2.id) + + # Referee CANNOT read v1 — it predates the submitted version. with pytest.raises(RecordPermissionDeniedError): service.read(identity=ep_referee.identity, id_=record_in_enrolled_community.id) - # Referee should be able to read v2 (the submitted version) - service.read(identity=ep_referee.identity, id_=v2.id) - - # Create v3. + # Create v3 after acceptance. v3_draft = service.new_version(system_identity, id_=record_in_enrolled_community.id) v3 = service.publish(system_identity, id_=v3_draft.id) - # Referee should be able to view v3 (created after request submitted) + # Referee CAN read v3 — newer versions inherit access from the grant. service.read(identity=ep_referee.identity, id_=v3.id) + + # Create v4. + v4_draft = service.new_version(system_identity, id_=record_in_enrolled_community.id) + v4 = service.publish(system_identity, id_=v4_draft.id) + + # Referee CAN read v4 — grant covers all versions >= submitted version index. + service.read(identity=ep_referee.identity, id_=v4.id) + + +# --------------------------------------------------------------------------- +# CommitteeApprovalComponent — apprn identifier guard +# --------------------------------------------------------------------------- + + +def test_apprn_identifier_cannot_be_manually_changed( + minimal_restricted_record, uploader, app, db +): + """apprn identifiers are system-managed; add/modify/remove all raise an error. + + Covers three paths: + - Non-system user tries to add or remove apprn → _validate_identifier_changes. + - System user tries to place apprn on a record with no committee_approval + → _regenerate_apprn_identifier else-branch. + """ + service = current_rdm_records.records_service + report_number = f"CERN-EP-{YEAR}-001" + + # --- Record A: has committee_approval + apprn set by system --- + draft_a = service.create(uploader.identity, minimal_restricted_record) + record_a = service.publish(uploader.identity, id_=draft_a.id) + + pid_obj = PersistentIdentifier.get("recid", record_a.id) + rec_obj = RDMRecord.get_record(pid_obj.object_uuid) + pf = rec_obj.parent.get("permission_flags") or {} + pf["committee_approval"] = { + "reportnumber": [report_number], + "source_internal_version": record_a.id, + } + rec_obj.parent["permission_flags"] = pf + rec_obj.parent.commit() + db.session.commit() + + sys_draft = service.edit(system_identity, id_=record_a.id) + record_a = service.publish(system_identity, id_=sys_draft.id) + + # Open a draft and read it back as the uploader so restricted fields + # (e.g. internal_notes) are stripped before we pass data back to update_draft. + service.edit(system_identity, id_=record_a.id) + draft_data = service.read_draft(uploader.identity, id_=record_a.id).data + identifiers_without_apprn = [ + i for i in draft_data["metadata"].get("identifiers", []) + if i.get("scheme") != "apprn" + ] + + # Non-system: remove apprn → rejected. + with pytest.raises(ValidationErrorWithMessageAsList): + service.update_draft( + uploader.identity, + id_=record_a.id, + data={**draft_data, "metadata": {**draft_data["metadata"], + "identifiers": identifiers_without_apprn}}, + ) + + # Non-system: add a different apprn → rejected. + with pytest.raises(ValidationErrorWithMessageAsList): + service.update_draft( + uploader.identity, + id_=record_a.id, + data={**draft_data, "metadata": {**draft_data["metadata"], + "identifiers": identifiers_without_apprn + [ + {"scheme": "apprn", "identifier": "CERN-EP-2099-999"} + ]}}, + ) + + # --- Record B: no committee_approval; system tries to add apprn --- + draft_b = service.create(uploader.identity, minimal_restricted_record) + record_b = service.publish(uploader.identity, id_=draft_b.id) + sys_draft_b = service.edit(system_identity, id_=record_b.id) + draft_b_data = sys_draft_b.data + + with pytest.raises(ValidationErrorWithMessageAsList): + service.update_draft( + system_identity, + id_=sys_draft_b.id, + data={**draft_b_data, "metadata": {**draft_b_data["metadata"], + "identifiers": draft_b_data["metadata"].get("identifiers", []) + [ + {"scheme": "apprn", "identifier": report_number} + ]}}, + )