From 328f9d11247a141d75bc5cde0d5569a3027efeab Mon Sep 17 00:00:00 2001
From: uhensler <urs.hensler@frentix.com>
Date: Fri, 27 Sep 2019 10:43:44 +0200
Subject: [PATCH] OO-4207: Course node children are not accessible if the
 parent isn't

---
 .../nodes/st/STCourseNodeRunController.java   |  3 +--
 .../run/navigation/NavigationHandler.java     | 17 +------------
 .../run/userview/CourseTreeModelBuilder.java  |  4 +++
 .../run/userview/CourseTreeVisitor.java       | 25 ++++++-------------
 .../java/org/olat/course/site/CourseSite.java |  4 +--
 .../ui/CourseTemplateSearchController.java    | 20 ++++++---------
 .../indexer/repository/CourseIndexer.java     | 10 ++------
 7 files changed, 25 insertions(+), 58 deletions(-)

diff --git a/src/main/java/org/olat/course/nodes/st/STCourseNodeRunController.java b/src/main/java/org/olat/course/nodes/st/STCourseNodeRunController.java
index ff8c581f8a3..2fbc8e467fb 100644
--- a/src/main/java/org/olat/course/nodes/st/STCourseNodeRunController.java
+++ b/src/main/java/org/olat/course/nodes/st/STCourseNodeRunController.java
@@ -56,7 +56,6 @@ import org.olat.course.nodes.CourseNode;
 import org.olat.course.nodes.CourseNodeFactory;
 import org.olat.course.nodes.ObjectivesHelper;
 import org.olat.course.nodes.STCourseNode;
-import org.olat.course.run.navigation.NavigationHandler;
 import org.olat.course.run.scoring.ScoreEvaluation;
 import org.olat.course.run.userview.CourseTreeNode;
 import org.olat.course.run.userview.TreeEvaluation;
@@ -137,7 +136,7 @@ public class STCourseNodeRunController extends BasicController {
 					CourseNode child = childCourseTreeNode.getCourseNode();
 					Controller childViewController = null;
 					Controller childPeekViewController = null;
-					boolean accessible = NavigationHandler.mayAccessWholeTreeUp(childCourseTreeNode);
+					boolean accessible = childCourseTreeNode.isAccessible();
 					if (displayType.equals(STCourseNodeEditController.CONFIG_VALUE_DISPLAY_PEEKVIEW)) {
 						if (peekviewChildNodes.size() == 0) {
 							// Special case: no child nodes configured. This is the case when
diff --git a/src/main/java/org/olat/course/run/navigation/NavigationHandler.java b/src/main/java/org/olat/course/run/navigation/NavigationHandler.java
index 58d5fd5e227..c7da6335aeb 100644
--- a/src/main/java/org/olat/course/run/navigation/NavigationHandler.java
+++ b/src/main/java/org/olat/course/run/navigation/NavigationHandler.java
@@ -346,10 +346,6 @@ public class NavigationHandler implements Disposable {
 			if (!newCalledTreeNode.isVisible()) {
 				throw new AssertException("node eval not visible!!");
 			}
-			// 2. start with the current NodeEvaluation, evaluate overall accessiblity
-			// per node bottom-up to see if all ancestors still grant access to the
-			// desired node
-			boolean mayAccessWholeTreeUp = mayAccessWholeTreeUp(newCalledTreeNode);
 			String newSelectedNodeId = newCalledTreeNode.getIdent();
 			Controller controller;
 			AdditionalConditionManager addMan = null;
@@ -359,7 +355,7 @@ public class NavigationHandler implements Disposable {
 				addMan = new AdditionalConditionManager((AbstractAccessableCourseNode)courseNode, courseId, identityEnv);
 			}
 			
-			if (!mayAccessWholeTreeUp|| (addMan != null && !addMan.evaluateConditions())) {
+			if (!newCalledTreeNode.isAccessible() || (addMan != null && !addMan.evaluateConditions())) {
 				// we cannot access the node anymore (since e.g. a time constraint
 				// changed), so give a (per-node-configured) explanation why and what
 				// the access conditions would be (a free form text, should be
@@ -644,17 +640,6 @@ public class NavigationHandler implements Disposable {
 		}
 	}
 	
-	public static boolean mayAccessWholeTreeUp(CourseTreeNode courseTreeNode) {
-		CourseTreeNode curTreeNode = courseTreeNode;
-		boolean mayAccess;
-		do {
-			mayAccess = curTreeNode.isAccessible();
-			curTreeNode = (CourseTreeNode) curTreeNode.getParent();
-		} while (curTreeNode != null && mayAccess);
-		// top reached or may not access node
-		return mayAccess;
-	}
-	
 	private static class SubTree {
 		private final Controller controller;
 		private final GenericTreeModel treeModel;
diff --git a/src/main/java/org/olat/course/run/userview/CourseTreeModelBuilder.java b/src/main/java/org/olat/course/run/userview/CourseTreeModelBuilder.java
index e259b405cf9..320999921c3 100644
--- a/src/main/java/org/olat/course/run/userview/CourseTreeModelBuilder.java
+++ b/src/main/java/org/olat/course/run/userview/CourseTreeModelBuilder.java
@@ -59,6 +59,10 @@ public abstract class CourseTreeModelBuilder {
 				CourseNode cn = (CourseNode) courseNode.getChildAt(i);
 				CourseTreeNode child = getCourseTreeNode(cn, treeEval, filter, childLevel);
 				if (child.isVisible()) {
+					// if the parent is not accessible the child is not accessible as well!
+					if (!treeNode.isAccessible()) {
+						child.setAccessible(false);
+					}
 					treeNode.addChild(child);
 				}
 			}
diff --git a/src/main/java/org/olat/course/run/userview/CourseTreeVisitor.java b/src/main/java/org/olat/course/run/userview/CourseTreeVisitor.java
index d6904d7cb4b..aba6127ff21 100644
--- a/src/main/java/org/olat/course/run/userview/CourseTreeVisitor.java
+++ b/src/main/java/org/olat/course/run/userview/CourseTreeVisitor.java
@@ -27,7 +27,6 @@ import org.olat.course.ICourse;
 import org.olat.course.nodeaccess.NodeAccessService;
 import org.olat.course.nodes.CourseNode;
 import org.olat.course.run.environment.CourseEnvironment;
-import org.olat.course.run.navigation.NavigationHandler;
 
 /**
  * 
@@ -63,33 +62,25 @@ public class CourseTreeVisitor {
 		TreeNode treeNode = nodeAccessService.getCourseTreeModelBuilder(uce)
 				.build(new TreeEvaluation(), filter)
 				.getNodeById(node.getIdent());
-		if (treeNode instanceof CourseTreeNode) {
-			boolean mayAccessWholeTreeUp = NavigationHandler.mayAccessWholeTreeUp((CourseTreeNode)treeNode);
-			if(mayAccessWholeTreeUp) {
-				return true;
-			}
-		}
-
-		return false;
+		return treeNode != null? treeNode.isAccessible(): false;
 	}
 	
 	public void visit(Visitor visitor, TreeFilter filter) {
 		UserCourseEnvironment userCourseEnv = new UserCourseEnvironmentImpl(ienv, courseEnv);
 		TreeEvaluation treeEval = new TreeEvaluation();
-		CourseTreeNode rootTreeNode = (CourseTreeNode)nodeAccessService.getCourseTreeModelBuilder(userCourseEnv)
+		TreeNode rootTreeNode = nodeAccessService.getCourseTreeModelBuilder(userCourseEnv)
 				.build(treeEval, filter)
 				.getRootNode();
-		visit(visitor, rootTreeNode, userCourseEnv, treeEval, filter);
+		visit(visitor, rootTreeNode);
 	}
 	
-	private void visit(Visitor visitor, CourseTreeNode node, UserCourseEnvironment userCourseEnv, TreeEvaluation treeEval, TreeFilter filter) {
-		boolean mayAccessWholeTreeUp = NavigationHandler.mayAccessWholeTreeUp(node);
-		if(mayAccessWholeTreeUp) {
+	private void visit(Visitor visitor, TreeNode node) {
+		if(node.isAccessible()) {
 			visitor.visit(node);
 			for(int i=0; i<node.getChildCount(); i++) {
-				CourseTreeNode childNode = (CourseTreeNode)node.getChildAt(i);
-				visit(visitor, childNode, userCourseEnv, treeEval, filter);
-			}	
+				TreeNode childNode = (TreeNode)node.getChildAt(i);
+				visit(visitor, childNode);
+			}
 		}
 	}
 }
diff --git a/src/main/java/org/olat/course/site/CourseSite.java b/src/main/java/org/olat/course/site/CourseSite.java
index 834dfb6c260..be80ca10a39 100644
--- a/src/main/java/org/olat/course/site/CourseSite.java
+++ b/src/main/java/org/olat/course/site/CourseSite.java
@@ -37,7 +37,6 @@ import org.olat.course.ICourse;
 import org.olat.course.nodeaccess.NodeAccessService;
 import org.olat.course.run.CourseRuntimeController;
 import org.olat.course.run.RunMainController;
-import org.olat.course.run.navigation.NavigationHandler;
 import org.olat.course.run.userview.CourseTreeNode;
 import org.olat.course.run.userview.TreeEvaluation;
 import org.olat.course.run.userview.UserCourseEnvironmentImpl;
@@ -112,8 +111,7 @@ public class CourseSite extends AbstractSiteInstance {
 				CourseTreeNode courseTreeNode = (CourseTreeNode)nodeAccessService.getCourseTreeModelBuilder(uce)
 						.build(new TreeEvaluation(), new VisibleTreeFilter())
 						.getRootNode();
-				boolean mayAccessWholeTreeUp = NavigationHandler.mayAccessWholeTreeUp(courseTreeNode);
-				hasAccess = mayAccessWholeTreeUp && courseTreeNode.isVisible();
+				hasAccess = courseTreeNode.isVisible() && courseTreeNode.isAccessible();
 			}
 		}
 		
diff --git a/src/main/java/org/olat/modules/portfolio/ui/CourseTemplateSearchController.java b/src/main/java/org/olat/modules/portfolio/ui/CourseTemplateSearchController.java
index 6d4c7a72a9f..9c8adc220d1 100644
--- a/src/main/java/org/olat/modules/portfolio/ui/CourseTemplateSearchController.java
+++ b/src/main/java/org/olat/modules/portfolio/ui/CourseTemplateSearchController.java
@@ -43,8 +43,6 @@ import org.olat.course.ICourse;
 import org.olat.course.nodeaccess.NodeAccessService;
 import org.olat.course.nodes.CourseNode;
 import org.olat.course.nodes.PortfolioCourseNode;
-import org.olat.course.run.navigation.NavigationHandler;
-import org.olat.course.run.userview.CourseTreeNode;
 import org.olat.course.run.userview.TreeEvaluation;
 import org.olat.course.run.userview.UserCourseEnvironment;
 import org.olat.course.run.userview.UserCourseEnvironmentImpl;
@@ -137,16 +135,14 @@ public class CourseTemplateSearchController extends FormBasicController {
 			TreeNode treeNode = nodeAccessService.getCourseTreeModelBuilder(uce)
 					.build(new TreeEvaluation(), new VisibleTreeFilter())
 					.getNodeById(pNode.getIdent());
-			if (treeNode instanceof CourseTreeNode) {
-				if(NavigationHandler.mayAccessWholeTreeUp((CourseTreeNode)treeNode)) {
-					RepositoryEntry refEntry = pNode.getReferencedRepositoryEntry();
-					if("BinderTemplate".equals(refEntry.getOlatResource().getResourceableTypeName())) {
-						RepositoryEntry courseEntry = uce.getCourseEnvironment().getCourseGroupManager().getCourseEntry();
-						
-						CurrentBinder binderKey = new CurrentBinder(courseEntry.getKey(), pNode.getIdent());
-						if(!currentSet.contains(binderKey)) {
-							rows.add(new CourseTemplateRow(courseEntry, pNode, refEntry));
-						}
+			if (treeNode != null && treeNode.isAccessible()) {
+				RepositoryEntry refEntry = pNode.getReferencedRepositoryEntry();
+				if("BinderTemplate".equals(refEntry.getOlatResource().getResourceableTypeName())) {
+					RepositoryEntry courseEntry = uce.getCourseEnvironment().getCourseGroupManager().getCourseEntry();
+					
+					CurrentBinder binderKey = new CurrentBinder(courseEntry.getKey(), pNode.getIdent());
+					if(!currentSet.contains(binderKey)) {
+						rows.add(new CourseTemplateRow(courseEntry, pNode, refEntry));
 					}
 				}
 			}
diff --git a/src/main/java/org/olat/search/service/indexer/repository/CourseIndexer.java b/src/main/java/org/olat/search/service/indexer/repository/CourseIndexer.java
index 26b941d31b8..430dee8a003 100644
--- a/src/main/java/org/olat/search/service/indexer/repository/CourseIndexer.java
+++ b/src/main/java/org/olat/search/service/indexer/repository/CourseIndexer.java
@@ -35,7 +35,6 @@ import org.olat.core.id.IdentityEnvironment;
 import org.olat.core.id.Roles;
 import org.olat.core.id.context.BusinessControl;
 import org.olat.core.id.context.ContextEntry;
-import org.olat.core.logging.AssertException;
 import org.olat.core.logging.StartupException;
 import org.olat.core.logging.Tracing;
 import org.olat.core.util.nodes.INode;
@@ -45,7 +44,6 @@ import org.olat.course.CourseModule;
 import org.olat.course.ICourse;
 import org.olat.course.nodeaccess.NodeAccessService;
 import org.olat.course.nodes.CourseNode;
-import org.olat.course.run.navigation.NavigationHandler;
 import org.olat.course.run.userview.CourseTreeNode;
 import org.olat.course.run.userview.TreeEvaluation;
 import org.olat.course.run.userview.UserCourseEnvironment;
@@ -202,12 +200,8 @@ public class CourseIndexer extends AbstractHierarchicalIndexer {
 			return false;
 		}
 
-		if (!newCalledTreeNode.isVisible()) throw new AssertException("node eval not visible!!");
-		if (log.isDebugEnabled()) log.debug("call mayAccessWholeTreeUp..." );
-		boolean mayAccessWholeTreeUp = NavigationHandler.mayAccessWholeTreeUp(courseTreeNode);
-		if (log.isDebugEnabled()) log.debug("call mayAccessWholeTreeUp=" + mayAccessWholeTreeUp );
-		
-		if (mayAccessWholeTreeUp) {
+		if (log.isDebugEnabled()) log.debug("call accessible=" + newCalledTreeNode.isAccessible() );
+		if (newCalledTreeNode.isAccessible()) {
 			CourseNodeIndexer courseNodeIndexer = getCourseNodeIndexer(courseNode);
 			bcContextEntry.setTransientState(new CourseNodeEntry(courseNode));
 			return courseNodeIndexer.checkAccess(bcContextEntry, businessControl, identity, roles)
-- 
GitLab