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

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

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-importer#22
diging/citesphere-messages#12

Anything else the reviewer needs to know?

... describe here ...

…srefController

diging-jenkins · 2024-06-13T18:57:33Z

Can one of the admins verify this patch?

…into story/CITE-177

jdamerow · 2024-06-25T15:49:16Z

citesphere/pom.xml

        <citesphere.model.version>1.19</citesphere.model.version>
+        <crossref-connect-version>0.1</crossref-connect-version>


is there a reason citesphere uses a different version than the importer?

Changed version to 0.3-SNAPSHOT, which is latest version in both repositories.

jdamerow · 2024-06-25T20:18:01Z

citesphere/src/main/java/edu/asu/diging/citesphere/web/user/jobs/ImportCrossrefController.java

+        Map<String, Object> response = new HashMap<>();
+        try {
+            jobManager.createJob((IUser) authentication.getPrincipal(), groupId, dois);
+            System.out.println("=============================================================================================================");


jdamerow · 2024-06-25T20:19:44Z

.../src/test/java/edu/asu/diging/citesphere/core/service/impl/ImportCrossrefJobManagerTest.java

please follow our testing naming convention

jdamerow · 2024-07-10T21:12:04Z

citesphere/pom.xml

        <citesphere.model.version>1.19</citesphere.model.version>
+        <crossref-connect-version>0.3-SNAPSHOT</crossref-connect-version>


there is not 0.3-SNAPSHOT version. if you made changes to this plugin, there needs to be a pr (and that needs to be linked here).

Changed it to 0.2 as citesphere-import

citesphere/src/main/webapp/WEB-INF/views/auth/import/crossref.html

jdamerow · 2024-07-10T21:25:05Z

citesphere/src/main/webapp/WEB-INF/views/auth/import/crossref.html

+		</div>
+		<div id="failAlert" style="display: none;" class="alert alert-danger alert-dismissible fade in" role="alert">
+			<button type="button" class="close" data-dismiss="alert" aria-label="Close"><span aria-hidden="true">x</span></button>
+			Import Failed! Please check the logs


there are no logs the user could check, so this message is confusing.

Changed the message to actual error received from controller.

jdamerow · 2024-07-10T21:27:12Z

.../src/test/java/edu/asu/diging/citesphere/core/service/impl/ImportCrossrefJobManagerTest.java

+        assertNotNull(job);
+        assertEquals(JobStatus.FAILURE, job.getStatus());
+        assertEquals(1, job.getPhases().size());
+        assertEquals("Error creating message", job.getPhases().get(0).getMessage());


this is not worth checking; if the text changes the method logic won't change but it'll make the test fail.

jdamerow · 2024-08-01T18:22:31Z

citesphere/src/main/java/edu/asu/diging/citesphere/api/v1/user/JobInfoController.java

+            ArrayNode doisNode = mapper.createArrayNode();
+            ((IImportCrossrefJob)job).getDois().forEach(d -> doisNode.add(d));
+            node.set("dois", doisNode);
+            node.put("groupId", ((IImportCrossrefJob)job).getCitationGroup());


I think it's time to clean this up. let's create a map that maps object type to methods that are then called here.

jdamerow · 2024-08-01T18:28:07Z

citesphere/src/main/webapp/WEB-INF/views/auth/import/crossref.html

+					$("#failAlert").show();
+				}
+            }
+        });


something is off with the indentation

jdamerow · 2024-09-04T16:32:01Z

citesphere/src/main/java/edu/asu/diging/citesphere/api/v1/user/JobInfoController.java

+            ArrayNode doisNode = mapper.createArrayNode();
+            ((IImportCrossrefJob)job).getDois().forEach(d -> doisNode.add(d));
+            node.set("dois", doisNode);
+            node.put("groupId", ((IImportCrossrefJob)job).getCitationGroup());


jdamerow · 2024-09-04T17:15:25Z

citesphere/src/main/webapp/WEB-INF/views/auth/import/crossref.html

+					$("#failAlert").show();
+				}
+            }
+        });


…into story/CITE-177

jdamerow · 2024-09-10T20:17:31Z

citesphere/src/main/java/edu/asu/diging/citesphere/api/v1/user/JobInfoController.java

+            List<String> doisList = new ArrayList<>();
+            ((IImportCrossrefJob)job).getDois().forEach(d -> doisList.add(d));
+            reponse.put("dois", doisList);
+            reponse.put("groupId", ((IImportCrossrefJob)job).getCitationGroup());


Not sure where the comment went, but I made one here before about handling this differently (put the code in methods and map those to job types).

jdamerow · 2024-10-08T19:54:46Z

citesphere/src/main/java/edu/asu/diging/citesphere/api/v1/user/JobInfoController.java

+        } else if (job instanceof IExportJob) {
+            handleExportJob(response, (IExportJob) job);
+        } else if (job instanceof IImportCrossrefJob) {
+            handleImportCrossrefJob(response, (IImportCrossrefJob) job);


I think it's time to clean this up. let's create a map that maps object type to methods that are then called here.

Create a map and map class to method, then call map.get(job.getClass()) to get method to execute.

jdamerow and others added 6 commits June 20, 2022 16:55

[CITE-177] setup and created service to query crossref

2b1ce80

[CITE-177] front end and backend changes to start crossref import

557f26f

[CITE-177] Clearing the div before fetching the results

dacc76f

[CITE-177] Added group ID to Kafka message

709aa15

[CITE-177] Updated JobInfoController to add groupId to response

4791750

[CITE-177] Added test cases for ImportCrossrefJobManager & ImportCros…

0e74de6

…srefController

Merge branch 'develop' into story/CITE-177

90e71d2

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

PradnyaC11 requested a review from jdamerow June 13, 2024 19:05

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

PradnyaC11 added 2 commits June 13, 2024 15:38

[CITE-177] Updated crossref.html to show alert

8faa45c

Merge branch 'story/CITE-177' of https://github.com/diging/citesphere …

8b219a6

…into story/CITE-177

PradnyaC11 marked this pull request as ready for review June 13, 2024 22:41

jdamerow requested changes Jun 25, 2024

View reviewed changes

jdamerow closed this Jun 25, 2024

[CITE-177] Addressing the PR comments

836705d

PradnyaC11 reopened this Jul 2, 2024

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

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

5495a62

jdamerow requested changes Jul 10, 2024

View reviewed changes

jdamerow closed this Jul 10, 2024

[CITE-177] Addressed PR comments

d9ad221

PradnyaC11 reopened this Jul 11, 2024

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

jdamerow requested changes Aug 1, 2024

View reviewed changes

jdamerow closed this Aug 1, 2024

PradnyaC11 added 2 commits August 1, 2024 16:45

[CITE-177] Addressing PR comments

3fffd0e

[CITE-177] Addressing PR comments

7324581

PradnyaC11 reopened this Aug 5, 2024

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

This was referenced Aug 27, 2024

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

Merged

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

Open

jdamerow requested changes Sep 4, 2024

View reviewed changes

jdamerow closed this Sep 4, 2024

PradnyaC11 and others added 2 commits September 6, 2024 09:48

[CITE-177] Addressed PR comments

fc238b9

[CITE-177] Corrected indentation for crossref.html

4c7f0d9

PradnyaC11 reopened this Sep 6, 2024

Merge branch 'story/CITE-177' of https://github.com/diging/citesphere …

4c8c5ac

…into story/CITE-177

jdamerow requested changes Sep 10, 2024

View reviewed changes

jdamerow closed this Sep 10, 2024

[CITE-177] Addressed PR comments

ba4a409

PradnyaC11 reopened this Sep 20, 2024

PradnyaC11 requested a review from jdamerow September 20, 2024 23:14

jdamerow requested changes Oct 8, 2024

View reviewed changes

jdamerow closed this Oct 8, 2024

PradnyaC11 added 2 commits October 25, 2024 16:40

[CITE-177] Addressing PR comments

baf03ad

[CITE-177] Addressed PR comments

9afba45

PradnyaC11 reopened this Oct 28, 2024

PradnyaC11 requested a review from jdamerow October 28, 2024 22:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

PradnyaC11 commented Jun 13, 2024 •

edited

Loading

diging-jenkins commented Jun 13, 2024

jdamerow Jun 25, 2024

PradnyaC11 Jul 2, 2024

jdamerow Jun 25, 2024

PradnyaC11 Jul 2, 2024

jdamerow Jun 25, 2024

jdamerow Jul 10, 2024

PradnyaC11 Jul 11, 2024

jdamerow Jul 10, 2024

PradnyaC11 Jul 11, 2024

jdamerow Jul 10, 2024

jdamerow Aug 1, 2024

jdamerow Sep 4, 2024

jdamerow Aug 1, 2024

jdamerow Sep 4, 2024

jdamerow Sep 4, 2024

jdamerow Sep 4, 2024

jdamerow Sep 10, 2024

jdamerow Oct 8, 2024

		<citesphere.model.version>1.19</citesphere.model.version>
		<crossref-connect-version>0.1</crossref-connect-version>

		<citesphere.model.version>1.19</citesphere.model.version>
		<crossref-connect-version>0.3-SNAPSHOT</crossref-connect-version>

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

Are you sure you want to change the base?

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

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

Choose a reason for hiding this comment

PradnyaC11 commented Jun 13, 2024 •

edited

Loading