Skip to content

change(ep): make sure correct env var is used for communities #559

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.

Sign up for GitHub

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

Merged
kpsherva merged 1 commit into from
Jul 24, 2026
Merged

change(ep): make sure correct env var is used for communities #559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion cds_migrator_kit/rdm/migration_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,14 @@ def resolve_record_pid(pid):
### EP Approval configuration only needed for local, it should use cds-rdm config for de/sandbox/prod
# ===========================
CDS_CERN_SCIENTIFIC_COMMUNITY_ID = "78b3c4aa-c4e6-4502-8226-67ba2d347afe"
"""The id of the CERN Scientific community."""
"""The id of the CERN Scientific community.

This is only a local-dev default: on other instances (sandbox/prod), set the
``INVENIO_CDS_CERN_SCIENTIFIC_COMMUNITY_ID`` environment variable to the real
community id for that instance. It's read via ``current_app.config`` (not a
direct module import) specifically so that env var override works - see
ep_approval_entry.py::_cern_scientific_community_id.
"""

CDS_COMMITTEE_APPROVAL_COMMUNITIES = {
"dd13404c-bcd6-4b15-aeef-38d678c61ff1": {
Expand Down
30 changes: 22 additions & 8 deletions cds_migrator_kit/rdm/records/load/ep_approval_entry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,28 @@
from collections import OrderedDict
from copy import deepcopy

from flask import current_app

from cds_migrator_kit.errors import UnexpectedValue
from cds_migrator_kit.rdm.migration_config import CDS_CERN_SCIENTIFIC_COMMUNITY_ID

EPPHAPP_FILE_TYPE = "EPPHAPP_FILE"
EP_APPROVAL_REPORT_NUMBER_PREFIX = "CERN-EP"
EP_APPROVAL_REPORT_NUMBER_RE = re.compile(r"^CERN-EP-\d{4}-\d{3}$")


def _cern_scientific_community_id():
"""Return the CERN Scientific community id from the app config.

Read via ``current_app.config`` (not a direct import of
``migration_config``) so environment-specific overrides (e.g. an
``INVENIO_CDS_CERN_SCIENTIFIC_COMMUNITY_ID`` env var on a given instance)
actually take effect - a plain module-level import would be frozen to
whatever migration_config.py hardcodes, invisible to Flask's config
loading/env var override mechanism entirely.
"""
return current_app.config["CDS_CERN_SCIENTIFIC_COMMUNITY_ID"]


class MetadataEntry:
"""Build a load entry for the public or restricted EP approval split."""

Expand Down Expand Up @@ -182,10 +196,11 @@ def _apply_entry_modifications(self, split):
self._add_cern_scientific_community(split)

def _add_cern_scientific_community(self, entry):
community_id = _cern_scientific_community_id()
communities = entry.get("parent", {}).get("json", {}).get("communities", {})
ids = list(communities.get("ids", []))
if CDS_CERN_SCIENTIFIC_COMMUNITY_ID not in ids:
ids.append(CDS_CERN_SCIENTIFIC_COMMUNITY_ID)
if community_id not in ids:
ids.append(community_id)
communities["ids"] = ids
entry.setdefault("parent", {}).setdefault("json", {})[
"communities"
Expand All @@ -203,16 +218,15 @@ def _remove_cern_scientific_community(self, entry):

The restricted record holds the internal-only EPPHAPP draft and must
not be discoverable via the broader community; only PublicEntry adds
CDS_CERN_SCIENTIFIC_COMMUNITY_ID (see _add_cern_scientific_community).
it (see _add_cern_scientific_community).
"""
community_id = _cern_scientific_community_id()
communities = entry.get("parent", {}).get("json", {}).get("communities", {})
ids = [
cid
for cid in communities.get("ids", [])
if cid != CDS_CERN_SCIENTIFIC_COMMUNITY_ID
cid for cid in communities.get("ids", []) if cid != community_id
]
communities["ids"] = ids
if communities.get("default") == CDS_CERN_SCIENTIFIC_COMMUNITY_ID:
if communities.get("default") == community_id:
communities["default"] = ids[0] if ids else None
entry.setdefault("parent", {}).setdefault("json", {})[
"communities"
Expand Down
4 changes: 2 additions & 2 deletions scripts/copy_collection_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ def get_dump_files_paths(json_dump_dir):
return dump_files


collection = "lep/ep/l3"
environment = "sandbox"
collection = "lep/aleph_drafts"
environment = "dev"

destination_prefix = "/eos/media/cds/cds-rdm/{0}/migration/{1}/files".format(
environment, collection
Expand Down
46 changes: 23 additions & 23 deletions tests/cds-rdm/test_ep_approval_entry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def _versions_public_only():
class TestPublicEntryVersions:
"""Test that PublicEntry filters out EPPHAPP files and deduplicates versions."""

def test_public_excludes_epphapp_files(self):
def test_public_excludes_epphapp_files(self, app):
entry = _make_entry(_versions_with_epphapp())
result = PublicEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -231,15 +231,15 @@ def test_public_excludes_epphapp_files(self):
fdata["type"] != EPPHAPP_FILE_TYPE
), f"EPPHAPP file {key} should not appear in public split"

def test_public_deduplicates_identical_versions(self):
def test_public_deduplicates_identical_versions(self, app):
entry = _make_entry(_versions_with_epphapp())
result = PublicEntry(
entry, _make_approval_request(), _make_migration_logger()
).build()

assert len(result["versions"]) == 1

def test_public_access_is_public(self):
def test_public_access_is_public(self, app):
entry = _make_entry(_versions_with_epphapp())
result = PublicEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -249,7 +249,7 @@ def test_public_access_is_public(self):
assert vdata["access"]["access_obj"]["record"] == "public"
assert vdata["access"]["access_obj"]["files"] == "public"

def test_public_raises_when_no_public_files(self):
def test_public_raises_when_no_public_files(self, app):
versions = OrderedDict(
[
(
Expand All @@ -272,7 +272,7 @@ def test_public_raises_when_no_public_files(self):
entry, _make_approval_request(), _make_migration_logger()
).build()

def test_public_excludes_restricted_non_epphapp_files(self):
def test_public_excludes_restricted_non_epphapp_files(self, app):
"""A record that isn't restricted as a whole can still ship individually
restricted, non-EPPHAPP files (e.g. via 506__m); those must be excluded
from the public split (and land in RestrictedEntry instead), not raise.
Expand Down Expand Up @@ -309,7 +309,7 @@ def test_public_excludes_restricted_non_epphapp_files(self):
assert "restricted.pdf" not in files
assert PUBLIC_FILE_KEY in files

def test_public_multiple_distinct_versions(self):
def test_public_multiple_distinct_versions(self, app):
versions = OrderedDict(
[
(
Expand Down Expand Up @@ -352,7 +352,7 @@ def test_public_multiple_distinct_versions(self):
class TestRestrictedEntryVersions:
"""Test that RestrictedEntry keeps EPPHAPP files when present."""

def test_restricted_keeps_only_epphapp_when_present(self):
def test_restricted_keeps_only_epphapp_when_present(self, app):
entry = _make_entry(_versions_with_epphapp())
result = RestrictedEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -364,15 +364,15 @@ def test_restricted_keeps_only_epphapp_when_present(self):
fdata["type"] == EPPHAPP_FILE_TYPE
), f"Non-EPPHAPP file {key} should not appear in restricted split"

def test_restricted_keeps_all_changing_epphapp_versions(self):
def test_restricted_keeps_all_changing_epphapp_versions(self, app):
entry = _make_entry(_versions_with_epphapp())
result = RestrictedEntry(
entry, _make_approval_request(), _make_migration_logger()
).build()

assert len(result["versions"]) == 4

def test_restricted_access_is_restricted(self):
def test_restricted_access_is_restricted(self, app):
entry = _make_entry(_versions_with_epphapp())
result = RestrictedEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -382,7 +382,7 @@ def test_restricted_access_is_restricted(self):
assert vdata["access"]["access_obj"]["record"] == "restricted"
assert vdata["access"]["access_obj"]["files"] == "restricted"

def test_restricted_uses_public_files_when_no_epphapp(self):
def test_restricted_uses_public_files_when_no_epphapp(self, app):
entry = _make_entry(_versions_public_only())
logger = _make_migration_logger()
result = RestrictedEntry(entry, _make_approval_request(), logger).build()
Expand All @@ -391,7 +391,7 @@ def test_restricted_uses_public_files_when_no_epphapp(self):
assert "document.pdf" in result["versions"][1]["files"]
logger.add_information.assert_called()

def test_restricted_raises_when_no_files_at_all(self):
def test_restricted_raises_when_no_files_at_all(self, app):
versions = OrderedDict(
[
(
Expand All @@ -414,7 +414,7 @@ def test_restricted_raises_when_no_files_at_all(self):
class TestPublicEntryIdentifiers:
"""Test identifier handling in the public split."""

def test_public_removes_cern_ep_report_numbers(self):
def test_public_removes_cern_ep_report_numbers(self, app):
entry = _make_entry(_versions_with_epphapp())
result = PublicEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -429,7 +429,7 @@ def test_public_removes_cern_ep_report_numbers(self):
for i in identifiers
)

def test_public_keeps_non_ep_cdsrn(self):
def test_public_keeps_non_ep_cdsrn(self, app):
identifiers = [
{"identifier": RECID, "scheme": "cds"},
{"scheme": "cdsrn", "identifier": APPROVED_REPORT_NUMBER},
Expand All @@ -452,7 +452,7 @@ def test_public_keeps_non_ep_cdsrn(self):
class TestRestrictedEntryIdentifiers:
"""Test identifier handling in the restricted split."""

def test_restricted_removes_matching_cern_ep_rn(self):
def test_restricted_removes_matching_cern_ep_rn(self, app):
entry = _make_entry(_versions_with_epphapp())
result = RestrictedEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -466,7 +466,7 @@ def test_restricted_removes_matching_cern_ep_rn(self):

assert APPROVED_REPORT_NUMBER not in cdsrn_values

def test_restricted_keeps_draft_report_number(self):
def test_restricted_keeps_draft_report_number(self, app):
entry = _make_entry(_versions_with_epphapp())
result = RestrictedEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -480,7 +480,7 @@ def test_restricted_keeps_draft_report_number(self):

assert DRAFT_REPORT_NUMBER in cdsrn_values

def test_restricted_raises_on_mismatched_report_number(self):
def test_restricted_raises_on_mismatched_report_number(self, app):
identifiers = [
{"identifier": RECID, "scheme": "cds"},
{"scheme": "cdsrn", "identifier": "CERN-EP-2020-999"},
Expand All @@ -491,7 +491,7 @@ def test_restricted_raises_on_mismatched_report_number(self):
entry, _make_approval_request(), _make_migration_logger()
).build()

def test_restricted_removes_doi_pid(self):
def test_restricted_removes_doi_pid(self, app):
entry = _make_entry(_versions_with_epphapp(), has_doi=True)
result = RestrictedEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -503,15 +503,15 @@ def test_restricted_removes_doi_pid(self):
class TestPublicEntryModifications:
"""Test record/parent level modifications on the public split."""

def test_public_removes_request_data(self):
def test_public_removes_request_data(self, app):
entry = _make_entry(_versions_with_epphapp())
result = PublicEntry(
entry, _make_approval_request(), _make_migration_logger()
).build()

assert "_request_data" not in result["record"]

def test_public_sets_owned_by_system(self):
def test_public_sets_owned_by_system(self, app):
entry = _make_entry(_versions_with_epphapp())
result = PublicEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -520,7 +520,7 @@ def test_public_sets_owned_by_system(self):
assert result["record"]["owned_by"] == "system"
assert result["parent"]["json"]["access"]["owned_by"] == {"user": "system"}

def test_public_adds_cern_scientific_community(self):
def test_public_adds_cern_scientific_community(self, app):
entry = _make_entry(_versions_with_epphapp())
result = PublicEntry(
entry, _make_approval_request(), _make_migration_logger()
Expand All @@ -530,7 +530,7 @@ def test_public_adds_cern_scientific_community(self):
result["parent"]["json"]["communities"]["ids"]
)

def test_public_does_not_duplicate_community(self):
def test_public_does_not_duplicate_community(self, app):
entry = _make_entry(_versions_with_epphapp())
entry["parent"]["json"]["communities"]["ids"] = [
"example-community",
Expand All @@ -547,7 +547,7 @@ def test_public_does_not_duplicate_community(self):
class TestEntryImmutability:
"""Ensure build() deep-copies and does not mutate the original entry."""

def test_public_build_does_not_mutate_original(self):
def test_public_build_does_not_mutate_original(self, app):
entry = _make_entry(_versions_with_epphapp())
original = deepcopy(entry)
PublicEntry(entry, _make_approval_request(), _make_migration_logger()).build()
Expand All @@ -557,7 +557,7 @@ def test_public_build_does_not_mutate_original(self):
== original["record"]["json"]["metadata"]["identifiers"]
)

def test_restricted_build_does_not_mutate_original(self):
def test_restricted_build_does_not_mutate_original(self, app):
entry = _make_entry(_versions_with_epphapp())
original = deepcopy(entry)
RestrictedEntry(
Expand Down
Loading