Story/cite 172 - From the managed authority page, there should be a link that lists all citations that reference that authority #272

PradnyaC11 · 2024-06-18T22:06:23Z

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

Authorities are used for editors, authors, etc. The managed authority page lists all authorities used by a user or group. When I'm on the managed authority page, there should be a link for each authority that brings up all the citations that use that specific authority.

... Put ticket description here and add link to ticket ...
https://diging.atlassian.net/browse/CITE-172

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

diging/citesphere-model#55

Anything else the reviewer needs to know?

... describe here ...

…er to service

diging-jenkins · 2024-06-18T22:06:26Z

Can one of the admins verify this patch?

jdamerow · 2024-06-26T16:40:11Z

top part of PR is not completely filled out.

jdamerow · 2024-09-06T18:39:10Z

citesphere/pom.xml

@@ -25,7 +25,7 @@
        <spring.kafka.version>2.2.6.RELEASE</spring.kafka.version>
        <spring-social-zotero.version>0.12</spring-social-zotero.version>
        <citesphere.messages.version>0.4</citesphere.messages.version>
-        <citesphere.model.version>1.19</citesphere.model.version>
+        <citesphere.model.version>1.22-SNAPSHOT</citesphere.model.version>


That's not a real version number.

jdamerow · 2024-09-06T18:43:15Z

...e/src/main/java/edu/asu/diging/citesphere/web/user/authorities/AuthorityItemsController.java

+
+    @RequestMapping("/auth/authority/items")
+    public String showPage(Model model, @RequestParam("uri") String uri, Authentication authentication) {
+        userEntries = authorityService.findByUri((IUser) authentication.getPrincipal(), uri);


why is this variable called userEntries? It's not users that are held in this list.

jdamerow · 2024-09-06T18:43:59Z

...e/src/main/java/edu/asu/diging/citesphere/web/user/authorities/AuthorityItemsController.java

+        userEntries = authorityService.findByUri((IUser) authentication.getPrincipal(), uri);
+
+        if(userEntries == null || userEntries.isEmpty()) {
+            logger.error("No entry found for URI: " + uri);


that would just mean there are no authority with the uri, it's not an exception in the system really. so no need to log as error, maybe info.

jdamerow · 2024-09-06T18:51:14Z

...phere/src/main/java/edu/asu/diging/citesphere/core/authority/impl/AuthorityItemsService.java

+import edu.asu.diging.citesphere.model.transfer.impl.Citations;
+
+@Service
+public class AuthorityItemsService implements IAuthorityItemsService {


Is there a reason this is not part of the citation manager class? Also, tests are missing.

jdamerow · 2024-10-25T21:23:09Z

Resolve conflicts please

jdamerow · 2024-11-26T18:46:00Z

...e/src/main/java/edu/asu/diging/citesphere/web/user/authorities/AuthorityItemsController.java

+
+    private final Logger logger = LoggerFactory.getLogger(getClass());
+
+    private List<IAuthorityEntry> authorityEntries = new ArrayList();


This class is a singleton, so using a global variable like this is a big no no (all request will have the same variable/share the same data).

jdamerow · 2024-11-26T18:46:31Z

...e/src/main/java/edu/asu/diging/citesphere/web/user/authorities/AuthorityItemsController.java

+        authorityEntries = authorityService.findByUri((IUser) authentication.getPrincipal(), uri);
+
+        if(authorityEntries == null || authorityEntries.isEmpty()) {
+            logger.info("No entry found for URI: " + uri);


remove, this is not really useful here.

jdamerow · 2024-11-26T18:51:21Z

...e/src/main/java/edu/asu/diging/citesphere/web/user/authorities/AuthorityItemsController.java

+            logger.error("Found multiple entries for URI: " + uri);
+            model.addAttribute("error", "Found multiple entries for URI: " + uri);
+        } else {
+            Citations citations = citationManager.findAuthorityItems(authorityEntries.get(0));


this seems to be missing the logged in person (only citations the user has access to should be shown).

jdamerow · 2025-01-09T14:30:42Z

citesphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/CitationManager.java

+
+            List<ICitation> filteredCitations = allCitations.stream()
+                    .filter(citation -> groupIds.contains(citation.getGroup().toString()))
+                    .collect(Collectors.toList());


this filtering should happen on the db level (when calling findCitationsByPersonUri)

jdamerow · 2025-01-09T14:31:31Z

citesphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/CitationManager.java

@@ -506,6 +510,27 @@ public CitationPage getPrevAndNextCitation(IUser user, String groupId, String co
    public void deleteLocalGroupCitations(String groupId) {
        citationStore.deleteCitationByGroupId(groupId);
    }
+
+    @Override
+    public Citations findAuthorityItems(IAuthorityEntry entry, IUser user) {       


that's a confusing name. Why "items"? and it's not finding "authority items", it's finding citations that refer to the given authority, doesn't it?

also needs tests

Trilekha Mylavarapu and others added 21 commits March 22, 2022 14:07

[CITE-172] list all citations that reference that authority

7d03861

[CITE-172] list all citations that reference that authority

cfc4a0a

[CITE-172] list all citations that reference that authority

fb6356c

Delete ListAuthoritiesByName.java

ada2f8e

[CITE-172] list all citations that reference that authority

d23444d

[CITE-172] list all citations that reference that authority

e3d42cb

[CITE-172] list all citations that reference that authority

24265ef

[CITE-172] list all citations that reference that authority

344ec06

[CITE-172] list all citations that reference that authority

0254edc

[CITE-172] list all citations that reference that authority

157a217

[CITE-172] list all citation for given authority

64c1045

[CITE-172] list all citation for given authority

48f5262

[CITE-172] list all citations that reference that authority

0098c03

Fixed the bugs found

6df134d

Fixed null pointer issue

0f32ec1

fixing code refactor issues

306a1cd

Update AuthorityItemsController.java

46b1928

Update AuthorityItemsController.java

c514475

Fixing code identation issues

5d42efe

[CITE-172] Created AuthorityItemsService and moved code from controll…

bbb0210

…er to service

[CITE-172] Added logging to AuthorityItemsController

f54059f

Merge branch 'develop' into story/CITE-172

634fc9c

PradnyaC11 requested a review from jdamerow June 18, 2024 22:18

[CITE-172] Resolving error in list.html

7c2eb19

jdamerow closed this Jun 26, 2024

PradnyaC11 reopened this Jul 2, 2024

jdamerow requested changes Sep 6, 2024

View reviewed changes

jdamerow closed this Sep 6, 2024

[CITE-172] Addressed PR comments

9edb72b

PradnyaC11 reopened this Sep 24, 2024

PradnyaC11 requested a review from jdamerow September 24, 2024 16:57

jdamerow closed this Oct 25, 2024

PradnyaC11 reopened this Oct 28, 2024

Merge branch 'develop' into story/CITE-172

f433b08

jdamerow requested changes Nov 26, 2024

View reviewed changes

jdamerow closed this Nov 26, 2024

PradnyaC11 added 2 commits December 6, 2024 16:41

[CITE-172] Addressed PR comments

aea3426

[CITE-172] Addressed PR comments

8f675cd

PradnyaC11 reopened this Dec 9, 2024

[CITE-172] Resolved code factor issues

aa657dc

jdamerow requested changes Jan 9, 2025

View reviewed changes

jdamerow closed this Jan 9, 2025

PradnyaC11 added 3 commits January 9, 2025 16:35

[CITE-172] Addressed PR comments

eb898b7

[CITE-172] Addressed PR comments

40d2942

[CITE-172] Updated citesphere-model version

8c2aa6d

PradnyaC11 reopened this Jan 10, 2025

PradnyaC11 requested a review from jdamerow January 10, 2025 23:38

PradnyaC11 mentioned this pull request Jan 10, 2025

Story/cite 172 - From the managed authority page, there should be a link that lists all citations that reference that authority diging/citesphere-model#55

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Story/cite 172 - From the managed authority page, there should be a link that lists all citations that reference that authority #272

Story/cite 172 - From the managed authority page, there should be a link that lists all citations that reference that authority #272

PradnyaC11 commented Jun 18, 2024 •

edited

Loading

diging-jenkins commented Jun 18, 2024

jdamerow commented Jun 26, 2024

jdamerow Sep 6, 2024

jdamerow Sep 6, 2024

jdamerow Sep 6, 2024

jdamerow Sep 6, 2024

jdamerow commented Oct 25, 2024

jdamerow Nov 26, 2024

jdamerow Nov 26, 2024

jdamerow Nov 26, 2024

jdamerow Jan 9, 2025

jdamerow Jan 9, 2025

jdamerow Jan 9, 2025


		private final Logger logger = LoggerFactory.getLogger(getClass());

		private List<IAuthorityEntry> authorityEntries = new ArrayList();

Story/cite 172 - From the managed authority page, there should be a link that lists all citations that reference that authority #272

Are you sure you want to change the base?

Story/cite 172 - From the managed authority page, there should be a link that lists all citations that reference that authority #272

Conversation

PradnyaC11 commented Jun 18, 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 18, 2024

jdamerow commented Jun 26, 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

jdamerow commented Oct 25, 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

PradnyaC11 commented Jun 18, 2024 •

edited

Loading