Merge 6422779 into e7f43d3

se-edu · Jan 23, 2018 · 425046c · 425046c
2 parents e7f43d3 + 6422779
commit 425046c
Show file tree

Hide file tree

Showing 10 changed files with 162 additions and 46 deletions.
diff --git a/src/main/java/seedu/address/logic/commands/DeleteCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCommand.java
@@ -24,22 +24,15 @@ public class DeleteCommand extends UndoableCommand {
 
     private final Index targetIndex;
 
+    private Person personToDelete;
+
     public DeleteCommand(Index targetIndex) {
         this.targetIndex = targetIndex;
     }
 
 
     @Override
     public CommandResult executeUndoableCommand() throws CommandException {
-
-        List<Person> lastShownList = model.getFilteredPersonList();
-
-        if (targetIndex.getZeroBased() >= lastShownList.size()) {
-            throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
-        }
-
-        Person personToDelete = lastShownList.get(targetIndex.getZeroBased());
-
         try {
             model.deletePerson(personToDelete);
         } catch (PersonNotFoundException pnfe) {
@@ -49,6 +42,17 @@ public CommandResult executeUndoableCommand() throws CommandException {
         return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, personToDelete));
     }
 
+    @Override
+    protected void preprocessUndoableCommand() throws CommandException {
+        List<Person> lastShownList = model.getFilteredPersonList();
+
+        if (targetIndex.getZeroBased() >= lastShownList.size()) {
+            throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+        }
+
+        personToDelete = lastShownList.get(targetIndex.getZeroBased());
+    }
+
     @Override
     public boolean equals(Object other) {
         return other == this // short circuit if same object

diff --git a/src/main/java/seedu/address/logic/commands/EditCommand.java b/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
@@ -68,15 +71,6 @@ public EditCommand(Index index, EditPersonDescriptor editPersonDescriptor) {
 
     @Override
     public CommandResult executeUndoableCommand() throws CommandException {
-        List<Person> lastShownList = model.getFilteredPersonList();
-
-        if (index.getZeroBased() >= lastShownList.size()) {
-            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 +82,18 @@ public CommandResult executeUndoableCommand() throws CommandException {
         return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, editedPerson));
     }
 
+    @Override
+    protected void preprocessUndoableCommand() throws CommandException {
+        List<Person> lastShownList = model.getFilteredPersonList();
+
+        if (index.getZeroBased() >= lastShownList.size()) {
+            throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+        }
+
+        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}.

diff --git a/src/main/java/seedu/address/logic/commands/UndoableCommand.java b/src/main/java/seedu/address/logic/commands/UndoableCommand.java
@@ -24,6 +24,12 @@ private void saveAddressBookSnapshot() {
         this.previousAddressBook = new AddressBook(model.getAddressBook());
     }
 
+    /**
+     * Preprocess the UndoableCommand if necessary.
+     * UndoableCommands that require this preprocessing step should override this method.
+     */
+    protected void preprocessUndoableCommand() throws CommandException {}
+
     /**
      * Reverts the AddressBook to the state before this command
      * was executed and updates the filtered person list to
@@ -53,6 +59,7 @@ protected final void redo() {
     @Override
     public final CommandResult execute() throws CommandException {
         saveAddressBookSnapshot();
+        preprocessUndoableCommand();
         return executeUndoableCommand();
     }
 }
diff --git a/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java b/src/test/java/seedu/address/logic/commands/DeleteCommandTest.java
@@ -5,10 +5,12 @@
 import static seedu.address.logic.commands.CommandTestUtil.assertCommandFailure;
 import static seedu.address.logic.commands.CommandTestUtil.assertCommandSuccess;
 import static seedu.address.logic.commands.CommandTestUtil.showFirstPersonOnly;
+import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON;
 import static seedu.address.testutil.TypicalIndexes.INDEX_SECOND_PERSON;
 import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook;
 
+import org.junit.Before;
 import org.junit.Test;
 
 import seedu.address.commons.core.Messages;
@@ -21,11 +23,21 @@
 import seedu.address.model.person.Person;
 
 /**
- * Contains integration tests (interaction with the Model) and unit tests for {@code DeleteCommand}.
+ * Contains integration tests (interaction with the Model, UndoCommand and RedoCommand) and unit tests for
+ * {@code DeleteCommand}.
  */
 public class DeleteCommandTest {
 
     private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
+    private UndoCommand undoCommand = new UndoCommand();
+    private RedoCommand redoCommand = new RedoCommand();
+    private UndoRedoStack undoRedoStack = new UndoRedoStack();
+
+    @Before
+    public void setUp() {
+        undoCommand.setData(model, new CommandHistory(), undoRedoStack);
+        redoCommand.setData(model, new CommandHistory(), undoRedoStack);
+    }
 
     @Test
     public void execute_validIndexUnfilteredList_success() throws Exception {
@@ -34,10 +46,22 @@ public void execute_validIndexUnfilteredList_success() throws Exception {
 
         String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS, personToDelete);
 
-        ModelManager expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs());
-        expectedModel.deletePerson(personToDelete);
+        ModelManager expectedModelBeforeDeleting = new ModelManager(model.getAddressBook(), new UserPrefs());
+        ModelManager expectedModelAfterDeleting = new ModelManager(model.getAddressBook(), new UserPrefs());
+        expectedModelAfterDeleting.deletePerson(personToDelete);
+
+        assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModelAfterDeleting);
+
+        // deleteCommand succeeded, pushed into undoRedoStaack
+        undoRedoStack.push(deleteCommand);
 
-        assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModel);
+        // ensure list shows all persons after undo
+        expectedModelBeforeDeleting.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        assertCommandSuccess(undoCommand, model, UndoCommand.MESSAGE_SUCCESS, expectedModelBeforeDeleting);
+
+        // ensure list shows all persons after redo
+        expectedModelAfterDeleting.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        assertCommandSuccess(redoCommand, model, RedoCommand.MESSAGE_SUCCESS, expectedModelAfterDeleting);
     }
 
     @Test
@@ -46,6 +70,10 @@ public void execute_invalidIndexUnfilteredList_throwsCommandException() throws E
         DeleteCommand deleteCommand = prepareCommand(outOfBoundIndex);
 
         assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+
+        // deleteCommand failed, not pushed into undoRedoStack
+        assertCommandFailure(undoCommand, model, UndoCommand.MESSAGE_FAILURE);
+        assertCommandFailure(redoCommand, model, RedoCommand.MESSAGE_FAILURE);
     }
 
     @Test
@@ -57,11 +85,23 @@ public void execute_validIndexFilteredList_success() throws Exception {
 
         String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS, personToDelete);
 
-        Model expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs());
-        expectedModel.deletePerson(personToDelete);
-        showNoPerson(expectedModel);
+        Model expectedModelBeforeDeletingFirst = new ModelManager(model.getAddressBook(), new UserPrefs());
+        Model expectedModelAfterDeletingFirst = new ModelManager(model.getAddressBook(), new UserPrefs());
+        expectedModelAfterDeletingFirst.deletePerson(personToDelete);
+        showNoPerson(expectedModelAfterDeletingFirst);
 
-        assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModel);
+        assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModelAfterDeletingFirst);
+
+        // deleteCommand succeeded, pushed into undoRedoStaack
+        undoRedoStack.push(deleteCommand);
+
+        // ensure list shows all persons after undo
+        expectedModelBeforeDeletingFirst.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        assertCommandSuccess(undoCommand, model, UndoCommand.MESSAGE_SUCCESS, expectedModelBeforeDeletingFirst);
+
+        // ensure list shows all persons after redo
+        expectedModelAfterDeletingFirst.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        assertCommandSuccess(redoCommand, model, RedoCommand.MESSAGE_SUCCESS, expectedModelAfterDeletingFirst);
     }
 
     @Test
@@ -75,6 +115,10 @@ public void execute_invalidIndexFilteredList_throwsCommandException() {
         DeleteCommand deleteCommand = prepareCommand(outOfBoundIndex);
 
         assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+
+        // deleteCommand failed, not pushed into undoRedoStack
+        assertCommandFailure(undoCommand, model, UndoCommand.MESSAGE_FAILURE);
+        assertCommandFailure(redoCommand, model, RedoCommand.MESSAGE_FAILURE);
     }
 
     @Test

diff --git a/src/test/java/seedu/address/logic/commands/EditCommandTest.java b/src/test/java/seedu/address/logic/commands/EditCommandTest.java
@@ -14,6 +14,7 @@
 import static seedu.address.testutil.TypicalIndexes.INDEX_SECOND_PERSON;
 import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook;
 
+import org.junit.Before;
 import org.junit.Test;
 
 import seedu.address.commons.core.Messages;
@@ -35,6 +36,15 @@
 public class EditCommandTest {
 
     private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
+    private UndoCommand undoCommand = new UndoCommand();
+    private RedoCommand redoCommand = new RedoCommand();
+    private UndoRedoStack undoRedoStack = new UndoRedoStack();
+
+    @Before
+    public void setUp() {
+        undoCommand.setData(model, new CommandHistory(), undoRedoStack);
+        redoCommand.setData(model, new CommandHistory(), undoRedoStack);
+    }
 
     @Test
     public void execute_allFieldsSpecifiedUnfilteredList_success() throws Exception {
@@ -44,10 +54,18 @@ public void execute_allFieldsSpecifiedUnfilteredList_success() throws Exception
 
         String expectedMessage = String.format(EditCommand.MESSAGE_EDIT_PERSON_SUCCESS, editedPerson);
 
-        Model expectedModel = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs());
-        expectedModel.updatePerson(model.getFilteredPersonList().get(0), editedPerson);
+        Model expectedModelBeforeEditing = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs());
+        Model expectedModelAfterEditing = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs());
+        expectedModelAfterEditing.updatePerson(model.getFilteredPersonList().get(0), editedPerson);
 
-        assertCommandSuccess(editCommand, model, expectedMessage, expectedModel);
+        assertCommandSuccess(editCommand, model, expectedMessage, expectedModelAfterEditing);
+
+        // editCommand succeeded, pushed into undoRedoStack
+        undoRedoStack.push(editCommand);
+
+        assertCommandSuccess(undoCommand, model, UndoCommand.MESSAGE_SUCCESS, expectedModelBeforeEditing);
+
+        assertCommandSuccess(redoCommand, model, RedoCommand.MESSAGE_SUCCESS, expectedModelAfterEditing);
     }
 
     @Test

diff --git a/src/test/java/seedu/address/logic/commands/RedoCommandTest.java b/src/test/java/seedu/address/logic/commands/RedoCommandTest.java
@@ -3,8 +3,6 @@
 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;
@@ -18,35 +16,37 @@
 import seedu.address.model.Model;
 import seedu.address.model.ModelManager;
 import seedu.address.model.UserPrefs;
+import seedu.address.testutil.TypicalPersons;
+
 
 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 AddCommand addCommandAmy = new AddCommand(TypicalPersons.AMY);
+    private final AddCommand addCommandBob = new AddCommand(TypicalPersons.BOB);
 
     @Before
     public void setUp() {
-        deleteCommandOne.setData(model, EMPTY_COMMAND_HISTORY, EMPTY_STACK);
-        deleteCommandTwo.setData(model, EMPTY_COMMAND_HISTORY, EMPTY_STACK);
+        addCommandAmy.setData(model, EMPTY_COMMAND_HISTORY, EMPTY_STACK);
+        addCommandBob.setData(model, EMPTY_COMMAND_HISTORY, EMPTY_STACK);
     }
 
     @Test
-    public void execute() {
+    public void execute() throws Exception {
         UndoRedoStack undoRedoStack = prepareStack(
-                Collections.emptyList(), Arrays.asList(deleteCommandTwo, deleteCommandOne));
+                Collections.emptyList(), Arrays.asList(addCommandBob, addCommandAmy));
         RedoCommand redoCommand = new RedoCommand();
         redoCommand.setData(model, EMPTY_COMMAND_HISTORY, undoRedoStack);
         Model expectedModel = new ModelManager(getTypicalAddressBook(), new UserPrefs());
 
         // multiple commands in redoStack
-        deleteFirstPerson(expectedModel);
+        expectedModel.addPerson(TypicalPersons.AMY);
         assertCommandSuccess(redoCommand, model, RedoCommand.MESSAGE_SUCCESS, expectedModel);
 
         // single command in redoStack
-        deleteFirstPerson(expectedModel);
+        expectedModel.addPerson(TypicalPersons.BOB);
         assertCommandSuccess(redoCommand, model, RedoCommand.MESSAGE_SUCCESS, expectedModel);
 
         // no command in redoStack

diff --git a/src/test/java/seedu/address/testutil/TestUtil.java b/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());
     }
 }
diff --git 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