Index: lams_central/src/java/org/lamsfoundation/lams/webservice/SPEnrolmentServlet.java =================================================================== diff -u -r3080c61fc08c0d5fdc28fb9ad6c8ab51363693d9 -r657c3ddcb79ba8772e462cc797d2937dd88292cc --- lams_central/src/java/org/lamsfoundation/lams/webservice/SPEnrolmentServlet.java (.../SPEnrolmentServlet.java) (revision 3080c61fc08c0d5fdc28fb9ad6c8ab51363693d9) +++ lams_central/src/java/org/lamsfoundation/lams/webservice/SPEnrolmentServlet.java (.../SPEnrolmentServlet.java) (revision 657c3ddcb79ba8772e462cc797d2937dd88292cc) @@ -474,135 +474,158 @@ boolean isStaffMode, AtomicInteger mappingsProcessed) throws UserInfoValidationException { String courseCode = course.getCode(); Integer courseId = course.getOrganisationId(); + ConcurrentMap existingSubcourses = allExistingParsedCoursesAndSubcourses.get(courseId); + Map nonProcessedSubcourses = existingSubcourses == null ? new HashMap<>() + : new HashMap<>(existingSubcourses); // go through each subcourse - for (Entry> subcourseEntry : mappings.get(courseCode).entrySet()) { - String subcourseCode = subcourseEntry.getKey(); - ConcurrentMap existingSubcourses = allExistingParsedCoursesAndSubcourses - .get(courseId); - Organisation subcourse = existingSubcourses == null ? null : existingSubcourses.get(subcourseCode); + Map> subcourseMappings = mappings.get(courseCode); + if (subcourseMappings != null) { + for (Entry> subcourseEntry : subcourseMappings.entrySet()) { + String subcourseCode = subcourseEntry.getKey(); + nonProcessedSubcourses.remove(subcourseCode); - // create subcourse - if (subcourse == null) { - ExtCourseClassMap extSubOrgMap = integrationService.createExtCourseClassMap(extServer, creatorId, - subcourseCode, subcourseCode, course.getOrganisationId().toString(), false); - subcourse = extSubOrgMap.getOrganisation(); - subcourse.setCode(subcourseCode); - userManagementService.save(subcourse); + Organisation subcourse = existingSubcourses == null ? null : existingSubcourses.get(subcourseCode); + // create subcourse + if (subcourse == null) { + ExtCourseClassMap extSubOrgMap = integrationService.createExtCourseClassMap(extServer, creatorId, + subcourseCode, subcourseCode, course.getOrganisationId().toString(), false); + subcourse = extSubOrgMap.getOrganisation(); + subcourse.setCode(subcourseCode); + userManagementService.save(subcourse); - if (existingSubcourses == null) { - existingSubcourses = new ConcurrentHashMap<>(); - allExistingParsedCoursesAndSubcourses.put(courseId, existingSubcourses); - } - existingSubcourses.put(subcourse.getCode(), subcourse); - allExistingParsedExtCourses.put(extSubOrgMap.getOrganisation().getOrganisationId(), extSubOrgMap); + if (existingSubcourses == null) { + existingSubcourses = new ConcurrentHashMap<>(); + allExistingParsedCoursesAndSubcourses.put(courseId, existingSubcourses); + } + existingSubcourses.put(subcourse.getCode(), subcourse); + allExistingParsedExtCourses.put(extSubOrgMap.getOrganisation().getOrganisationId(), extSubOrgMap); - String message = "Subcourse created with code and name \"" + courseCode + "\" and ID " - + subcourse.getOrganisationId(); - logger.info(message); - logEventService.logEvent(LogEvent.TYPE_USER_ORG_ADMIN, creatorId, null, null, null, - "SPEnrolment: " + message); - } else { - String name = subcourse.getName(); - - ExtCourseClassMap extOrgMap = allExistingParsedExtCourses.get(subcourse.getOrganisationId()); - if (extOrgMap == null) { - extOrgMap = new ExtCourseClassMap(); - extOrgMap.setCourseid(name); - extOrgMap.setExtServer(extServer); - extOrgMap.setOrganisation(subcourse); - userManagementService.save(extOrgMap); - - String message = "External subcourse created for existing subcourse with code \"" + subcourseCode - + "\" and name \"" + name + "\" and ID " + subcourse.getOrganisationId(); + String message = "Subcourse created with code and name \"" + courseCode + "\" and ID " + + subcourse.getOrganisationId(); logger.info(message); logEventService.logEvent(LogEvent.TYPE_USER_ORG_ADMIN, creatorId, null, null, null, "SPEnrolment: " + message); + } else { + String name = subcourse.getName(); + + ExtCourseClassMap extOrgMap = allExistingParsedExtCourses.get(subcourse.getOrganisationId()); + if (extOrgMap == null) { + extOrgMap = new ExtCourseClassMap(); + extOrgMap.setCourseid(name); + extOrgMap.setExtServer(extServer); + extOrgMap.setOrganisation(subcourse); + userManagementService.save(extOrgMap); + + String message = "External subcourse created for existing subcourse with code \"" + + subcourseCode + "\" and name \"" + name + "\" and ID " + + subcourse.getOrganisationId(); + logger.info(message); + logEventService.logEvent(LogEvent.TYPE_USER_ORG_ADMIN, creatorId, null, null, null, + "SPEnrolment: " + message); + } } - } - Integer subcourseId = subcourse.getOrganisationId(); + Integer subcourseId = subcourse.getOrganisationId(); - // get existing learners/staff members for given subcourse - Collection subcourseMonitorsOrLearners = userManagementService - .getUsersFromOrganisationByRole(subcourseId, isStaffMode ? Role.MONITOR : Role.LEARNER, true); - Collection subcourseUsers = new HashSet<>(subcourseMonitorsOrLearners); - if (isStaffMode) { - // make sure that staff has both monitor and author roles in subcourse, - // even if they are course managers in the parent organisations - // and they already have a monitor role in subcourse - Collection authors = userManagementService.getUsersFromOrganisationByRole(subcourseId, - Role.AUTHOR, true); - subcourseUsers.retainAll(authors); - } + // get existing learners/staff members for given subcourse + Collection subcourseMonitorsOrLearners = userManagementService + .getUsersFromOrganisationByRole(subcourseId, isStaffMode ? Role.MONITOR : Role.LEARNER, true); + Collection subcourseUsers = new HashSet<>(subcourseMonitorsOrLearners); + if (isStaffMode) { + // make sure that staff has both monitor and author roles in subcourse, + // even if they are course managers in the parent organisations + // and they already have a monitor role in subcourse + Collection authors = userManagementService.getUsersFromOrganisationByRole(subcourseId, + Role.AUTHOR, true); + subcourseUsers.retainAll(authors); + } - // go through each user - for (String login : subcourseEntry.getValue()) { + // go through each user + for (String login : subcourseEntry.getValue()) { - logger.info("Processing \"" + login + "\""); + logger.info("Processing \"" + login + "\""); - mappingsProcessed.incrementAndGet(); + mappingsProcessed.incrementAndGet(); - // check if the user is already a learner/staff member in the subcourse - // if so, there is nothing to do - boolean userAlreadyAssigned = false; - Integer userId = userIDs.get(login); - Iterator subcourseUserIterator = subcourseUsers.iterator(); - while (subcourseUserIterator.hasNext()) { - User user = subcourseUserIterator.next(); - if (userId.equals(user.getUserId())) { - // IMPORTANT: if we found a matching existing learner/staff member, we get remove him from this collection - // so after this loop he does not get removed from subcourses - subcourseUserIterator.remove(); - subcourseMonitorsOrLearners.remove(user); - userAlreadyAssigned = true; - break; + // check if the user is already a learner/staff member in the subcourse + // if so, there is nothing to do + boolean userAlreadyAssigned = false; + Integer userId = userIDs.get(login); + Iterator subcourseUserIterator = subcourseUsers.iterator(); + while (subcourseUserIterator.hasNext()) { + User user = subcourseUserIterator.next(); + if (userId.equals(user.getUserId())) { + // IMPORTANT: if we found a matching existing learner/staff member, we get remove him from this collection + // so after this loop he does not get removed from subcourses + subcourseUserIterator.remove(); + subcourseMonitorsOrLearners.remove(user); + userAlreadyAssigned = true; + break; + } } - } - if (userAlreadyAssigned) { - continue; - } + if (userAlreadyAssigned) { + continue; + } - // the user is not a learner/staff member yet, so assign him the role and add him to lessons - Map> existingSubcoursesRoles = allExistingRoles.get(login); - Set existingSubcourseRoles = existingSubcoursesRoles == null ? null - : allExistingRoles.get(login).get(subcourseId); - if (existingSubcourseRoles == null) { - existingSubcourseRoles = new HashSet<>(); - } - if (isStaffMode) { - existingSubcourseRoles.add(Role.ROLE_AUTHOR); - existingSubcourseRoles.add(Role.ROLE_MONITOR); - } else { - existingSubcourseRoles.add(Role.ROLE_LEARNER); - } - User user = allExistingParsedUsers.get(login); - userManagementService.setRolesForUserOrganisation(user, subcourse, - existingSubcourseRoles.stream().map(String::valueOf).collect(Collectors.toList()), false); - - for (Lesson lesson : lessonService.getLessonsByGroup(subcourseId)) { + // the user is not a learner/staff member yet, so assign him the role and add him to lessons + Map> existingSubcoursesRoles = allExistingRoles.get(login); + Set existingSubcourseRoles = existingSubcoursesRoles == null ? null + : allExistingRoles.get(login).get(subcourseId); + if (existingSubcourseRoles == null) { + existingSubcourseRoles = new HashSet<>(); + } if (isStaffMode) { - lessonService.addStaffMember(lesson.getLessonId(), userId); + existingSubcourseRoles.add(Role.ROLE_AUTHOR); + existingSubcourseRoles.add(Role.ROLE_MONITOR); } else { - lessonService.addLearner(lesson.getLessonId(), userId); + existingSubcourseRoles.add(Role.ROLE_LEARNER); } + User user = allExistingParsedUsers.get(login); + userManagementService.setRolesForUserOrganisation(user, subcourse, + existingSubcourseRoles.stream().map(String::valueOf).collect(Collectors.toList()), false); + + for (Lesson lesson : lessonService.getLessonsByGroup(subcourseId)) { + if (isStaffMode) { + lessonService.addStaffMember(lesson.getLessonId(), userId); + } else { + lessonService.addLearner(lesson.getLessonId(), userId); + } + } + + String message = (isStaffMode ? "Teacher" : "Learner") + " \"" + login + "\" added to subcourse " + + subcourseId + " and its lessons"; + logger.info(message); + logEventService.logEvent(LogEvent.TYPE_USER_ORG_ADMIN, creatorId, null, null, null, + "SPEnrolment: " + message); } - String message = (isStaffMode ? "Teacher" : "Learner") + " \"" + login + "\" added to subcourse " - + subcourseId + " and its lessons"; - logger.info(message); - logEventService.logEvent(LogEvent.TYPE_USER_ORG_ADMIN, creatorId, null, null, null, - "SPEnrolment: " + message); + // user is a learner/staff member, but he should not; remove him role from subcourse, course and lessons + for (User user : subcourseMonitorsOrLearners) { + + boolean removedFromSubcourse = removeFromCourse(subcourse, user, + isStaffMode ? Mode.STAFF : Mode.LEARNER, allExistingRoles); + if (removedFromSubcourse) { + String message = (isStaffMode ? "Teacher" : "Learner") + " \"" + user.getLogin() + + "\" removed from subcourse " + subcourseId + " and its lessons"; + logger.info(message); + logEventService.logEvent(LogEvent.TYPE_USER_ORG_ADMIN, creatorId, null, null, null, + "SPEnrolment: " + message); + } + } } + } - // user is a learner/staff member, but he should not; remove him role from subcourse, course and lessons + for (Organisation subcourse : nonProcessedSubcourses.values()) { + // get all learners/staff members for given subcourse and remove them + Collection subcourseMonitorsOrLearners = userManagementService.getUsersFromOrganisationByRole( + subcourse.getOrganisationId(), isStaffMode ? Role.MONITOR : Role.LEARNER, true); for (User user : subcourseMonitorsOrLearners) { boolean removedFromSubcourse = removeFromCourse(subcourse, user, - isStaffMode ? Mode.STAFF : Mode.LEARNER, allExistingParsedCoursesAndSubcourses, - allExistingRoles); + isStaffMode ? Mode.STAFF : Mode.LEARNER, allExistingRoles); if (removedFromSubcourse) { String message = (isStaffMode ? "Teacher" : "Learner") + " \"" + user.getLogin() - + "\" removed from subcourse " + subcourseId + " and its lessons"; + + "\" removed from subcourse " + subcourse.getOrganisationId() + " and its lessons"; logger.info(message); logEventService.logEvent(LogEvent.TYPE_USER_ORG_ADMIN, creatorId, null, null, null, "SPEnrolment: " + message); @@ -612,7 +635,6 @@ } private boolean removeFromCourse(Organisation course, User user, Mode mode, - Map> allExistingParsedCoursesAndSubcourses, Map>> allExistingRoles) { // no existing roles and the user should be removed - nothing to do Map> existingCoursesRoles = allExistingRoles.get(user.getLogin()); @@ -662,7 +684,7 @@ return true; } } - removeFromCourse(parentCourse, user, mode, allExistingParsedCoursesAndSubcourses, allExistingRoles); + removeFromCourse(parentCourse, user, mode, allExistingRoles); } } @@ -726,7 +748,7 @@ // user is a group manager, but he should not; remove him role from course for (User user : courseUsers) { - boolean removedFromCourse = removeFromCourse(course, user, Mode.MANAGER, null, allExistingRoles); + boolean removedFromCourse = removeFromCourse(course, user, Mode.MANAGER, allExistingRoles); if (removedFromCourse) { String message = "Manager \"" + user.getLogin() + "\" removed from course " + courseId; logger.info(message);