Story/cite-177 There needs to be an import from Crossref #22

PradnyaC11 · 2024-06-13T18:30:26Z

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]).

I have removed all code style changes that are not necessary (e.g. changing blanks across the whole file that don’t need to be changed, adding empty lines in parts other than your own code)
I am not making any changes to files that don’t have any effect (e.g. imports added that don’t need to be added)
I do not have any sysout statements in my code or commented out code that isn’t needed anymore
I am not reformatting any files in the wrong format or without cause.
I am not changing file encoding or line endings to something else than UTF-8, LF
My pull request does not show an insane amount of files being changed although my ticket only requires a few files being changed
I have added Javadoc/documentation where appropriate
I have added test cases where appropriate
I have explained any part of my code/implementation decisions that is not be self-explanatory

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 ...

…rossrefReferenceImportProcessor accordingly

diging-jenkins · 2024-06-13T18:30:29Z

Can one of the admins verify this patch?

jdamerow · 2024-07-05T19:04:05Z

citesphere-importer/pom.xml

@@ -15,7 +15,8 @@
        <spring-data.version>Lovelace-RELEASE</spring-data.version>
        <thymeleaf.version>3.0.11.RELEASE</thymeleaf.version>
        <spring.kafka.version>2.2.6.RELEASE</spring.kafka.version>
-        <citesphere.messages.version>0.2</citesphere.messages.version>
+        <citesphere.messages.version>0.5</citesphere.messages.version>
+        <crossref-connect-version>0.3-SNAPSHOT</crossref-connect-version>


the current version of crossref-connect is 0.2. Where there changes and is there a pull request? If so, it needs to be linked. If not, this needs to be the correct version.

jdamerow · 2024-07-05T19:07:55Z

...a/edu/asu/diging/citesphere/importer/core/service/impl/CrossrefReferenceImportProcessor.java

+
+        sendMessage(null, message.getId(), Status.PROCESSING, ResponseCode.P00);
+        BibEntryIterator bibIterator = null;
+        bibIterator = new CrossRefIterator(info);


merge with line 82

jdamerow · 2024-07-05T19:10:42Z

...a/edu/asu/diging/citesphere/importer/core/service/impl/CrossrefReferenceImportProcessor.java

+
+        response = response != null ? response : new ItemCreationResponse();
+        sendMessage(response, message.getId(), Status.DONE, ResponseCode.S00);  // giving error 500 as response code mentioned
+    }


this is a lot of duplicate code. This method should be in the superclass and in line 83 should getBibIterator() be called. The rest should then be the same. The logic of how the bibiterator is created will then be in that method.

jdamerow · 2024-07-05T19:17:09Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+        articles = new ArrayList<>();
+        for (String doi : info.getDois()) {
+            try {
+                Item item = crossrefService.get(doi);


The way it is written right now, this should be done in next (only one) and also paced to make sure we don't bombard crossref with requests. Even better would probably be to pass in a list of dois and make only one request but that might not be implemented either in our connector or in the crossref api.

jdamerow · 2024-07-05T19:20:11Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+            chair.setIds(Arrays.asList(contributorID));
+            contributors.add(chair);
+        }
+        // added Editors & translators to article meta. 


Removed the comment.

jdamerow · 2024-07-05T19:26:36Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+                ref.setReferenceId(itemRef.getKey());
+                ref.setReferenceString(itemRef.getUnstructured());
+                ref.setReferenceStringRaw(itemRef.getUnstructured());
+                references.add(ref);


content of for loop should go in its own method

jdamerow · 2024-07-05T19:26:49Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+        return meta;
+    }
+
+    public List<Contributor> mapPersonToContributor(List<Person> personList) {


does this need to be public?

Changed to private

jdamerow · 2024-07-05T19:29:41Z

...java/edu/asu/diging/citesphere/importer/core/zotero/template/impl/JsonGenerationService.java

@@ -41,8 +42,11 @@ public ObjectNode generateJson(JsonNode template, BibEntry entry) {
        ItemJsonGenerator generator = generators.get(entry.getArticleType());
        if (generator != null) {
            return generator.generate(template, entry);
+        } else if (entry instanceof Publication){
+            generator = generators.get("CrossRef");
+            return generator.generate(template, entry);


what is this code for?

Removed this block, as crossref generator is removed.

jdamerow · 2024-07-05T19:30:24Z

...va/edu/asu/diging/citesphere/importer/core/service/CrossrefReferenceImportProcessorTest.java

+    }
+
+    @Test
+    public void testStartImport_successful() throws Exception {


test naming conventions not followed

Deleted the tests as CrossrefRefernceImport does not have much now.

jdamerow · 2024-07-05T19:32:26Z

...va/edu/asu/diging/citesphere/importer/core/service/CrossrefReferenceImportProcessorTest.java

+    @Test
+    public void testStartImport_successful() throws Exception {
+        when(info.getDois()).thenReturn(Arrays.asList("10.1234/example1", "10.5678/example2"));
+        when(handlerRegistry.handleFile(info, null)).thenReturn(bibIterator);


the crossref importer doesn't have a handlerregistry, does it?

Deleted the tests as CrossrefRefernceImport does not have much now.

jdamerow · 2024-07-12T14:12:32Z

no need to make comments like "done" or "removed", etc. I just get more notifications and it drowns out the comments I really need to read and respond to.

jdamerow · 2024-08-01T16:40:10Z

...r/src/main/java/edu/asu/diging/citesphere/importer/core/service/AbstractImportProcessor.java

-        }
-        return file;
-    }
+    protected abstract BibEntryIterator getbibIterator(KafkaJobMessage message, JobInfo info);


should be getBibEntryIterator

jdamerow · 2024-08-01T16:42:58Z

...e-importer/src/main/java/edu/asu/diging/citesphere/importer/core/model/impl/Publication.java

+    public final static String EDITED_BOOK = "edited-book";
+    public final static String PROCEEDINGS_ARTICLE = "proceedings-article";
+    public final static String DISSERTATION = "dissertation";
+    public final static String BOOK_SECTION = "book-section";


this already exists (BOOK_CHAPTER)

jdamerow · 2024-08-01T16:44:36Z

...e-importer/src/main/java/edu/asu/diging/citesphere/importer/core/model/impl/Publication.java

@@ -12,6 +12,34 @@ public class Publication implements BibEntry {
    public final static String NEWS_ITEM = "newspaperArticle";
    public final static String PROCEEDINGS_PAPER = "conferencePaper";
    public final static String DOCUMENT = "document";
+    // publication types in CrossRef
+    public final static String MONOGRAPH = "monograph";


we can use BOOK instead

jdamerow · 2024-08-01T16:45:33Z

...e-importer/src/main/java/edu/asu/diging/citesphere/importer/core/model/impl/Publication.java

+    public final static String POSTED_CONTENT = "posted-content";
+    public final static String COMPONENT = "component";
+    public final static String EDITED_BOOK = "edited-book";
+    public final static String PROCEEDINGS_ARTICLE = "proceedings-article";


this is the same as PROCEEDINGS_PAPER I believe

jdamerow · 2024-08-01T16:49:34Z

...r/src/main/java/edu/asu/diging/citesphere/importer/core/service/AbstractImportProcessor.java

@@ -81,45 +65,105 @@ public void init() {
        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.MONOGRAPH, ItemType.BOOK);
+        itemTypeMapping.put(Publication.JOURNAL_ISSUE, ItemType.JOURNAL_ARTICLE);


this is wrong I believe, an issue is not an article. I think we will simply not deal with those and not import issues.

jdamerow · 2024-08-01T16:52:38Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+        meta.setIssns(issnList);
+
+        List<Contributor> contributors = new ArrayList<>();
+        if(item.getChair() != null) {


jdamerow · 2024-08-01T16:52:50Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+
+    private ArticleMeta parseArticleMeta(Item item) {
+        ArticleMeta meta = new ArticleMeta();
+        meta.setArticleTitle(String.join(", ", item.getTitle()));


jdamerow · 2024-08-01T16:53:07Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+                ref.setReferenceId(itemRef.getKey());
+                ref.setReferenceString(itemRef.getUnstructured());
+                ref.setReferenceStringRaw(itemRef.getUnstructured());
+                references.add(ref);


jdamerow · 2024-08-01T16:57:20Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+        BibEntry nextEntry = new Publication();;
+
+        try {
+            Item item = crossrefService.get(info.getDois().get(currentIndex));


instead of keeping an int around, just get an interator from the dois list and keep that

jdamerow · 2024-08-01T16:58:14Z

...java/edu/asu/diging/citesphere/importer/core/zotero/template/impl/JsonGenerationService.java

@@ -13,6 +13,7 @@
 import com.fasterxml.jackson.databind.node.ObjectNode;

 import edu.asu.diging.citesphere.importer.core.model.BibEntry;
+import edu.asu.diging.citesphere.importer.core.model.impl.Publication;


is this needed?

jdamerow · 2024-09-04T16:14:08Z

...r/src/main/java/edu/asu/diging/citesphere/importer/core/service/AbstractImportProcessor.java

I think the abstract import processor should still stay in the impl package, it's part implementation after all.

jdamerow · 2024-09-04T16:16:20Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+
+    private ArticleMeta parseArticleMeta(Item item) {
+        ArticleMeta meta = new ArticleMeta();
+        meta.setArticleTitle(String.join(", ", item.getTitle()));


jdamerow · 2024-09-04T16:16:51Z

...n/java/edu/asu/diging/citesphere/importer/core/service/parse/iterators/CrossRefIterator.java

+                ref.setReferenceId(itemRef.getKey());
+                ref.setReferenceString(itemRef.getUnstructured());
+                ref.setReferenceStringRaw(itemRef.getUnstructured());
+                references.add(ref);


jdamerow

unaddressed comments

jdamerow · 2024-10-10T15:58:01Z

Looks fine. Once the Citesphere PR is ready to be tested, I'll deploy this.

jdamerow and others added 22 commits July 7, 2022 16:57

[CITE-177] started on processor to import from crossref

0106646

[CITE-177] Trying to add functionality for starting the import

656fab7

[CITE-177] Trying to add functionality for picking the job.

a82d996

[CITE-177] Adding function for starting the import

d3aa18f

[CITE-177] Trying to add crossref import functionality

1710f06

[CITE-177] Trying to create Iterator for Crossref

10d8372

[CITE-177] Iterator changes.

27ee091

[CITE-177] Updating the Iterator and ImportProcessor

f0c4b61

[CITE-177] Adding iterator

58f195c

Adding iterator.

a2d4979

[CITE-177] Correcting the iterator

9616319

[CITE-177] Updating the Crossref Iterator

e3856ea

[CITE-177] CrossrefIterator and identifier

8c4545b

[CITE-177] Resolved error CrossrefReferenceImportProcessor class

c2bfbd0

[CITE-177] Added CrossRef types to CrossRefPublication, and updated C…

74bcd5f

…rossrefReferenceImportProcessor accordingly

[CITE-177] Updated CrossRefIterator

7ecdfd0

[CITE-177] Updated CrossRefIterator

41fed60

[CITE-177] updated generateJson method of JsonGenerationService

7f097df

[CITE-177] Updated itemTypeMapping in crossref import processer

46c2904

[CITE-177] Added more mapping in CrossRefInportProcessor

e36ff3d

[CITE-177] Udpated CrossRefIterator for typeMap and iterator logic

1c94f97

[CITE-177] Added test cases for CrossrefReferenceImportProcessor

bed4968

PradnyaC11 changed the title ~~Story/cite 177~~ Story/cite-177 There needs to be an import from Crossref Jun 13, 2024

PradnyaC11 added 2 commits June 13, 2024 11:38

[CITE-177] Refactoring code to fix issues.

e48be13

[CITE-177] Renamed file to remove unwanted file commit

c086f44

PradnyaC11 requested a review from jdamerow June 13, 2024 18:54

PradnyaC11 mentioned this pull request Jun 13, 2024

Story/cite 177: There needs to be an import from Crossref diging/citesphere#271

Open

9 tasks

PradnyaC11 marked this pull request as draft June 13, 2024 21:11

[CITE-177] Updated CrossRefIterator.java

287ffed

PradnyaC11 added 3 commits June 27, 2024 16:34

[CITE-177] Addressing PR comments

c4c4f13

[CITE-177] Addressed PR comments

eb6a3a1

[CITE-177] Addressed PR comments

a06f8af

PradnyaC11 reopened this Jul 2, 2024

PradnyaC11 requested a review from jdamerow July 2, 2024 22:53

[CITE-177] Changed crossref-connect-version in pom.xml

5767556

jdamerow requested changes Jul 5, 2024

View reviewed changes

jdamerow closed this Jul 5, 2024

PradnyaC11 added 2 commits July 10, 2024 16:42

[CITE-177] Addressed PR comments

bfcf53f

[CITE-177] Addressed PR comments

6670084

PradnyaC11 reopened this Jul 11, 2024

PradnyaC11 requested a review from jdamerow July 11, 2024 23:47

jdamerow closed this Jul 12, 2024

PradnyaC11 reopened this Jul 23, 2024

jdamerow requested changes Aug 1, 2024

View reviewed changes

jdamerow closed this Aug 1, 2024

[CITE-177] Addressing PR comments

0023dd2

PradnyaC11 reopened this Aug 5, 2024

PradnyaC11 requested a review from jdamerow August 5, 2024 22:17

PradnyaC11 mentioned this pull request Aug 27, 2024

[CITE-177] Adding the Kafka topic for Crossref diging/citesphere-messages#12

Merged

9 tasks

jdamerow requested changes Sep 4, 2024

View reviewed changes

jdamerow closed this Sep 4, 2024

[CITE-177] Addressing PR comments

d954231

PradnyaC11 reopened this Sep 5, 2024

jdamerow requested changes Sep 6, 2024

View reviewed changes

jdamerow closed this Sep 6, 2024

[CITE-177] Addressed PR comments

b7ded34

PradnyaC11 reopened this Sep 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Story/cite-177 There needs to be an import from Crossref #22

Story/cite-177 There needs to be an import from Crossref #22

PradnyaC11 commented Jun 13, 2024 •

edited

Loading

diging-jenkins commented Jun 13, 2024

jdamerow Jul 5, 2024

jdamerow Jul 5, 2024

jdamerow Jul 5, 2024

jdamerow Jul 5, 2024

jdamerow Jul 5, 2024

PradnyaC11 Jul 11, 2024

jdamerow Jul 5, 2024

jdamerow Aug 1, 2024

jdamerow Sep 4, 2024

jdamerow Jul 5, 2024

PradnyaC11 Jul 11, 2024

jdamerow Jul 5, 2024

PradnyaC11 Jul 11, 2024

jdamerow Jul 5, 2024

PradnyaC11 Jul 11, 2024

jdamerow Jul 5, 2024

PradnyaC11 Jul 11, 2024

jdamerow commented Jul 12, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Aug 1, 2024

jdamerow Sep 4, 2024

jdamerow Sep 4, 2024

jdamerow Sep 4, 2024

jdamerow left a comment

jdamerow commented Oct 10, 2024

Story/cite-177 There needs to be an import from Crossref #22

Are you sure you want to change the base?

Story/cite-177 There needs to be an import from Crossref #22

Conversation

PradnyaC11 commented Jun 13, 2024 • edited Loading

Guidelines for Pull Requests

Please provide a brief description of your ticket

Are there any other pull requests that this one depends on?

Anything else the reviewer needs to know?

diging-jenkins commented Jun 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdamerow commented Jul 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdamerow left a comment

Choose a reason for hiding this comment

jdamerow commented Oct 10, 2024

PradnyaC11 commented Jun 13, 2024 •

edited

Loading