Skip to content

fix(harvester): merge new abstracts and author identifiers on update - #950

Open
TahaKhan998 wants to merge 1 commit into
CERNDocumentServer:masterfrom
TahaKhan998:fix/harvester-update-strategies
Open

fix(harvester): merge new abstracts and author identifiers on update#950
TahaKhan998 wants to merge 1 commit into
CERNDocumentServer:masterfrom
TahaKhan998:fix/harvester-update-strategies

Conversation

@TahaKhan998

Copy link
Copy Markdown

Two things Fleur reported after editing records in INSPIRE didn't make it into CDS. The first one is the Italian abstract she added, which the mapper picks up fine but the update strategies didn't have an entry for additional descriptions, so it just got dropped, and now it's added as an append-only merge. The second one is the ORCID she added to the author, which got lost because each author gets one key for matching and it's the ORCID if there is one and the name otherwise, so the stored author was keyed by name and the incoming one by ORCID, they never matched and nothing was updated. Now every author lists all their keys, identifiers and name, and we try the identifiers first and fall back to the name, so an author who gains an ORCID upstream still matches the one we have and the ORCID gets merged in. The only exception is when both sides already have an ORCID and they're different, then it's two different people with the same name and we don't merge them.

@TahaKhan998 TahaKhan998 moved this to In review 🔍 in Sprint Q3 2026 ☀️ Aug 31, 2026
@TahaKhan998
TahaKhan998 force-pushed the fix/harvester-update-strategies branch from e4e62a3 to 18c4b99 Compare August 31, 2026 15:47
Comment on lines +96 to +97
(p.get("family_name") or "").lower(),
(p.get("given_name") or "").lower(),

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.

I am not sure I understand how this part will behave - we can't be relying on matching only family name or only given name because it will lead to false positives, no?
ex. Karolina ABC and Karolina EDF are different names but if you use given name as matching key, they will be matched incorrectly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So for your examples if INSPIRE sends Karolina ABC and Karolina EDF, theres no comma in the name so the mapper cant split it and the whole string just goes into family name. That gives you keys like ("name", "karolina abc", "", "") and ("name", "karolina edf", "", "") . Different keys so they dont match. If INSPIRE sends first name and last name separately instead you get ("name", "abc", "karolina", "abc, karolina") and ("name", "edf", "karolina", "edf, karolina"). Still different because the family name part is different even though given name is the same both times. So we are not matching on given name alone, the whole tuple has to match.
What happens is it first tries to match on ORCID if there is one and only if that doesnt work it tries the name key. The case we're fixing is CDS has the author keyed by name only INSPIRE now has an ORCID and the ORCID lookup finds nothing but the name key matches so we merge and the ORCID gets added. But if both sides already have an ORCID and theyre different we dont fall back to the name even if it would match so two different people called Karolina dont get merged.

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.

do we have tests covering different possible corner cases?

@TahaKhan998
TahaKhan998 force-pushed the fix/harvester-update-strategies branch from 18c4b99 to 274d6fa Compare September 1, 2026 13:21
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.

2 participants