From db30e35913209cdd715785c83ea098e320c4a936 Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Tue, 26 Mar 2019 16:17:40 +0100
Subject: [PATCH] OO-3996: fix filter for inactive curriculum elements

---
 .../CurriculumElementWithViewsDataModel.java  |  3 +-
 .../CurriculumElementViewsRowComparator.java  | 31 +++++------
 ...rriculumElementViewsRowComparatorTest.java | 53 ++++++++++++++++++-
 3 files changed, 66 insertions(+), 21 deletions(-)

diff --git a/src/main/java/org/olat/modules/curriculum/ui/CurriculumElementWithViewsDataModel.java b/src/main/java/org/olat/modules/curriculum/ui/CurriculumElementWithViewsDataModel.java
index 7037208b65f..6cc9c805ea6 100644
--- a/src/main/java/org/olat/modules/curriculum/ui/CurriculumElementWithViewsDataModel.java
+++ b/src/main/java/org/olat/modules/curriculum/ui/CurriculumElementWithViewsDataModel.java
@@ -68,10 +68,9 @@ public class CurriculumElementWithViewsDataModel extends DefaultFlexiTreeTableDa
 		if(row.isCurriculumElementOnly() || row.isCurriculumElementWithEntry()) {
 			active = row.getCurriculumElementStatus() == CurriculumElementStatus.active;
 		}
-			
 		if(active) {
 			for(CurriculumElementWithViewsRow parent=row.getParent(); parent != null; parent=parent.getParent()) {
-				if(row.isCurriculumElementOnly() || row.isCurriculumElementWithEntry()) {
+				if(parent.isCurriculumElementOnly() || parent.isCurriculumElementWithEntry()) {
 					active &= row.getCurriculumElementStatus() == CurriculumElementStatus.active;
 				}
 			}
diff --git a/src/main/java/org/olat/modules/curriculum/ui/component/CurriculumElementViewsRowComparator.java b/src/main/java/org/olat/modules/curriculum/ui/component/CurriculumElementViewsRowComparator.java
index f609d7dccc6..07a39346aa7 100644
--- a/src/main/java/org/olat/modules/curriculum/ui/component/CurriculumElementViewsRowComparator.java
+++ b/src/main/java/org/olat/modules/curriculum/ui/component/CurriculumElementViewsRowComparator.java
@@ -90,12 +90,7 @@ public class CurriculumElementViewsRowComparator extends FlexiTreeNodeComparator
 	}
 	
 	private int compareCurriculumElements(CurriculumElementWithViewsRow c1, CurriculumElementWithViewsRow c2) {
-		int c = 0;
-		if(c1.isClosedOrInactive() && !c2.isClosedOrInactive()) {
-			c = 1;
-		} else if(!c1.isClosedOrInactive() && c2.isClosedOrInactive()) {
-			c = -1;
-		}
+		int c = compareClosed(c1, c2);
 		
 		if(c == 0) {
 			if(c1.getCurriculumElementBeginDate() == null || c2.getCurriculumElementBeginDate() == null) {
@@ -128,13 +123,7 @@ public class CurriculumElementViewsRowComparator extends FlexiTreeNodeComparator
 	}
 	
 	private int compareRepositoryEntry(CurriculumElementWithViewsRow c1, CurriculumElementWithViewsRow c2) {
-		int c = 0;
-
-		if(c1.isClosedOrInactive() && !c2.isClosedOrInactive()) {
-			c = 1;
-		} else if(!c1.isClosedOrInactive() && c2.isClosedOrInactive()) {
-			c = -1;
-		}
+		int c = compareClosed(c1, c2);
 		
 		if(c == 0) {
 			if(c1.getRepositoryEntryDisplayName() == null || c2.getRepositoryEntryDisplayName() == null) {
@@ -158,17 +147,23 @@ public class CurriculumElementViewsRowComparator extends FlexiTreeNodeComparator
 		return c;
 	}
 	
+	private int compareClosed(CurriculumElementWithViewsRow c1, CurriculumElementWithViewsRow c2) {
+		int c = 0;
+		if(c1.isClosedOrInactive() && !c2.isClosedOrInactive()) {
+			c = 1;
+		} else if(!c1.isClosedOrInactive() && c2.isClosedOrInactive()) {
+			c = -1;
+		}
+		return c;
+	}
+	
 	private int compareDisplayName(CurriculumElementWithViewsRow c1, CurriculumElementWithViewsRow c2) {
 		String d1 = getDisplayName(c1);
 		String d2 = getDisplayName(c2);
 		if(d1 == null || d2 == null) {
 			return compareNullObjects(d1, d2);
 		}
-		int c = d1.compareTo(d2);
-		if(c == 0) {
-			
-		}
-		return c;
+		return d1.compareTo(d2);
 	}
 	
 	private String getDisplayName(CurriculumElementWithViewsRow row) {
diff --git a/src/test/java/org/olat/modules/curriculum/ui/component/CurriculumElementViewsRowComparatorTest.java b/src/test/java/org/olat/modules/curriculum/ui/component/CurriculumElementViewsRowComparatorTest.java
index e9783362759..9e099d5e85c 100644
--- a/src/test/java/org/olat/modules/curriculum/ui/component/CurriculumElementViewsRowComparatorTest.java
+++ b/src/test/java/org/olat/modules/curriculum/ui/component/CurriculumElementViewsRowComparatorTest.java
@@ -143,6 +143,57 @@ public class CurriculumElementViewsRowComparatorTest extends OlatTestCase {
 		Assert.assertEquals(entry2.getKey(), rows.get(3).getRepositoryEntryKey());
 	}
 	
+	@Test
+	public void testRepositoryEntryClosed_underParent() {
+		Identity author = JunitTestHelper.createAndPersistIdentityAsRndAuthor("sort-cur-el");
+
+		Curriculum curriculum = curriculumDao.createAndPersist("Cur-for-el-1", "Curriculum for element", "Curriculum", null);
+		CurriculumElement element = curriculumElementDao.createCurriculumElement("Element-1", "1. Element", CurriculumElementStatus.inactive,
+				new Date(), new Date(), null, null, CurriculumCalendars.disabled, CurriculumLectures.disabled, curriculum);
+		dbInstance.commitAndCloseSession();
+
+		RepositoryEntry entry1 = JunitTestHelper.deployBasicCourse(author, "1 course", RepositoryEntryStatusEnum.closed, false, false);
+		RepositoryEntry entry2 = JunitTestHelper.deployBasicCourse(author, "2 course", RepositoryEntryStatusEnum.trash, false, false);
+		RepositoryEntry entry3 = JunitTestHelper.deployBasicCourse(author, "3 course", RepositoryEntryStatusEnum.published, false, false);
+		RepositoryEntry entry4 = JunitTestHelper.deployBasicCourse(author, "4 course", RepositoryEntryStatusEnum.published, false, false);
+		// add the course and a participant to the curriculum
+		curriculumService.addRepositoryEntry(element, entry1, false);
+		curriculumService.addRepositoryEntry(element, entry2, false);
+		curriculumService.addRepositoryEntry(element, entry3, false);
+		curriculumService.addRepositoryEntry(element, entry4, false);
+		dbInstance.commitAndCloseSession();
+		
+
+		CurriculumElementWithViewsRow parent = new CurriculumElementWithViewsRow(element, null, 4);
+		
+		CurriculumElementWithViewsRow row1 = new CurriculumElementWithViewsRow(element, null,
+				new RepositoryEntryMyCourseImpl(entry1, null, false, 0, 0), false);
+		row1.setParent(parent);
+		CurriculumElementWithViewsRow row2 = new CurriculumElementWithViewsRow(element, null,
+				new RepositoryEntryMyCourseImpl(entry2, null, false, 0, 0), false);
+		row2.setParent(parent);
+		CurriculumElementWithViewsRow row3 = new CurriculumElementWithViewsRow(element, null,
+				new RepositoryEntryMyCourseImpl(entry3, null, false, 0, 0), false);
+		row3.setParent(parent);
+		CurriculumElementWithViewsRow row4 = new CurriculumElementWithViewsRow(element, null,
+				new RepositoryEntryMyCourseImpl(entry4, null, false, 0, 0), false);
+		row4.setParent(parent);
+
+		List<CurriculumElementWithViewsRow> rows = new ArrayList<>();
+		rows.add(parent);
+		rows.add(row1);
+		rows.add(row2);
+		rows.add(row3);
+		rows.add(row4);
+		
+		Collections.sort(rows, new CurriculumElementViewsRowComparator(Locale.ENGLISH));
+
+		Assert.assertEquals(entry3.getKey(), rows.get(1).getRepositoryEntryKey());
+		Assert.assertEquals(entry4.getKey(), rows.get(2).getRepositoryEntryKey());
+		Assert.assertEquals(entry1.getKey(), rows.get(3).getRepositoryEntryKey());
+		Assert.assertEquals(entry2.getKey(), rows.get(4).getRepositoryEntryKey());
+	}
+	
 	/**
 	 * Simulate a list of repository entries under their own curriculum element.
 	 * 
@@ -172,7 +223,7 @@ public class CurriculumElementViewsRowComparatorTest extends OlatTestCase {
 		curriculumService.addRepositoryEntry(element3, entry3, false);
 		curriculumService.addRepositoryEntry(element4, entry4, false);
 		dbInstance.commitAndCloseSession();
-		
+
 		CurriculumElementWithViewsRow row1 = new CurriculumElementWithViewsRow(element1, null,
 				new RepositoryEntryMyCourseImpl(entry1, null, false, 0, 0), true);
 		CurriculumElementWithViewsRow row2 = new CurriculumElementWithViewsRow(element2, null,
-- 
GitLab