From 1441a0503e7574d8ef412020974543db0ee2e29d Mon Sep 17 00:00:00 2001 From: uhensler <urs.hensler@frentix.com> Date: Mon, 28 Oct 2019 15:57:46 +0100 Subject: [PATCH] OO-4207: Access evaluation of learning paths when course structure changes --- .../DefaultLinearStatusEvaluator.java | 49 ++----- .../assessment/STLinearStatusEvaluator.java | 32 +++- .../st/assessment/ScoreStatusEvaluator.java | 4 +- .../scoring/AccountingEvaluatorsFactory.java | 4 +- .../run/scoring/AssessmentAccounting.java | 6 +- .../course/run/scoring/StatusEvaluator.java | 17 ++- .../DefaultLinearStatusEvaluatorTest.java | 137 +++++++----------- .../STLinearStatusEvaluatorTest.java | 51 +++++++ 8 files changed, 170 insertions(+), 130 deletions(-) diff --git a/src/main/java/org/olat/course/learningpath/evaluation/DefaultLinearStatusEvaluator.java b/src/main/java/org/olat/course/learningpath/evaluation/DefaultLinearStatusEvaluator.java index e6925ce4a6a..db10d767594 100644 --- a/src/main/java/org/olat/course/learningpath/evaluation/DefaultLinearStatusEvaluator.java +++ b/src/main/java/org/olat/course/learningpath/evaluation/DefaultLinearStatusEvaluator.java @@ -39,57 +39,40 @@ public class DefaultLinearStatusEvaluator implements StatusEvaluator { private static final Logger log = Tracing.createLoggerFor(DefaultLinearStatusEvaluator.class); @Override - public AssessmentEntryStatus getStatus(AssessmentEvaluation previousEvaluation, - AssessmentEvaluation currentEvaluation, boolean firstChild) { + public AssessmentEntryStatus getStatus(AssessmentEvaluation currentEvaluation, + Blocker blocker) { AssessmentEntryStatus currentStatus = currentEvaluation.getAssessmentStatus(); AssessmentEntryStatus status = currentStatus; - if (isNotReadyYet(currentStatus)) { - if (isAccessible(previousEvaluation, firstChild)) { + if (isNotInProgressYet(currentStatus)) { + if (isNotBlocked(blocker)) { status = AssessmentEntryStatus.notStarted; } else { status = AssessmentEntryStatus.notReady; } } + if (blocker != null && AssessmentObligation.mandatory.equals(currentEvaluation.getObligation()) + && !Boolean.TRUE.equals(currentEvaluation.getFullyAssessed())) { + blocker.block(); + } - log.debug("Previous: fully assessed '{}', obligation '{}', status '{}'" - , previousEvaluation != null? previousEvaluation.getFullyAssessed(): null - , previousEvaluation != null? previousEvaluation.getObligation(): null - , previousEvaluation != null? previousEvaluation.getAssessmentStatus(): null); - log.debug("Current: fully assessed '{}', obligation '{}, status '{}', new status '{}'" + log.debug("fully assessed '{}', obligation '{}, blocked: '{}', status '{}', new status '{}'" , currentEvaluation.getFullyAssessed() , currentEvaluation.getObligation() + , !isNotBlocked(blocker) , currentEvaluation.getAssessmentStatus() , status); return status; } - private boolean isNotReadyYet(AssessmentEntryStatus currentStatus) { - return currentStatus == null || AssessmentEntryStatus.notReady.equals(currentStatus); - } - - private boolean isAccessible(AssessmentEvaluation assessmentEvaluation, boolean firstChild) { - return isRoot(assessmentEvaluation) - || isFullyAssessed(assessmentEvaluation) - || isOptionalAndReady(assessmentEvaluation) - || isFirstChildAndNotStarted(assessmentEvaluation, firstChild); - } - - private boolean isRoot(AssessmentEvaluation assessmentEvaluation) { - return assessmentEvaluation == null; - } - - private boolean isFullyAssessed(AssessmentEvaluation assessmentEvaluation) { - return assessmentEvaluation.getFullyAssessed() != null && assessmentEvaluation.getFullyAssessed(); - } - - private boolean isOptionalAndReady(AssessmentEvaluation assessmentEvaluation) { - return !AssessmentObligation.mandatory.equals(assessmentEvaluation.getObligation()) - && !AssessmentEntryStatus.notReady.equals(assessmentEvaluation.getAssessmentStatus()); + private boolean isNotInProgressYet(AssessmentEntryStatus currentStatus) { + return currentStatus == null + || AssessmentEntryStatus.notReady.equals(currentStatus) + || AssessmentEntryStatus.notStarted.equals(currentStatus); } - private boolean isFirstChildAndNotStarted(AssessmentEvaluation assessmentEvaluation, boolean firstChild) { - return firstChild && AssessmentEntryStatus.notStarted.equals(assessmentEvaluation.getAssessmentStatus()); + private boolean isNotBlocked(Blocker blocker) { + return blocker == null || !blocker.isBlocked(); } @Override diff --git a/src/main/java/org/olat/course/nodes/st/assessment/STLinearStatusEvaluator.java b/src/main/java/org/olat/course/nodes/st/assessment/STLinearStatusEvaluator.java index dbbb765c1f6..a177fc5e54f 100644 --- a/src/main/java/org/olat/course/nodes/st/assessment/STLinearStatusEvaluator.java +++ b/src/main/java/org/olat/course/nodes/st/assessment/STLinearStatusEvaluator.java @@ -21,7 +21,8 @@ package org.olat.course.nodes.st.assessment; import java.util.List; -import org.olat.course.learningpath.evaluation.DefaultLinearStatusEvaluator; +import org.apache.logging.log4j.Logger; +import org.olat.core.logging.Tracing; import org.olat.course.run.scoring.AssessmentEvaluation; import org.olat.course.run.scoring.StatusEvaluator; import org.olat.modules.assessment.model.AssessmentEntryStatus; @@ -34,13 +35,32 @@ import org.olat.modules.assessment.model.AssessmentObligation; * */ public class STLinearStatusEvaluator implements StatusEvaluator { - - private final StatusEvaluator defaultEvaluator = new DefaultLinearStatusEvaluator(); + + private static final Logger log = Tracing.createLoggerFor(STLinearStatusEvaluator.class); @Override - public AssessmentEntryStatus getStatus(AssessmentEvaluation previousEvaluation, - AssessmentEvaluation currentEvaluation, boolean firstChild) { - return defaultEvaluator.getStatus(previousEvaluation, currentEvaluation, firstChild); + public AssessmentEntryStatus getStatus(AssessmentEvaluation currentEvaluation, + Blocker blocker) { + AssessmentEntryStatus currentStatus = currentEvaluation.getAssessmentStatus(); + AssessmentEntryStatus status = currentStatus; + if (isBlocked(blocker)) { + status = AssessmentEntryStatus.notReady; + } else { + status = AssessmentEntryStatus.notStarted; + } + + log.debug(" fully assessed '{}', obligation '{}, blocked: '{}', status '{}', new status '{}'" + , currentEvaluation.getFullyAssessed() + , currentEvaluation.getObligation() + , isBlocked(blocker) + , currentEvaluation.getAssessmentStatus() + , status); + + return status; + } + + private boolean isBlocked(Blocker blocker) { + return blocker != null && blocker.isBlocked(); } @Override diff --git a/src/main/java/org/olat/course/nodes/st/assessment/ScoreStatusEvaluator.java b/src/main/java/org/olat/course/nodes/st/assessment/ScoreStatusEvaluator.java index ca679c440b7..39823e3caa9 100644 --- a/src/main/java/org/olat/course/nodes/st/assessment/ScoreStatusEvaluator.java +++ b/src/main/java/org/olat/course/nodes/st/assessment/ScoreStatusEvaluator.java @@ -34,8 +34,8 @@ import org.olat.modules.assessment.model.AssessmentEntryStatus; public class ScoreStatusEvaluator implements StatusEvaluator { @Override - public AssessmentEntryStatus getStatus(AssessmentEvaluation previousEvaluation, - AssessmentEvaluation currentEvaluation, boolean firstChild) { + public AssessmentEntryStatus getStatus(AssessmentEvaluation currentEvaluation, + Blocker blocker) { if (currentEvaluation.getPassed() != null && currentEvaluation.getPassed().booleanValue()) { return AssessmentEntryStatus.done; } else if (currentEvaluation.getScore() != 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 20840356838..34da3224c89 100644 --- a/src/main/java/org/olat/course/run/scoring/AccountingEvaluatorsFactory.java +++ b/src/main/java/org/olat/course/run/scoring/AccountingEvaluatorsFactory.java @@ -150,8 +150,8 @@ class AccountingEvaluatorsFactory { private static class UnchangingStatusEvaluator implements StatusEvaluator { @Override - public AssessmentEntryStatus getStatus(AssessmentEvaluation previousEvaluation, - AssessmentEvaluation currentEvaluation, boolean firstChild) { + public AssessmentEntryStatus getStatus(AssessmentEvaluation currentEvaluation, + Blocker blocker) { return currentEvaluation.getAssessmentStatus(); } 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 f74940b0bba..4da1e7a378c 100644 --- a/src/main/java/org/olat/course/run/scoring/AssessmentAccounting.java +++ b/src/main/java/org/olat/course/run/scoring/AssessmentAccounting.java @@ -37,6 +37,7 @@ import org.olat.course.assessment.CourseAssessmentService; import org.olat.course.config.CourseConfig; import org.olat.course.nodes.CourseNode; import org.olat.course.run.scoring.LastModificationsEvaluator.LastModifications; +import org.olat.course.run.scoring.StatusEvaluator.Blocker; import org.olat.course.run.userview.UserCourseEnvironment; import org.olat.modules.assessment.AssessmentEntry; import org.olat.modules.assessment.model.AssessmentEntryStatus; @@ -58,6 +59,7 @@ public class AssessmentAccounting implements ScoreAccounting { private Map<String, AssessmentEntry> identToEntry = new HashMap<>(); private final Map<CourseNode, AssessmentEvaluation> courseNodeToEval = new HashMap<>(); private AssessmentEvaluation previousEvaluation; + private Blocker blocker; @Autowired private CourseAssessmentService courseAssessmentService; @@ -91,6 +93,7 @@ public class AssessmentAccounting implements ScoreAccounting { @Override public boolean evaluateAll(boolean update) { previousEvaluation = null; + blocker = new Blocker(); courseNodeToEval.clear(); identToEntry = loadAssessmentEntries(getIdentity()); @@ -174,9 +177,10 @@ public class AssessmentAccounting implements ScoreAccounting { result.setPassed(passed); StatusEvaluator statusEvaluator = evaluators.getStatusEvaluator(); - AssessmentEntryStatus status = statusEvaluator.getStatus(previousEvaluation, result, firstChild); + AssessmentEntryStatus status = statusEvaluator.getStatus(result, blocker); result.setStatus(status); + previousEvaluation = result; int childCount = courseNode.getChildCount(); List<AssessmentEvaluation> children = new ArrayList<>(childCount); diff --git a/src/main/java/org/olat/course/run/scoring/StatusEvaluator.java b/src/main/java/org/olat/course/run/scoring/StatusEvaluator.java index c20f4a90e64..2b74e46f8b1 100644 --- a/src/main/java/org/olat/course/run/scoring/StatusEvaluator.java +++ b/src/main/java/org/olat/course/run/scoring/StatusEvaluator.java @@ -31,8 +31,21 @@ import org.olat.modules.assessment.model.AssessmentEntryStatus; */ public interface StatusEvaluator { - public AssessmentEntryStatus getStatus(AssessmentEvaluation previousEvaluation, AssessmentEvaluation currentEvaluation, boolean firstChild); + public AssessmentEntryStatus getStatus(AssessmentEvaluation currentEvaluation, Blocker blocker); + + public AssessmentEntryStatus getStatus(AssessmentEvaluation currentEvaluation, List<AssessmentEvaluation> children); - public AssessmentEntryStatus getStatus(AssessmentEvaluation currentEvaluation, List<AssessmentEvaluation>children); + public static class Blocker { + + private boolean blocked = false; + + public boolean isBlocked() { + return blocked; + } + + public void block() { + blocked = true; + } + } } diff --git a/src/test/java/org/olat/course/learningpath/evaluation/DefaultLinearStatusEvaluatorTest.java b/src/test/java/org/olat/course/learningpath/evaluation/DefaultLinearStatusEvaluatorTest.java index fa740e41adb..5161f28a4fe 100644 --- a/src/test/java/org/olat/course/learningpath/evaluation/DefaultLinearStatusEvaluatorTest.java +++ b/src/test/java/org/olat/course/learningpath/evaluation/DefaultLinearStatusEvaluatorTest.java @@ -20,17 +20,10 @@ package org.olat.course.learningpath.evaluation; import static org.assertj.core.api.Assertions.assertThat; -import static org.olat.modules.assessment.model.AssessmentEntryStatus.done; -import static org.olat.modules.assessment.model.AssessmentEntryStatus.notReady; -import static org.olat.modules.assessment.model.AssessmentEntryStatus.notStarted; -import static org.olat.modules.assessment.model.AssessmentObligation.mandatory; -import static org.olat.modules.assessment.model.AssessmentObligation.optional; - -import java.util.Arrays; -import java.util.List; import org.junit.Test; import org.olat.course.run.scoring.AssessmentEvaluation; +import org.olat.course.run.scoring.StatusEvaluator.Blocker; import org.olat.modules.assessment.model.AssessmentEntryStatus; import org.olat.modules.assessment.model.AssessmentObligation; @@ -43,115 +36,91 @@ import org.olat.modules.assessment.model.AssessmentObligation; public class DefaultLinearStatusEvaluatorTest { private DefaultLinearStatusEvaluator sut = new DefaultLinearStatusEvaluator(); - + @Test - public void shouldReturnDoneIfAssessmentStatusIsDone() { - AssessmentEvaluation assessmentEvaluation = getAssessmentEvaluation(null, done, null); - - AssessmentEntryStatus status = sut.getStatus(null, assessmentEvaluation, false); + public void shouldBlockIfMandatoryAndNotFullyAssessed() { + Blocker blocker = new Blocker(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(Boolean.FALSE, null, AssessmentObligation.mandatory); + + sut.getStatus(currentEvaluation, blocker); - assertThat(status).isEqualTo(done); + assertThat(blocker.isBlocked()).isTrue(); } @Test - public void shouldReturnReadyIfIsRootNodeAndNotAlreadyDone() { - AssessmentEvaluation assessmentEvaluation = getAssessmentEvaluation(null, notStarted, null); - - AssessmentEntryStatus status = sut.getStatus(null, assessmentEvaluation, false); + public void shouldNotBlockIfNotMandatoryAndNotFullyAssessed() { + Blocker blocker = new Blocker(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(Boolean.FALSE, null, AssessmentObligation.optional); + + sut.getStatus(currentEvaluation, blocker); - assertThat(status).isEqualTo(notStarted); + assertThat(blocker.isBlocked()).isFalse(); } @Test - public void shouldReturnReadyIfIsRootNodeAndIsNotStartedYet() { - AssessmentEvaluation assessmentEvaluation = getAssessmentEvaluation(null, notReady, null); - - AssessmentEntryStatus status = sut.getStatus(null, assessmentEvaluation, false); + public void shouldNotBlockIfMandatoryAndFullyAssessed() { + Blocker blocker = new Blocker(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(Boolean.TRUE, null, AssessmentObligation.mandatory); + + sut.getStatus(currentEvaluation, blocker); - assertThat(status).isEqualTo(notStarted); + assertThat(blocker.isBlocked()).isFalse(); } @Test - public void shouldReturnReadyIfIsRootNodeAndHasNoStatusYet() { - AssessmentEvaluation assessmentEvaluation = getAssessmentEvaluation(null, null, null); + public void shouldNotChangeStatusIfBlockedButInProgress() { + Blocker blocker = new Blocker(); + blocker.block(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, AssessmentEntryStatus.inProgress, null); - AssessmentEntryStatus status = sut.getStatus(null, assessmentEvaluation, false); + AssessmentEntryStatus status = sut.getStatus(currentEvaluation, blocker); - assertThat(status).isEqualTo(notStarted); - } - - @Test - public void shouldReturnReadyIfPreviousNodeFullyAssessed() { - assertStatus(Boolean.TRUE, null, null, null, notStarted); - } - - @Test - public void shouldReturnReadyIfPreviousNodeFullyAssessedAndIsMandatory() { - assertStatus(Boolean.TRUE, null, mandatory, null, notStarted); - } - - @Test - public void shouldReturnReadyIfPreviousNodeIsOptionalAndIsReady() { - assertStatus(Boolean.FALSE, notStarted, optional, notReady, notStarted); + assertThat(status).isEqualTo(AssessmentEntryStatus.inProgress); } - - @Test - public void shouldReturnReadyIfPreviousNodeHasNoObligationAndIsReady() { - // No obligation means optional. E.g. the STCourseNode has no obligation and is - // always the first node in a course tree - assertStatus(Boolean.FALSE, notStarted, null, notReady, notStarted); - } - - @Test - public void shouldReturnNotReadyIfPreviousNodeIsOptionalAndIsNotReady() { - assertStatus(Boolean.FALSE, notReady, optional, notReady, notReady); - } - + @Test - public void shouldReturnNotReadyIfPreviousNodeIsMandatoryAndNotFullyAssessedYet() { - assertStatus(Boolean.FALSE, null, mandatory, null, notReady); + public void shouldNotChangeStatusIfBlockedButInReview() { + Blocker blocker = new Blocker(); + blocker.block(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, AssessmentEntryStatus.inReview, null); + + AssessmentEntryStatus status = sut.getStatus(currentEvaluation, blocker); + + assertThat(status).isEqualTo(AssessmentEntryStatus.inReview); } - + @Test - public void shouldReturnNotReadyIfPreviousNodeIsMandatoryAndFullyAssessedIsNull() { - assertStatus(null, null, mandatory, null, notReady); - } - - private void assertStatus(Boolean previousFullyAssessd, AssessmentEntryStatus previousStatus, - AssessmentObligation previousObligation, AssessmentEntryStatus currentStatus, - AssessmentEntryStatus expected) { - AssessmentEvaluation previousEvaluation = getAssessmentEvaluation(previousFullyAssessd, previousStatus, previousObligation); - AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, currentStatus, null); + public void shouldNotChangeStatusIfBlockedButDone() { + Blocker blocker = new Blocker(); + blocker.block(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, AssessmentEntryStatus.done, null); - AssessmentEntryStatus status = sut.getStatus(previousEvaluation, currentEvaluation, false); + AssessmentEntryStatus status = sut.getStatus(currentEvaluation, blocker); - assertThat(status).isEqualTo(expected); + assertThat(status).isEqualTo(AssessmentEntryStatus.done); } - + @Test - public void shouldRetrunNotStartedIfisFirstChildAndPreviuosIsNotStartedAndMandatory() { - AssessmentEvaluation previousEvaluation = getAssessmentEvaluation(null, AssessmentEntryStatus.notStarted, AssessmentObligation.mandatory); - AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, AssessmentEntryStatus.notReady, null); + public void shouldSetNotStartedIfNotBlocked() { + Blocker blocker = new Blocker(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, null, null); - AssessmentEntryStatus status = sut.getStatus(previousEvaluation, currentEvaluation, true); + AssessmentEntryStatus status = sut.getStatus(currentEvaluation, blocker); assertThat(status).isEqualTo(AssessmentEntryStatus.notStarted); } - @Test - public void shouldNotChangeStatusWhenCheckingAgainstChildren() { - AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, notStarted, null); - AssessmentEvaluation child1 = getAssessmentEvaluation(Boolean.TRUE, done, null); - AssessmentEvaluation child2 = getAssessmentEvaluation(Boolean.TRUE, done, null); - List<AssessmentEvaluation> children = Arrays.asList(child1, child2); + public void shouldSetNotReadyIfBlocked() { + Blocker blocker = new Blocker(); + blocker.block(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, null, null); - AssessmentEntryStatus status = sut.getStatus(currentEvaluation, children); + AssessmentEntryStatus status = sut.getStatus(currentEvaluation, blocker); - assertThat(status).isEqualTo(notStarted); + assertThat(status).isEqualTo(AssessmentEntryStatus.notReady); } - private AssessmentEvaluation getAssessmentEvaluation(Boolean fullyAssessd, AssessmentEntryStatus assessmentStatus, AssessmentObligation obligation) { return new AssessmentEvaluation(null, null, null, null, assessmentStatus, null, fullyAssessd, null, null, null, null, null, 0, null, null, null, null, obligation, null); } diff --git a/src/test/java/org/olat/course/nodes/st/assessment/STLinearStatusEvaluatorTest.java b/src/test/java/org/olat/course/nodes/st/assessment/STLinearStatusEvaluatorTest.java index 9dc3815a459..625c23ec4cc 100644 --- a/src/test/java/org/olat/course/nodes/st/assessment/STLinearStatusEvaluatorTest.java +++ b/src/test/java/org/olat/course/nodes/st/assessment/STLinearStatusEvaluatorTest.java @@ -26,6 +26,7 @@ import java.util.List; import org.junit.Test; import org.olat.course.run.scoring.AssessmentEvaluation; +import org.olat.course.run.scoring.StatusEvaluator.Blocker; import org.olat.modules.assessment.model.AssessmentEntryStatus; import org.olat.modules.assessment.model.AssessmentObligation; @@ -38,6 +39,56 @@ import org.olat.modules.assessment.model.AssessmentObligation; public class STLinearStatusEvaluatorTest { private STLinearStatusEvaluator sut = new STLinearStatusEvaluator(); + @Test + public void shouldNotBlockIfMandatoryAndNotFullyAssessed() { + Blocker blocker = new Blocker(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(Boolean.FALSE, null, AssessmentObligation.mandatory); + + sut.getStatus(currentEvaluation, blocker); + + assertThat(blocker.isBlocked()).isFalse(); + } + + @Test + public void shouldNotBlockIfNotMandatoryAndNotFullyAssessed() { + Blocker blocker = new Blocker(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(Boolean.FALSE, null, AssessmentObligation.optional); + + sut.getStatus(currentEvaluation, blocker); + + assertThat(blocker.isBlocked()).isFalse(); + } + + @Test + public void shouldNotBlockIfMandatoryAndFullyAssessed() { + Blocker blocker = new Blocker(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(Boolean.TRUE, null, AssessmentObligation.mandatory); + + sut.getStatus(currentEvaluation, blocker); + + assertThat(blocker.isBlocked()).isFalse(); + } + + @Test + public void shouldInitStatusNotStartedIfNotBlocked() { + Blocker blocker = new Blocker(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, AssessmentEntryStatus.done, null); + + AssessmentEntryStatus status = sut.getStatus(currentEvaluation, blocker); + + assertThat(status).isEqualTo(AssessmentEntryStatus.notStarted); + } + + @Test + public void shouldInitStatusNotReadyIfBlocked() { + Blocker blocker = new Blocker(); + blocker.block(); + AssessmentEvaluation currentEvaluation = getAssessmentEvaluation(null, AssessmentEntryStatus.done, null); + + AssessmentEntryStatus status = sut.getStatus(currentEvaluation, blocker); + + assertThat(status).isEqualTo(AssessmentEntryStatus.notReady); + } @Test public void shouldReturnInProgressIfChildIsInProgressNull() { -- GitLab