add(load): EP approval record migration stream#541
Conversation
4619216 to
dc83813
Compare
cf8176b to
6de53c6
Compare
| "mint_legacy_recid": True, | ||
| "save_original_dump": True, | ||
| "clc_sync": True, | ||
| "record_state": True, |
There was a problem hiding this comment.
what does that mean? to generate or not the record state?
There was a problem hiding this comment.
it means we'll not add the restricted record to record_state_logger, i can change the variable name if it's confusing
There was a problem hiding this comment.
why would we have to skip a restricted one to the state logger?
There was a problem hiding this comment.
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", [] |
There was a problem hiding this comment.
shall we raise and identify if there is any case?
There was a problem hiding this comment.
since it's removed from ep-approval metadata, i'm also removing so we dont need to check
| for _, version_data in split.get("versions", {}).items(): | ||
| current_version_files = OrderedDict() | ||
|
|
||
| for key, file_data in version_data.get("files", {}).items(): |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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
| "Can also be set per-collection in streams.yaml under transform.workers." | ||
| ), | ||
| ) | ||
| @click.option( |
There was a problem hiding this comment.
I explained in the PR description
#541 (comment)
invenio migration run --collection <collection-name> --ep-approval --dry-run
There was a problem hiding this comment.
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
| # 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(): |
There was a problem hiding this comment.
are there cases when we shouldn't log record state? what cases are that?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
dedbdb7 to
7b9b117
Compare
7b9b117 to
2bcb422
Compare
palkerecsenyi
left a comment
There was a problem hiding this comment.
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")) |
There was a problem hiding this comment.
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": {}, |

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
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.CERN-EP-*): removed from public record before load.Files split
EPPHAPP_FILE(restricted file for EP) files only.Loader
The loader creates 2 records and links them via:
isversionof/isvariantformof)How to run
Records with
ep_approvalcannot use the standard record stream it'll raise a migration error. They must be migrated with--ep-approval.Result
For now this only handles records with a approved EP approval history (exactly one
waitingand oneapprovedentry).Questions
waitingorrejectedstate, we would need a different split/load mechanism. Is this a case?EPPHAPP_FILEhttps://cds.cern.ch/record/2864686/files/