From 6d97bfb2fb6599160986a66e709d16ce60fb799f Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Tue, 10 Jul 2018 13:44:16 +0200
Subject: [PATCH] OO-3286: fix some issues with members count in courses linked
 to curriculum elements

---
 .../MembersCourseNodeRunController.java       | 10 ++--
 .../course/nodes/members/MembersHelpers.java  | 51 ++++++-------------
 .../ui/main/_i18n/LocalStrings_de.properties  |  2 +-
 .../modules/curriculum/CurriculumService.java |  7 +++
 .../manager/CurriculumServiceImpl.java        |  8 +++
 .../olat/repository/RepositoryService.java    |  9 +++-
 .../manager/RepositoryEntryRelationDAO.java   | 27 ++++------
 .../manager/RepositoryServiceImpl.java        | 14 ++---
 .../RepositoryEntryRelationDAOTest.java       |  6 ++-
 9 files changed, 64 insertions(+), 70 deletions(-)

diff --git a/src/main/java/org/olat/course/nodes/members/MembersCourseNodeRunController.java b/src/main/java/org/olat/course/nodes/members/MembersCourseNodeRunController.java
index ce2f9da3c0f..9c146c23d22 100644
--- a/src/main/java/org/olat/course/nodes/members/MembersCourseNodeRunController.java
+++ b/src/main/java/org/olat/course/nodes/members/MembersCourseNodeRunController.java
@@ -54,8 +54,7 @@ import org.springframework.beans.factory.annotation.Autowired;
  */
 public class MembersCourseNodeRunController extends BasicController {
 	
-	private List<Identity> owners, coaches, participants;
-
+	
 	private MembersDisplayRunController membersDisplayRunController;
 	
 	@Autowired
@@ -69,9 +68,10 @@ public class MembersCourseNodeRunController extends BasicController {
 		super(ureq, wControl);
 		
 		CourseEnvironment courseEnv = userCourseEnv.getCourseEnvironment();
-		
-		this.coaches = new ArrayList<>();
-		this.participants = new ArrayList<>();
+
+		List<Identity> owners;
+		List<Identity> coaches = new ArrayList<>();
+		List<Identity> participants = new ArrayList<>();
 
 		boolean showOwners = config.getBooleanSafe(MembersCourseNode.CONFIG_KEY_SHOWOWNER);
 		
diff --git a/src/main/java/org/olat/course/nodes/members/MembersHelpers.java b/src/main/java/org/olat/course/nodes/members/MembersHelpers.java
index 204fff6b698..3466f73e7c2 100644
--- a/src/main/java/org/olat/course/nodes/members/MembersHelpers.java
+++ b/src/main/java/org/olat/course/nodes/members/MembersHelpers.java
@@ -72,8 +72,7 @@ public class MembersHelpers {
 			list.addAll(retrieveCoachesFromAreas(coachAreaKeys, cgm));
 		}
 		
-		if(moduleConfiguration.anyTrue(MembersCourseNode.CONFIG_KEY_COACHES_COURSE
-				, MembersCourseNode.CONFIG_KEY_COACHES_ALL)) {
+		if(moduleConfiguration.anyTrue(MembersCourseNode.CONFIG_KEY_COACHES_COURSE, MembersCourseNode.CONFIG_KEY_COACHES_ALL)) {
 			list.addAll(retrieveCoachesFromCourse(cgm));
 		}
 		if(moduleConfiguration.anyTrue(MembersCourseNode.CONFIG_KEY_COACHES_ALL)) {
@@ -83,32 +82,22 @@ public class MembersHelpers {
 	
 	public static List<Identity> retrieveCoachesFromAreas(List<Long> areaKeys, CourseGroupManager cgm) {
 		List<Identity> coaches = cgm.getCoachesFromAreas(areaKeys);
-		Set<Identity> coachesWithoutDuplicates = new HashSet<Identity>(coaches);
-		coaches = new ArrayList<Identity>(coachesWithoutDuplicates);
-		return coaches;
+		return new ArrayList<>(new HashSet<>(coaches));
 	}
 	
 	public static List<Identity> retrieveCoachesFromGroups(List<Long> groupKeys, CourseGroupManager cgm) {
-		List<Identity> coaches = new ArrayList<Identity>(new HashSet<Identity>(cgm.getCoachesFromBusinessGroups(groupKeys)));
-		return coaches;
+		return new ArrayList<>(new HashSet<>(cgm.getCoachesFromBusinessGroups(groupKeys)));
 	}
 	
 	public static List<Identity> retrieveCoachesFromCourse(CourseGroupManager cgm) {
-		List<Identity> coaches = cgm.getCoaches();
-		return coaches;
+		return cgm.getCoaches();
 	}
 
 	public static List<Identity> retrieveCoachesFromCourseGroups(CourseGroupManager cgm) {
-		Set<Identity> uniq = new HashSet<Identity>();
-		{
-			List<Identity> coaches = cgm.getCoachesFromAreas();
-			uniq.addAll(coaches);
-		}
-		{
-			List<Identity> coaches = cgm.getCoachesFromBusinessGroups();
-			uniq.addAll(coaches);
-		}
-		return new ArrayList<Identity>(uniq);
+		Set<Identity> uniq = new HashSet<>();
+		uniq.addAll(cgm.getCoachesFromAreas());
+		uniq.addAll(cgm.getCoachesFromBusinessGroups());
+		return new ArrayList<>(uniq);
 	}
 	
 	// -----------------------------------------------------
@@ -143,31 +132,21 @@ public class MembersHelpers {
 	}
 	
 	public static List<Identity> retrieveParticipantsFromAreas(List<Long> areaKeys, CourseGroupManager cgm) {
-		List<Identity> participiants = cgm.getParticipantsFromAreas(areaKeys);
-		return participiants;
+		return cgm.getParticipantsFromAreas(areaKeys);
 	}
 	
 	public static List<Identity> retrieveParticipantsFromGroups(List<Long> groupKeys, CourseGroupManager cgm) {
-		List<Identity> participiants = cgm.getParticipantsFromBusinessGroups(groupKeys);
-		return participiants;
+		return cgm.getParticipantsFromBusinessGroups(groupKeys);
 	}
 	
 	public static List<Identity> retrieveParticipantsFromCourse(CourseGroupManager cgm) {
-		List<Identity> participiants = cgm.getParticipants();
-		return participiants;
+		return cgm.getParticipants();
 	}
 	
 	public static List<Identity> retrieveParticipantsFromCourseGroups(CourseGroupManager cgm) {
-		Set<Identity> uniq = new HashSet<Identity>();
-		{
-			List<Identity> participiants = cgm.getParticipantsFromAreas();
-			uniq.addAll(participiants);
-		}
-		{
-			List<Identity> participiants = cgm.getParticipantsFromBusinessGroups();
-			uniq.addAll(participiants);
-		}
-		return new ArrayList<Identity>(uniq);
+		Set<Identity> uniq = new HashSet<>();
+		uniq.addAll(cgm.getParticipantsFromAreas());
+		uniq.addAll(cgm.getParticipantsFromBusinessGroups());
+		return new ArrayList<>(uniq);
 	}
-	
 }
diff --git a/src/main/java/org/olat/group/ui/main/_i18n/LocalStrings_de.properties b/src/main/java/org/olat/group/ui/main/_i18n/LocalStrings_de.properties
index b1d8678d3d3..32185fdb51d 100644
--- a/src/main/java/org/olat/group/ui/main/_i18n/LocalStrings_de.properties
+++ b/src/main/java/org/olat/group/ui/main/_i18n/LocalStrings_de.properties
@@ -162,7 +162,7 @@ import.member=Mitglieder hinzuf\u00FCgen
 
 
 role.curriculum.participant=Curriculumteilnehmer
-role.curriculum.coach=Curriculumbetreuers
+role.curriculum.coach=Curriculumbetreuer
 role.curriculum.owner=Curriculumkursbesitzer
 role.group.tutor=Gruppenbetreuer
 role.group.participant=Gruppenteilnehmer
diff --git a/src/main/java/org/olat/modules/curriculum/CurriculumService.java b/src/main/java/org/olat/modules/curriculum/CurriculumService.java
index d70d5db3641..b528dc26d93 100644
--- a/src/main/java/org/olat/modules/curriculum/CurriculumService.java
+++ b/src/main/java/org/olat/modules/curriculum/CurriculumService.java
@@ -346,6 +346,13 @@ public interface CurriculumService {
 
 	public void removeRepositoryEntry(CurriculumElement element, RepositoryEntryRef entry);
 	
+	/**
+	 * Remove the repository entry from all the curriculum elements.
+	 * 
+	 * @param entry
+	 */
+	public void removeRepositoryEntry(RepositoryEntry entry);
+	
 	/**
 	 * The list of taxonomy levels of the curriculum element.
 	 * 
diff --git a/src/main/java/org/olat/modules/curriculum/manager/CurriculumServiceImpl.java b/src/main/java/org/olat/modules/curriculum/manager/CurriculumServiceImpl.java
index fe632c29c85..f67620ee140 100644
--- a/src/main/java/org/olat/modules/curriculum/manager/CurriculumServiceImpl.java
+++ b/src/main/java/org/olat/modules/curriculum/manager/CurriculumServiceImpl.java
@@ -379,6 +379,14 @@ public class CurriculumServiceImpl implements CurriculumService {
 		return repositoryEntryRelationDao.hasRelation(element.getGroup(), entry);
 	}
 
+	@Override
+	public void removeRepositoryEntry(RepositoryEntry entry) {
+		List<CurriculumElement> elements = curriculumRepositoryEntryRelationDao.getCurriculumElements(entry);
+		for(CurriculumElement element:elements) {
+			repositoryEntryRelationDao.removeRelation(element.getGroup(), entry);
+		}
+	}
+
 	@Override
 	public void removeRepositoryEntry(CurriculumElement element, RepositoryEntryRef entry) {
 		repositoryEntryRelationDao.removeRelation(element.getGroup(), entry);
diff --git a/src/main/java/org/olat/repository/RepositoryService.java b/src/main/java/org/olat/repository/RepositoryService.java
index 2a17c0bf9ae..1e9340059fa 100644
--- a/src/main/java/org/olat/repository/RepositoryService.java
+++ b/src/main/java/org/olat/repository/RepositoryService.java
@@ -183,7 +183,14 @@ public interface RepositoryService {
 
 	public void filterMembership(IdentityRef identity, Collection<Long> entries);
 
-	public int countMembers(RepositoryEntryRef re, String... roles);
+	/**
+	 * Count the number of member with the specified role.
+	 * 
+	 * @param re The repository entry
+	 * @param role The role (mandatory)
+	 * @return
+	 */
+	public int countMembers(RepositoryEntryRef re, String role);
 
 	/**
 	 * Count all members (following up to business groups waiting list) with the following
diff --git a/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java b/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java
index e2de12b33a9..449ee793b57 100644
--- a/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java
+++ b/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java
@@ -274,31 +274,22 @@ public class RepositoryEntryRelationDAO {
 	 * the organizations.
 	 * 
 	 * @param re The repository entry
-	 * @param roles The roles (optional)
+	 * @param roles The role (mandatory)
 	 * @return
 	 */
-	public int countMembers(RepositoryEntryRef re, String... roles) {
-		List<String> roleList = GroupRoles.toList(roles);
-		
-		StringBuilder sb = new StringBuilder();
+	public int countMembers(RepositoryEntryRef re, String role) {
+		StringBuilder sb = new StringBuilder(256);
 		sb.append("select count(members) from ").append(RepositoryEntry.class.getName()).append(" as v")
 		  .append(" inner join v.groups as relGroup")
 		  .append(" inner join relGroup.group as baseGroup")
 		  .append(" inner join baseGroup.members as members")
-		  .append(" left join businessgroup as businessGroup on (businessGroup.baseGroup.key=baseGroup.key)")
-		  .append(" where v.key=:repoKey and (relGroup.defaultGroup=true or businessGroup.key is not null)");
-		if(!roleList.isEmpty()) {
-				sb.append(" and members.role in (:roles)");
-		}
-		
-		TypedQuery<Number> query = dbInstance.getCurrentEntityManager()
+		  .append(" where v.key=:repoKey and members.role=:role");
+
+		Number count = dbInstance.getCurrentEntityManager()
 				.createQuery(sb.toString(), Number.class)
-				.setParameter("repoKey", re.getKey());
-		if(!roleList.isEmpty()) {
-				query.setParameter("roles", roleList);
-		}
-		
-		Number count = query.getSingleResult();
+				.setParameter("repoKey", re.getKey())
+				.setParameter("role", role)
+				.getSingleResult();
 		return count == null ? 0 : count.intValue();
 	}
 	
diff --git a/src/main/java/org/olat/repository/manager/RepositoryServiceImpl.java b/src/main/java/org/olat/repository/manager/RepositoryServiceImpl.java
index 49534947063..4b4bc7ea24e 100644
--- a/src/main/java/org/olat/repository/manager/RepositoryServiceImpl.java
+++ b/src/main/java/org/olat/repository/manager/RepositoryServiceImpl.java
@@ -63,6 +63,7 @@ import org.olat.course.assessment.manager.UserCourseInformationsManager;
 import org.olat.course.certificate.CertificatesManager;
 import org.olat.ims.qti21.manager.AssessmentTestSessionDAO;
 import org.olat.modules.assessment.manager.AssessmentEntryDAO;
+import org.olat.modules.curriculum.CurriculumService;
 import org.olat.modules.lecture.LectureService;
 import org.olat.modules.portfolio.PortfolioService;
 import org.olat.modules.reminder.manager.ReminderDAO;
@@ -451,6 +452,9 @@ public class RepositoryServiceImpl implements RepositoryService {
 		//detach portfolio if there are some lost
 		CoreSpringFactory.getImpl(PortfolioService.class).detachCourseFromBinders(entry);
 		dbInstance.commit();
+		//detach from curriculum
+		CoreSpringFactory.getImpl(CurriculumService.class).removeRepositoryEntry(entry);
+		dbInstance.commit();
 
 		// inform handler to do any cleanup work... handler must delete the
 		// referenced resourceable a swell.
@@ -616,11 +620,10 @@ public class RepositoryServiceImpl implements RepositoryService {
 	}
 
 	@Override
-	public int countMembers(RepositoryEntryRef re, String... roles) {
-		return reToGroupDao.countMembers(re, roles);
+	public int countMembers(RepositoryEntryRef re, String role) {
+		return reToGroupDao.countMembers(re, role);
 	}
 
-
 	@Override
 	public int countMembers(List<? extends RepositoryEntryRef> res, Identity excludeMe) {
 		return reToGroupDao.countMembers(res, excludeMe);
@@ -653,10 +656,7 @@ public class RepositoryServiceImpl implements RepositoryService {
 
 	@Override
 	public List<Identity> getIdentitiesWithRole(String role) {
-		long start = System.nanoTime();
-		List<Identity> ids = reToGroupDao.getIdentitiesWithRole(role);
-		CodeHelper.printNanoTime(start, "Repository ids with role");
-		return ids;
+		return reToGroupDao.getIdentitiesWithRole(role);
 	}
 
 	@Override
diff --git a/src/test/java/org/olat/repository/manager/RepositoryEntryRelationDAOTest.java b/src/test/java/org/olat/repository/manager/RepositoryEntryRelationDAOTest.java
index 9076f886844..ab912039775 100644
--- a/src/test/java/org/olat/repository/manager/RepositoryEntryRelationDAOTest.java
+++ b/src/test/java/org/olat/repository/manager/RepositoryEntryRelationDAOTest.java
@@ -154,13 +154,15 @@ public class RepositoryEntryRelationDAOTest extends OlatTestCase {
 
 		//all members
 		List<Identity> members = repositoryEntryRelationDao.getMembers(re, RepositoryEntryRelationType.defaultGroup);
-		int numOfMembers = repositoryEntryRelationDao.countMembers(re);
 		Assert.assertNotNull(members);
 		Assert.assertEquals(2, members.size());
-		Assert.assertEquals(2, numOfMembers);
 		Assert.assertTrue(members.contains(id1));
 		Assert.assertTrue(members.contains(id2));
 		
+		// owner
+		int numOfOwners = repositoryEntryRelationDao.countMembers(re, GroupRoles.owner.name());
+		Assert.assertEquals(1, numOfOwners);
+		
 		//participant
 		List<Identity> participants = repositoryEntryRelationDao.getMembers(re, RepositoryEntryRelationType.defaultGroup, GroupRoles.participant.name());
 		int numOfParticipants = repositoryEntryRelationDao.countMembers(re, GroupRoles.participant.name());
-- 
GitLab