From ec8a7453cb6c1e638c9bab5d277fa7ca3a7dccb9 Mon Sep 17 00:00:00 2001 From: Daniel Palafox Date: Thu, 28 Sep 2023 16:29:34 -0500 Subject: [PATCH 1/2] feature: Add sorting by source functionality to funding and works --- .../v3/activities/FundingComparators.java | 19 +++++- .../web/controllers/FundingsController.java | 2 +- .../controllers/PublicProfileController.java | 2 +- .../web/pagination/WorksPaginator.java | 56 ++++++++++++++--- .../controllers/FundingsControllerTest.java | 13 ++++ .../web/pagination/WorksPaginatorTest.java | 63 +++++++++++-------- 6 files changed, 118 insertions(+), 37 deletions(-) diff --git a/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/FundingComparators.java b/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/FundingComparators.java index 5cd98d3c3a8..7a99d715356 100644 --- a/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/FundingComparators.java +++ b/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/FundingComparators.java @@ -3,6 +3,7 @@ import java.util.Collections; import java.util.Comparator; +import org.orcid.core.utils.v3.SourceUtils; import org.orcid.pojo.ajaxForm.FundingForm; import org.orcid.pojo.grouping.FundingGroup; @@ -13,8 +14,12 @@ public class FundingComparators { private static final String DATE_SORT_KEY = "date"; private static final String TYPE_SORT_KEY = "type"; - - public static Comparator getInstance(String key, boolean sortAsc) { + + private static final String SOURCE_SORT_KEY = "source"; + + private static String orcid = null; + + public static Comparator getInstance(String key, boolean sortAsc, String orcid) { Comparator comparator = null; if (DATE_SORT_KEY.equals(key)) { comparator = FundingComparators.DATE_COMPARATOR; @@ -22,8 +27,11 @@ public static Comparator getInstance(String key, boolean sortAsc) comparator = FundingComparators.TITLE_COMPARATOR; } else if (TYPE_SORT_KEY.equals(key)) { comparator = FundingComparators.TYPE_COMPARATOR; + } else if (SOURCE_SORT_KEY.equals(key)) { + FundingComparators.orcid = orcid; + comparator = FundingComparators.SOURCE_COMPARATOR; } - + if (sortAsc) { return comparator; } else { @@ -94,4 +102,9 @@ public static Comparator getInstance(String key, boolean sortAsc) return g1.getStartDate().compareTo(g2.getStartDate()); }; + public static Comparator SOURCE_COMPARATOR = (g1, g2) -> Boolean.compare(isSelfAsserted(g1), isSelfAsserted(g2)); + + private static boolean isSelfAsserted(FundingGroup fundingGroup) { + return SourceUtils.isSelfAsserted(fundingGroup.getSource(), FundingComparators.orcid); + } } diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/FundingsController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/FundingsController.java index 599f1b6a504..63b9dd6b6dc 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/FundingsController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/FundingsController.java @@ -196,7 +196,7 @@ FundingForm getFunding() { fundingGroups.add(fundingGroup); } - fundingGroups.sort(FundingComparators.getInstance(sort, sortAsc)); + fundingGroups.sort(FundingComparators.getInstance(sort, sortAsc, getEffectiveUserOrcid())); return fundingGroups; } diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicProfileController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicProfileController.java index 1e43c046d99..0af6121d717 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicProfileController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicProfileController.java @@ -428,7 +428,7 @@ private boolean isRecordReadyForIndexing(ProfileEntity profile) { fundingGroups.add(fundingGroup); } - fundingGroups.sort(FundingComparators.getInstance(sort, sortAsc)); + fundingGroups.sort(FundingComparators.getInstance(sort, sortAsc, orcid)); return fundingGroups; } diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/pagination/WorksPaginator.java b/orcid-web/src/main/java/org/orcid/frontend/web/pagination/WorksPaginator.java index 24f8c0d780f..ccb7796241f 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/pagination/WorksPaginator.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/pagination/WorksPaginator.java @@ -11,6 +11,7 @@ import org.orcid.core.manager.v3.WorksCacheManager; import org.orcid.core.manager.v3.WorksExtendedCacheManager; import org.orcid.core.manager.v3.read_only.WorkManagerReadOnly; +import org.orcid.core.utils.v3.SourceUtils; import org.orcid.jaxb.model.v3.release.common.PublicationDate; import org.orcid.jaxb.model.v3.release.common.Visibility; import org.orcid.jaxb.model.v3.release.record.summary.WorkSummary; @@ -27,7 +28,9 @@ public class WorksPaginator { static final String DATE_SORT_KEY = "date"; static final String TYPE_SORT_KEY = "type"; - + + static final String SOURCE_SORT_KEY = "source"; + @Resource(name = "workManagerReadOnlyV3") private WorkManagerReadOnly workManagerReadOnly; @@ -42,7 +45,7 @@ public Page getWorksPage(String orcid, int offset, int pageSize, bool Page worksPage = new Page(); if (works != null) { List filteredGroups = filter(works, justPublic); - filteredGroups = sort(filteredGroups, sort, sortAsc); + filteredGroups = sort(filteredGroups, sort, sortAsc, orcid); worksPage.setTotalGroups(filteredGroups.size()); @@ -62,7 +65,7 @@ public Page getWorksExtendedPage(String orcid, int offset, int pageSi Page worksPage = new Page(); if (works != null) { List filteredGroups = filterWorksExtended(works, justPublic); - filteredGroups = sortExtended(filteredGroups, sort, sortAsc); + filteredGroups = sortExtended(filteredGroups, sort, sortAsc, orcid); worksPage.setTotalGroups(filteredGroups.size()); @@ -79,7 +82,7 @@ public Page getWorksExtendedPage(String orcid, int offset, int pageSi public Page refreshWorks(String orcid, int limit, String sort, boolean sortAsc) { Works works = worksCacheManager.getGroupedWorks(orcid); - List sortedGroups = sort(works.getWorkGroup(), sort, sortAsc); + List sortedGroups = sort(works.getWorkGroup(), sort, sortAsc, orcid); Page worksPage = new Page(); worksPage.setTotalGroups(sortedGroups.size()); @@ -100,7 +103,7 @@ public Page getAllWorks(String orcid, boolean justPublic, String sort Page worksPage = new Page(); if (works != null) { List filteredGroups = filter(works, justPublic); - filteredGroups = sort(filteredGroups, sort, sortAsc); + filteredGroups = sort(filteredGroups, sort, sortAsc, orcid); worksPage.setTotalGroups(filteredGroups.size()); @@ -116,13 +119,15 @@ public Page getAllWorks(String orcid, boolean justPublic, String sort return worksPage; } - private List sort(List list, String sort, boolean sortAsc) { + private List sort(List list, String sort, boolean sortAsc, String orcid) { if (TITLE_SORT_KEY.equals(sort)) { Collections.sort(list, new TitleComparator()); } else if (DATE_SORT_KEY.equals(sort)) { Collections.sort(list, new DateComparator()); } else if (TYPE_SORT_KEY.equals(sort)) { Collections.sort(list, new TypeComparator()); + } else if (SOURCE_SORT_KEY.equals(sort)) { + Collections.sort(list, new SourceComparator(orcid)); } if (!sortAsc) { @@ -131,13 +136,15 @@ private List sort(List return list; } - private List sortExtended(List list, String sort, boolean sortAsc) { + private List sortExtended(List list, String sort, boolean sortAsc, String orcid) { if (TITLE_SORT_KEY.equals(sort)) { Collections.sort(list, new TitleComparatorWorkGroupExtended()); } else if (DATE_SORT_KEY.equals(sort)) { Collections.sort(list, new DateComparatorWorkGroupExtended()); } else if (TYPE_SORT_KEY.equals(sort)) { Collections.sort(list, new TypeComparatorWorkGroupExtended()); + } else if (SOURCE_SORT_KEY.equals(sort)) { + Collections.sort(list, new SourceComparatorWorkGroupExtended(orcid)); } if (!sortAsc) { @@ -312,6 +319,24 @@ public int compare(org.orcid.jaxb.model.v3.release.record.summary.WorkGroup o1, } } + private class SourceComparator implements Comparator { + + private String orcid; + + SourceComparator(String orcid) { + this.orcid = orcid; + } + + @Override + public int compare(org.orcid.jaxb.model.v3.release.record.summary.WorkGroup o1, org.orcid.jaxb.model.v3.release.record.summary.WorkGroup o2) { + return Boolean.compare(isSelfAsserted(o1.getWorkSummary().get(0)), isSelfAsserted(o2.getWorkSummary().get(0))); + } + + private boolean isSelfAsserted(WorkSummary workSummary) { + return SourceUtils.isSelfAsserted(workSummary.getSource(), orcid); + } + } + private class DateComparatorWorkGroupExtended implements Comparator { @Override @@ -432,4 +457,21 @@ public int compare(WorkGroupExtended o1, WorkGroupExtended o2) { } } + private class SourceComparatorWorkGroupExtended implements Comparator { + private String orcid; + + SourceComparatorWorkGroupExtended(String orcid) { + this.orcid = orcid; + } + + @Override + public int compare(WorkGroupExtended o1, WorkGroupExtended o2) { + return Boolean.compare(isSelfAsserted(o1.getWorkSummary().get(0)), isSelfAsserted(o2.getWorkSummary().get(0))); + } + + private boolean isSelfAsserted(WorkSummaryExtended workSummary) { + return SourceUtils.isSelfAsserted(workSummary.getSource(), orcid); + } + } + } diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/FundingsControllerTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/FundingsControllerTest.java index 0125692b203..711dd988f02 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/FundingsControllerTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/FundingsControllerTest.java @@ -637,6 +637,19 @@ public void testAddFundingWithInvalidDates() throws Exception { assertEquals(fundingController.getMessage("fundings.endDate.after"), funding.getErrors().get(0)); } + @Test + public void testGetFundingsJsonSortedBySource() { + HttpSession session = mock(HttpSession.class); + when(servletRequest.getSession()).thenReturn(session); + when(localeManager.getLocale()).thenReturn(new Locale("us", "EN")); + + List fundings = fundingController.getFundingsJson("source", true); + assertNotNull(fundings); + assertEquals(3, fundings.size()); + assertEquals("4444-4444-4444-4441", fundings.get(0).getFundings().get(0).getSource()); + assertEquals("4444-4444-4444-4443", fundings.get(2).getFundings().get(0).getSource()); + } + private FundingForm getFundingForm() { FundingForm funding = fundingController.getFunding(); funding.setFundingType(Text.valueOf("award")); diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/pagination/WorksPaginatorTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/pagination/WorksPaginatorTest.java index 2114bfd36e3..232199e79fd 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/web/pagination/WorksPaginatorTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/web/pagination/WorksPaginatorTest.java @@ -4,11 +4,11 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Random; import java.util.UUID; +import java.util.stream.Collectors; import org.junit.Before; import org.junit.Test; @@ -74,7 +74,7 @@ public void setUp() { public void testGetWorksPage() { int pageSize = 100; - Mockito.when(worksCacheManager.getGroupedWorks(Mockito.anyString())).thenReturn(get1000PublicWorkGroups()); + Mockito.when(worksCacheManager.getGroupedWorks(Mockito.anyString())).thenReturn(getPublicWorkGroups(1000)); Page page = worksPaginator.getWorksPage("orcid", 0, pageSize, false, WorksPaginator.DATE_SORT_KEY, true); assertEquals(pageSize, page.getGroups().size()); org.orcid.pojo.grouping.WorkGroup workGroupPage1 = page.getGroups().get(0); @@ -167,28 +167,22 @@ public void testGetPublicWorksExtendedPage() { } @Test - public void testTitleSortCaseInsensitive() { - int pageSize = 100; - - Works works = get1000PublicWorkGroups(); - for (WorkGroup workGroup : works.getWorkGroup()) { - if (new Random().nextBoolean()) { - for (WorkSummary summary : workGroup.getWorkSummary()) { - summary.getTitle().setTitle(new Title(summary.getTitle().getTitle().getContent().toUpperCase())); - } + public void testSourceSort() { + int pageSize = 50; + + Works works = getPublicWorkGroups(50); + List workGroups = works.getWorkGroup().stream().limit(2).collect(Collectors.toList()); + for (WorkGroup workGroup : workGroups) { + for (WorkSummary summary : workGroup.getWorkSummary()) { + summary.setSource(new Source("orcid")); } } + Mockito.when(worksCacheManager.getGroupedWorks(Mockito.anyString())).thenReturn(works); - Page page = worksPaginator.getWorksPage("orcid", 0, pageSize, false, WorksPaginator.TITLE_SORT_KEY, true); + Page page = worksPaginator.getWorksPage("orcid", 0, pageSize, false, WorksPaginator.SOURCE_SORT_KEY, false); - org.orcid.pojo.grouping.WorkGroup previous = page.getGroups().remove(0); - while (!page.getGroups().isEmpty()) { - org.orcid.pojo.grouping.WorkGroup next = page.getGroups().remove(0); - String previousTitle = previous.getWorks().get(0).getTitle().getValue(); - String nextTitle = next.getWorks().get(0).getTitle().getValue(); - assertTrue(previousTitle.toLowerCase().compareTo(nextTitle.toLowerCase()) <= 0); - previous = next; - } + assertEquals("orcid", page.getGroups().get(0).getWorks().get(0).getSource()); + assertEquals("orcid", page.getGroups().get(0).getWorks().get(49).getSource()); } @Test @@ -208,10 +202,29 @@ public void testReverseSecondaryTitleSortForNullDates() { previous = next; } } - + + @Test + public void testTitleSortCase() { + int pageSize = 50; + + Works works = getPublicWorkGroups(50); + List workGroups = works.getWorkGroup().stream().limit(2).collect(Collectors.toList()); + for (WorkGroup workGroup : workGroups) { + for (WorkSummary summary : workGroup.getWorkSummary()) { + summary.setSource(new Source("orcid")); + } + } + + Mockito.when(worksCacheManager.getGroupedWorks(Mockito.anyString())).thenReturn(works); + Page page = worksPaginator.getWorksPage("orcid", 0, pageSize, false, WorksPaginator.SOURCE_SORT_KEY, false); + + assertEquals("APP-5555-5555-5555-5555", page.getGroups().get(0).getWorks().get(0).getSource()); + assertEquals("orcid", page.getGroups().get(49).getWorks().get(0).getSource()); + } + @Test public void testGetAllWorks() { - Works works = get1000PublicWorkGroups(); + Works works = getPublicWorkGroups(1000); Mockito.when(worksCacheManager.getGroupedWorks(Mockito.anyString())).thenReturn(works); Page page = worksPaginator.getAllWorks("orcid", false, WorksPaginator.TITLE_SORT_KEY, true); assertEquals(1000, page.getTotalGroups()); @@ -257,7 +270,7 @@ public void testGetWorkWithNulltitle() { } private Works getWorkGroupsWithNullDates() { - Works works = get1000PublicWorkGroups(); + Works works = getPublicWorkGroups(1000); for (WorkGroup workGroup : works.getWorkGroup()) { for (WorkSummary workSummary : workGroup.getWorkSummary()) { workSummary.setPublicationDate(null); @@ -266,12 +279,12 @@ private Works getWorkGroupsWithNullDates() { return works; } - private Works get1000PublicWorkGroups() { + private Works getPublicWorkGroups(int numberOfWorks) { Works works = new Works(); works.setLastModifiedDate(new LastModifiedDate(DateUtils.convertToXMLGregorianCalendar(System.currentTimeMillis()))); works.setPath("some path"); - for (int i = 0; i < 1000; i++) { + for (int i = 0; i < numberOfWorks; i++) { works.getWorkGroup().add(getPublicWorkGroup(i)); } return works; From f1afedb7f93431436dcfbb5ce4aa9a28e7e02012 Mon Sep 17 00:00:00 2001 From: Daniel Palafox Date: Thu, 28 Sep 2023 17:02:46 -0500 Subject: [PATCH 2/2] fix: Sort by source unit test --- .../org/orcid/frontend/web/pagination/WorksPaginatorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/pagination/WorksPaginatorTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/pagination/WorksPaginatorTest.java index 232199e79fd..2a10808f122 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/web/pagination/WorksPaginatorTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/web/pagination/WorksPaginatorTest.java @@ -181,8 +181,8 @@ public void testSourceSort() { Mockito.when(worksCacheManager.getGroupedWorks(Mockito.anyString())).thenReturn(works); Page page = worksPaginator.getWorksPage("orcid", 0, pageSize, false, WorksPaginator.SOURCE_SORT_KEY, false); - assertEquals("orcid", page.getGroups().get(0).getWorks().get(0).getSource()); - assertEquals("orcid", page.getGroups().get(0).getWorks().get(49).getSource()); + assertEquals("APP-5555-5555-5555-5555", page.getGroups().get(0).getWorks().get(0).getSource()); + assertEquals("orcid", page.getGroups().get(49).getWorks().get(0).getSource()); } @Test