From 810b038c7fb278e4cb7ec57c4743e2256adfce2f Mon Sep 17 00:00:00 2001 From: Karolina Przerwa Date: Fri, 24 Jul 2026 14:49:07 +0200 Subject: [PATCH] change(ep): make sure correct env var is used for communities --- cds_migrator_kit/rdm/migration_config.py | 9 +++- .../rdm/records/load/ep_approval_entry.py | 30 ++++++++---- scripts/copy_collection_files.py | 4 +- tests/cds-rdm/test_ep_approval_entry.py | 46 +++++++++---------- 4 files changed, 55 insertions(+), 34 deletions(-) diff --git a/cds_migrator_kit/rdm/migration_config.py b/cds_migrator_kit/rdm/migration_config.py index 75068681..51b226f7 100644 --- a/cds_migrator_kit/rdm/migration_config.py +++ b/cds_migrator_kit/rdm/migration_config.py @@ -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": { diff --git a/cds_migrator_kit/rdm/records/load/ep_approval_entry.py b/cds_migrator_kit/rdm/records/load/ep_approval_entry.py index de15e082..108d9473 100644 --- a/cds_migrator_kit/rdm/records/load/ep_approval_entry.py +++ b/cds_migrator_kit/rdm/records/load/ep_approval_entry.py @@ -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.""" @@ -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" @@ -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" diff --git a/scripts/copy_collection_files.py b/scripts/copy_collection_files.py index e6302bb3..22ae352a 100644 --- a/scripts/copy_collection_files.py +++ b/scripts/copy_collection_files.py @@ -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 diff --git a/tests/cds-rdm/test_ep_approval_entry.py b/tests/cds-rdm/test_ep_approval_entry.py index 6d0ba064..33bd9d3b 100644 --- a/tests/cds-rdm/test_ep_approval_entry.py +++ b/tests/cds-rdm/test_ep_approval_entry.py @@ -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() @@ -231,7 +231,7 @@ 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() @@ -239,7 +239,7 @@ def test_public_deduplicates_identical_versions(self): 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() @@ -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( [ ( @@ -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. @@ -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( [ ( @@ -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() @@ -364,7 +364,7 @@ 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() @@ -372,7 +372,7 @@ def test_restricted_keeps_all_changing_epphapp_versions(self): 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() @@ -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() @@ -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( [ ( @@ -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() @@ -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}, @@ -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() @@ -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() @@ -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"}, @@ -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() @@ -503,7 +503,7 @@ 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() @@ -511,7 +511,7 @@ def test_public_removes_request_data(self): 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() @@ -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() @@ -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", @@ -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() @@ -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(