diff --git a/src/main/java/org/olat/course/nodes/st/assessment/ConventionalSTCompletionEvaluator.java b/src/main/java/org/olat/course/nodes/st/assessment/ConventionalSTCompletionEvaluator.java index a3bb1149f24b70d4dd8d350b94916d69f543fe8a..a91a38ab9d1503bb621621729b1d2df188bb9084 100644 --- a/src/main/java/org/olat/course/nodes/st/assessment/ConventionalSTCompletionEvaluator.java +++ b/src/main/java/org/olat/course/nodes/st/assessment/ConventionalSTCompletionEvaluator.java @@ -20,10 +20,8 @@ package org.olat.course.nodes.st.assessment; import org.olat.core.CoreSpringFactory; -import org.olat.core.util.tree.TreeVisitor; import org.olat.course.assessment.CourseAssessmentService; import org.olat.course.assessment.handler.AssessmentConfig; -import org.olat.course.nodes.CollectingVisitor; import org.olat.course.nodes.CourseNode; import org.olat.course.run.scoring.AssessmentEvaluation; import org.olat.course.run.scoring.CompletionEvaluator; @@ -52,37 +50,17 @@ public class ConventionalSTCompletionEvaluator implements CompletionEvaluator { ScoreAccounting scoreAccounting) { Double completion = null; - if (isPassed(currentEvaluation)) { - completion = Double.valueOf(1.0); - } else { - // get all children - CollectingVisitor visitor = CollectingVisitor.testing(cn -> !cn.getIdent().equals(courseNode.getIdent())); - TreeVisitor tv = new TreeVisitor(visitor, courseNode, true); - tv.visitAll(); - - int countIsPassedConfig = 0; - int countIsPassed = 0; - for (CourseNode child: visitor.getCourseNodes()) { - AssessmentConfig assessmentConfig = courseAssessmentService.getAssessmentConfig(child); - if (isPassedConfigurated(assessmentConfig)) { - countIsPassedConfig++; - AssessmentEvaluation assessmentEvaluation = scoreAccounting.evalCourseNode(child); - if (isNodePassed(assessmentEvaluation)) { - countIsPassed++; - } - } - } - if (countIsPassedConfig > 0) { - completion = Double.valueOf((double)countIsPassed / countIsPassedConfig); + + if (courseNode.getParent() == null) { + AssessmentConfig assessmentConfig = courseAssessmentService.getAssessmentConfig(courseNode); + if (isPassedConfigurated(assessmentConfig)) { + completion = isNodePassed(currentEvaluation)? Double.valueOf(1.0): Double.valueOf(0.0); } } + return completion; } - private boolean isPassed(AssessmentEvaluation currentEvaluation) { - return Boolean.TRUE.equals(currentEvaluation.getPassed()); - } - private boolean isNodePassed(AssessmentEvaluation assessmentEvaluation) { return Boolean.TRUE.equals(assessmentEvaluation.getPassed()) && Boolean.TRUE.equals(assessmentEvaluation.getUserVisible()); diff --git a/src/main/java/org/olat/repository/ui/list/RepositoryEntryListController.java b/src/main/java/org/olat/repository/ui/list/RepositoryEntryListController.java index 115ee70dc5c916e7aadf6a5476ac235676b315be..b7bdfc2d65dbcbfc7cdce6e440892e1758e55efc 100644 --- a/src/main/java/org/olat/repository/ui/list/RepositoryEntryListController.java +++ b/src/main/java/org/olat/repository/ui/list/RepositoryEntryListController.java @@ -79,6 +79,9 @@ import org.olat.core.util.StringHelper; import org.olat.core.util.Util; import org.olat.core.util.resource.OresHelper; import org.olat.course.CorruptedCourseException; +import org.olat.course.CourseFactory; +import org.olat.course.ICourse; +import org.olat.course.condition.ConditionNodeAccessProvider; import org.olat.repository.RepositoryEntry; import org.olat.repository.RepositoryEntryStatusEnum; import org.olat.repository.RepositoryManager; @@ -635,6 +638,9 @@ public class RepositoryEntryListController extends FormBasicController @Override public void forgeCompletion(RepositoryEntryRow row) { if(!guestOnly && row.getCompletion() != null) { + if (isConventionalUnpassedCourse(row)) { + return; + } ProgressBarItem completionItem = new ProgressBarItem("completion_" + row.getKey(), 100, row.getCompletion().floatValue(), Float.valueOf(1), null); completionItem.setWidthInPercent(true); @@ -656,6 +662,15 @@ public class RepositoryEntryListController extends FormBasicController row.setCompletionItem(completionItem); } } + + private boolean isConventionalUnpassedCourse(RepositoryEntryRow row) { + if ("CourseModule".equals(row.getOLATResourceable().getResourceableTypeName())) { + ICourse course = CourseFactory.loadCourse(row.getOLATResourceable()); + return ConditionNodeAccessProvider.TYPE.equals(course.getCourseConfig().getNodeAccessType().getType()) + && row.getCompletion().doubleValue() < 1.0; + } + return false; + } @Override public void forgeSelectLink(RepositoryEntryRow row) { diff --git a/src/main/java/org/olat/upgrade/OLATUpgrade_15_pre_0.java b/src/main/java/org/olat/upgrade/OLATUpgrade_15_pre_0.java index 76f085791642a00ee03b4870ea7368d71fbc7237..9157bd0b66355db3be4ebac7b331d2241dcf481a 100644 --- a/src/main/java/org/olat/upgrade/OLATUpgrade_15_pre_0.java +++ b/src/main/java/org/olat/upgrade/OLATUpgrade_15_pre_0.java @@ -77,6 +77,11 @@ public class OLATUpgrade_15_pre_0 extends OLATUpgrade { uhd.setInstallationComplete(allOk); upgradeManager.setUpgradesHistory(uhd, VERSION); + + // OLATUpgrade_15_pre_6_ae has only to run if this upgrade (OLATUpgrade_15_pre_0) + // is run before the fix for the completion of assessment entries of conventional courses. + upgradeManager.setUpgradesHistory(uhd, OLATUpgrade_15_pre_6_ae.VERSION); + if(allOk) { log.info(Tracing.M_AUDIT, "Finished OLATUpgrade_15_pre_0 successfully!"); } else { diff --git a/src/main/java/org/olat/upgrade/OLATUpgrade_15_pre_6_ae.java b/src/main/java/org/olat/upgrade/OLATUpgrade_15_pre_6_ae.java new file mode 100644 index 0000000000000000000000000000000000000000..6968dbee5574b6f3a20ee168f64b26f2d1f45420 --- /dev/null +++ b/src/main/java/org/olat/upgrade/OLATUpgrade_15_pre_6_ae.java @@ -0,0 +1,163 @@ +/** + * <a href="http://www.openolat.org"> + * OpenOLAT - Online Learning and Training</a><br> + * <p> + * Licensed under the Apache License, Version 2.0 (the "License"); <br> + * you may not use this file except in compliance with the License.<br> + * You may obtain a copy of the License at the + * <a href="http://www.apache.org/licenses/LICENSE-2.0">Apache homepage</a> + * <p> + * Unless required by applicable law or agreed to in writing,<br> + * software distributed under the License is distributed on an "AS IS" BASIS, <br> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. <br> + * See the License for the specific language governing permissions and <br> + * limitations under the License. + * <p> + * Initial code contributed and copyrighted by<br> + * frentix GmbH, http://www.frentix.com + * <p> + */ +package org.olat.upgrade; + +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.logging.log4j.Logger; +import org.olat.core.commons.persistence.DB; +import org.olat.core.commons.persistence.QueryBuilder; +import org.olat.core.logging.Tracing; +import org.olat.course.CorruptedCourseException; +import org.olat.course.CourseFactory; +import org.olat.course.ICourse; +import org.olat.course.Structure; +import org.olat.course.assessment.CourseAssessmentService; +import org.olat.course.condition.ConditionNodeAccessProvider; +import org.olat.course.nodeaccess.NodeAccessType; +import org.olat.repository.RepositoryEntry; +import org.springframework.beans.factory.annotation.Autowired; + +/** + * + * Initial date: 17.03.2020<br> + * @author uhensler, urs.hensler@frentix.com, http://www.frentix.com + * + */ +public class OLATUpgrade_15_pre_6_ae extends OLATUpgrade { + + private static final Logger log = Tracing.createLoggerFor(OLATUpgrade_15_pre_6_ae.class); + + static final String VERSION = "OLAT_15.pre.6.ae"; + private static final String CONDITION_COURSE_COMPLETION = "CONDITION COURSES COMPLETION"; + + @Autowired + private DB dbInstance; + @Autowired + private CourseAssessmentService courseAssessmentService; + + public OLATUpgrade_15_pre_6_ae() { + super(); + } + + @Override + public String getVersion() { + return VERSION; + } + + @Override + public boolean doPostSystemInitUpgrade(UpgradeManager upgradeManager) { + UpgradeHistoryData uhd = upgradeManager.getUpgradesHistory(VERSION); + if (uhd == null) { + // has never been called, initialize + uhd = new UpgradeHistoryData(); + } else if (uhd.isInstallationComplete()) { + return false; + } + + boolean allOk = true; + allOk &= migrateAssessmentEntrCompletion(upgradeManager, uhd); + + uhd.setInstallationComplete(allOk); + upgradeManager.setUpgradesHistory(uhd, VERSION); + if(allOk) { + log.info(Tracing.M_AUDIT, "Finished OLATUpgrade_15_pre_6_ae successfully!"); + } else { + log.info(Tracing.M_AUDIT, "OLATUpgrade_15_pre_6_ae not finished, try to restart OpenOlat!"); + } + return allOk; + } + + + private boolean migrateAssessmentEntrCompletion(UpgradeManager upgradeManager, UpgradeHistoryData uhd) { + boolean allOk = true; + if (!uhd.getBooleanDataValue(CONDITION_COURSE_COMPLETION)) { + try { + migrateCourseAssessmentEntries(); + dbInstance.commitAndCloseSession(); + log.info("All assessment entry completions migrated."); + } catch (Exception e) { + log.error("", e); + allOk = false; + } + + uhd.setBooleanDataValue(CONDITION_COURSE_COMPLETION, allOk); + upgradeManager.setUpgradesHistory(uhd, VERSION); + } + return allOk; + } + + private void migrateCourseAssessmentEntries() { + List<RepositoryEntry> courseEntries = getCourseEntries(); + + AtomicInteger migrationCounter = new AtomicInteger(0); + for (RepositoryEntry repositoryEntry : courseEntries) { + calculateCourseAssessmentEntries(repositoryEntry); + migrationCounter.incrementAndGet(); + if(migrationCounter.get() % 25 == 0) { + dbInstance.commitAndCloseSession(); + } else { + dbInstance.commit(); + } + if(migrationCounter.get() % 100 == 0) { + log.info("Assessment entries: num. of courses migrated: {}", migrationCounter); + } + } + } + + private List<RepositoryEntry> getCourseEntries() { + QueryBuilder sb = new QueryBuilder(); + sb.append("select re"); + sb.append(" from repositoryentry re"); + sb.append(" inner join re.olatResource as ores"); + sb.and().append(" ores.resName = 'CourseModule'"); + sb.append(" order by re.statistics.lastUsage desc"); + + return dbInstance.getCurrentEntityManager() + .createQuery(sb.toString(), RepositoryEntry.class) + .getResultList(); + } + + private void calculateCourseAssessmentEntries(RepositoryEntry repositoryEntry) { + try { + ICourse course = CourseFactory.loadCourse(repositoryEntry); + if (course != null) { + NodeAccessType nodeAccessType = NodeAccessType.of(course); + if (ConditionNodeAccessProvider.TYPE.equals(nodeAccessType.getType())) { + Structure runStructure = course.getCourseEnvironment().getRunStructure(); + if (runStructure != null) { + courseAssessmentService.evaluateAll(course); + log.info("Assessment entry completions calculated: course {} ({}).", + repositoryEntry.getKey(), repositoryEntry.getDisplayname()); + } + } + } + } catch (CorruptedCourseException cce) { + log.warn("CorruptedCourseException in migration of assessment entry completion of course {} ({})", + repositoryEntry.getKey(), repositoryEntry.getDisplayname()); + } catch (Exception e) { + log.error("Error in migration of assessment entry completion of course {} ({}).", repositoryEntry.getKey(), + repositoryEntry.getDisplayname()); + log.error("", e); + } + } + +} diff --git a/src/main/java/org/olat/upgrade/_spring/upgradeContext.xml b/src/main/java/org/olat/upgrade/_spring/upgradeContext.xml index 3da015e50dabc04a3130c369e14e88469001f57d..c788ce63b379e049c6426a23cb4ce73ea0ed3c3d 100644 --- a/src/main/java/org/olat/upgrade/_spring/upgradeContext.xml +++ b/src/main/java/org/olat/upgrade/_spring/upgradeContext.xml @@ -56,6 +56,7 @@ <bean id="upgrade_14_2_0" class="org.olat.upgrade.OLATUpgrade_14_2_0"/> <bean id="upgrade_15_pre_6" class="org.olat.upgrade.OLATUpgrade_15_pre_6"/><!-- because really quicker as pre.0 --> <bean id="upgrade_15_pre_0" class="org.olat.upgrade.OLATUpgrade_15_pre_0"/> + <bean id="upgrade_15_pre_6_ae" class="org.olat.upgrade.OLATUpgrade_15_pre_6_ae"/> </list> </property> </bean> diff --git a/src/test/java/org/olat/course/nodes/st/assessment/ConventionalSTCompletionEvaluatorTest.java b/src/test/java/org/olat/course/nodes/st/assessment/ConventionalSTCompletionEvaluatorTest.java index 358d63ca2e2dbeaf760fd280cc8734e9da03cfde..466ff56ff1a6b96f7801ae77f8263626a9c4963b 100644 --- a/src/test/java/org/olat/course/nodes/st/assessment/ConventionalSTCompletionEvaluatorTest.java +++ b/src/test/java/org/olat/course/nodes/st/assessment/ConventionalSTCompletionEvaluatorTest.java @@ -29,7 +29,6 @@ import org.mockito.MockitoAnnotations; import org.olat.course.assessment.CourseAssessmentService; import org.olat.course.assessment.MappedScoreAccounting; import org.olat.course.assessment.handler.AssessmentConfig; -import org.olat.course.nodes.Card2BrainCourseNode; import org.olat.course.nodes.CourseNode; import org.olat.course.nodes.STCourseNode; import org.olat.course.run.scoring.AssessmentEvaluation; @@ -63,53 +62,89 @@ public class ConventionalSTCompletionEvaluatorTest { } @Test - public void shouldReturn1IfCondittionIsPassed() { + public void shouldReturnNullIfItIsNotRoot() { MappedScoreAccounting scoreAccounting = new MappedScoreAccounting(); - CourseNode parent = new STCourseNode(); + CourseNode root = new STCourseNode(); + CourseNode child = new STCourseNode(); + root.addChild(child); + AssessmentEvaluation parrentEvaluation = createAssessmentEvaluation(Boolean.TRUE, Boolean.TRUE); + scoreAccounting.put(child, parrentEvaluation); + when(courseAssessmentService.getAssessmentConfig(child)).thenReturn(configWithPassed); + + Double completion = sut.getCompletion(parrentEvaluation, child, scoreAccounting); + + assertThat(completion).isNull(); + } + + @Test + public void shouldReturnNullIfHasNoPassConfig() { + MappedScoreAccounting scoreAccounting = new MappedScoreAccounting(); + + CourseNode root = new STCourseNode(); + AssessmentEvaluation parrentEvaluation = createAssessmentEvaluation(Boolean.TRUE, Boolean.TRUE); + scoreAccounting.put(root, parrentEvaluation); + when(courseAssessmentService.getAssessmentConfig(root)).thenReturn(configWithoutPassed); + + Double completion = sut.getCompletion(parrentEvaluation, root, scoreAccounting); + + assertThat(completion).isNull(); + } + + @Test + public void shouldReturn0IfIsFailed() { + MappedScoreAccounting scoreAccounting = new MappedScoreAccounting(); + + CourseNode root = new STCourseNode(); + AssessmentEvaluation parrentEvaluation = createAssessmentEvaluation(Boolean.FALSE, Boolean.TRUE); + scoreAccounting.put(root, parrentEvaluation); + when(courseAssessmentService.getAssessmentConfig(root)).thenReturn(configWithPassed); + + Double completion = sut.getCompletion(parrentEvaluation, root, scoreAccounting); + + assertThat(completion).isEqualTo(0.0); + } + + @Test + public void shouldReturn0IfNOtPassedYet() { + MappedScoreAccounting scoreAccounting = new MappedScoreAccounting(); + + CourseNode root = new STCourseNode(); + AssessmentEvaluation parrentEvaluation = createAssessmentEvaluation(null, Boolean.TRUE); + scoreAccounting.put(root, parrentEvaluation); + when(courseAssessmentService.getAssessmentConfig(root)).thenReturn(configWithPassed); + + Double completion = sut.getCompletion(parrentEvaluation, root, scoreAccounting); + + assertThat(completion).isEqualTo(0.0); + } + + @Test + public void shouldReturn0IfIsPassedButNotUserVisible() { + MappedScoreAccounting scoreAccounting = new MappedScoreAccounting(); + + CourseNode root = new STCourseNode(); AssessmentEvaluation parrentEvaluation = createAssessmentEvaluation(Boolean.TRUE, null); - scoreAccounting.put(parent, parrentEvaluation); + scoreAccounting.put(root, parrentEvaluation); + when(courseAssessmentService.getAssessmentConfig(root)).thenReturn(configWithPassed); - Double completion = sut.getCompletion(parrentEvaluation, parent, scoreAccounting); + Double completion = sut.getCompletion(parrentEvaluation, root, scoreAccounting); - assertThat(completion).isEqualByComparingTo(1.0); + assertThat(completion).isEqualTo(0.0); } @Test - public void shouldReturnNumberOfPassedNodes() { + public void shouldReturn1IfIsPassedAndUserVisible() { MappedScoreAccounting scoreAccounting = new MappedScoreAccounting(); - CourseNode parent = new STCourseNode(); - AssessmentEvaluation parrentEvaluation = createAssessmentEvaluation(Boolean.FALSE, null); - scoreAccounting.put(parent, parrentEvaluation); - // Child with passed configured: passed - CourseNode child1 = new Card2BrainCourseNode(); - parent.addChild(child1); - AssessmentEvaluation assessedEvaluation1 = createAssessmentEvaluation(Boolean.TRUE, Boolean.TRUE); - scoreAccounting.put(child1, assessedEvaluation1); - when(courseAssessmentService.getAssessmentConfig(child1)).thenReturn(configWithPassed); - // Child with passed configured: passed but not user visible - CourseNode child2= new Card2BrainCourseNode(); - parent.addChild(child2); - AssessmentEvaluation assessedEvaluation2 = createAssessmentEvaluation(Boolean.TRUE, Boolean.FALSE); - scoreAccounting.put(child2, assessedEvaluation2); - when(courseAssessmentService.getAssessmentConfig(child2)).thenReturn(configWithPassed); - // Child with passed configured: not passed - CourseNode child3 = new Card2BrainCourseNode(); - parent.addChild(child3); - AssessmentEvaluation assessedEvaluation3 = createAssessmentEvaluation(Boolean.FALSE, Boolean.TRUE); - scoreAccounting.put(child3, assessedEvaluation3); - when(courseAssessmentService.getAssessmentConfig(child3)).thenReturn(configWithPassed); - // Child without passed configured: - CourseNode child4 = new Card2BrainCourseNode(); - parent.addChild(child4); - AssessmentEvaluation assessedEvaluation4 = createAssessmentEvaluation(Boolean.TRUE, Boolean.TRUE); - scoreAccounting.put(child4, assessedEvaluation4); - when(courseAssessmentService.getAssessmentConfig(child4)).thenReturn(configWithPassed); - - Double completion = sut.getCompletion(parrentEvaluation, parent, scoreAccounting); - - assertThat(completion).isEqualByComparingTo(0.5); + CourseNode root = new STCourseNode(); + AssessmentEvaluation parrentEvaluation = createAssessmentEvaluation(Boolean.TRUE, Boolean.TRUE); + scoreAccounting.put(root, parrentEvaluation); + when(courseAssessmentService.getAssessmentConfig(root)).thenReturn(configWithPassed); + + Double completion = sut.getCompletion(parrentEvaluation, root, scoreAccounting); + + assertThat(completion).isEqualTo(1.0); } private AssessmentEvaluation createAssessmentEvaluation(Boolean passed, Boolean userVisibility) { diff --git a/src/test/java/org/olat/modules/assessment/manager/AssessmentEntryDAOTest.java b/src/test/java/org/olat/modules/assessment/manager/AssessmentEntryDAOTest.java index 4931e15723a71d5546e934ccdcaaa3f3e5d4e000..8d3ec6f13a09a0ba371f4b1adbe71cbfe6c257bc 100644 --- a/src/test/java/org/olat/modules/assessment/manager/AssessmentEntryDAOTest.java +++ b/src/test/java/org/olat/modules/assessment/manager/AssessmentEntryDAOTest.java @@ -776,6 +776,10 @@ public class AssessmentEntryDAOTest extends OlatTestCase { random(), Boolean.TRUE, null); nodeAssessment2_2Root.setCompletion(0.2); assessmentEntryDao.updateAssessmentEntry(nodeAssessment2_2Root); + AssessmentEntry nodeAssessment2_3Root = assessmentEntryDao.createAssessmentEntry(assessedIdentity2, null, entry3, + random(), Boolean.TRUE, null); + nodeAssessment2_3Root.setCompletion(null); + assessmentEntryDao.updateAssessmentEntry(nodeAssessment2_3Root); // other identity AssessmentEntry nodeAssessment1OtherIdent = assessmentEntryDao.createAssessmentEntry(assessedIdentityOther, null,