add(load): EP approval record migration stream by zubeydecivelek · Pull Request #541 · CERNDocumentServer/cds-migrator-kit

zubeydecivelek · 2026-06-25T10:26:47Z

needs https://github.com/CERNDocumentServer/cds-rdm/tree/feature/ep-approval

This PR covers the migration of EP approved records.

We'll have a new loader (CDSEPApprovalRecordServiceLoad) for records that went through EP approval. It splits one legacy record into two RDM records, creates the EP approval request with the original history (submitter, approver, dates, report number), and links everything together with related identifiers and an APPRN.

Metadata split

EP report number (CERN-EP-YYYY-*): removed from both splits before load. After the request is created, the APPRN is minted on the restricted record and added to the public record metadata.
EP draft report number (CERN-EP-*): removed from public record before load.
DOI: removed from the restricted split only.
Access: public record is set to public; restricted record stays restricted.
Community: CERN Scientific community is added to the public record parent.
Legacy identifiers / side effects: legacy recid minting, original dump, CLC sync, and record-state logging only run on the public record.

Files split

Restricted record: EPPHAPP_FILE(restricted file for EP) files only if record has the restricted file, otherwise use all.
Public record: all other files (and it must not contain any remaining restricted files).

Loader

The loader creates 2 records and links them via:

an accepted EP approval request (submitted + accepted timeline from legacy history)
related identifiers (isversionof / isvariantformof)
APPRN on the public record

How to run

Records with ep_approval cannot use the standard record stream it'll raise a migration error. They must be migrated with --ep-approval.

invenio migration run --collection <collection-name> --ep-approval --dry-run

Result

For now this only handles records with a approved EP approval history (exactly one waiting and one approved entry).

Questions

If we also need to migrate records with waiting or rejected state, we would need a different split/load mechanism. Is this a case?
We'll migrate the accepted state records.
This record doesn’t have the epphapp restricted file, how do we split? we need restricted EPPHAPP_FILE https://cds.cern.ch/record/2864686/files/

zzacharo · 2026-06-29T12:59:08Z

+            "mint_legacy_recid": True,
+            "save_original_dump": True,
+            "clc_sync": True,
+            "record_state": True,


what does that mean? to generate or not the record state?

it means we'll not add the restricted record to record_state_logger, i can change the variable name if it's confusing

why would we have to skip a restricted one to the state logger?

this is due to the EP migration. Legacy CDS with EP approval is one public record which in the new system will become 2: one restricted internal + one public with the EP number. The latter will be used for redirection of the legacy one.

I think we discussed that it will change in the end, right?

kpsherva · 2026-07-03T14:50:04Z

        "Can also be set per-collection in streams.yaml under transform.workers."
    ),
 )
+@click.option(


how will we use it?

I explained in the PR description
#541 (comment)

invenio migration run --collection <collection-name> --ep-approval --dry-run

please remember to create a recipe following the other migrations we have done or doing https://gitlab.cern.ch/cds-team/cds-rdm-openshift/-/work_items?show=eyJpaWQiOiIxNSIsImZ1bGxfcGF0aCI6ImNkcy10ZWFtL2Nkcy1yZG0tb3BlbnNoaWZ0IiwiaWQiOjM3NTE2MX0%3D and take it into account there

kpsherva · 2026-07-03T14:54:51Z

            # Dump the computed record state. This is useful to migrate then the record stats
            if record_state_context:
-                self.record_state_logger.add_record_state(record_state_context)
+                if self._should_log_record_state():


are there cases when we shouldn't log record state? what cases are that?

We'll have 2 cds-rdm records and we want to use the public one for stats migration and redirections, that's why i'm setting it False if it's the restricted/draft record

an alternative would be to log every record and introduce a special attribute for the internal EP that then is not taken into account on stats and redirections. The only advantage with this is that record_state depicts the full picture of the migration

I added `internal record ` to the log, stats and redirections doesnt effected. But comments migration needs to change because it uses legacy_recid to get the migrated record. @kpsherva

palkerecsenyi

I don't yet have a huge amount of understanding of the migrator kit in general, but it looks good to me! Just two questions, but I can't see any major problems

palkerecsenyi · 2026-07-16T14:43:57Z

+        self._resolve_user_by_email(approved.get("submitted_by"), "approver")
+
+        # Check if record approved after the deadline
+        waiting_deadline = self._parse_legacy_datetime(waiting.get("deadline"))


What is this deadline? Is this the date after which if EP has not responded it gets auto-approved?

It's not auto-approved, it gives permission to the user requested the ep approval to approve. so it goes back to requester. For migration i'm just adding the deadline to the request. we'll not need this feature

palkerecsenyi · 2026-07-16T14:51:51Z

+                data={
+                    "title": f'EP approval for "{publication_title}"',
+                    # Use the default
+                    "payload": {},


The payload contains the following items, are we not storing these in CDS legacy?

I only have these https://github.com/CERNDocumentServer/cds-migrator-kit/pull/541/changes#diff-3e3d2c21565c476bee42ebd3dbf0ad13124747435717f3e1d51bf6cd74cc0e73R633-R640

I checked the legacy code, these info only used in emails in legacy so we dont have this information in DB or record marc

kpsherva · 2026-07-20T08:23:11Z

+                field="validation",
+                stage="load",
+                recid=recid,
+                priority="warning",


I think if there is an unknown error then it should be critical not only warning

kpsherva · 2026-07-20T08:25:35Z

+            "record_state": True,
+        }
+
+    def _restricted_load_flags(self):


it doesn't need to be a method, it is used only in one place - I would prefer if you place it in load split record, it will be better for readability of the code. Same for the method above - it only defines the dictionary that is used in one place

kpsherva · 2026-07-20T09:40:06Z

+
+        self._ep_approval_parsed = (waiting_entry, approved_entry, report_number)
+
+    def _create_ep_approval(self, restricted_record_state, legacy_recid):


this method should be moved to ApprovalRequest class

kpsherva · 2026-07-20T09:40:10Z

+        self._finalise_on_load = finalise
+        return super()._load(entry)
+
+    def _validate_ep_approval(self, ep_approval, legacy_recid):


this method should be moved to ApprovalRequest class

kpsherva · 2026-07-20T09:41:37Z

+            )
+        return {"user": str(user.id)}
+
+    def _existing_ep_approval_request(self, legacy_recid):


this method should be moved to ApprovalRequest class

kpsherva · 2026-07-20T09:41:42Z

+                continue
+        return None
+
+    def _get_ep_referee_group(self, restricted_parent):


this method should be moved to ApprovalRequest class

kpsherva · 2026-07-20T09:46:10Z

+EP_APPROVAL_REPORT_NUMBER_RE = re.compile(r"^CERN-EP-\d{4}-\d{3}$")
+
+
+class CDSEPApprovalRecordServiceLoad(CDSRecordServiceLoad):


Suggested change

class CDSEPApprovalRecordServiceLoad(CDSRecordServiceLoad):

class CDSEPApprovalRecordServiceLoad(Load):

kpsherva · 2026-07-20T13:10:10Z

+            self.approval_request = ApprovalRequest(
+                ep_approval=ep_approval,
+                legacy_recid=recid,
+                title=metadata.get("title"),
+                resource_type=metadata.get("resource_type"),
+                dry_run=self.dry_run,
+            )
+            self.approval_request.validate()


I would move these lines just before where you call .create for readability

When I split the entries i need the validated approval request

kpsherva · 2026-07-20T13:13:37Z

+        """Return waiting/approved history entries and the report number."""
+        if len(self.ep_approval) != 2:
+            raise UnexpectedValue(
+                message="EP approval history has more/less than 2 entries",


what does that mean in practice if it has 2 entries?

we should have waiting and accepted entries. if we are missing it should raise, now it doesnt support only waiting state

kpsherva · 2026-07-20T13:14:49Z

+        return ep_config["referee_group"]
+
+    @staticmethod
+    def _resolve_user_by_email(email, role):


do we always have emails of referees?

kpsherva · 2026-07-20T13:17:08Z

+        expires_at = self.parse_legacy_datetime(self.waiting_entry.get("deadline"))
+        referee_group = self._get_referee_group(restricted_parent)
+
+        with UnitOfWork() as uow:


why is uow needed here?

I change the request after create, and i think uow is useful so create doesn’t commit early, so all those changes can be registered and committed together. you think it's not needed?

kpsherva · 2026-07-20T13:20:53Z

+            self.migration_logger.add_log(exc, record=entry)
+
+    def _link_ep_approval_records(
+        self, restricted_record_state, public_record_state, legacy_recid


this method is mixing two actions - adding the request to parent of restricted record and public record and linking them together. I would rename this method as "add_request_to_parents" and move the related_identifier creation to have them right after the records are created.

kpsherva · 2026-07-20T13:22:19Z

+        ] = communities
+
+    def _should_remove_ep_report_number(self, identifier, public_split):
+        """Return whether an EP report number should be stripped from metadata."""


I am a bit confused why this check is needed - you will remove the EP report number only from the restricted_entry when you are preparing the metadata, right? so you know already that you should remove it because you are preparing the restricted entry

it should be apprn not cdsrn, so i'm removing from both records. Usually there's a draft report number like CERN-EP-DRAFT-FASER-2024-001 and we need to keep that one in the restricted record

kpsherva · 2026-07-20T13:36:43Z

+            public_entry = self._split_entry(entry, include_epphapp=False)
+            restricted_entry = self._split_entry(entry, include_epphapp=True)


ok sorry for digging deeper on the pattern but I see the opportunity here to encapsulate the responsibilities a bit futher

Suggested change

public_entry = self._split_entry(entry, include_epphapp=False)

restricted_entry = self._split_entry(entry, include_epphapp=True)

public_entry = PublicEntry(entry, include_epphapp=False).build()

restricted_entry = RestrictedEntry(entry, include_epphapp=True).build()

where

class MetadataEntry() def identifiers(entry): raise NotImplementedError def build(): return { "metadata": { "identifiers": self.idenfifiers(entry) ... and any other key of the entry that you need to modify based on if this is restricted record or not } } class PublicEntry(MetadataEntry): # all the dict manipulation for the public record go here, like adding cern research community def __init__(): def identifiers(): # you probably keep the identifiers unchanged here return identifiers class RestrictedEntry(MetadataEntry): # all the dict manipulations needed for the restricted record go here def identifiers(): # you remove the ep report number here return identifers

please let me know if we need to discuss the details in person

kpsherva · 2026-07-21T08:55:35Z

+class PublicEntry(MetadataEntry):
+    """Build the public EP approval split entry."""
+
+    def _build_versions(self, split):


are there any similarities or duplications we could make general for both public and restricted entry ?

zubeydecivelek force-pushed the ep-approval-load branch 2 times, most recently from 4619216 to dc83813 Compare June 25, 2026 13:49

zubeydecivelek moved this to In review 🔍 in Sprint Q3 2026 ☀️ Jun 26, 2026

zubeydecivelek added this to Sprint Q3 2026 ☀️ Jun 26, 2026

zubeydecivelek force-pushed the ep-approval-load branch 2 times, most recently from cf8176b to 6de53c6 Compare June 26, 2026 14:58

zubeydecivelek requested review from 0einstein0, kpsherva and zzacharo and removed request for 0einstein0 and kpsherva June 29, 2026 08:51

0einstein0 reviewed Jun 29, 2026

View reviewed changes

Comment thread cds_migrator_kit/rdm/records/load/ep_approval_load.py Outdated

Comment thread cds_migrator_kit/rdm/records/load/ep_approval_load.py Outdated

zzacharo reviewed Jun 29, 2026

View reviewed changes

kpsherva reviewed Jul 3, 2026

View reviewed changes

Comment thread cds_migrator_kit/rdm/records/transform/transform.py Outdated

kpsherva reviewed Jul 3, 2026

View reviewed changes

TahaKhan998 moved this from In review 🔍 to In progress in Sprint Q3 2026 ☀️ Jul 6, 2026

TahaKhan998 assigned zubeydecivelek Jul 6, 2026

kpsherva reviewed Jul 9, 2026

View reviewed changes

Comment thread cds_migrator_kit/rdm/records/load/ep_approval_load.py Outdated

zubeydecivelek force-pushed the ep-approval-load branch 2 times, most recently from dedbdb7 to 7b9b117 Compare July 14, 2026 08:47

zubeydecivelek moved this from In progress to Ready in Sprint Q3 2026 ☀️ Jul 14, 2026

zubeydecivelek moved this from Ready to In progress in Sprint Q3 2026 ☀️ Jul 14, 2026

zubeydecivelek force-pushed the ep-approval-load branch from 7b9b117 to 2bcb422 Compare July 14, 2026 15:56

zubeydecivelek requested a review from palkerecsenyi July 15, 2026 16:55

palkerecsenyi reviewed Jul 16, 2026

View reviewed changes

kpsherva reviewed Jul 20, 2026

View reviewed changes

Comment thread cds_migrator_kit/rdm/records/load/ep_approval_load.py Outdated

zubeydecivelek force-pushed the ep-approval-load branch 2 times, most recently from e4d2f5f to 52c508e Compare July 20, 2026 12:45

kpsherva reviewed Jul 20, 2026

View reviewed changes

Comment thread cds_migrator_kit/rdm/records/load/ep_approval_load.py Outdated

kpsherva reviewed Jul 20, 2026

View reviewed changes

Comment thread cds_migrator_kit/rdm/records/load/ep_approval_load.py Outdated

kpsherva reviewed Jul 20, 2026

View reviewed changes

Comment thread cds_migrator_kit/rdm/records/load/ep_approval_load.py Outdated

kpsherva reviewed Jul 20, 2026

View reviewed changes

zubeydecivelek force-pushed the ep-approval-load branch from 52c508e to 488a3ad Compare July 20, 2026 13:59

zubeydecivelek added 3 commits July 21, 2026 10:51

add(load): EP approval record migration stream

3ba20ba

change(ep-approve-load): add approval request class

aa864e7

add(ep-approval): new entry to split the records

f46917b

zubeydecivelek force-pushed the ep-approval-load branch from 1877efc to f46917b Compare July 21, 2026 08:54

kpsherva reviewed Jul 21, 2026

View reviewed changes

add(tests): ep approval split entry

2bffd96

zubeydecivelek force-pushed the ep-approval-load branch from ad8d63a to 2bffd96 Compare July 21, 2026 09:41

zubeydecivelek moved this from In progress to In review 🔍 in Sprint Q3 2026 ☀️ Jul 21, 2026

kpsherva merged commit d56ce92 into CERNDocumentServer:master Jul 21, 2026
2 checks passed

github-project-automation Bot moved this from In review 🔍 to To release 🤖 in Sprint Q3 2026 ☀️ Jul 21, 2026

kpsherva moved this from To release 🤖 to Released ✔️ in Sprint Q3 2026 ☀️ Jul 21, 2026


		self._ep_approval_parsed = (waiting_entry, approved_entry, report_number)

		def _create_ep_approval(self, restricted_record_state, legacy_recid):

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


		class CDSEPApprovalRecordServiceLoad(CDSRecordServiceLoad):

	class CDSEPApprovalRecordServiceLoad(CDSRecordServiceLoad):
	class CDSEPApprovalRecordServiceLoad(Load):

		public_entry = self._split_entry(entry, include_epphapp=False)
		restricted_entry = self._split_entry(entry, include_epphapp=True)

Uh oh!

Conversation

zubeydecivelek commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Metadata split

Files split

Loader

How to run

Result

Questions

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kpsherva Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

palkerecsenyi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zubeydecivelek commented Jun 25, 2026 •

edited

Loading

kpsherva Jul 9, 2026 •

edited

Loading

zubeydecivelek Jul 20, 2026 •

edited

Loading

kpsherva Jul 20, 2026 •

edited

Loading

kpsherva Jul 20, 2026 •

edited

Loading