Skip to content
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 Crossref REST translator #3359

Merged
merged 4 commits into from
Sep 14, 2024
Merged

Conversation

mrtcode
Copy link
Member

@mrtcode mrtcode commented Sep 12, 2024

I noticed there are some differences in the available data compared to the XML API. For example, fields like ISBN, pages, editors, contributors, and language, etc. can sometimes be missing, but not always. However, this translator performs better with various book-related items.

It was tested with all item types provided by the Crossref API, and each item type is also included in the tests.

All in all, I think it's pretty good now.

The hyphen has been removed from the translator file name to match the actual translator name (there was an issue with Scaffold). I wonder if this could have any negative consequences?

@mrtcode
Copy link
Member Author

mrtcode commented Sep 12, 2024

To run test with Scaffold detectSearch has to be changed to return true.

@dstillman
Copy link
Member

We should set Library Catalog to just "Crossref", not "Crossref REST".

Crossref REST.js Outdated
}
// Add DOI to extra for unsupported items
else {
item.extra = (item.extra ? item.extra + '\n' : '') + 'DOI: ' + result.DOI;
Copy link
Member

@dstillman dstillman Sep 12, 2024

Choose a reason for hiding this comment

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

We shouldn't need to do this manually. It should get set automatically if not valid for the type.

Copy link
Member

Choose a reason for hiding this comment

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

(But that might not show up in tests.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it was exactly like that.

@mrtcode
Copy link
Member Author

mrtcode commented Sep 12, 2024

We should set Library Catalog to just "Crossref", not "Crossref REST".

Fixed. However, I imagine it will be overridden, as is the case with the XML translator.

@dstillman dstillman merged commit 4167397 into zotero:master Sep 14, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants