From ff19f6a37a7f7b07a6b21a258f2594e67ba4fc1f Mon Sep 17 00:00:00 2001
From: uhensler <urs.hensler@frentix.com>
Date: Tue, 26 May 2020 11:30:35 +0200
Subject: [PATCH] OO-4710: Release date is not used if steps are not sequential

---
 .../STWithoutSequenceBlockerEvaluator.java    |   6 +-
 .../st/assessment/SequentialBlocker.java      |   5 +
 .../st/assessment/WithoutSequenceBlocker.java |  31 ++--
 .../scoring/AccountingEvaluatorsFactory.java  |   2 +-
 .../run/scoring/AssessmentAccounting.java     |   1 +
 .../org/olat/course/run/scoring/Blocker.java  |   2 +
 .../run/scoring/AssessmentAccountingTest.java | 150 ++++++++++++++++++
 7 files changed, 181 insertions(+), 16 deletions(-)

diff --git a/src/main/java/org/olat/course/nodes/st/assessment/STWithoutSequenceBlockerEvaluator.java b/src/main/java/org/olat/course/nodes/st/assessment/STWithoutSequenceBlockerEvaluator.java
index 719c8867752..766e8fd1572 100644
--- a/src/main/java/org/olat/course/nodes/st/assessment/STWithoutSequenceBlockerEvaluator.java
+++ b/src/main/java/org/olat/course/nodes/st/assessment/STWithoutSequenceBlockerEvaluator.java
@@ -19,8 +19,6 @@
  */
 package org.olat.course.nodes.st.assessment;
 
-import java.util.Date;
-
 import org.olat.course.run.scoring.Blocker;
 import org.olat.course.run.scoring.BlockerEvaluator;
 
@@ -34,9 +32,7 @@ public class STWithoutSequenceBlockerEvaluator implements BlockerEvaluator {
 
 	@Override
 	public Blocker getChildrenBlocker(Blocker blocker) {
-		boolean blocked = blocker != null? blocker.isBlocked(): false;
-		Date startDate = blocker != null? blocker.getStartDate(): null;
-		return new WithoutSequenceBlocker(blocked, startDate);
+		return new WithoutSequenceBlocker(blocker);
 	}
 
 	@Override
diff --git a/src/main/java/org/olat/course/nodes/st/assessment/SequentialBlocker.java b/src/main/java/org/olat/course/nodes/st/assessment/SequentialBlocker.java
index 6885f05ac28..5080c771029 100644
--- a/src/main/java/org/olat/course/nodes/st/assessment/SequentialBlocker.java
+++ b/src/main/java/org/olat/course/nodes/st/assessment/SequentialBlocker.java
@@ -48,4 +48,9 @@ public class SequentialBlocker implements Blocker {
 	public Date getStartDate() {
 		return startDate;
 	}
+
+	@Override
+	public void nextCourseNode() {
+		//
+	}
 }
\ No newline at end of file
diff --git a/src/main/java/org/olat/course/nodes/st/assessment/WithoutSequenceBlocker.java b/src/main/java/org/olat/course/nodes/st/assessment/WithoutSequenceBlocker.java
index 5722a82cd98..cb6a861f25d 100644
--- a/src/main/java/org/olat/course/nodes/st/assessment/WithoutSequenceBlocker.java
+++ b/src/main/java/org/olat/course/nodes/st/assessment/WithoutSequenceBlocker.java
@@ -31,32 +31,43 @@ import org.olat.course.run.scoring.Blocker;
  */
 public class WithoutSequenceBlocker implements Blocker {
 
-	private final boolean blocked;
-	private final Date startDate;
+	private final Blocker parent;
+	private boolean blocked;
+	private Date startDate;
 	
-	public WithoutSequenceBlocker(boolean blocked, Date startDate) {
-		this.blocked = blocked;
-		this.startDate = startDate;
+	public WithoutSequenceBlocker(Blocker parent) {
+		this.parent = parent;
 	}
 
 	@Override
 	public boolean isBlocked() {
-		return blocked;
+		return blocked
+				? true 
+				: parent != null? parent.isBlocked(): false;
 	}
 
 	@Override
 	public void block() {
-		//
+		blocked = true;
 	}
-
+	
 	@Override
 	public void block(Date startDate) {
-		//
+		this.startDate = startDate;
+		block();
 	}
 
 	@Override
 	public Date getStartDate() {
-		return startDate;
+		return startDate != null
+				? startDate
+				: parent != null? parent.getStartDate(): null;
+	}
+
+	@Override
+	public void nextCourseNode() {
+		blocked = false;
+		startDate = null;
 	}
 
 }
diff --git a/src/main/java/org/olat/course/run/scoring/AccountingEvaluatorsFactory.java b/src/main/java/org/olat/course/run/scoring/AccountingEvaluatorsFactory.java
index 14c852e0afe..5a7ecd1a519 100644
--- a/src/main/java/org/olat/course/run/scoring/AccountingEvaluatorsFactory.java
+++ b/src/main/java/org/olat/course/run/scoring/AccountingEvaluatorsFactory.java
@@ -113,7 +113,7 @@ class AccountingEvaluatorsFactory {
 
 		@Override
 		public Blocker getChildrenBlocker(Blocker blocker) {
-			return blocker != null? blocker: new WithoutSequenceBlocker(false, null);
+			return blocker != null? blocker: new WithoutSequenceBlocker(null);
 		}
 
 		@Override
diff --git a/src/main/java/org/olat/course/run/scoring/AssessmentAccounting.java b/src/main/java/org/olat/course/run/scoring/AssessmentAccounting.java
index 41d2d1311c4..0ba567cfb1d 100644
--- a/src/main/java/org/olat/course/run/scoring/AssessmentAccounting.java
+++ b/src/main/java/org/olat/course/run/scoring/AssessmentAccounting.java
@@ -152,6 +152,7 @@ public class AssessmentAccounting implements ScoreAccounting {
 		
 		AssessmentEvaluation currentEvaluation = evalCourseNode(courseNode);
 		AccountingResult result = new AccountingResult(currentEvaluation);
+		blocker.nextCourseNode();
 		
 		AccountingEvaluators evaluators = courseAssessmentService.getEvaluators(courseNode, courseConfig);
 		
diff --git a/src/main/java/org/olat/course/run/scoring/Blocker.java b/src/main/java/org/olat/course/run/scoring/Blocker.java
index ca662ed99ef..cf6128487aa 100644
--- a/src/main/java/org/olat/course/run/scoring/Blocker.java
+++ b/src/main/java/org/olat/course/run/scoring/Blocker.java
@@ -36,5 +36,7 @@ public interface Blocker {
 	void block(Date startDate);
 
 	Date getStartDate();
+	
+	void nextCourseNode();
 
 }
\ No newline at end of file
diff --git a/src/test/java/org/olat/course/run/scoring/AssessmentAccountingTest.java b/src/test/java/org/olat/course/run/scoring/AssessmentAccountingTest.java
index 9424df32d2e..772fc967f64 100644
--- a/src/test/java/org/olat/course/run/scoring/AssessmentAccountingTest.java
+++ b/src/test/java/org/olat/course/run/scoring/AssessmentAccountingTest.java
@@ -19,15 +19,20 @@
  */
 package org.olat.course.run.scoring;
 
+import java.util.Date;
+
 import org.assertj.core.api.SoftAssertions;
 import org.junit.Test;
 import org.olat.core.commons.persistence.DB;
 import org.olat.core.id.Identity;
 import org.olat.core.id.IdentityEnvironment;
+import org.olat.core.util.DateUtils;
 import org.olat.course.CourseFactory;
 import org.olat.course.ICourse;
 import org.olat.course.Structure;
+import org.olat.course.learningpath.LearningPathService;
 import org.olat.course.learningpath.manager.LearningPathNodeAccessProvider;
+import org.olat.course.nodes.CourseNode;
 import org.olat.course.nodes.SPCourseNode;
 import org.olat.course.nodes.STCourseNode;
 import org.olat.course.run.environment.CourseEnvironment;
@@ -53,6 +58,8 @@ public class AssessmentAccountingTest extends OlatTestCase {
 	private DB dbInstance;
 	@Autowired
 	private AssessmentService assessmentService;
+	@Autowired
+	private LearningPathService learningPathService;
 	
 	/**
 	 * Course (sequential)
@@ -436,6 +443,145 @@ public class AssessmentAccountingTest extends OlatTestCase {
 		softly.assertAll();
 		
 	}
+	
+	/**
+	 * Course (without sequence)
+	 *   - SP: no date
+	 *   - ST: (without sequence)
+	 *     - SP: no date
+	 *     - SP: date
+	 *     - SP: no date
+	 *     - SP: date
+	 *     - ST: without sequence
+	 *       - SP: no date
+	 *   - ST (sequential)
+	 *     - SP: no date
+	 *     - SP: date
+	 *     - SP: no date
+	 *     - ST: (without sequence)
+	 *       - SP: no date
+	 *   - SP: no date
+	 */
+	@Test
+	public void shouldRespectStartDates() {
+		// Create course
+		Identity author = JunitTestHelper.createAndPersistIdentityAsAuthor("author");
+		RepositoryEntry courseEntry = JunitTestHelper.deployEmptyCourse(author, "Learning Path",
+				RepositoryEntryStatusEnum.published, true, false);
+		ICourse course = CourseFactory.loadCourse(courseEntry);
+		course.getCourseConfig().setNodeAccessType(LearningPathNodeAccessProvider.TYPE);
+		CourseEnvironment courseEnv = course.getCourseEnvironment();
+		
+		// Make the course runtime structure
+		Structure runStructure = courseEnv.getRunStructure();
+		STCourseNode root = (STCourseNode)runStructure.getRootNode();
+		root.getModuleConfiguration().setStringValue(STCourseNode.CONFIG_LP_SEQUENCE_KEY,
+				STCourseNode.CONFIG_LP_SEQUENCE_VALUE_WITHOUT);
+		SPCourseNode sp_0_1 = new SPCourseNode();
+		root.addChild(sp_0_1);
+		// Structure 1
+		STCourseNode st_1 = new STCourseNode();
+		st_1.getModuleConfiguration().setStringValue(STCourseNode.CONFIG_LP_SEQUENCE_KEY,
+				STCourseNode.CONFIG_LP_SEQUENCE_VALUE_WITHOUT);
+		root.addChild(st_1);
+		SPCourseNode sp_1_1 = new SPCourseNode();
+		st_1.addChild(sp_1_1);
+		SPCourseNode sp_1_2 = new SPCourseNode();
+		setStartInFuture(sp_1_2);
+		st_1.addChild(sp_1_2);
+		SPCourseNode sp_1_3 = new SPCourseNode();
+		st_1.addChild(sp_1_3);
+		SPCourseNode sp_1_4 = new SPCourseNode();
+		setStartInFuture(sp_1_4);
+		st_1.addChild(sp_1_4);
+		// Structure 11
+		STCourseNode st_11 = new STCourseNode();
+		st_11.getModuleConfiguration().setStringValue(STCourseNode.CONFIG_LP_SEQUENCE_KEY,
+				STCourseNode.CONFIG_LP_SEQUENCE_VALUE_WITHOUT);
+		st_1.addChild(st_11);
+		SPCourseNode sp_11_1 = new SPCourseNode();
+		st_11.addChild(sp_11_1);
+		// Structure 2
+		STCourseNode st_2 = new STCourseNode();
+		st_2.getModuleConfiguration().setStringValue(STCourseNode.CONFIG_LP_SEQUENCE_KEY,
+				STCourseNode.CONFIG_LP_SEQUENCE_VALUE_SEQUENTIAL);
+		root.addChild(st_2);
+		SPCourseNode sp_2_1 = new SPCourseNode();
+		st_2.addChild(sp_2_1);
+		SPCourseNode sp_2_2 = new SPCourseNode();
+		setStartInFuture(sp_2_2);
+		st_2.addChild(sp_2_2);
+		SPCourseNode sp_2_3 = new SPCourseNode();
+		st_2.addChild(sp_2_3);
+		// Structure 21
+		STCourseNode st_21 = new STCourseNode();
+		st_21.getModuleConfiguration().setStringValue(STCourseNode.CONFIG_LP_SEQUENCE_KEY,
+				STCourseNode.CONFIG_LP_SEQUENCE_VALUE_WITHOUT);
+		st_2.addChild(st_21);
+		SPCourseNode sp_21_1 = new SPCourseNode();
+		st_21.addChild(sp_21_1);
+		//Structure 0 again
+		SPCourseNode sp_0_2 = new SPCourseNode();
+		root.addChild(sp_0_2);
+		
+		
+		// Add a participant to the course
+		Identity participant = JunitTestHelper.createAndPersistIdentityAsUser("participant");
+		IdentityEnvironment identityEnv = new IdentityEnvironment();
+		identityEnv.setIdentity(participant);
+		UserCourseEnvironmentImpl userCourseEnv = new UserCourseEnvironmentImpl(identityEnv, courseEnv);
+		userCourseEnv.setUserRoles(false, false, true);
+		dbInstance.commitAndCloseSession();
+		
+		
+		// Init the AssessmentAccounting
+		ScoreAccounting scoreAccounting = userCourseEnv.getScoreAccounting();
+		scoreAccounting.evaluateAll(true);
+		dbInstance.commitAndCloseSession();
+
+		SoftAssertions softly = new SoftAssertions();
+		softly.assertThat(scoreAccounting.evalCourseNode(root).getAssessmentStatus()).as("root").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_0_1).getAssessmentStatus()).as("sp_0_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(st_1).getAssessmentStatus()).as("st_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_1_1).getAssessmentStatus()).as("sp_1_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_1_2).getAssessmentStatus()).as("sp_1_2").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_1_3).getAssessmentStatus()).as("sp_1_3").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_1_4).getAssessmentStatus()).as("sp_1_4").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(st_11).getAssessmentStatus()).as("st_11").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_11_1).getAssessmentStatus()).as("sp_11_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(st_2).getAssessmentStatus()).as("st_2").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_2_1).getAssessmentStatus()).as("sp_2_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_2_2).getAssessmentStatus()).as("sp_2_2").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_2_3).getAssessmentStatus()).as("sp_2_3").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(st_21).getAssessmentStatus()).as("st_21").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_21_1).getAssessmentStatus()).as("sp_21_1").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_0_2).getAssessmentStatus()).as("sp_0_2").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertAll();
+		
+		// The participant executes sp_1_3
+		setDone(participant, courseEntry, sp_1_3);
+		scoreAccounting.evaluateAll(true);
+		dbInstance.commitAndCloseSession();
+		
+		softly = new SoftAssertions();
+		softly.assertThat(scoreAccounting.evalCourseNode(root).getAssessmentStatus()).as("root").isEqualTo(AssessmentEntryStatus.inProgress);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_0_1).getAssessmentStatus()).as("sp_0_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(st_1).getAssessmentStatus()).as("st_1").isEqualTo(AssessmentEntryStatus.inProgress);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_1_1).getAssessmentStatus()).as("sp_1_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_1_2).getAssessmentStatus()).as("sp_1_2").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_1_3).getAssessmentStatus()).as("sp_1_3").isEqualTo(AssessmentEntryStatus.done);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_1_4).getAssessmentStatus()).as("sp_1_4").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(st_11).getAssessmentStatus()).as("st_11").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_11_1).getAssessmentStatus()).as("sp_11_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(st_2).getAssessmentStatus()).as("st_2").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_2_1).getAssessmentStatus()).as("sp_2_1").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_2_2).getAssessmentStatus()).as("sp_2_2").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_2_3).getAssessmentStatus()).as("sp_2_3").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(st_21).getAssessmentStatus()).as("st_21").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_21_1).getAssessmentStatus()).as("sp_21_1").isEqualTo(AssessmentEntryStatus.notReady);
+		softly.assertThat(scoreAccounting.evalCourseNode(sp_0_2).getAssessmentStatus()).as("sp_0_2").isEqualTo(AssessmentEntryStatus.notStarted);
+		softly.assertAll();
+	}
 
 	private void setDone(Identity identity, RepositoryEntry entry, SPCourseNode courseNode) {
 		AssessmentEntry assessmentEntry = assessmentService.loadAssessmentEntry(identity, entry, courseNode.getIdent());
@@ -445,5 +591,9 @@ public class AssessmentAccountingTest extends OlatTestCase {
 		dbInstance.commitAndCloseSession();
 	}
 
+	private void setStartInFuture(CourseNode courseNode) {
+		Date future = DateUtils.addDays(new Date(), 1);
+		learningPathService.getConfigs(courseNode).setStartDate(future);
+	}
 
 }
-- 
GitLab