fix(harvester): do not create records for unmatched EP approvals - #922
Open
TahaKhan998 wants to merge 1 commit into
Open
fix(harvester): do not create records for unmatched EP approvals#922TahaKhan998 wants to merge 1 commit into
TahaKhan998 wants to merge 1 commit into
Conversation
6 tasks
kpsherva
reviewed
Aug 18, 2026
| ) | ||
| if record.to_dict().get("access", {}).get("record") == "restricted": | ||
| raise WriterError( | ||
| "EP approval number matched a restricted record. " |
Contributor
There was a problem hiding this comment.
Suggested change
| "EP approval number matched a restricted record. " | |
| "EP approval number matched a restricted record - record must be public to be updated by the harvester" |
just to explain to the curators what should happen
kpsherva
reviewed
Aug 18, 2026
| else: | ||
| if apprn: | ||
| raise WriterError( | ||
| "EP approval number did not match an existing record. " |
Contributor
There was a problem hiding this comment.
Suggested change
| "EP approval number did not match an existing record. " | |
| "EP approval number did not match an existing record - EP approval numbers can't be assigned outside CDS publishing workflow." |
TahaKhan998
force-pushed
the
fix/issue-906-ep-approval-no-create
branch
from
August 19, 2026 11:30
8913cba to
7585b20
Compare
kpsherva
reviewed
Aug 19, 2026
Comment on lines
+91
to
+94
| apprn = self.matcher._retrieve_identifier( | ||
| stream_entry.entry.get("metadata", {}).get("identifiers", []), | ||
| "apprn", | ||
| ) |
Contributor
There was a problem hiding this comment.
these checks should not be implemented in the route step - if you also consider the other PR which you worked on the legacy recid matching - we will end up with a very unreadable method which has multiple responsibilities to handle - which is not the best practice.
these checks should have an additional validation class, organised similarly (composition) as the mappers - different validation workflow while updating the record and different while creating.
we are aiming to see, inside _update_record method something like
self.record_validator.validate(mode="update")
similar for the create mode
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
part of #906
If an INSPIRE record has an EP approval number and it does not match an existing CDS record, the harvester raises an error and does not create a new one. Same if it matches a restricted record, that means they forgot to publish the internal version after the approval. If it matches a public record we update it as usual.