From 964f2c2f9369070737f99ccd1de1473c08dc47bc Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 11:03:40 +0800 Subject: [PATCH 01/13] Change DeleteCommand to support deleting with StudentId --- .../java/seedu/address/logic/Messages.java | 2 + .../address/logic/commands/DeleteCommand.java | 43 +++++++++++++------ .../logic/parser/DeleteCommandParser.java | 20 ++++++--- 3 files changed, 46 insertions(+), 19 deletions(-) diff --git a/src/main/java/seedu/address/logic/Messages.java b/src/main/java/seedu/address/logic/Messages.java index 9b8087ce0a6..63251c26968 100644 --- a/src/main/java/seedu/address/logic/Messages.java +++ b/src/main/java/seedu/address/logic/Messages.java @@ -19,6 +19,8 @@ public class Messages { public static final String MESSAGE_DUPLICATE_FIELDS = "Multiple values specified for the following single-valued field(s): "; + public static final String MESSAGE_INVALID_STUDENT_ID = "No student is found with Student ID: %1$s"; + /** * Returns an error message indicating the duplicate prefixes. */ diff --git a/src/main/java/seedu/address/logic/commands/DeleteCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCommand.java index 1135ac19b74..7ee022d187b 100644 --- a/src/main/java/seedu/address/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/address/logic/commands/DeleteCommand.java @@ -4,12 +4,16 @@ import java.util.List; -import seedu.address.commons.core.index.Index; +import static seedu.address.logic.parser.CliSyntax.PREFIX_STUDENTID; +import static seedu.address.model.person.StudentId.isValidStudentId; + import seedu.address.commons.util.ToStringBuilder; import seedu.address.logic.Messages; import seedu.address.logic.commands.exceptions.CommandException; import seedu.address.model.Model; import seedu.address.model.person.Person; +import seedu.address.model.person.StudentId; +import seedu.address.model.person.exceptions.InvalidStudentIdException; /** * Deletes a person identified using it's displayed index from the address book. @@ -19,28 +23,41 @@ public class DeleteCommand extends Command { public static final String COMMAND_WORD = "delete"; public static final String MESSAGE_USAGE = COMMAND_WORD - + ": Deletes the person identified by the index number used in the displayed person list.\n" - + "Parameters: INDEX (must be a positive integer)\n" - + "Example: " + COMMAND_WORD + " 1"; + + ": Deletes the student identified by the Student ID used in the displayed person list.\n" + + "Parameters: " + + PREFIX_STUDENTID + "ID\n" + + "Example: " + COMMAND_WORD + " " + + PREFIX_STUDENTID + "12345678"; - public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Person: %1$s"; + public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Student: %1$s"; - private final Index targetIndex; + private final StudentId studentId; - public DeleteCommand(Index targetIndex) { - this.targetIndex = targetIndex; + public DeleteCommand(StudentId studentId) { + requireNonNull(studentId); + if (!isValidStudentId(studentId.studentId)) { + throw new InvalidStudentIdException(studentId.studentId); + } + this.studentId = studentId; } @Override public CommandResult execute(Model model) throws CommandException { requireNonNull(model); List lastShownList = model.getFilteredPersonList(); + Person personToDelete = null; + + for (Person person : lastShownList) { + if (person.getStudentId().equals(studentId)) { + personToDelete = person; + break; + } + } - if (targetIndex.getZeroBased() >= lastShownList.size()) { - throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); + if (personToDelete == null) { + throw new CommandException(String.format(Messages.MESSAGE_INVALID_STUDENT_ID, studentId)); } - Person personToDelete = lastShownList.get(targetIndex.getZeroBased()); model.deletePerson(personToDelete); return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(personToDelete))); } @@ -57,13 +74,13 @@ public boolean equals(Object other) { } DeleteCommand otherDeleteCommand = (DeleteCommand) other; - return targetIndex.equals(otherDeleteCommand.targetIndex); + return studentId.equals(otherDeleteCommand.studentId); } @Override public String toString() { return new ToStringBuilder(this) - .add("targetIndex", targetIndex) + .add("studentId", studentId) .toString(); } } diff --git a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java index 3527fe76a3e..2941e990d71 100644 --- a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java @@ -2,9 +2,9 @@ import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; -import seedu.address.commons.core.index.Index; import seedu.address.logic.commands.DeleteCommand; import seedu.address.logic.parser.exceptions.ParseException; +import seedu.address.model.person.StudentId; /** * Parses input arguments and creates a new DeleteCommand object @@ -17,13 +17,21 @@ public class DeleteCommandParser implements Parser { * @throws ParseException if the user input does not conform the expected format */ public DeleteCommand parse(String args) throws ParseException { + String trimmedArg = args.trim(); try { - Index index = ParserUtil.parseIndex(args); - return new DeleteCommand(index); - } catch (ParseException pe) { + + if (!StudentId.isValidStudentId(trimmedArg)) { + throw new ParseException( + String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE)); + } + + StudentId studentId = new StudentId(trimmedArg); + return new DeleteCommand(studentId); + + } catch (IllegalArgumentException e) { throw new ParseException( - String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), pe); + String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), e); } } - } + From e86725fb9b060aa5617219bf72c8967fb0ce9ff5 Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 12:04:20 +0800 Subject: [PATCH 02/13] Update tests to support delete with StudentId instead of Index --- .../address/logic/commands/DeleteCommand.java | 3 - .../logic/parser/DeleteCommandParser.java | 1 - .../logic/commands/DeleteCommandTest.java | 61 +++++++++++++------ .../logic/parser/AddressBookParserTest.java | 7 ++- .../logic/parser/DeleteCommandParserTest.java | 4 +- 5 files changed, 49 insertions(+), 27 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/DeleteCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCommand.java index 7ee022d187b..f07b2548b6a 100644 --- a/src/main/java/seedu/address/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/address/logic/commands/DeleteCommand.java @@ -35,9 +35,6 @@ public class DeleteCommand extends Command { public DeleteCommand(StudentId studentId) { requireNonNull(studentId); - if (!isValidStudentId(studentId.studentId)) { - throw new InvalidStudentIdException(studentId.studentId); - } this.studentId = studentId; } diff --git a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java index 2941e990d71..6f200a0e4f7 100644 --- a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java @@ -34,4 +34,3 @@ public DeleteCommand parse(String args) throws ParseException { } } } - diff --git a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java index b6f332eabca..9278e4af7fb 100644 --- a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java @@ -18,6 +18,7 @@ import seedu.address.model.ModelManager; import seedu.address.model.UserPrefs; import seedu.address.model.person.Person; +import seedu.address.model.person.StudentId; /** * Contains integration tests (interaction with the Model) and unit tests for @@ -28,9 +29,10 @@ public class DeleteCommandTest { private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs()); @Test - public void execute_validIndexUnfilteredList_success() { + public void execute_validStudentIdUnfilteredList_success() { Person personToDelete = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()); - DeleteCommand deleteCommand = new DeleteCommand(INDEX_FIRST_PERSON); + StudentId studentIdToDelete = personToDelete.getStudentId(); + DeleteCommand deleteCommand = new DeleteCommand(studentIdToDelete); String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(personToDelete)); @@ -42,19 +44,21 @@ public void execute_validIndexUnfilteredList_success() { } @Test - public void execute_invalidIndexUnfilteredList_throwsCommandException() { - Index outOfBoundIndex = Index.fromOneBased(model.getFilteredPersonList().size() + 1); - DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex); + public void execute_invalidStudentIdUnfilteredList_throwsCommandException() { + StudentId invalidStudentId = new StudentId("invalid123"); - assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); + DeleteCommand deleteCommand = new DeleteCommand(invalidStudentId); + + assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_STUDENT_ID); } @Test - public void execute_validIndexFilteredList_success() { + public void execute_validStudentIdFilteredList_success() { showPersonAtIndex(model, INDEX_FIRST_PERSON); Person personToDelete = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()); - DeleteCommand deleteCommand = new DeleteCommand(INDEX_FIRST_PERSON); + StudentId studentIdToDelete = personToDelete.getStudentId(); + DeleteCommand deleteCommand = new DeleteCommand(studentIdToDelete); String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(personToDelete)); @@ -67,28 +71,43 @@ public void execute_validIndexFilteredList_success() { } @Test - public void execute_invalidIndexFilteredList_throwsCommandException() { + public void execute_invalidStudentIdFilteredList_throwsCommandException() { +// showPersonAtIndex(model, INDEX_FIRST_PERSON); +// +// Index outOfBoundIndex = INDEX_SECOND_PERSON; +// // ensures that outOfBoundIndex is still in bounds of address book list +// assertTrue(outOfBoundIndex.getZeroBased() < model.getAddressBook().getPersonList().size()); +// +// DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex); +// +// assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); +// showPersonAtIndex(model, INDEX_FIRST_PERSON); - Index outOfBoundIndex = INDEX_SECOND_PERSON; - // ensures that outOfBoundIndex is still in bounds of address book list - assertTrue(outOfBoundIndex.getZeroBased() < model.getAddressBook().getPersonList().size()); + StudentId invalidStudentId = new StudentId("invalid123"); + + DeleteCommand deleteCommand = new DeleteCommand(invalidStudentId); - DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex); + assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_STUDENT_ID); - assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); } @Test public void equals() { - DeleteCommand deleteFirstCommand = new DeleteCommand(INDEX_FIRST_PERSON); - DeleteCommand deleteSecondCommand = new DeleteCommand(INDEX_SECOND_PERSON); + Person firstPerson = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()); + Person secondPerson = model.getFilteredPersonList().get(INDEX_SECOND_PERSON.getZeroBased()); + + StudentId firstStudentId = firstPerson.getStudentId(); + StudentId secondStudentId = secondPerson.getStudentId(); + + DeleteCommand deleteFirstCommand = new DeleteCommand(firstStudentId); + DeleteCommand deleteSecondCommand = new DeleteCommand(secondStudentId); // same object -> returns true assertTrue(deleteFirstCommand.equals(deleteFirstCommand)); // same values -> returns true - DeleteCommand deleteFirstCommandCopy = new DeleteCommand(INDEX_FIRST_PERSON); + DeleteCommand deleteFirstCommandCopy = new DeleteCommand(firstStudentId); assertTrue(deleteFirstCommand.equals(deleteFirstCommandCopy)); // different types -> returns false @@ -103,9 +122,11 @@ public void equals() { @Test public void toStringMethod() { - Index targetIndex = Index.fromOneBased(1); - DeleteCommand deleteCommand = new DeleteCommand(targetIndex); - String expected = DeleteCommand.class.getCanonicalName() + "{targetIndex=" + targetIndex + "}"; + Person firstPerson = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()); + StudentId targetStudentId = firstPerson.getStudentId(); + + DeleteCommand deleteCommand = new DeleteCommand(targetStudentId); + String expected = DeleteCommand.class.getCanonicalName() + "{targetStudentId=" + targetStudentId + "}"; assertEquals(expected, deleteCommand.toString()); } diff --git a/src/test/java/seedu/address/logic/parser/AddressBookParserTest.java b/src/test/java/seedu/address/logic/parser/AddressBookParserTest.java index 5a1ab3dbc0c..618573edfe4 100644 --- a/src/test/java/seedu/address/logic/parser/AddressBookParserTest.java +++ b/src/test/java/seedu/address/logic/parser/AddressBookParserTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; import static seedu.address.logic.Messages.MESSAGE_UNKNOWN_COMMAND; +import static seedu.address.logic.parser.CliSyntax.PREFIX_STUDENTID; import static seedu.address.testutil.Assert.assertThrows; import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON; @@ -25,6 +26,7 @@ import seedu.address.logic.parser.exceptions.ParseException; import seedu.address.model.person.NameContainsKeywordsPredicate; import seedu.address.model.person.Person; +import seedu.address.model.person.StudentId; import seedu.address.testutil.EditPersonDescriptorBuilder; import seedu.address.testutil.PersonBuilder; import seedu.address.testutil.PersonUtil; @@ -48,9 +50,10 @@ public void parseCommand_clear() throws Exception { @Test public void parseCommand_delete() throws Exception { + StudentId validStudentId = new StudentId("12345678"); DeleteCommand command = (DeleteCommand) parser.parseCommand( - DeleteCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased()); - assertEquals(new DeleteCommand(INDEX_FIRST_PERSON), command); + DeleteCommand.COMMAND_WORD + " " + PREFIX_STUDENTID + validStudentId); + assertEquals(new DeleteCommand(validStudentId), command); } @Test diff --git a/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java b/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java index 6a40e14a649..8eb37cdd7a2 100644 --- a/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.api.Test; import seedu.address.logic.commands.DeleteCommand; +import seedu.address.model.person.StudentId; /** * As we are only doing white-box testing, our test cases do not cover path variations @@ -22,7 +23,8 @@ public class DeleteCommandParserTest { @Test public void parse_validArgs_returnsDeleteCommand() { - assertParseSuccess(parser, "1", new DeleteCommand(INDEX_FIRST_PERSON)); + StudentId validStudentId = new StudentId("12345678"); + assertParseSuccess(parser, validStudentId.toString(), new DeleteCommand(validStudentId)); } @Test From 1c697dab054f49895ca2345c673109310da0cfa9 Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 12:26:36 +0800 Subject: [PATCH 03/13] Update DeleteCommand and parser to support prefix --- .../java/seedu/address/logic/commands/DeleteCommand.java | 2 +- .../seedu/address/logic/parser/DeleteCommandParser.java | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/DeleteCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCommand.java index f07b2548b6a..843d1ad9088 100644 --- a/src/main/java/seedu/address/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/address/logic/commands/DeleteCommand.java @@ -77,7 +77,7 @@ public boolean equals(Object other) { @Override public String toString() { return new ToStringBuilder(this) - .add("studentId", studentId) + .add("targetStudentId", studentId) .toString(); } } diff --git a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java index 6f200a0e4f7..cb06ab4702c 100644 --- a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java @@ -19,13 +19,19 @@ public class DeleteCommandParser implements Parser { public DeleteCommand parse(String args) throws ParseException { String trimmedArg = args.trim(); try { + if (!trimmedArg.startsWith("id/")) { + throw new ParseException( + String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE)); + } + + String studentIdString = trimmedArg.substring(3).trim(); if (!StudentId.isValidStudentId(trimmedArg)) { throw new ParseException( String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE)); } - StudentId studentId = new StudentId(trimmedArg); + StudentId studentId = new StudentId(studentIdString); return new DeleteCommand(studentId); } catch (IllegalArgumentException e) { From a6c4984730217c21849898fd3e1da519be4c5de2 Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 12:28:42 +0800 Subject: [PATCH 04/13] Fix LogicManagerTest --- src/test/java/seedu/address/logic/LogicManagerTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/java/seedu/address/logic/LogicManagerTest.java b/src/test/java/seedu/address/logic/LogicManagerTest.java index e516f48df5c..93e0f4d1873 100644 --- a/src/test/java/seedu/address/logic/LogicManagerTest.java +++ b/src/test/java/seedu/address/logic/LogicManagerTest.java @@ -1,8 +1,7 @@ package seedu.address.logic; import static org.junit.jupiter.api.Assertions.assertEquals; -import static seedu.address.logic.Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX; -import static seedu.address.logic.Messages.MESSAGE_UNKNOWN_COMMAND; +import static seedu.address.logic.Messages.*; import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.COURSE_DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_AMY; @@ -63,8 +62,8 @@ public void execute_invalidCommandFormat_throwsParseException() { @Test public void execute_commandExecutionError_throwsCommandException() { - String deleteCommand = "delete 9"; - assertCommandException(deleteCommand, MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); + String deleteCommand = "delete 00000000"; + assertCommandException(deleteCommand, String.format(MESSAGE_INVALID_STUDENT_ID, "00000000")); } @Test From 1141af03fb501a17218a73efe5a5711644092556 Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 12:53:32 +0800 Subject: [PATCH 05/13] Update related tests --- .../logic/parser/DeleteCommandParser.java | 6 ------ .../seedu/address/logic/LogicManagerTest.java | 2 +- .../logic/commands/DeleteCommandTest.java | 17 ++++++++--------- .../logic/parser/DeleteCommandParserTest.java | 3 ++- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java index cb06ab4702c..dea9426b8b7 100644 --- a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java @@ -25,12 +25,6 @@ public DeleteCommand parse(String args) throws ParseException { } String studentIdString = trimmedArg.substring(3).trim(); - - if (!StudentId.isValidStudentId(trimmedArg)) { - throw new ParseException( - String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE)); - } - StudentId studentId = new StudentId(studentIdString); return new DeleteCommand(studentId); diff --git a/src/test/java/seedu/address/logic/LogicManagerTest.java b/src/test/java/seedu/address/logic/LogicManagerTest.java index 93e0f4d1873..120ae0c08cb 100644 --- a/src/test/java/seedu/address/logic/LogicManagerTest.java +++ b/src/test/java/seedu/address/logic/LogicManagerTest.java @@ -62,7 +62,7 @@ public void execute_invalidCommandFormat_throwsParseException() { @Test public void execute_commandExecutionError_throwsCommandException() { - String deleteCommand = "delete 00000000"; + String deleteCommand = "delete id/00000000"; assertCommandException(deleteCommand, String.format(MESSAGE_INVALID_STUDENT_ID, "00000000")); } diff --git a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java index 9278e4af7fb..5a56e9fd09f 100644 --- a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java @@ -45,11 +45,14 @@ public void execute_validStudentIdUnfilteredList_success() { @Test public void execute_invalidStudentIdUnfilteredList_throwsCommandException() { - StudentId invalidStudentId = new StudentId("invalid123"); +// Index outOfBoundIndex = Index.fromOneBased(model.getFilteredPersonList().size() + 1); +// DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex); +// +// assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); + StudentId invalidStudentId = new StudentId("12345679"); DeleteCommand deleteCommand = new DeleteCommand(invalidStudentId); - - assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_STUDENT_ID); + assertCommandFailure(deleteCommand, model, String.format(Messages.MESSAGE_INVALID_STUDENT_ID, invalidStudentId)); } @Test @@ -81,14 +84,10 @@ public void execute_invalidStudentIdFilteredList_throwsCommandException() { // DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex); // // assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); -// showPersonAtIndex(model, INDEX_FIRST_PERSON); - - StudentId invalidStudentId = new StudentId("invalid123"); - + StudentId invalidStudentId = new StudentId("00000000"); DeleteCommand deleteCommand = new DeleteCommand(invalidStudentId); - - assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_STUDENT_ID); + assertCommandFailure(deleteCommand, model, String.format(Messages.MESSAGE_INVALID_STUDENT_ID, invalidStudentId)); } diff --git a/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java b/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java index 8eb37cdd7a2..b4f83e1ea09 100644 --- a/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java @@ -24,7 +24,8 @@ public class DeleteCommandParserTest { @Test public void parse_validArgs_returnsDeleteCommand() { StudentId validStudentId = new StudentId("12345678"); - assertParseSuccess(parser, validStudentId.toString(), new DeleteCommand(validStudentId)); + String input = "id/" + validStudentId.toString(); + assertParseSuccess(parser, input, new DeleteCommand(validStudentId)); } @Test From 817d85a0c513240dd3b4529287ce6a5c0926721f Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 12:55:07 +0800 Subject: [PATCH 06/13] Remove unused import statements --- src/main/java/seedu/address/logic/commands/DeleteCommand.java | 2 -- .../java/seedu/address/logic/commands/DeleteCommandTest.java | 2 +- .../seedu/address/logic/parser/DeleteCommandParserTest.java | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/DeleteCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCommand.java index 843d1ad9088..0c8d1b0249f 100644 --- a/src/main/java/seedu/address/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/address/logic/commands/DeleteCommand.java @@ -5,7 +5,6 @@ import java.util.List; import static seedu.address.logic.parser.CliSyntax.PREFIX_STUDENTID; -import static seedu.address.model.person.StudentId.isValidStudentId; import seedu.address.commons.util.ToStringBuilder; import seedu.address.logic.Messages; @@ -13,7 +12,6 @@ import seedu.address.model.Model; import seedu.address.model.person.Person; import seedu.address.model.person.StudentId; -import seedu.address.model.person.exceptions.InvalidStudentIdException; /** * Deletes a person identified using it's displayed index from the address book. diff --git a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java index 5a56e9fd09f..65990bc8de3 100644 --- a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java @@ -12,7 +12,7 @@ import org.junit.jupiter.api.Test; -import seedu.address.commons.core.index.Index; +//import seedu.address.commons.core.index.Index; import seedu.address.logic.Messages; import seedu.address.model.Model; import seedu.address.model.ModelManager; diff --git a/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java b/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java index b4f83e1ea09..7802dd0923c 100644 --- a/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java @@ -3,7 +3,6 @@ import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; import static seedu.address.logic.parser.CommandParserTestUtil.assertParseFailure; import static seedu.address.logic.parser.CommandParserTestUtil.assertParseSuccess; -import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON; import org.junit.jupiter.api.Test; From a9c9c63918788313af7eee79ed109a32f701664c Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 14:23:11 +0800 Subject: [PATCH 07/13] Fix bugs related to parsing --- .../address/logic/commands/DeleteCommand.java | 14 +++++++------- .../address/logic/parser/DeleteCommandParser.java | 2 +- .../logic/parser/DeleteCommandParserTest.java | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/DeleteCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCommand.java index 0c8d1b0249f..91d0d8a887f 100644 --- a/src/main/java/seedu/address/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/address/logic/commands/DeleteCommand.java @@ -28,7 +28,7 @@ public class DeleteCommand extends Command { + PREFIX_STUDENTID + "12345678"; public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Student: %1$s"; - + public static final String MESSAGE_PERSON_NOT_FOUND = "No student is found with Student ID: %1$s"; private final StudentId studentId; public DeleteCommand(StudentId studentId) { @@ -40,21 +40,21 @@ public DeleteCommand(StudentId studentId) { public CommandResult execute(Model model) throws CommandException { requireNonNull(model); List lastShownList = model.getFilteredPersonList(); - Person personToDelete = null; + Person toDelete = null; for (Person person : lastShownList) { if (person.getStudentId().equals(studentId)) { - personToDelete = person; + toDelete = person; break; } } - if (personToDelete == null) { - throw new CommandException(String.format(Messages.MESSAGE_INVALID_STUDENT_ID, studentId)); + if (toDelete == null) { + throw new CommandException(String.format(MESSAGE_PERSON_NOT_FOUND, studentId)); } - model.deletePerson(personToDelete); - return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(personToDelete))); + model.deletePerson(toDelete); + return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(toDelete))); } @Override diff --git a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java index dea9426b8b7..36a99a161a6 100644 --- a/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/DeleteCommandParser.java @@ -25,7 +25,7 @@ public DeleteCommand parse(String args) throws ParseException { } String studentIdString = trimmedArg.substring(3).trim(); - StudentId studentId = new StudentId(studentIdString); + StudentId studentId = ParserUtil.parseStudentId(studentIdString); return new DeleteCommand(studentId); } catch (IllegalArgumentException e) { diff --git a/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java b/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java index 7802dd0923c..c30c8936f2d 100644 --- a/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java @@ -23,7 +23,7 @@ public class DeleteCommandParserTest { @Test public void parse_validArgs_returnsDeleteCommand() { StudentId validStudentId = new StudentId("12345678"); - String input = "id/" + validStudentId.toString(); + String input = "id/" + validStudentId; assertParseSuccess(parser, input, new DeleteCommand(validStudentId)); } From 4dc8ba633403deb0190163fdca1855d4f18f6182 Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 14:50:49 +0800 Subject: [PATCH 08/13] Fix minor bugs --- .../java/seedu/address/logic/Messages.java | 2 -- .../seedu/address/logic/LogicManagerTest.java | 3 ++- .../logic/commands/DeleteCommandTest.java | 27 ++----------------- 3 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/main/java/seedu/address/logic/Messages.java b/src/main/java/seedu/address/logic/Messages.java index 63251c26968..9b8087ce0a6 100644 --- a/src/main/java/seedu/address/logic/Messages.java +++ b/src/main/java/seedu/address/logic/Messages.java @@ -19,8 +19,6 @@ public class Messages { public static final String MESSAGE_DUPLICATE_FIELDS = "Multiple values specified for the following single-valued field(s): "; - public static final String MESSAGE_INVALID_STUDENT_ID = "No student is found with Student ID: %1$s"; - /** * Returns an error message indicating the duplicate prefixes. */ diff --git a/src/test/java/seedu/address/logic/LogicManagerTest.java b/src/test/java/seedu/address/logic/LogicManagerTest.java index 120ae0c08cb..8139fe24bd9 100644 --- a/src/test/java/seedu/address/logic/LogicManagerTest.java +++ b/src/test/java/seedu/address/logic/LogicManagerTest.java @@ -22,6 +22,7 @@ import seedu.address.logic.commands.AddCommand; import seedu.address.logic.commands.CommandResult; +import seedu.address.logic.commands.DeleteCommand; import seedu.address.logic.commands.ListCommand; import seedu.address.logic.commands.exceptions.CommandException; import seedu.address.logic.parser.exceptions.ParseException; @@ -63,7 +64,7 @@ public void execute_invalidCommandFormat_throwsParseException() { @Test public void execute_commandExecutionError_throwsCommandException() { String deleteCommand = "delete id/00000000"; - assertCommandException(deleteCommand, String.format(MESSAGE_INVALID_STUDENT_ID, "00000000")); + assertCommandException(deleteCommand, String.format(DeleteCommand.MESSAGE_PERSON_NOT_FOUND, "00000000")); } @Test diff --git a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java index 65990bc8de3..ee1bfddf7d7 100644 --- a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java @@ -45,14 +45,9 @@ public void execute_validStudentIdUnfilteredList_success() { @Test public void execute_invalidStudentIdUnfilteredList_throwsCommandException() { -// Index outOfBoundIndex = Index.fromOneBased(model.getFilteredPersonList().size() + 1); -// DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex); -// -// assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); - StudentId invalidStudentId = new StudentId("12345679"); DeleteCommand deleteCommand = new DeleteCommand(invalidStudentId); - assertCommandFailure(deleteCommand, model, String.format(Messages.MESSAGE_INVALID_STUDENT_ID, invalidStudentId)); + assertCommandFailure(deleteCommand, model, String.format(DeleteCommand.MESSAGE_PERSON_NOT_FOUND, invalidStudentId)); } @Test @@ -72,25 +67,7 @@ public void execute_validStudentIdFilteredList_success() { assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModel); } - - @Test - public void execute_invalidStudentIdFilteredList_throwsCommandException() { -// showPersonAtIndex(model, INDEX_FIRST_PERSON); -// -// Index outOfBoundIndex = INDEX_SECOND_PERSON; -// // ensures that outOfBoundIndex is still in bounds of address book list -// assertTrue(outOfBoundIndex.getZeroBased() < model.getAddressBook().getPersonList().size()); -// -// DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex); -// -// assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); - showPersonAtIndex(model, INDEX_FIRST_PERSON); - StudentId invalidStudentId = new StudentId("00000000"); - DeleteCommand deleteCommand = new DeleteCommand(invalidStudentId); - assertCommandFailure(deleteCommand, model, String.format(Messages.MESSAGE_INVALID_STUDENT_ID, invalidStudentId)); - - } - + @Test public void equals() { Person firstPerson = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()); From b423e24e0cb838b76c27d771ebd4abfa5504b830 Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 14:51:23 +0800 Subject: [PATCH 09/13] Update test related to the use of index --- .../java/seedu/address/logic/commands/DeleteCommandTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java index ee1bfddf7d7..b9ff68b687e 100644 --- a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java @@ -67,7 +67,7 @@ public void execute_validStudentIdFilteredList_success() { assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModel); } - + @Test public void equals() { Person firstPerson = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()); From 4165be715e0aa6743516f517a8d87a84a0d0d62b Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 15:04:53 +0800 Subject: [PATCH 10/13] Fix minor bugs --- src/main/java/seedu/address/logic/commands/AddCommand.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/AddCommand.java b/src/main/java/seedu/address/logic/commands/AddCommand.java index 9ba0554a285..6387339c89c 100644 --- a/src/main/java/seedu/address/logic/commands/AddCommand.java +++ b/src/main/java/seedu/address/logic/commands/AddCommand.java @@ -32,13 +32,13 @@ public class AddCommand extends Command { + PREFIX_COURSE + "COURSE " + PREFIX_TAG + "TAG\n" + "Example: " + COMMAND_WORD + " " - + PREFIX_STUDENTID + "12345678" + + PREFIX_STUDENTID + "12345678 " + PREFIX_NAME + "John Doe " + PREFIX_PHONE + "98765432 " + PREFIX_EMAIL + "johnd@example.com " + PREFIX_ADDRESS + "311, Clementi Ave 2, #02-25 " - + PREFIX_COURSE + "Computer Science" - + PREFIX_TAG + "Student"; + + PREFIX_COURSE + "Computer Science " + + PREFIX_TAG + "Student "; public static final String MESSAGE_SUCCESS = "New person added: %1$s"; From f3cb1616c40dad8806d2dd61dbf08b9b8226527d Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 15:30:16 +0800 Subject: [PATCH 11/13] Fix checkstyle issue --- .../seedu/address/logic/commands/DeleteCommand.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/DeleteCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCommand.java index 91d0d8a887f..458c5b4b48f 100644 --- a/src/main/java/seedu/address/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/address/logic/commands/DeleteCommand.java @@ -1,11 +1,10 @@ package seedu.address.logic.commands; import static java.util.Objects.requireNonNull; +import static seedu.address.logic.parser.CliSyntax.PREFIX_STUDENTID; import java.util.List; -import static seedu.address.logic.parser.CliSyntax.PREFIX_STUDENTID; - import seedu.address.commons.util.ToStringBuilder; import seedu.address.logic.Messages; import seedu.address.logic.commands.exceptions.CommandException; @@ -36,6 +35,13 @@ public DeleteCommand(StudentId studentId) { this.studentId = studentId; } + /** + * Executes the delete command and removes a person identified by the given studentID. + * + * @param model the model that contains the data of persons + * @return a CommandResult that shows the outcome of the command + * @throws CommandException if the studentID is invalid or not found + */ @Override public CommandResult execute(Model model) throws CommandException { requireNonNull(model); From 8110ce65f0264790a96ca85e7abb36707925f81a Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 15:34:56 +0800 Subject: [PATCH 12/13] Fix checkstyle issue --- .../java/seedu/address/logic/commands/DeleteCommand.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/seedu/address/logic/commands/DeleteCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCommand.java index 458c5b4b48f..3acecc36de4 100644 --- a/src/main/java/seedu/address/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/address/logic/commands/DeleteCommand.java @@ -30,6 +30,12 @@ public class DeleteCommand extends Command { public static final String MESSAGE_PERSON_NOT_FOUND = "No student is found with Student ID: %1$s"; private final StudentId studentId; + /** + * Creates a DeleteCommand to delete the person identified by the specified {@code StudentId}. + * + * @param studentId The student ID of the person to be deleted. + * @throws NullPointerException if the {@code studentId} is null. + */ public DeleteCommand(StudentId studentId) { requireNonNull(studentId); this.studentId = studentId; From df9a1d942e67d5571c5ed48026441f19ac5fbc8f Mon Sep 17 00:00:00 2001 From: jessica2828 Date: Wed, 9 Oct 2024 15:40:56 +0800 Subject: [PATCH 13/13] Fix checkstyle issue --- src/test/java/seedu/address/logic/LogicManagerTest.java | 2 +- .../java/seedu/address/logic/commands/DeleteCommandTest.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/seedu/address/logic/LogicManagerTest.java b/src/test/java/seedu/address/logic/LogicManagerTest.java index 8139fe24bd9..bd8ed826e01 100644 --- a/src/test/java/seedu/address/logic/LogicManagerTest.java +++ b/src/test/java/seedu/address/logic/LogicManagerTest.java @@ -1,7 +1,7 @@ package seedu.address.logic; import static org.junit.jupiter.api.Assertions.assertEquals; -import static seedu.address.logic.Messages.*; +import static seedu.address.logic.Messages.MESSAGE_UNKNOWN_COMMAND; import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.COURSE_DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_AMY; diff --git a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java index b9ff68b687e..7cb5b09ed3b 100644 --- a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java @@ -47,7 +47,8 @@ public void execute_validStudentIdUnfilteredList_success() { public void execute_invalidStudentIdUnfilteredList_throwsCommandException() { StudentId invalidStudentId = new StudentId("12345679"); DeleteCommand deleteCommand = new DeleteCommand(invalidStudentId); - assertCommandFailure(deleteCommand, model, String.format(DeleteCommand.MESSAGE_PERSON_NOT_FOUND, invalidStudentId)); + assertCommandFailure( + deleteCommand, model, String.format(DeleteCommand.MESSAGE_PERSON_NOT_FOUND, invalidStudentId)); } @Test