From 6e7c5977a5401ce3ad508f7603e738bcd27d6ee0 Mon Sep 17 00:00:00 2001
From: uhensler <urs.hensler@frentix.com>
Date: Thu, 2 Jul 2020 16:27:55 +0200
Subject: [PATCH] OO-4630: Do not add an organizer twice to an appointment
---
.../AppointmentsSecurityCallbackFactory.java | 5 -
.../AppointmentsSecurityCallback.java | 4 -
.../AppointmentsSecurityCallbackFactory.java | 111 ------------------
.../appointments/AppointmentsService.java | 4 +-
.../manager/AppointmentsServiceImpl.java | 54 +++++++--
.../ui/TopicCreateController.java | 36 ++----
.../appointments/ui/TopicEditController.java | 18 +--
.../appointments/AppointmentsServiceTest.java | 23 ++++
8 files changed, 83 insertions(+), 172 deletions(-)
delete mode 100644 src/main/java/org/olat/modules/appointments/AppointmentsSecurityCallbackFactory.java
diff --git a/src/main/java/org/olat/course/nodes/appointments/AppointmentsSecurityCallbackFactory.java b/src/main/java/org/olat/course/nodes/appointments/AppointmentsSecurityCallbackFactory.java
index 6bfc8edd3a5..7473b9f1502 100644
--- a/src/main/java/org/olat/course/nodes/appointments/AppointmentsSecurityCallbackFactory.java
+++ b/src/main/java/org/olat/course/nodes/appointments/AppointmentsSecurityCallbackFactory.java
@@ -74,11 +74,6 @@ public class AppointmentsSecurityCallbackFactory {
return admin || coachCanEditTopic;
}
- @Override
- public Identity getDefaultOrganizer() {
- return coachCanEditTopic? identity: null;
- }
-
@Override
public boolean canEditTopic(List<Organizer> organizers) {
if (readOnly) return false;
diff --git a/src/main/java/org/olat/modules/appointments/AppointmentsSecurityCallback.java b/src/main/java/org/olat/modules/appointments/AppointmentsSecurityCallback.java
index 843df6ec6de..438ede5aeda 100644
--- a/src/main/java/org/olat/modules/appointments/AppointmentsSecurityCallback.java
+++ b/src/main/java/org/olat/modules/appointments/AppointmentsSecurityCallback.java
@@ -21,8 +21,6 @@ package org.olat.modules.appointments;
import java.util.List;
-import org.olat.core.id.Identity;
-
/**
*
* Initial date: 13 Apr 2020<br>
@@ -35,8 +33,6 @@ public interface AppointmentsSecurityCallback {
public boolean canCreateTopic();
- public Identity getDefaultOrganizer();
-
public boolean canEditTopic(List<Organizer> organizers);
public boolean canViewAppointment(List<Organizer> organizers);
diff --git a/src/main/java/org/olat/modules/appointments/AppointmentsSecurityCallbackFactory.java b/src/main/java/org/olat/modules/appointments/AppointmentsSecurityCallbackFactory.java
deleted file mode 100644
index 16fd5804902..00000000000
--- a/src/main/java/org/olat/modules/appointments/AppointmentsSecurityCallbackFactory.java
+++ /dev/null
@@ -1,111 +0,0 @@
-/**
- * <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.modules.appointments;
-
-import java.util.List;
-
-import org.olat.core.id.Identity;
-import org.olat.course.nodes.AppointmentsCourseNode;
-import org.olat.course.run.userview.UserCourseEnvironment;
-import org.olat.modules.ModuleConfiguration;
-
-/**
- *
- * Initial date: 13 Apr 2020<br>
- * @author uhensler, urs.hensler@frentix.com, http://www.frentix.com
- *
- */
-public class AppointmentsSecurityCallbackFactory {
-
- public static final AppointmentsSecurityCallback create(ModuleConfiguration config,
- UserCourseEnvironment userCourseEnv) {
- return new UserAppointmentsSecurityCallback(config, userCourseEnv);
- }
-
- private static class UserAppointmentsSecurityCallback implements AppointmentsSecurityCallback {
-
- private final Identity identity;
- private final boolean admin;
- private final boolean coach;
- private final boolean coachCanEditTopic;
- private final boolean coachCanEditAppointment;
- private final boolean participant;
- private final boolean readOnly;
-
- public UserAppointmentsSecurityCallback(ModuleConfiguration config, UserCourseEnvironment userCourseEnv) {
- this.identity = userCourseEnv.getIdentityEnvironment().getIdentity();
- this.admin = userCourseEnv.isAdmin();
- this.coach = userCourseEnv.isCoach();
- this.participant = userCourseEnv.isParticipant();
- this.readOnly = userCourseEnv.isCourseReadOnly();
-
- this.coachCanEditTopic = coach && config.getBooleanSafe(AppointmentsCourseNode.CONFIG_COACH_EDIT_TOPIC);
- this.coachCanEditAppointment = coach && config.getBooleanSafe(AppointmentsCourseNode.CONFIG_COACH_EDIT_APPOINTMENT);
- }
-
- @Override
- public boolean canEditTopics() {
- return admin;
- }
-
- @Override
- public boolean canCreateTopic() {
- if (readOnly) return false;
-
- return admin || coachCanEditTopic;
- }
-
- @Override
- public Identity getDefaultOrganizer() {
- return coachCanEditTopic? identity: null;
- }
-
- @Override
- public boolean canEditTopic(List<Organizer> organizers) {
- if (readOnly) return false;
-
- return admin || (coachCanEditTopic && isOrganizer(organizers));
- }
-
- @Override
- public boolean canViewAppointment(List<Organizer> organizers) {
- return admin || (coach && isOrganizer(organizers));
- }
-
- @Override
- public boolean canEditAppointment(List<Organizer> organizers) {
- return admin || (coachCanEditAppointment && isOrganizer(organizers));
- }
-
- @Override
- public boolean canSelectAppointments() {
- if (readOnly) return false;
-
- return participant;
- }
-
- private boolean isOrganizer(List<Organizer> organizers) {
- return organizers.stream()
- .anyMatch(o -> o.getIdentity().getKey().equals(identity.getKey()));
- }
-
- }
-
-}
diff --git a/src/main/java/org/olat/modules/appointments/AppointmentsService.java b/src/main/java/org/olat/modules/appointments/AppointmentsService.java
index 0597009cc72..43337edf995 100644
--- a/src/main/java/org/olat/modules/appointments/AppointmentsService.java
+++ b/src/main/java/org/olat/modules/appointments/AppointmentsService.java
@@ -59,9 +59,7 @@ public interface AppointmentsService {
*/
public List<Topic> getRestictedTopic(RepositoryEntryRef entry, String subIdent, IdentityRef identity);
- public Organizer createOrganizer(Topic topic, Identity identity);
-
- public void deleteOrganizers(TopicRef topic, Collection<Organizer> organizers);
+ public void updateOrganizers(Topic topic, Collection<Identity> identities);
public List<Organizer> getOrganizers(TopicRef topic);
diff --git a/src/main/java/org/olat/modules/appointments/manager/AppointmentsServiceImpl.java b/src/main/java/org/olat/modules/appointments/manager/AppointmentsServiceImpl.java
index 7c4a9dd620f..3489742990b 100644
--- a/src/main/java/org/olat/modules/appointments/manager/AppointmentsServiceImpl.java
+++ b/src/main/java/org/olat/modules/appointments/manager/AppointmentsServiceImpl.java
@@ -198,10 +198,48 @@ public class AppointmentsServiceImpl implements AppointmentsService {
groupDao.removeGroup(group);
}
}
-
+
@Override
- public Organizer createOrganizer(Topic topic, Identity identity) {
- Organizer createOrganizer = organizerDao.createOrganizer(topic, identity);
+ public void updateOrganizers(Topic topic, Collection<Identity> identities) {
+ List<Organizer> organizers = organizerDao.loadOrganizers(topic);
+ List<Identity> organizersToCreate = new ArrayList<>(identities.size());
+ for (Identity identity : identities) {
+ boolean found = false;
+ for (Organizer organizer : organizers) {
+ if (organizer.getIdentity().equals(identity)) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ organizersToCreate.add(identity);
+ }
+ }
+
+ if (!organizersToCreate.isEmpty()) {
+ List<Organizer> createdOrganizers = createOrganizers(topic, organizersToCreate);
+ organizers.addAll(createdOrganizers);
+ }
+
+ ArrayList<Organizer> organizersToDelete = new ArrayList<>(organizers.size());
+ for (Iterator<Organizer> organizersIt = organizers.iterator(); organizersIt.hasNext(); ) {
+ Organizer organizer = organizersIt.next();
+ if (!identities.contains(organizer.getIdentity())) {
+ organizersToDelete.add(organizer);
+ }
+ }
+ if (!organizersToDelete.isEmpty()) {
+ deleteOrganizers(topic, organizersToDelete);
+ }
+ }
+
+ private List<Organizer> createOrganizers(Topic topic, Collection<Identity> identities) {
+ List<Organizer> organizers = new ArrayList<>(identities.size());
+ for (Identity identity : identities) {
+ Organizer organizer = organizerDao.createOrganizer(topic, identity);
+ organizers.add(organizer);
+ }
AppointmentSearchParams params = new AppointmentSearchParams();
params.setTopic(topic);
@@ -210,13 +248,15 @@ public class AppointmentsServiceImpl implements AppointmentsService {
params.setStatus(Status.confirmed);
}
List<Appointment> appointments = appointmentDao.loadAppointments(params);
- calendarSyncher.syncCalendar(appointments, identity);
- return createOrganizer;
+ for (Organizer organizer : organizers) {
+ calendarSyncher.syncCalendar(appointments, organizer.getIdentity());
+ }
+
+ return organizers;
}
- @Override
- public void deleteOrganizers(TopicRef topic, Collection<Organizer> organizers) {
+ private void deleteOrganizers(TopicRef topic, Collection<Organizer> organizers) {
AppointmentSearchParams params = new AppointmentSearchParams();
params.setTopic(topic);
params.setStatus(Status.confirmed);
diff --git a/src/main/java/org/olat/modules/appointments/ui/TopicCreateController.java b/src/main/java/org/olat/modules/appointments/ui/TopicCreateController.java
index 4ef345aa1a6..f3a65eea1a2 100644
--- a/src/main/java/org/olat/modules/appointments/ui/TopicCreateController.java
+++ b/src/main/java/org/olat/modules/appointments/ui/TopicCreateController.java
@@ -29,7 +29,6 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Date;
import java.util.List;
-import java.util.Set;
import java.util.stream.Collectors;
import org.olat.basesecurity.GroupRoles;
@@ -55,7 +54,6 @@ import org.olat.core.util.StringHelper;
import org.olat.modules.appointments.Appointment;
import org.olat.modules.appointments.AppointmentsSecurityCallback;
import org.olat.modules.appointments.AppointmentsService;
-import org.olat.modules.appointments.Organizer;
import org.olat.modules.appointments.Topic;
import org.olat.modules.appointments.Topic.Type;
import org.olat.repository.RepositoryEntry;
@@ -94,7 +92,6 @@ public class TopicCreateController extends FormBasicController {
private String subIdent;
private AppointmentsSecurityCallback secCallback;
private Topic topic;
- private List<Organizer> organizers;
private List<Identity> coaches;
private List<AppointmentWrapper> appointmentWrappers;
private boolean multiParticipationsSelected = true;
@@ -115,7 +112,6 @@ public class TopicCreateController extends FormBasicController {
this.entry = entry;
this.subIdent = subIdent;
- organizers = new ArrayList<>(0);
coaches = repositoryService.getMembers(entry, RepositoryEntryRelationType.all, GroupRoles.coach.name());
initForm(ureq);
@@ -153,13 +149,14 @@ public class TopicCreateController extends FormBasicController {
}
coachesKV.sort(VALUE_ASC);
organizerEl = uifactory.addCheckboxesDropdown("organizer", "organizer", formLayout, coachesKV.keys(), coachesKV.values());
- for (Organizer organizer : organizers) {
- Long organizerKey = organizer.getIdentity().getKey();
- if (coaches.stream().anyMatch(coach -> organizerKey.equals(coach.getKey()))) {
- organizerEl.select(organizerKey.toString(), true);
+ organizerEl.setVisible(!coaches.isEmpty());
+
+ if (organizerEl.isVisible()) {
+ String defaultOrganizerKey = getIdentity().getKey().toString();
+ if (organizerEl.getKeys().contains(defaultOrganizerKey)) {
+ organizerEl.select(defaultOrganizerKey, true);
}
}
- organizerEl.setVisible(!coaches.isEmpty());
// Appointments
locationEl = uifactory.addTextElement("appointment.location", 128, null, formLayout);
@@ -369,10 +366,6 @@ public class TopicCreateController extends FormBasicController {
private void doSaveTopic() {
topic = appointmentsService.createTopic(entry, subIdent);
- Identity organizer = secCallback.getDefaultOrganizer();
- if (organizer != null) {
- appointmentsService.createOrganizer(topic, organizer);
- }
String title = titleEl.getValue();
topic.setTitle(title);
@@ -397,21 +390,10 @@ public class TopicCreateController extends FormBasicController {
private void doSaveOrganizers() {
Collection<String> selectedOrganizerKeys = organizerEl.getSelectedKeys();
-
- // delete unselected
- List<Organizer> organizersToDelete = organizers.stream()
- .filter(organizer -> !selectedOrganizerKeys.contains(organizer.getIdentity().getKey().toString()))
+ List<Identity> selectedOrganizers = coaches.stream()
+ .filter(i -> selectedOrganizerKeys.contains(i.getKey().toString()))
.collect(Collectors.toList());
- appointmentsService.deleteOrganizers(topic, organizersToDelete);
-
- // create newly selected
- Set<String> currentOrganizerKeys = organizers.stream()
- .map(o -> o.getIdentity().getKey().toString())
- .collect(Collectors.toSet());
- selectedOrganizerKeys.removeAll(currentOrganizerKeys);
- coaches.stream()
- .filter(coach -> selectedOrganizerKeys.contains(coach.getKey().toString()))
- .forEach(coach -> appointmentsService.createOrganizer(topic, coach));
+ appointmentsService.updateOrganizers(topic, selectedOrganizers);
}
private void doSaveSingleAppointments() {
diff --git a/src/main/java/org/olat/modules/appointments/ui/TopicEditController.java b/src/main/java/org/olat/modules/appointments/ui/TopicEditController.java
index 900936f6bbb..e8817c263bd 100644
--- a/src/main/java/org/olat/modules/appointments/ui/TopicEditController.java
+++ b/src/main/java/org/olat/modules/appointments/ui/TopicEditController.java
@@ -25,7 +25,6 @@ import static org.olat.core.util.ArrayHelper.emptyStrings;
import java.util.Collection;
import java.util.List;
-import java.util.Set;
import java.util.stream.Collectors;
import org.olat.basesecurity.GroupRoles;
@@ -241,21 +240,10 @@ public class TopicEditController extends FormBasicController {
private void doSaveOrgianzers() {
Collection<String> selectedOrganizerKeys = organizerEl.getSelectedKeys();
-
- // delete unselected
- List<Organizer> organizersToDelete = organizers.stream()
- .filter(organizer -> !selectedOrganizerKeys.contains(organizer.getIdentity().getKey().toString()))
+ List<Identity> selectedOrganizers = coaches.stream()
+ .filter(i -> selectedOrganizerKeys.contains(i.getKey().toString()))
.collect(Collectors.toList());
- appointmentsService.deleteOrganizers(topic, organizersToDelete);
-
- // create newly selected
- Set<String> currentOrganizerKeys = organizers.stream()
- .map(o -> o.getIdentity().getKey().toString())
- .collect(Collectors.toSet());
- selectedOrganizerKeys.removeAll(currentOrganizerKeys);
- coaches.stream()
- .filter(coach -> selectedOrganizerKeys.contains(coach.getKey().toString()))
- .forEach(coach -> appointmentsService.createOrganizer(topic, coach));
+ appointmentsService.updateOrganizers(topic, selectedOrganizers);
}
private boolean isConfigChanged() {
diff --git a/src/test/java/org/olat/modules/appointments/AppointmentsServiceTest.java b/src/test/java/org/olat/modules/appointments/AppointmentsServiceTest.java
index 34dd3ec2ec2..992c455d951 100644
--- a/src/test/java/org/olat/modules/appointments/AppointmentsServiceTest.java
+++ b/src/test/java/org/olat/modules/appointments/AppointmentsServiceTest.java
@@ -53,6 +53,29 @@ public class AppointmentsServiceTest extends OlatTestCase {
@Autowired
private AppointmentsService sut;
+ @Test
+ public void shouldUpdateOrganizers() {
+ Identity organizerDelete = JunitTestHelper.createAndPersistIdentityAsRndUser("ap");
+ Identity organizerKeep = JunitTestHelper.createAndPersistIdentityAsRndUser("ap");
+ Identity organizerNew1 = JunitTestHelper.createAndPersistIdentityAsRndUser("ap");
+ Identity organizerNew2 = JunitTestHelper.createAndPersistIdentityAsRndUser("ap");
+ Topic topic = createRandomTopic();
+ sut.updateOrganizers(topic, asList(organizerDelete, organizerKeep));
+ dbInstance.commitAndCloseSession();
+
+ sut.updateOrganizers(topic, asList(organizerKeep, organizerNew1, organizerNew2));
+ dbInstance.commitAndCloseSession();
+
+ List<Organizer> organizers = sut.getOrganizers(topic);
+ assertThat(organizers).extracting(Organizer::getIdentity)
+ .containsExactlyInAnyOrder(
+ organizerKeep,
+ organizerNew1,
+ organizerNew2)
+ .doesNotContain(
+ organizerDelete);
+ }
+
@Test
public void createParticipationShouldCreateParticiption() {
Identity participant1 = JunitTestHelper.createAndPersistIdentityAsRndUser("ap");
--
GitLab