-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update google drive action #869
Conversation
: undefined, | ||
"arcadia:content": comment.text, | ||
"arcadia:publication-date": comment.createdAt, | ||
"arcadia:full-name": comment.author.fullName, | ||
"arcadia:orcid": comment.author.orcid, | ||
"arcadia:orcid": `https://orcid.org/${comment.author.orcid}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current use pages on legacy just render the orcid value - (and construct the URL for the href). Is there a reason to store the whole URL, as opposed to just the ORCID? Somehow feels cleaner to me to construct the URL when we need, than to try and parse out just the ID value when we need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Storing them as just the ID part causes a bunch of problems on Legacy because ORCIDs are technically fully qualified RDF IRIs — the https:// is part of the ID — and people are used to pasting in the full URL on every other system. All the validators are built assuming this. Etc. So on Legacy we're parsing in and parsing out all the time anyway. They're useless as the numbers alone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does fix the anchor validation issue. The problem I'm bumping into now is that it doesn't remove or check for duplicate IDs on versions or discussions when re-run, and errors on that. I knew that was one of the things still to fix.
My preference would be for ORCID to be a string, not URL type, and to just store the ID - not the full https...
. If there's a good reason not to do it that way though, this should work as is.
See comment above for why we should store ORCID as a URL. Re: duplicates...what's the error? My assumption would be that it would successfully create duplicates via the upsert. |
b492252
to
2569317
Compare
Issue(s) Resolved
High-level Explanation of PR
Test Plan
Screenshots (if applicable)
Notes