fix(Manage Students): Remove student does not remove from all workgro…

…ups (WISE-Community#243)
I-Sparkle-Germany · Jan 10, 2024 · 39e6cfb · 39e6cfb
1 parent 667abb3
commit 39e6cfb
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 15 deletions.
diff --git a/...se/portal/presentation/web/controllers/teacher/management/RemoveStudentRunController.java b/...se/portal/presentation/web/controllers/teacher/management/RemoveStudentRunController.java
@@ -28,7 +28,7 @@ public class RemoveStudentRunController {
   private UserService userService;
 
   @DeleteMapping("/api/teacher/run/{runId}/student/{studentId}/remove")
-  void changeWorkgroupPeriod(Authentication auth, @PathVariable Long runId,
+  public void removeStudent(Authentication auth, @PathVariable Long runId,
       @PathVariable Long studentId) throws ObjectNotFoundException {
     Run run = runService.retrieveById(runId);
     if (runService.hasWritePermission(auth, run)) {

diff --git a/src/main/java/org/wise/portal/service/student/impl/StudentServiceImpl.java b/src/main/java/org/wise/portal/service/student/impl/StudentServiceImpl.java
@@ -24,6 +24,7 @@
 package org.wise.portal.service.student.impl;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
@@ -122,18 +123,21 @@ public boolean isStudentAssociatedWithTeacher(User studentUser, User teacherUser
   }
 
   public void removeStudentFromRun(User studentUser, Run run) {
-    if (run.isStudentAssociatedToThisRun(studentUser)) {
-      StudentRunInfo studentRunInfo = getStudentRunInfo(studentUser, run);
-      Group period = run.getPeriodOfStudent(studentUser);
-      Set<User> membersToRemove = new HashSet<User>();
-      membersToRemove.add(studentUser);
-      groupService.removeMembers(period, membersToRemove);
-      Workgroup workgroup = studentRunInfo.getWorkgroup();
-      if (workgroup != null) {
-        Set<User> membersToRemoveFromWorkgroup = new HashSet<User>();
-        membersToRemoveFromWorkgroup.add(studentUser);
-        workgroupService.removeMembers(workgroup, membersToRemoveFromWorkgroup);
-      }
+    removeStudentFromWorkgroupsInRun(studentUser, run);
+    removeStudentFromPeriodInRun(studentUser, run);
+  }
+
+  private void removeStudentFromWorkgroupsInRun(User studentUser, Run run) {
+    List<Workgroup> workgroups = workgroupService.getWorkgroupListByRunAndUser(run, studentUser);
+    for (Workgroup workgroup : workgroups) {
+      workgroupService.removeMembers(workgroup, Collections.singleton(studentUser));
+    }
+  }
+
+  private void removeStudentFromPeriodInRun(User studentUser, Run run) {
+    Group period = run.getPeriodOfStudent(studentUser);
+    if (period != null) {
+      groupService.removeMembers(period, Collections.singleton(studentUser));
     }
   }
 

diff --git a/src/test/java/org/wise/portal/service/student/impl/StudentServiceImplTest.java b/src/test/java/org/wise/portal/service/student/impl/StudentServiceImplTest.java
@@ -28,8 +28,10 @@
 import static org.easymock.EasyMock.reset;
 import static org.easymock.EasyMock.verify;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyString;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
@@ -61,6 +63,7 @@
 import org.wise.portal.service.run.RunService;
 import org.wise.portal.service.student.StudentService;
 import org.wise.portal.service.workgroup.WorkgroupService;
+import org.wise.portal.spring.data.redis.MessagePublisher;
 
 /**
  * @author Hiroki Terashima
@@ -71,6 +74,9 @@ public class StudentServiceImplTest {
   @TestSubject
   private StudentService studentService = new StudentServiceImpl();
 
+  @Mock
+  private MessagePublisher redisPublisher;
+
   @Mock
   private RunService runService;
 
@@ -129,7 +135,6 @@ public void addStudentToRun_ExistingUserAndRun_ShouldAddStudentToRun()
     groupService.addMember(period.getId(), studentUser);
     expectLastCall();
     replay(groupService);
-
     studentService.addStudentToRun(studentUser, projectcode);
     verify(runService);
     verify(groupService);
@@ -185,6 +190,8 @@ public void addStudentsToRun_StudentIsAlreadyInTheRun_ShouldThrowStudentAlreadyA
     groupService.addMember(period.getId(), studentUser);
     expectLastCall();
     replay(groupService);
+    redisPublisher.publish(anyString());
+    expectLastCall();
     try {
       studentService.addStudentToRun(studentUser, projectcode);
     } catch (Exception e) {
@@ -229,7 +236,6 @@ public void removeStudentFromRun_studentIsInRun_ShouldRemoveStudentFromRunAndWor
     workgroupService.removeMembers(workgroup, membersToRemoveFromWorkgroup);
     expectLastCall();
     replay(workgroupService);
-
     studentService.removeStudentFromRun(studentUser, run);
     verify(groupService);
     verify(workgroupService);
@@ -239,6 +245,8 @@ public void removeStudentFromRun_studentIsInRun_ShouldRemoveStudentFromRunAndWor
   public void removeStudentFromRun_studentIsNotInRun_ShouldDoNothing() {
     replay(runService);
     replay(groupService);
+    expect(workgroupService.getWorkgroupListByRunAndUser(run, studentUser))
+        .andReturn(Collections.emptyList());
     replay(workgroupService);
     studentService.removeStudentFromRun(studentUser, run);
     verify(runService);