deal with another alternate orcid format for contributors #1059
Conversation
app/services/orcid_builder.rb
Outdated
| @@ -48,6 +48,9 @@ def orcidid(contributor) | |||
| return identifier.uri if identifier.uri | |||
| return identifier.value if identifier.value.start_with?('https://orcid.org/') | |||
|
|
|||
| # some records have just the ORCIDID without the URL prefix, add it if so, e.g. druid:tp865ng1792 | |||
| return URI.join('https://orcid.org/', identifier.value).to_s if identifier.source.uri.blank? | |||
There was a problem hiding this comment.
Is there ever a case in which identifier.source.uri isn't https://orcid.org or blank? If not, how about getting rid of L52 and replacing L54 with URI.join('https://orcid.org/', identifier.value).to_s.
There was a problem hiding this comment.
That's a good question ... I support in theory it could be "sandbox" for test data, or maybe it might change in the future? Seems unlikely though.
Our test do verify it works with an alternate URI and I think it was speced this way, but unclear if that's a common situation.
There was a problem hiding this comment.
Since the rest of orcid_builder assumes that you can just used https://orcid.org when it is missing, seems fine to always use it.
There was a problem hiding this comment.
changed (and also in orcid_client PR)
There was a problem hiding this comment.
Is it ok to put a [HOLD] on this until we've got some clarity around how ORCID identifiers are structured in Cocina?
Why was this change made? 🤔
Same thing as sul-dlss/orcid_client#30, i.e. we have varying formats of how ORCIDs are represented in contributors. This deals with another case that caused https://app.honeybadger.io/projects/50568/faults/102209790
We also needed to add to orcid_client until we consolidate as described in sul-dlss-deprecated/dor_indexing#15
How was this change tested? 🤨
Updated spec