Skip to content

add(load): EP approval record migration stream#541

Open
zubeydecivelek wants to merge 1 commit into
CERNDocumentServer:masterfrom
zubeydecivelek:ep-approval-load
Open

add(load): EP approval record migration stream#541
zubeydecivelek wants to merge 1 commit into
CERNDocumentServer:masterfrom
zubeydecivelek:ep-approval-load

Conversation

@zubeydecivelek

@zubeydecivelek zubeydecivelek commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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.
  • 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?
  • 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/

@zubeydecivelek
zubeydecivelek force-pushed the ep-approval-load branch 2 times, most recently from 4619216 to dc83813 Compare June 25, 2026 13:49
@zubeydecivelek zubeydecivelek moved this to In review 🔍 in Sprint Q2 2026 ☀️ Jun 26, 2026
@zubeydecivelek
zubeydecivelek force-pushed the ep-approval-load branch 2 times, most recently from cf8176b to 6de53c6 Compare June 26, 2026 14:58
@zubeydecivelek
zubeydecivelek requested review from 0einstein0, kpsherva and zzacharo and removed request for 0einstein0 and kpsherva June 29, 2026 08:51
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
"mint_legacy_recid": True,
"save_original_dump": True,
"clc_sync": True,
"record_state": True,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@kpsherva kpsherva Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# TODO: What if there are multiple experiments?
experiments = record_json.get("custom_fields", {}).get(
"cern:experiments", []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we raise and identify if there is any case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it's removed from ep-approval metadata, i'm also removing so we dont need to check

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
for _, version_data in split.get("versions", {}).items():
current_version_files = OrderedDict()

for key, file_data in version_data.get("files", {}).items():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking....what about if we just copy all files in the restricted record unconditionally and keep only the public ones for the public record? if an EPPHAPP_FILE_TYPE exsits then we could use this restriction for the restricted record for all files otherwise just restricted files i.e members of the community. That will solve also the record you found that didnt have any restricted file. wdyt @kpsherva ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed, We'll have duplicated files in both records if there's no EPPHAPP_FILE_TYPE. Draft record(the record requested the approval) will be always restricted record

Comment thread cds_migrator_kit/rdm/migration_config.py Outdated
"Can also be set per-collection in streams.yaml under transform.workers."
),
)
@click.option(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how will we use it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I explained in the PR description
#541 (comment)

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread cds_migrator_kit/rdm/records/transform/transform.py Outdated
# 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():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@TahaKhan998 TahaKhan998 moved this from In review 🔍 to In progress in Sprint Q2 2026 ☀️ Jul 6, 2026
Comment thread cds_migrator_kit/rdm/records/load/ep_approval_load.py Outdated
@zubeydecivelek
zubeydecivelek force-pushed the ep-approval-load branch 2 times, most recently from dedbdb7 to 7b9b117 Compare July 14, 2026 08:47
@zubeydecivelek zubeydecivelek moved this from In progress to Ready in Sprint Q2 2026 ☀️ Jul 14, 2026
@zubeydecivelek zubeydecivelek moved this from Ready to In progress in Sprint Q2 2026 ☀️ Jul 14, 2026

@palkerecsenyi palkerecsenyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants