fix(harvester): merge new abstracts and author identifiers on update - #950
fix(harvester): merge new abstracts and author identifiers on update#950TahaKhan998 wants to merge 1 commit into
Conversation
e4e62a3 to
18c4b99
Compare
| (p.get("family_name") or "").lower(), | ||
| (p.get("given_name") or "").lower(), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
do we have tests covering different possible corner cases?
18c4b99 to
274d6fa
Compare
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.