UndoableCommand: Update redo() to restore previous view before executing

The method undo() sets the Addressbook view to show all persons
after restoring the Addressbook state.

This could cause the redoing  of commands that are index-based
such as DeleteCommand and EditCommand to execute itself on the
incorrect person due to different indexing in different views.

Let's update
* UndoableCommand to store the predicate for the current view
* redo() method to restore to its previous view right before
executing its command to ensure index-based Commands will
execute on the correct Person
* index-based CommandTests to include testing of undo()/redo()
on filtered person list and any other affected tests

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<ReadOnlyPerson> 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();
     }
 }

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);

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

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
 

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
          */

src/test/java/systemtests/EditCommandSystemTest.java

@@ -102,13 +102,25 @@ 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;
         personToEdit = getModel().getFilteredPersonList().get(index.getZeroBased());
         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
          */