diff --git a/src/main/java/seedu/address/logic/commands/UndoableCommand.java b/src/main/java/seedu/address/logic/commands/UndoableCommand.java index 1ba888ead594..41c774e83854 100644 --- a/src/main/java/seedu/address/logic/commands/UndoableCommand.java +++ b/src/main/java/seedu/address/logic/commands/UndoableCommand.java @@ -4,15 +4,19 @@ import static seedu.address.commons.util.CollectionUtil.requireAllNonNull; import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS; +import java.util.function.Predicate; + import seedu.address.logic.commands.exceptions.CommandException; import seedu.address.model.AddressBook; import seedu.address.model.ReadOnlyAddressBook; +import seedu.address.model.person.ReadOnlyPerson; /** * Represents a command which can be undone and redone. */ public abstract class UndoableCommand extends Command { private ReadOnlyAddressBook previousAddressBook; + private Predicate previousPredicate; protected abstract CommandResult executeUndoableCommand() throws CommandException; @@ -24,6 +28,13 @@ private void saveAddressBookSnapshot() { this.previousAddressBook = new AddressBook(model.getAddressBook()); } + /** + * Stores the predicate used in {@code model#filteredPersons}. + */ + private void savePredicateSnapshot() { + previousPredicate = model.getFilteredPersonListPredicate(); + } + /** * Reverts the AddressBook to the state before this command * was executed and updates the filtered person list to @@ -42,6 +53,9 @@ protected final void undo() { protected final void redo() { requireNonNull(model); try { + // restore filtered person list to the same previous view to ensure command executes itself on the same + // correct {@code Person} as before + model.updateFilteredPersonList(previousPredicate); executeUndoableCommand(); } catch (CommandException ce) { throw new AssertionError("The command has been successfully executed previously; " @@ -53,6 +67,7 @@ protected final void redo() { @Override public final CommandResult execute() throws CommandException { saveAddressBookSnapshot(); + savePredicateSnapshot(); return executeUndoableCommand(); } } diff --git a/src/main/java/seedu/address/model/Model.java b/src/main/java/seedu/address/model/Model.java index 9c6aa4e766a0..1b0121562e45 100644 --- a/src/main/java/seedu/address/model/Model.java +++ b/src/main/java/seedu/address/model/Model.java @@ -39,6 +39,9 @@ void updatePerson(ReadOnlyPerson target, ReadOnlyPerson editedPerson) /** Returns an unmodifiable view of the filtered person list */ ObservableList getFilteredPersonList(); + /** Returns the predicate used in the filtered person list */ + Predicate getFilteredPersonListPredicate(); + /** * Updates the filter of the filtered person list to filter by the given {@code predicate}. * @throws NullPointerException if {@code predicate} is null. diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java index 095c831cfbf8..b1b57caea7c8 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -37,6 +37,8 @@ public ModelManager(ReadOnlyAddressBook addressBook, UserPrefs userPrefs) { this.addressBook = new AddressBook(addressBook); filteredPersons = new FilteredList<>(this.addressBook.getPersonList()); + // set a show all person as default predicate to prevent filteredPersons having a null value predicate + filteredPersons.setPredicate(PREDICATE_SHOW_ALL_PERSONS); } public ModelManager() { @@ -92,6 +94,15 @@ public ObservableList getFilteredPersonList() { return FXCollections.unmodifiableObservableList(filteredPersons); } + @Override + @SuppressWarnings("unchecked") + public Predicate getFilteredPersonListPredicate() { + Predicate toReturn = (Predicate) filteredPersons.getPredicate(); + // the predicate set inside filteredPersons should never be null + assert toReturn != null; + return toReturn; + } + @Override public void updateFilteredPersonList(Predicate predicate) { requireNonNull(predicate); diff --git a/src/test/java/seedu/address/logic/commands/AddCommandTest.java b/src/test/java/seedu/address/logic/commands/AddCommandTest.java index b327ef2d805b..e6cb430dda9a 100644 --- a/src/test/java/seedu/address/logic/commands/AddCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/AddCommandTest.java @@ -129,6 +129,12 @@ public ObservableList getFilteredPersonList() { return null; } + @Override + public Predicate getFilteredPersonListPredicate() { + fail("This method should not be called."); + return null; + } + @Override public void updateFilteredPersonList(Predicate predicate) { fail("This method should not be called."); @@ -144,10 +150,17 @@ public void addPerson(ReadOnlyPerson person) throws DuplicatePersonException { throw new DuplicatePersonException(); } + /* ------------ These methods below are called prior to calling addPerson(ReadOnlyPerson) ------------------- */ + @Override public ReadOnlyAddressBook getAddressBook() { return new AddressBook(); } + + @Override + public Predicate getFilteredPersonListPredicate() { + return PREDICATE_SHOW_ALL_PERSONS; + } } /** @@ -161,10 +174,17 @@ public void addPerson(ReadOnlyPerson person) throws DuplicatePersonException { personsAdded.add(new Person(person)); } + /* ------------ These methods below are called prior to calling addPerson(ReadOnlyPerson) ------------------- */ + @Override public ReadOnlyAddressBook getAddressBook() { return new AddressBook(); } + + @Override + public Predicate getFilteredPersonListPredicate() { + return PREDICATE_SHOW_ALL_PERSONS; + } } } diff --git a/src/test/java/seedu/address/logic/commands/RedoCommandTest.java b/src/test/java/seedu/address/logic/commands/RedoCommandTest.java index 840fea5031d7..580cae44bfae 100644 --- a/src/test/java/seedu/address/logic/commands/RedoCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/RedoCommandTest.java @@ -34,13 +34,25 @@ public void setUp() { } @Test - public void execute() { - UndoRedoStack undoRedoStack = prepareStack( + public void execute() throws Exception { + UndoRedoStack undoRedoStackOne = prepareStack( + Arrays.asList(deleteCommandOne, deleteCommandTwo), Collections.emptyList()); + UndoRedoStack undoRedoStackTwo = prepareStack( Collections.emptyList(), Arrays.asList(deleteCommandTwo, deleteCommandOne)); + UndoCommand undoCommand = new UndoCommand(); RedoCommand redoCommand = new RedoCommand(); - redoCommand.setData(model, EMPTY_COMMAND_HISTORY, undoRedoStack); + undoCommand.setData(model, EMPTY_COMMAND_HISTORY, undoRedoStackOne); + redoCommand.setData(model, EMPTY_COMMAND_HISTORY, undoRedoStackTwo); Model expectedModel = new ModelManager(getTypicalAddressBook(), new UserPrefs()); + // run the commands first to save their states + deleteCommandOne.execute(); + deleteCommandTwo.execute(); + + // revert back to original state + undoCommand.execute(); + undoCommand.execute(); + // multiple commands in redoStack deleteFirstPerson(expectedModel); assertCommandSuccess(redoCommand, model, RedoCommand.MESSAGE_SUCCESS, expectedModel); diff --git a/src/test/java/seedu/address/logic/commands/UndoableCommandTest.java b/src/test/java/seedu/address/logic/commands/UndoableCommandTest.java index 41d97ab15c60..9446c8438d91 100644 --- a/src/test/java/seedu/address/logic/commands/UndoableCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/UndoableCommandTest.java @@ -36,7 +36,13 @@ public void executeUndo() throws Exception { } @Test - public void redo() { + public void redo() throws Exception { + // run the command once to save the states + dummyCommand.execute(); + + // revert back to original state + dummyCommand.undo(); + showFirstPersonOnly(model); // redo() should cause the model's filtered list to show all persons diff --git a/src/test/java/seedu/address/testutil/TestUtil.java b/src/test/java/seedu/address/testutil/TestUtil.java index 29e457791c8b..973d4409a49c 100644 --- a/src/test/java/seedu/address/testutil/TestUtil.java +++ b/src/test/java/seedu/address/testutil/TestUtil.java @@ -35,20 +35,20 @@ public static String getFilePathInSandboxFolder(String fileName) { * Returns the middle index of the person in the {@code model}'s person list. */ public static Index getMidIndex(Model model) { - return Index.fromOneBased(model.getAddressBook().getPersonList().size() / 2); + return Index.fromOneBased(model.getFilteredPersonList().size() / 2); } /** * Returns the last index of the person in the {@code model}'s person list. */ public static Index getLastIndex(Model model) { - return Index.fromOneBased(model.getAddressBook().getPersonList().size()); + return Index.fromOneBased(model.getFilteredPersonList().size()); } /** * Returns the person in the {@code model}'s person list at {@code index}. */ public static ReadOnlyPerson getPerson(Model model, Index index) { - return model.getAddressBook().getPersonList().get(index.getZeroBased()); + return model.getFilteredPersonList().get(index.getZeroBased()); } } diff --git a/src/test/java/seedu/address/testutil/TypicalPersons.java b/src/test/java/seedu/address/testutil/TypicalPersons.java index 11144bfd356c..538932547aad 100644 --- a/src/test/java/seedu/address/testutil/TypicalPersons.java +++ b/src/test/java/seedu/address/testutil/TypicalPersons.java @@ -57,6 +57,7 @@ public class TypicalPersons { .build(); public static final String KEYWORD_MATCHING_MEIER = "Meier"; // A keyword that matches MEIER + public static final String KEYWORD_MATCHING_FIONA = "Fiona"; // A keyword that matches FIONA private TypicalPersons() {} // prevents instantiation diff --git a/src/test/java/systemtests/DeleteCommandSystemTest.java b/src/test/java/systemtests/DeleteCommandSystemTest.java index 4a12f622d5a3..dd03f2435f9f 100644 --- a/src/test/java/systemtests/DeleteCommandSystemTest.java +++ b/src/test/java/systemtests/DeleteCommandSystemTest.java @@ -4,10 +4,12 @@ import static seedu.address.commons.core.Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX; import static seedu.address.commons.core.Messages.MESSAGE_UNKNOWN_COMMAND; import static seedu.address.logic.commands.DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS; +import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS; import static seedu.address.testutil.TestUtil.getLastIndex; import static seedu.address.testutil.TestUtil.getMidIndex; import static seedu.address.testutil.TestUtil.getPerson; import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON; +import static seedu.address.testutil.TypicalPersons.KEYWORD_MATCHING_FIONA; import static seedu.address.testutil.TypicalPersons.KEYWORD_MATCHING_MEIER; import org.junit.Test; @@ -60,11 +62,25 @@ public void delete() { /* ------------------ Performing delete operation while a filtered list is being shown ---------------------- */ /* Case: filtered person list, delete index within bounds of address book and person list -> deleted */ - showPersonsWithName(KEYWORD_MATCHING_MEIER); + Model modelBeforeDeletingFirstFiltered = getModel(); + showPersonsWithName(KEYWORD_MATCHING_FIONA); + Model modelAfterDeletingFirstFiltered = getModel(); Index index = INDEX_FIRST_PERSON; assertTrue(index.getZeroBased() < getModel().getFilteredPersonList().size()); assertCommandSuccess(index); + /* Case: undo deleting the first person in the filtered list -> first person in filtered list restored */ + command = UndoCommand.COMMAND_WORD; + expectedResultMessage = UndoCommand.MESSAGE_SUCCESS; + assertCommandSuccess(command, modelBeforeDeletingFirstFiltered, expectedResultMessage); + + /* Case: redo deleting the first person in the filtered list -> first person in filtered list deleted again */ + command = RedoCommand.COMMAND_WORD; + removePerson(modelAfterDeletingFirstFiltered, index); + modelAfterDeletingFirstFiltered.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS); + expectedResultMessage = RedoCommand.MESSAGE_SUCCESS; + assertCommandSuccess(command, modelAfterDeletingFirstFiltered, expectedResultMessage); + /* Case: filtered person list, delete index within bounds of address book but out of bounds of person list * -> rejected */ diff --git a/src/test/java/systemtests/EditCommandSystemTest.java b/src/test/java/systemtests/EditCommandSystemTest.java index 71796d3fa19a..6700b89106f5 100644 --- a/src/test/java/systemtests/EditCommandSystemTest.java +++ b/src/test/java/systemtests/EditCommandSystemTest.java @@ -102,6 +102,7 @@ public void edit() throws Exception { /* Case: filtered person list, edit index within bounds of address book and person list -> edited */ showPersonsWithName(KEYWORD_MATCHING_MEIER); + model = getModel(); index = INDEX_FIRST_PERSON; assertTrue(index.getZeroBased() < getModel().getFilteredPersonList().size()); command = EditCommand.COMMAND_WORD + " " + index.getOneBased() + " " + NAME_DESC_BOB; @@ -109,6 +110,17 @@ public void edit() throws Exception { editedPerson = new PersonBuilder(personToEdit).withName(VALID_NAME_BOB).build(); assertCommandSuccess(command, index, editedPerson); + /* Case: undo editing the first person in the filtered list -> first person in filtered list restored */ + command = UndoCommand.COMMAND_WORD; + expectedResultMessage = UndoCommand.MESSAGE_SUCCESS; + assertCommandSuccess(command, model, expectedResultMessage); + + /* Case: redo editing the first person in the filtered list -> first person in filtered list edited again */ + command = RedoCommand.COMMAND_WORD; + expectedResultMessage = RedoCommand.MESSAGE_SUCCESS; + model.updatePerson(personToEdit, editedPerson); + assertCommandSuccess(command, model, expectedResultMessage); + /* Case: filtered person list, edit index within bounds of address book but out of bounds of person list * -> rejected */