Delete student from team (AY2324S2-CS2103T-T09-4#137)

* Add assertion test * Add delete student from team command * Add more test cases and fix /delete_student_from_team message formatting * Add successful test cases * Fix error message for delete by id command * Fix merge issues * Put delete student from team success message back into DeleteStudentFromTeamCommand class * Move getModuleAndTutorialPair into ModuleTutorialPair class, fixed magic number issues and implementation errors
qinxutan · Apr 4, 2024 · 0d70478 · 0d70478
1 parent 107e2f2
commit 0d70478
Show file tree

Hide file tree

Showing 35 changed files with 881 additions and 121 deletions.
diff --git a/src/main/java/seedu/address/logic/commands/AddTeamCommand.java b/src/main/java/seedu/address/logic/commands/AddTeamCommand.java
@@ -8,7 +8,6 @@
 import static seedu.address.logic.parser.CliSyntax.PREFIX_TUTORIALCLASS;
 
 import seedu.address.logic.commands.exceptions.CommandException;
-import seedu.address.logic.messages.ModuleMessages;
 import seedu.address.model.Model;
 import seedu.address.model.module.ModuleCode;
 import seedu.address.model.module.ModuleTutorialPair;
@@ -70,7 +69,8 @@ public AddTeamCommand(ModuleCode module, TutorialClass tutorialClass, String tea
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        ModuleTutorialPair moduleAndTutorialClass = getModuleAndTutorialClass(model);
+        ModuleTutorialPair moduleAndTutorialClass = ModuleTutorialPair.getModuleAndTutorialClass(model,
+                module, tutorialClass);
         ModuleCode module = moduleAndTutorialClass.getModule();
         TutorialClass tutorialClass = moduleAndTutorialClass.getTutorialClass();
 
@@ -89,22 +89,6 @@ public CommandResult execute(Model model) throws CommandException {
         }
     }
 
-    protected ModuleTutorialPair getModuleAndTutorialClass(Model model) throws CommandException {
-        requireNonNull(model);
-        ModuleCode module = getModule();
-        TutorialClass tutorialClass = getTutorialClass();
-        ModuleCode existingModule = model.findModuleFromList(module);
-        TutorialClass existingTutorialClass = model.findTutorialClassFromList(tutorialClass, existingModule);
-        if (existingModule == null) {
-            throw new CommandException(String.format(ModuleMessages.MESSAGE_MODULE_NOT_FOUND, module));
-        }
-        if (existingTutorialClass == null) {
-            throw new CommandException(
-                    String.format(ModuleMessages.MESSAGE_TUTORIAL_DOES_NOT_BELONG_TO_MODULE, tutorialClass, module));
-        }
-        return new ModuleTutorialPair(existingModule, existingTutorialClass);
-    }
-
     protected ModuleCode getModule() {
         return module;
     }

diff --git a/src/main/java/seedu/address/logic/commands/DeleteTeamCommand.java b/src/main/java/seedu/address/logic/commands/DeleteTeamCommand.java
@@ -7,7 +7,6 @@
 import static seedu.address.logic.parser.CliSyntax.PREFIX_TUTORIALCLASS;
 
 import seedu.address.logic.commands.exceptions.CommandException;
-import seedu.address.logic.messages.ModuleMessages;
 import seedu.address.model.Model;
 import seedu.address.model.module.ModuleCode;
 import seedu.address.model.module.ModuleTutorialPair;
@@ -51,7 +50,8 @@ public DeleteTeamCommand(ModuleCode module, TutorialClass tutorialClass, String
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
 
-        ModuleTutorialPair moduleAndTutorialClass = getModuleAndTutorialClass(model);
+        ModuleTutorialPair moduleAndTutorialClass = ModuleTutorialPair.getModuleAndTutorialClass(model,
+                getModule(), getTutorialClass());
         ModuleCode module = moduleAndTutorialClass.getModule();
         TutorialClass tutorialClass = moduleAndTutorialClass.getTutorialClass();
         if (tutorialClass.hasTeam(team)) {
@@ -63,21 +63,6 @@ public CommandResult execute(Model model) throws CommandException {
         return new CommandResult(generateSuccessMessage(module, tutorialClass, team));
     }
 
-    protected ModuleTutorialPair getModuleAndTutorialClass(Model model) throws CommandException {
-        requireNonNull(model);
-        ModuleCode module = getModule();
-        TutorialClass tutorialClass = getTutorialClass();
-        ModuleCode existingModule = model.findModuleFromList(module);
-        TutorialClass existingTutorialClass = model.findTutorialClassFromList(tutorialClass, existingModule);
-        if (existingModule == null) {
-            throw new CommandException(String.format(ModuleMessages.MESSAGE_MODULE_NOT_FOUND, module));
-        }
-        if (existingTutorialClass == null) {
-            throw new CommandException(
-                    String.format(ModuleMessages.MESSAGE_TUTORIAL_DOES_NOT_BELONG_TO_MODULE, tutorialClass, module));
-        }
-        return new ModuleTutorialPair(existingModule, existingTutorialClass);
-    }
 
     protected ModuleCode getModule() {
         return module;

diff --git a/...edu/address/logic/commands/addstudenttoclasscommands/AddStudentToClassByEmailCommand.java b/...edu/address/logic/commands/addstudenttoclasscommands/AddStudentToClassByEmailCommand.java
@@ -40,7 +40,8 @@ public AddStudentToClassByEmailCommand(Email email, ModuleCode module, TutorialC
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        ModuleTutorialPair moduleAndTutorialClass = getModuleAndTutorialClass(model);
+        ModuleTutorialPair moduleAndTutorialClass = ModuleTutorialPair.getModuleAndTutorialClass(model,
+                getModule(), getTutorialClass());
         TutorialClass tutorialClass = moduleAndTutorialClass.getTutorialClass();
         ModuleCode module = moduleAndTutorialClass.getModule();
         Person personToAdd;

diff --git a/.../seedu/address/logic/commands/addstudenttoclasscommands/AddStudentToClassByIdCommand.java b/.../seedu/address/logic/commands/addstudenttoclasscommands/AddStudentToClassByIdCommand.java
@@ -39,7 +39,8 @@ public AddStudentToClassByIdCommand(StudentId studentId, ModuleCode module, Tuto
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        ModuleTutorialPair moduleAndTutorialClass = getModuleAndTutorialClass(model);
+        ModuleTutorialPair moduleAndTutorialClass = ModuleTutorialPair.getModuleAndTutorialClass(model,
+                getModule(), getTutorialClass());
         ModuleCode module = moduleAndTutorialClass.getModule();
         TutorialClass tutorialClass = moduleAndTutorialClass.getTutorialClass();
         Person personToAdd;

diff --git a/...edu/address/logic/commands/addstudenttoclasscommands/AddStudentToClassByIndexCommand.java b/...edu/address/logic/commands/addstudenttoclasscommands/AddStudentToClassByIndexCommand.java
@@ -34,7 +34,8 @@ public AddStudentToClassByIndexCommand(Index targetIndex, ModuleCode module, Tut
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        ModuleTutorialPair moduleAndTutorialClass = getModuleAndTutorialClass(model);
+        ModuleTutorialPair moduleAndTutorialClass = ModuleTutorialPair.getModuleAndTutorialClass(model,
+                getModule(), getTutorialClass());
         TutorialClass tutorialClass = moduleAndTutorialClass.getTutorialClass();
         ModuleCode module = moduleAndTutorialClass.getModule();
         Person personToAdd;

diff --git a/...java/seedu/address/logic/commands/addstudenttoclasscommands/AddStudentToClassCommand.java b/...java/seedu/address/logic/commands/addstudenttoclasscommands/AddStudentToClassCommand.java
@@ -1,6 +1,5 @@
 package seedu.address.logic.commands.addstudenttoclasscommands;
 
-import static java.util.Objects.requireNonNull;
 import static seedu.address.commons.util.CollectionUtil.requireAllNonNull;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_MODULECODE;
@@ -9,10 +8,8 @@
 import seedu.address.logic.commands.Command;
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.exceptions.CommandException;
-import seedu.address.logic.messages.ModuleMessages;
 import seedu.address.model.Model;
 import seedu.address.model.module.ModuleCode;
-import seedu.address.model.module.ModuleTutorialPair;
 import seedu.address.model.module.TutorialClass;
 
 /**
@@ -39,22 +36,6 @@ public AddStudentToClassCommand(ModuleCode module, TutorialClass tutorialClass)
         this.tutorialClass = tutorialClass;
     }
 
-    protected ModuleTutorialPair getModuleAndTutorialClass(Model model) throws CommandException {
-        requireNonNull(model);
-        ModuleCode module = getModule();
-        TutorialClass tutorialClass = getTutorialClass();
-        ModuleCode existingModule = model.findModuleFromList(module);
-        TutorialClass existingTutorialClass = model.findTutorialClassFromList(tutorialClass, existingModule);
-        if (existingModule == null) {
-            throw new CommandException(String.format(ModuleMessages.MESSAGE_MODULE_NOT_FOUND, module));
-        }
-        if (existingTutorialClass == null) {
-            throw new CommandException(
-                    String.format(ModuleMessages.MESSAGE_TUTORIAL_DOES_NOT_BELONG_TO_MODULE, tutorialClass, module));
-        }
-        return new ModuleTutorialPair(existingModule, existingTutorialClass);
-    }
-
     protected ModuleCode getModule() {
         return module;
     }

diff --git a/...ess/logic/commands/allocatestudenttoteamcommands/AllocateStudentToTeamByEmailCommand.java b/...ess/logic/commands/allocatestudenttoteamcommands/AllocateStudentToTeamByEmailCommand.java
@@ -9,6 +9,7 @@
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.exceptions.CommandException;
+import seedu.address.logic.messages.TeamMessages;
 import seedu.address.model.Model;
 import seedu.address.model.module.ModuleCode;
 import seedu.address.model.module.TutorialClass;
@@ -70,7 +71,7 @@ public CommandResult execute(Model model) throws CommandException {
         }
 
         if (tutTeam == null) {
-            throw new CommandException(String.format(MESSAGE_TEAM_DOES_NOT_EXIST, tutorialTeam, tutClass));
+            throw new CommandException(String.format(TeamMessages.MESSAGE_TEAM_DOES_NOT_EXIST, tutorialTeam, tutClass));
         }
 
         // throws commandException if any condition fails

diff --git a/...ess/logic/commands/allocatestudenttoteamcommands/AllocateStudentToTeamByIndexCommand.java b/...ess/logic/commands/allocatestudenttoteamcommands/AllocateStudentToTeamByIndexCommand.java
@@ -11,6 +11,7 @@
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.exceptions.CommandException;
+import seedu.address.logic.messages.TutorialClassMessages;
 import seedu.address.model.Model;
 import seedu.address.model.module.ModuleCode;
 import seedu.address.model.module.TutorialClass;
@@ -35,9 +36,6 @@ public class AllocateStudentToTeamByIndexCommand extends AllocateStudentToTeamCo
             + PREFIX_MODULECODE + "CS2101 "
             + PREFIX_TUTORIALCLASS + "T01 "
             + PREFIX_TEAMNAME + "Team 1 ";
-
-    public static final String MESSAGE_PERSON_INDEX_NOT_FOUND =
-            "Student at index %d not found inside tutorial class %s";
     private final Index index;
     private final ModuleCode moduleCode;
     private final TutorialClass tutorialClass;
@@ -71,7 +69,7 @@ public CommandResult execute(Model model) throws CommandException {
             studentToAllocate = model.getStudentsInTutorialClass(tutClass).get(index.getZeroBased());
         } catch (IndexOutOfBoundsException err) {
             throw new CommandException(
-                    String.format(MESSAGE_PERSON_INDEX_NOT_FOUND, index.getOneBased(), tutClass));
+                    String.format(TutorialClassMessages.MESSAGE_PERSON_INDEX_NOT_FOUND, index.getOneBased(), tutClass));
         }
 
         TutorialTeam tutTeam = tutClass.getTutorialTeam(tutClass, tutorialTeam);

diff --git a/...ess/logic/commands/allocatestudenttoteamcommands/AllocateStudentToTeamByStuIdCommand.java b/...ess/logic/commands/allocatestudenttoteamcommands/AllocateStudentToTeamByStuIdCommand.java
@@ -10,6 +10,7 @@
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.exceptions.CommandException;
+import seedu.address.logic.messages.TeamMessages;
 import seedu.address.model.Model;
 import seedu.address.model.module.ModuleCode;
 import seedu.address.model.module.TutorialClass;
@@ -72,7 +73,7 @@ public CommandResult execute(Model model) throws CommandException {
         }
 
         if (tutTeam == null) {
-            throw new CommandException(String.format(MESSAGE_TEAM_DOES_NOT_EXIST, tutorialTeam, tutClass));
+            throw new CommandException(String.format(TeamMessages.MESSAGE_TEAM_DOES_NOT_EXIST, tutorialTeam, tutClass));
         }
 
         // throws commandException if any condition fails

diff --git a/...du/address/logic/commands/allocatestudenttoteamcommands/AllocateStudentToTeamCommand.java b/...du/address/logic/commands/allocatestudenttoteamcommands/AllocateStudentToTeamCommand.java
@@ -9,6 +9,7 @@
 import seedu.address.logic.commands.Command;
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.exceptions.CommandException;
+import seedu.address.logic.messages.TeamMessages;
 import seedu.address.model.Model;
 import seedu.address.model.module.TutorialClass;
 import seedu.address.model.module.TutorialTeam;
@@ -36,13 +37,8 @@ public abstract class AllocateStudentToTeamCommand extends Command {
     public static final String MESSAGE_SUCCESS = "Allocate student to team: %s";
     public static final String MESSAGE_STUDENT_NOT_IN_TUTORIAL = "Student needs to be in that tutorial group first.";
     public static final String MESSAGE_STUDENT_DOES_NOT_EXIST = "Student does not exist in system";
-    public static final String MESSAGE_TEAM_DOES_NOT_EXIST = "Team %s does not exist in tutorial class %s";
     public static final String MESSAGE_CLASS_DOES_NOT_EXIST = "Tutorial class %s does not exist in module %s";
 
-    public static final String MESSAGE_DUPLICATE_PERSON_IN_TEAM = "This person already exists in a team"
-            + " in the tutorial class %s!";
-    public static final String MESSAGE_TEAM_SIZE_EXCEEDED = "Max team size of %d reached";
-
     public AllocateStudentToTeamCommand() {}
 
     @Override
@@ -64,15 +60,16 @@ public void checkAllocateCondition(Person student, TutorialClass tutClass, Tutor
         }
 
         if (!tutClass.hasTeamInTutorial(tutClass, tutorialTeam)) {
-            throw new CommandException(String.format(MESSAGE_TEAM_DOES_NOT_EXIST, tutorialTeam, tutClass));
+            throw new CommandException(String.format(TeamMessages.MESSAGE_TEAM_DOES_NOT_EXIST, tutorialTeam, tutClass));
         }
 
         if (tutClass.isStudentInAnyTeam(student, tutClass)) {
-            throw new CommandException(String.format(MESSAGE_DUPLICATE_PERSON_IN_TEAM, tutClass));
+            throw new CommandException(String.format(TeamMessages.MESSAGE_DUPLICATE_PERSON_IN_TEAM, tutClass));
         }
 
         if (tutorialTeam.hasTeamSizeExceeded(tutorialTeam)) {
-            throw new CommandException(String.format(MESSAGE_TEAM_SIZE_EXCEEDED, tutorialTeam.getTeamSize()));
+            throw new CommandException(String.format(TeamMessages.MESSAGE_TEAM_SIZE_EXCEEDED,
+                    tutorialTeam.getTeamSize()));
         }
     }
 

diff --git a/...s/logic/commands/deletestudentfromclasscommands/DeleteStudentFromClassByEmailCommand.java b/...s/logic/commands/deletestudentfromclasscommands/DeleteStudentFromClassByEmailCommand.java
@@ -40,7 +40,8 @@ public DeleteStudentFromClassByEmailCommand(Email email, ModuleCode module, Tuto
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        ModuleTutorialPair moduleAndTutorialClass = getModuleAndTutorialClass(model);
+        ModuleTutorialPair moduleAndTutorialClass = ModuleTutorialPair.getModuleAndTutorialClass(model,
+                getModule(), getTutorialClass());
         TutorialClass tutorialClass = moduleAndTutorialClass.getTutorialClass();
         ModuleCode module = moduleAndTutorialClass.getModule();
         Person personToDelete;

diff --git a/...ress/logic/commands/deletestudentfromclasscommands/DeleteStudentFromClassByIdCommand.java b/...ress/logic/commands/deletestudentfromclasscommands/DeleteStudentFromClassByIdCommand.java
@@ -41,7 +41,8 @@ public DeleteStudentFromClassByIdCommand(StudentId studentId, ModuleCode module,
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        ModuleTutorialPair moduleAndTutorialClass = getModuleAndTutorialClass(model);
+        ModuleTutorialPair moduleAndTutorialClass = ModuleTutorialPair.getModuleAndTutorialClass(model,
+                getModule(), getTutorialClass());
         ModuleCode module = moduleAndTutorialClass.getModule();
         TutorialClass tutorialClass = moduleAndTutorialClass.getTutorialClass();
         Person personToDelete;

diff --git a/...s/logic/commands/deletestudentfromclasscommands/DeleteStudentFromClassByIndexCommand.java b/...s/logic/commands/deletestudentfromclasscommands/DeleteStudentFromClassByIndexCommand.java
@@ -36,15 +36,17 @@ public DeleteStudentFromClassByIndexCommand(Index targetIndex, ModuleCode module
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        ModuleTutorialPair moduleAndTutorialClass = getModuleAndTutorialClass(model);
+        ModuleTutorialPair moduleAndTutorialClass = ModuleTutorialPair.getModuleAndTutorialClass(model,
+                getModule(), getTutorialClass());
         TutorialClass tutorialClass = moduleAndTutorialClass.getTutorialClass();
         ModuleCode module = moduleAndTutorialClass.getModule();
         Person personToDelete;
         try {
-            personToDelete = model.getFilteredPersonList().get(targetIndex.getZeroBased());
+            personToDelete = model.getStudentsInTutorialClass(tutorialClass).get(targetIndex.getZeroBased());
         } catch (IndexOutOfBoundsException e) {
             throw new CommandException(
-                    String.format(PersonMessages.MESSAGE_PERSON_INDEX_NOT_FOUND, targetIndex.getOneBased()));
+                    String.format(TutorialClassMessages.MESSAGE_PERSON_INDEX_NOT_FOUND, targetIndex.getOneBased(),
+                            tutorialClass));
         }
 
         if (!(tutorialClass.hasStudent(personToDelete))) {

diff --git a/.../address/logic/commands/deletestudentfromclasscommands/DeleteStudentFromClassCommand.java b/.../address/logic/commands/deletestudentfromclasscommands/DeleteStudentFromClassCommand.java
@@ -1,6 +1,5 @@
 package seedu.address.logic.commands.deletestudentfromclasscommands;
 
-import static java.util.Objects.requireNonNull;
 import static seedu.address.commons.util.CollectionUtil.requireAllNonNull;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_MODULECODE;
@@ -9,10 +8,8 @@
 import seedu.address.logic.commands.Command;
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.exceptions.CommandException;
-import seedu.address.logic.messages.ModuleMessages;
 import seedu.address.model.Model;
 import seedu.address.model.module.ModuleCode;
-import seedu.address.model.module.ModuleTutorialPair;
 import seedu.address.model.module.TutorialClass;
 
 /**
@@ -39,22 +36,6 @@ public DeleteStudentFromClassCommand(ModuleCode module, TutorialClass tutorialCl
         this.tutorialClass = tutorialClass;
     }
 
-    protected ModuleTutorialPair getModuleAndTutorialClass(Model model) throws CommandException {
-        requireNonNull(model);
-        ModuleCode module = getModule();
-        TutorialClass tutorialClass = getTutorialClass();
-        ModuleCode existingModule = model.findModuleFromList(module);
-        TutorialClass existingTutorialClass = model.findTutorialClassFromList(tutorialClass, existingModule);
-        if (existingModule == null) {
-            throw new CommandException(String.format(ModuleMessages.MESSAGE_MODULE_NOT_FOUND, module));
-        }
-        if (existingTutorialClass == null) {
-            throw new CommandException(
-                    String.format(ModuleMessages.MESSAGE_TUTORIAL_DOES_NOT_BELONG_TO_MODULE, tutorialClass, module));
-        }
-        return new ModuleTutorialPair(existingModule, existingTutorialClass);
-    }
-
     protected ModuleCode getModule() {
         return module;
     }