diff --git a/assets/js/components/record_details/CommitteeApproval.js b/assets/js/components/record_details/CommitteeApproval.js
index 354a45a4..27f49784 100644
--- a/assets/js/components/record_details/CommitteeApproval.js
+++ b/assets/js/components/record_details/CommitteeApproval.js
@@ -86,7 +86,7 @@ export class CommitteeApprovalManageSection extends Component {
{pubRn && {pubRn}}
- {canViewReviewedVersion && draftRecordId && (
+ {canViewReviewedVersion && draftRecordId && draftRecordId !== record.id && (
<>
{" · "}
diff --git a/site/cds_rdm/components.py b/site/cds_rdm/components.py
index 10a1be58..26f5bb99 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 _
@@ -130,22 +129,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.
@@ -160,9 +156,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", [])
@@ -177,7 +173,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 EP approval report number is system-managed and cannot be "
"added, modified, or removed manually."
)
@@ -201,47 +197,31 @@ def _validate_identifier_changes(self, identity, data, record):
raise ValidationErrorWithMessageAsList(errors)
- # Block cdsrn values that look like committee report numbers.
- ep_prefixes = self._committee_approval_prefixes()
- if ep_prefixes:
- errors = []
- for index, ident in enumerate(incoming_identifiers):
- if ident.get("scheme") == "cdsrn":
- val = ident.get("identifier", "")
- if any(val.startswith(p) for p in ep_prefixes):
- errors.append(
- {
- "field": f"metadata.identifiers.{index}.identifier",
- "messages": [
- _(
- f"The value '{val}' matches an EP approval "
- "report number pattern and cannot be used as "
- "a CDS report number."
- )
- ],
- }
- )
- 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")
+ approved_internal = committee_approval.get("approved_internal_version")
+ source_internal = committee_approval.get("source_internal_version")
+ if not reportnumber or not source_internal:
+ return False
+ # Migrated case: both flags point at the same version, so only the current version carries apprn
+ if approved_internal == source_internal:
+ return record["id"] == source_internal
+ # Public copy: source points back to a different (internal) record, always sync
+ return True
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.
- """
- ea = (
+ """Keep apprn in metadata.identifiers in sync with parent committee_approval."""
+ 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")
+ reportnumber = committee_approval.get("reportnumber")
identifiers = [
i
for i in (data.get("metadata") or {}).get("identifiers", [])
if i.get("scheme") != "apprn"
]
- if reportnumber and source_internal:
+ if self._should_sync_apprn(record, committee_approval):
identifiers = [
{"scheme": "apprn", "identifier": reportnumber}
] + identifiers
@@ -249,11 +229,11 @@ def _regenerate_apprn_identifier(self, record, data):
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/views.py b/site/cds_rdm/requests/views.py
index 3d2d7b71..6acd9e69 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={
@@ -289,6 +289,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/tests/test_committee_approval.py b/site/tests/test_committee_approval.py
index ce640721..8962ef26 100644
--- a/site/tests/test_committee_approval.py
+++ b/site/tests/test_committee_approval.py
@@ -10,10 +10,13 @@
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_records_resources.services.errors import (
PermissionDeniedError,
RecordPermissionDeniedError,
@@ -22,14 +25,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_ORIGIN_PREFIX,
+ COMMITTEE_APPROVAL_GRANT_PERMISSION,
+)
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 +88,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 +103,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 +114,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",
@@ -164,7 +168,9 @@ def test_approval_rn_invalid_formats():
def test_committee_approval_request_type_is_registered(app):
"""CommitteeApprovalRequest must be discoverable via the request type registry."""
- request_type = current_request_type_registry.lookup("committee-approval", quiet=True)
+ request_type = current_request_type_registry.lookup(
+ "committee-approval", quiet=True
+ )
assert request_type is not None
assert request_type.type_id == "committee-approval"
@@ -286,11 +292,13 @@ 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(
- community_manager.identity, minimal_restricted_record, committee_enrolled_community, service
+ community_manager.identity,
+ minimal_restricted_record,
+ committee_enrolled_community,
+ service,
)
r2 = current_requests_service.create(
@@ -407,67 +415,139 @@ 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,
+ "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) == 1 and apprn_ids[0]["identifier"] == report_number
+
+ # 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,
+ "approved_internal_version": internal.id,
+ }
+ internal_rec_obj.parent["permission_flags"] = pf
+ internal_rec_obj.parent.commit()
+ db.session.commit()
+
+ # 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 record.data.get("metadata", {}).get("identifiers", [])
+ i
+ for i in internal_v2.data.get("metadata", {}).get("identifiers", [])
if i.get("scheme") == "apprn"
]
- assert len(apprn_ids) == 0, "Internal draft must NOT carry the apprn identifier"
+ assert apprn_ids == []
- # Simulate public record: set source_internal_version in permission_flags.
- pf = rec_obj.parent.get("permission_flags") or {}
+ # 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,
- "source_internal_version": record.id,
+ "approved_internal_version": internal.id,
+ "approved_public_version": "9999999",
}
- 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)
+ 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 == []
- apprn_ids = [
- i for i in record2.data.get("metadata", {}).get("identifiers", [])
+ # 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_ids) == 1 and apprn_ids[0]["identifier"] == report_number
+ assert len(apprn_ids3) == 1 and apprn_ids3[0]["identifier"] == report_number
# ---------------------------------------------------------------------------
@@ -500,7 +580,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"))
@@ -530,19 +609,10 @@ 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")
- pid_obj = PersistentIdentifier.get(
- "recid", record_in_enrolled_community.id
- )
+ pid_obj = PersistentIdentifier.get("recid", record_in_enrolled_community.id)
record_uuid = pid_obj.object_uuid
expected_origin = f"{COMMITTEE_APPROVAL_GRANT_ORIGIN_PREFIX}{record_uuid}"
@@ -557,7 +627,8 @@ def test_referee_grant_added_on_submit_removed_on_decline(
# Grant must be present after submit.
rec = RDMRecord.get_record(record_uuid)
grants = [
- g for g in rec.parent.access.grants
+ g
+ for g in rec.parent.access.grants
if g.permission == COMMITTEE_APPROVAL_GRANT_PERMISSION
and g.origin == expected_origin
]
@@ -574,7 +645,8 @@ def test_referee_grant_added_on_submit_removed_on_decline(
# Grant must be removed after decline.
rec = RDMRecord.get_record(record_uuid)
remaining = [
- g for g in rec.parent.access.grants
+ g
+ for g in rec.parent.access.grants
if g.permission == COMMITTEE_APPROVAL_GRANT_PERMISSION
and g.origin == expected_origin
]
@@ -590,19 +662,10 @@ 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")
- pid_obj = PersistentIdentifier.get(
- "recid", record_in_enrolled_community.id
- )
+ pid_obj = PersistentIdentifier.get("recid", record_in_enrolled_community.id)
record_uuid = pid_obj.object_uuid
expected_origin = f"{COMMITTEE_APPROVAL_GRANT_ORIGIN_PREFIX}{record_uuid}"
@@ -622,7 +685,8 @@ def test_referee_grant_retained_after_accept(
rec = RDMRecord.get_record(record_uuid)
grants = [
- g for g in rec.parent.access.grants
+ g
+ for g in rec.parent.access.grants
if g.permission == COMMITTEE_APPROVAL_GRANT_PERMISSION
and g.origin == expected_origin
]
@@ -638,7 +702,6 @@ def test_referee_grant_scoped_to_submitted_version(
db,
):
"""Referee can read the submitted version but not a new version created afterwards."""
- from invenio_rdm_records.proxies import current_rdm_records
request_type = current_request_type_registry.lookup("committee-approval")
service = current_rdm_records.records_service
@@ -659,9 +722,7 @@ def test_referee_grant_scoped_to_submitted_version(
)
# Referee can read v1.
- v1 = service.read(
- identity=ep_referee.identity, id_=record_in_enrolled_community.id
- )
+ v1 = service.read(identity=ep_referee.identity, id_=record_in_enrolled_community.id)
assert v1.id == record_in_enrolled_community.id
# Create v2.