Merge 7b5b306 into e7f43d3

src/main/java/seedu/address/logic/commands/DeleteCommand.java

@@ -24,22 +24,20 @@ public class DeleteCommand extends UndoableCommand {
 
     private final Index targetIndex;
 
+    private Person personToDelete;
+
     public DeleteCommand(Index targetIndex) {
         this.targetIndex = targetIndex;
+        personToDelete = null;
     }
 
 
     @Override
     public CommandResult executeUndoableCommand() throws CommandException {
-
-        List<Person> lastShownList = model.getFilteredPersonList();
-
-        if (targetIndex.getZeroBased() >= lastShownList.size()) {
+        if (personToDelete == null) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
         }
 
-        Person personToDelete = lastShownList.get(targetIndex.getZeroBased());
-
         try {
             model.deletePerson(personToDelete);
         } catch (PersonNotFoundException pnfe) {
@@ -49,6 +47,15 @@ public CommandResult executeUndoableCommand() throws CommandException {
         return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, personToDelete));
     }
 
+    @Override
+    protected void preprocessUndoableCommand() {
+        List<Person> lastShownList = model.getFilteredPersonList();
+
+        if (targetIndex.getZeroBased() < lastShownList.size()) {
+            personToDelete = lastShownList.get(targetIndex.getZeroBased());
+        }
+    }
+
     @Override
     public boolean equals(Object other) {
         return other == this // short circuit if same object

src/main/java/seedu/address/logic/commands/EditCommand.java

@@ -54,6 +54,9 @@ public class EditCommand extends UndoableCommand {
     private final Index index;
     private final EditPersonDescriptor editPersonDescriptor;
 
+    private Person personToEdit;
+    private Person editedPerson;
+
     /**
      * @param index of the person in the filtered person list to edit
      * @param editPersonDescriptor details to edit the person with
@@ -64,19 +67,15 @@ public EditCommand(Index index, EditPersonDescriptor editPersonDescriptor) {
 
         this.index = index;
         this.editPersonDescriptor = new EditPersonDescriptor(editPersonDescriptor);
+        personToEdit = null;
     }
 
     @Override
     public CommandResult executeUndoableCommand() throws CommandException {
-        List<Person> lastShownList = model.getFilteredPersonList();
-
-        if (index.getZeroBased() >= lastShownList.size()) {
+        if (personToEdit == null) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
         }
 
-        Person personToEdit = lastShownList.get(index.getZeroBased());
-        Person editedPerson = createEditedPerson(personToEdit, editPersonDescriptor);
-
         try {
             model.updatePerson(personToEdit, editedPerson);
         } catch (DuplicatePersonException dpe) {
@@ -88,6 +87,16 @@ public CommandResult executeUndoableCommand() throws CommandException {
         return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, editedPerson));
     }
 
+    @Override
+    protected void preprocessUndoableCommand() {
+        List<Person> lastShownList = model.getFilteredPersonList();
+
+        if (index.getZeroBased() < lastShownList.size()) {
+            personToEdit = lastShownList.get(index.getZeroBased());
+            editedPerson = createEditedPerson(personToEdit, editPersonDescriptor);
+        }
+    }
+
     /**
      * Creates and returns a {@code Person} with the details of {@code personToEdit}
      * edited with {@code editPersonDescriptor}.

src/main/java/seedu/address/logic/commands/UndoableCommand.java

@@ -24,6 +24,14 @@ private void saveAddressBookSnapshot() {
         this.previousAddressBook = new AddressBook(model.getAddressBook());
     }
 
+    /**
+     * Preprocess the UndoableCommand if necessary.
+     * Commands that require this preprocessing should override this method.
+     */
+    protected void preprocessUndoableCommand() {
+        return;
+    }
+
     /**
      * Reverts the AddressBook to the state before this command
      * was executed and updates the filtered person list to
@@ -53,6 +61,7 @@ protected final void redo() {
     @Override
     public final CommandResult execute() throws CommandException {
         saveAddressBookSnapshot();
+        preprocessUndoableCommand();
         return executeUndoableCommand();
     }
 }

src/test/java/seedu/address/logic/commands/RedoCommandTest.java

@@ -1,10 +1,10 @@
 package seedu.address.logic.commands;
 
+import static org.junit.Assert.fail;
 import static seedu.address.logic.UndoRedoStackUtil.prepareStack;
 import static seedu.address.logic.commands.CommandTestUtil.assertCommandFailure;
 import static seedu.address.logic.commands.CommandTestUtil.assertCommandSuccess;
 import static seedu.address.logic.commands.CommandTestUtil.deleteFirstPerson;
-import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON;
 import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook;
 
 import java.util.Arrays;
@@ -15,28 +15,31 @@
 
 import seedu.address.logic.CommandHistory;
 import seedu.address.logic.UndoRedoStack;
+import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 import seedu.address.model.ModelManager;
 import seedu.address.model.UserPrefs;
+import seedu.address.model.person.Person;
+import seedu.address.model.person.exceptions.PersonNotFoundException;
 
 public class RedoCommandTest {
     private static final CommandHistory EMPTY_COMMAND_HISTORY = new CommandHistory();
     private static final UndoRedoStack EMPTY_STACK = new UndoRedoStack();
 
     private final Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
-    private final DeleteCommand deleteCommandOne = new DeleteCommand(INDEX_FIRST_PERSON);
-    private final DeleteCommand deleteCommandTwo = new DeleteCommand(INDEX_FIRST_PERSON);
+    private final DummyCommand dummyCommandOne = new DummyCommand(model);
+    private final DummyCommand dummyCommandTwo = new DummyCommand(model);
 
     @Before
     public void setUp() {
-        deleteCommandOne.setData(model, EMPTY_COMMAND_HISTORY, EMPTY_STACK);
-        deleteCommandTwo.setData(model, EMPTY_COMMAND_HISTORY, EMPTY_STACK);
+        dummyCommandOne.setData(model, EMPTY_COMMAND_HISTORY, EMPTY_STACK);
+        dummyCommandTwo.setData(model, EMPTY_COMMAND_HISTORY, EMPTY_STACK);
     }
 
     @Test
     public void execute() {
         UndoRedoStack undoRedoStack = prepareStack(
-                Collections.emptyList(), Arrays.asList(deleteCommandTwo, deleteCommandOne));
+                Collections.emptyList(), Arrays.asList(dummyCommandTwo, dummyCommandOne));
         RedoCommand redoCommand = new RedoCommand();
         redoCommand.setData(model, EMPTY_COMMAND_HISTORY, undoRedoStack);
         Model expectedModel = new ModelManager(getTypicalAddressBook(), new UserPrefs());
@@ -52,4 +55,24 @@ public void execute() {
         // no command in redoStack
         assertCommandFailure(redoCommand, model, RedoCommand.MESSAGE_FAILURE);
     }
+
+    /**
+     * Deletes the first person in the model's filtered list.
+     */
+    class DummyCommand extends UndoableCommand {
+        DummyCommand(Model model) {
+            this.model = model;
+        }
+
+        @Override
+        public CommandResult executeUndoableCommand() throws CommandException {
+            Person personToDelete = model.getFilteredPersonList().get(0);
+            try {
+                model.deletePerson(personToDelete);
+            } catch (PersonNotFoundException pnfe) {
+                fail("Impossible: personToDelete was retrieved from model.");
+            }
+            return new CommandResult("");
+        }
+    }
 }

src/test/java/seedu/address/testutil/TestUtil.java

@@ -32,23 +32,23 @@ public static String getFilePathInSandboxFolder(String fileName) {
     }
 
     /**
-     * Returns the middle index of the person in the {@code model}'s person list.
+     * Returns the middle index of the person in the {@code model}'s filtered 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.
+     * Returns the last index of the person in the {@code model}'s filtered 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}.
+     * Returns the person in the {@code model}'s filtered person list at {@code index}.
      */
     public static Person getPerson(Model model, Index index) {
-        return model.getAddressBook().getPersonList().get(index.getZeroBased());
+        return model.getFilteredPersonList().get(index.getZeroBased());
     }
 }

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

@@ -1,13 +1,16 @@
 package systemtests;
 
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 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 +63,28 @@ 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);
         Index index = INDEX_FIRST_PERSON;
+        Model modelBeforeDeletingFirstFiltered = getModel();
+        showPersonsWithName(KEYWORD_MATCHING_FIONA);
+        Person firstPersonAddressBook = getModel().getAddressBook().getPersonList().get(index.getZeroBased());
+        Person firstPersonFilteredList = getModel().getFilteredPersonList().get(index.getZeroBased());
+        assertNotEquals(firstPersonAddressBook, firstPersonFilteredList);
+        Model modelAfterDeletingFirstFiltered = getModel();
         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

@@ -1,6 +1,7 @@
 package systemtests;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY;
 import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_BOB;
@@ -100,14 +101,29 @@ public void edit() throws Exception {
         /* ------------------ Performing edit operation while a filtered list is being shown ------------------------ */
 
         /* Case: filtered person list, edit index within bounds of address book and person list -> edited */
-        showPersonsWithName(KEYWORD_MATCHING_MEIER);
         index = INDEX_FIRST_PERSON;
+        showPersonsWithName(KEYWORD_MATCHING_MEIER);
+        Person firstPersonAddressBook = getModel().getAddressBook().getPersonList().get(index.getZeroBased());
+        Person firstPersonFilteredList = getModel().getFilteredPersonList().get(index.getZeroBased());
+        assertNotEquals(firstPersonAddressBook, firstPersonFilteredList);
+        model  = getModel();
         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
          */