Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix for add to lesson #319

Merged
merged 2 commits into from
Nov 11, 2024
Merged

Bugfix for add to lesson #319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f391576
Merge branch 'master' into bugfix/addToLesson
Clarenceeey Nov 10, 2024
256c0ce
addtolessoncommand now adds when multiple of same students added in s…
Clarenceeey Nov 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 32 additions & 11 deletions src/main/java/seedu/address/logic/commands/lesson/AddToLessonCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static seedu.address.logic.parser.CliSyntax.PREFIX_INDEX;
import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand All @@ -22,7 +23,6 @@
import seedu.address.model.lesson.Lesson;
import seedu.address.model.student.Name;
import seedu.address.model.student.Student;
import seedu.address.model.student.exceptions.DuplicateStudentException;

/**
* Adds students to a specific lesson.
Expand All @@ -46,6 +46,7 @@ public class AddToLessonCommand extends Command {
public static final String MESSAGE_DUPLICATE_STUDENT_IN_LESSON_BY_INDEX =
"%s at index %d is already added to the lesson!";


private final Index index;
private final List<Name> studentNames;
private final List<Index> indices;
Expand Down Expand Up @@ -83,19 +84,29 @@ public CommandResult execute(Model model) throws CommandException {

logger.info("Lesson to edit: " + targetLesson.toString());

Set<Student> studentsToAddSet = new HashSet<>();
ArrayList<Student> studentsToAddArr = new ArrayList<>();

for (Name studentName : studentNames) {
Student student = model.findStudentByName(studentName)
.orElseThrow(() -> new CommandException(String.format(MESSAGE_STUDENT_NOT_FOUND, studentName)));
try {
editedLesson.addStudent(student);
} catch (DuplicateStudentException e) {

// if student was already in the lesson before this command, throw error
if (editedLesson.hasStudent(student)) {
logger.warning("Students were not added to lesson " + targetLesson.toString()
+ " because there were duplicate names");
+ " because the student " + studentName + " was already in lesson");
throw new CommandException(
String.format(MESSAGE_DUPLICATE_STUDENT_IN_LESSON_BY_NAME, studentName));
}

//if student was not previously in the lesson before this command, add it to the set
if (!studentsToAddSet.contains(student)) {
studentsToAddArr.add(student);
}
studentsToAddSet.add(student);
}

// checking if the indices are out of bounds
boolean throwException = false;
Set<Index> outOfBounds = new HashSet<>();

Expand All @@ -107,26 +118,36 @@ public CommandResult execute(Model model) throws CommandException {
}

if (throwException) {
logger.warning("Students were not added to lesson " + targetLesson.toString()
+ " because there were indices of students that were out of bounds");
String formattedOutOfBoundIndices = outOfBounds.stream()
.map(index -> String.valueOf(index.getOneBased()))
.collect(Collectors.joining(", "));
logger.warning("Students were not added to lesson " + targetLesson.toString()
+ " because indices " + formattedOutOfBoundIndices + " were out of bounds");
throw new CommandException(String.format(Messages.MESSAGE_INVALID_INDEX_SHOWN,
formattedOutOfBoundIndices));
}

// checking if the students at these indices were already in the lesson
for (Index studentIndex : indices) {
Student student = lastShownStudentList.get(studentIndex.getZeroBased());
try {
editedLesson.addStudent(student);
} catch (DuplicateStudentException e) {

if (editedLesson.hasStudent(student)) {
logger.warning("Students were not added to lesson " + targetLesson.toString()
+ " because there were duplicate names");
+ " because student " + student.getName() + " was already in the lesson");
throw new CommandException(
String.format(MESSAGE_DUPLICATE_STUDENT_IN_LESSON_BY_INDEX, student.getName(),
studentIndex.getOneBased()));
}

if (!studentsToAddSet.contains(student)) {
studentsToAddArr.add(student);
}
studentsToAddSet.add(student);
}

// actually adding the students into the lesson
for (Student student: studentsToAddArr) {
editedLesson.addStudent(student);
}

model.setLesson(targetLesson, editedLesson);
Expand Down
49 changes: 40 additions & 9 deletions src/test/java/seedu/address/logic/commands/lesson/AddToLessonCommandTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,27 +202,58 @@ public void execute_someInvalidStudentNames_throwsCommandException() {
}

@Test
public void execute_duplicateStudentByIndexInCommand_throwsCommandException() {
public void execute_duplicateStudentByIndexInCommandSuccess() throws Exception {

AddToLessonCommand command = new AddToLessonCommand(validLessonIndex5,
validStudentNames, duplicateIndices);

String error = String.format(AddToLessonCommand.MESSAGE_DUPLICATE_STUDENT_IN_LESSON_BY_INDEX,
ALICE.getName().fullName,
INDEX_FIRST_STUDENT.getOneBased());
Model expectedModel = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs());

assertCommandFailure(command, model, error);
CommandResult result = command.execute(model);

Lesson targetLesson = expectedModel.getFilteredLessonList()
.get(validLessonIndex5.getZeroBased());
Lesson editedLesson = new Lesson(targetLesson);

editedLesson.addStudent(CARL);
editedLesson.addStudent(DANIEL);
editedLesson.addStudent(ALICE);
expectedModel.setLesson(targetLesson, editedLesson);

String expectedMessage = String.format(AddToLessonCommand.MESSAGE_ADD_TO_LESSON_SUCCESS,
Messages.format(editedLesson));

assertEquals(result.getFeedbackToUser(), expectedMessage);
assertEquals(expectedModel.getFilteredLessonList().get(validLessonIndex5.getZeroBased()),
model.getFilteredLessonList().get(validLessonIndex5.getZeroBased()));
}

@Test
public void execute_duplicateStudentByNameInCommand_throwsCommandException() {
public void execute_duplicateStudentByNameInCommandSuccess() throws Exception {
AddToLessonCommand command = new AddToLessonCommand(validLessonIndex5,
duplicateStudentNames, validStudentIndicesContainsAlice);

String error = String.format(AddToLessonCommand.MESSAGE_DUPLICATE_STUDENT_IN_LESSON_BY_NAME,
CARL.getName().fullName);
Model expectedModel = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs());

CommandResult result = command.execute(model);

Lesson targetLesson = expectedModel.getFilteredLessonList()
.get(validLessonIndex5.getZeroBased());
Lesson editedLesson = new Lesson(targetLesson);

editedLesson.addStudent(CARL);
editedLesson.addStudent(ALICE);
editedLesson.addStudent(BENSON);
expectedModel.setLesson(targetLesson, editedLesson);

String expectedMessage = String.format(AddToLessonCommand.MESSAGE_ADD_TO_LESSON_SUCCESS,
Messages.format(editedLesson));

assertEquals(result.getFeedbackToUser(), expectedMessage);
assertEquals(expectedModel.getFilteredLessonList().get(validLessonIndex5.getZeroBased()),
model.getFilteredLessonList().get(validLessonIndex5.getZeroBased()));


assertCommandFailure(command, model, error);
}

@Test
Expand Down