Story/cite-177 There needs to be an import from Crossref#22
Story/cite-177 There needs to be an import from Crossref#22PradnyaC11 wants to merge 41 commits into
Conversation
…rossrefReferenceImportProcessor accordingly
|
Can one of the admins verify this patch? |
|
Looks fine. Once the Citesphere PR is ready to be tested, I'll deploy this. |
| private ArticleMeta parseArticleMeta(Item item) { | ||
| ArticleMeta meta = new ArticleMeta(); | ||
| meta.setArticleTitle(item.getTitle().get(0)); | ||
| meta.setShortTitle(String.join(", ", item.getTitle().subList(1, item.getTitle().size()))); |
There was a problem hiding this comment.
That doesn't seem right. Why would the short title be a concatenation of all other titles? If zotero doesn't allow additional titles, then we might just want to drop them for now.
| contributors.addAll(mapPersonToContributor(item.getTranslator(), ContributionType.TRANSLATOR)); | ||
| } | ||
| // List of chair | ||
| if(item.getChair() != null) { |
There was a problem hiding this comment.
is that all contributor types that crossref knows?
There was a problem hiding this comment.
Yes. crossref has 4 types of contributors - authors, editors, translators & chair
| } | ||
| meta.setContributors(contributors); | ||
|
|
||
| meta.setAuthorNotesCorrespondence(null); |
| meta.setLanguage(item.getLanguage()); | ||
| ReviewInfo review = new ReviewInfo(); | ||
| if (item.getReview() != null) { | ||
| review.setFullDescription(item.getReview().getCompetingInterestStatement()); |
There was a problem hiding this comment.
is full description the same as competing interest statement?
| private Reference mapSingleReference(edu.asu.diging.crossref.model.Reference itemRef) { | ||
| Reference ref = new Reference(); | ||
| ref.setAuthorString(itemRef.getAuthor()); | ||
| ref.setContributors(null); |
There was a problem hiding this comment.
same as before, isn't that the default value?
| itemTypeMapping.put(Publication.NEWS_ITEM, ItemType.NEWSPAPER_ARTICLE); | ||
| itemTypeMapping.put(Publication.PROCEEDINGS_PAPER, ItemType.CONFERENCE_PAPER); | ||
| itemTypeMapping.put(Publication.DOCUMENT, ItemType.DOCUMENT); | ||
| itemTypeMapping.put(Publication.BOOK, ItemType.BOOK); |
| itemTypeMapping.put(Publication.EDITED_BOOK, ItemType.BOOK); | ||
| itemTypeMapping.put(Publication.PROCEEDINGS_PAPER, ItemType.CONFERENCE_PAPER); | ||
| itemTypeMapping.put(Publication.DISSERTATION, ItemType.THESIS); | ||
| itemTypeMapping.put(Publication.BOOK_CHAPTER, ItemType.BOOK_SECTION); |
| @@ -0,0 +1,157 @@ | |||
| package edu.asu.diging.citesphere.importer.core.service.parse.crossref; | |||
There was a problem hiding this comment.
should be moved to an impl package
| import edu.asu.diging.crossref.model.Person; | ||
|
|
||
| @Component | ||
| public class CrossRefParser implements ICrossRefParser { |
| } | ||
|
|
||
| @Test | ||
| public void testParseJournalMeta_WithValidData() { |
There was a problem hiding this comment.
test method names should follow our naming convention (documented in confluence)
| @Override | ||
| public ContainerMeta parseJournalMeta(Item item) { | ||
| ContainerMeta meta = new ContainerMeta(); | ||
| meta.setContainerTitle(item.getContainerTitle().get(0)); |
There was a problem hiding this comment.
Yes, crossref API is returning list of strings for title
| @Override | ||
| public ArticleMeta parseArticleMeta(Item item) { | ||
| ArticleMeta meta = new ArticleMeta(); | ||
| meta.setArticleTitle(item.getTitle().get(0)); |
There was a problem hiding this comment.
same here, are there more than one?
There was a problem hiding this comment.
Yes, crossref API is returning list of strings for title
| if(dateParts != null) { | ||
| publicationDate.setPublicationDate(dateParts.get(2).toString()); | ||
| publicationDate.setPublicationMonth(dateParts.get(1).toString()); | ||
| publicationDate.setPublicationYear(dateParts.get(0).toString()); |
There was a problem hiding this comment.
are all of these always set or could date be null for example?
There was a problem hiding this comment.
publicationDate will be null, only if the published date is missing. That was case for some citations. Hance, the check for dateParts is added.
There was a problem hiding this comment.
I mean, can dateParts.get(2) return null? If so, toString() would throw an exception I believe?
|
Make it so, Jenkins. |
2 similar comments
|
Make it so, Jenkins. |
|
Make it so, Jenkins. |
|
I get compilation errors on Jenkins. I think there are some dependencies missing that are needed for the upgrade. Also, can you please update the Java version in the pom to 11? |
Guidelines for Pull Requests
If you haven't yet read our code review guidelines, please do so, You can find them here.
Please confirm the following by adding an x for each item (turn
[ ]into[x]).Please provide a brief description of your ticket
The user should be able to search in crossref to references to import. whatever search results from Crossref a user selects should then be imported into citesphere. This could be all search results or just a few or one.
For now, let's put add an entry to the import menu and let the user select which group to import the results into from a dropdown menu.
The importing should be done from Citesphere Importer (not Citesphere), similar how entries from files are imported.
For querying crossref, this library should be used: https://github.com/diging/crossref-connect. However, this is not yet released into maven central, so talk to Julia before picking up this ticket.
... Put ticket description here and add link to ticket ...
https://diging.atlassian.net/browse/CITE-177
Are there any other pull requests that this one depends on?
diging/citesphere-messages#12
diging/citesphere#271
Anything else the reviewer needs to know?
... describe here ...