Fix GUI task editing not being undoable

src/main/java/seedu/address/logic/LogicManager.java

@@ -10,6 +10,7 @@
 import seedu.address.commons.core.LogsCenter;
 import seedu.address.logic.commands.Command;
 import seedu.address.logic.commands.CommandResult;
+import seedu.address.logic.commands.UndoableCommand;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.logic.parser.AddressBookParser;
 import seedu.address.logic.parser.exceptions.ParseException;
@@ -54,7 +55,10 @@ public CommandResult execute(String commandText) throws CommandException, ParseE
     @Override
     public CommandResult executeCommand(Command command) throws CommandException {
         CommandResult commandResult = command.execute(model);
-        model.getCommandHistory().pushCommand(command);
+
+        if (command instanceof UndoableCommand) {
+            model.getCommandHistory().pushCommand((UndoableCommand) command);
+        }
 
         try {
             storage.saveAddressBook(model.getAddressBook());

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

@@ -2,8 +2,6 @@
 
 import static java.util.Objects.requireNonNull;
 
-import seedu.address.commons.core.Messages;
-import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.AddressBook;
 import seedu.address.model.Model;
 import seedu.address.model.ReadOnlyAddressBook;

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

@@ -1,37 +1,18 @@
 package seedu.address.logic.commands;
 
-import seedu.address.commons.core.Messages;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 
 /**
  * Represents a command with hidden internal logic and the ability to be executed.
  */
 public interface Command {
-
-//    protected boolean canExecute;
-
-    /**
-     * Default constructor for all classes that implement Command.
-     * @param canExecute Boolean for if the class is able to run the execute method.
-     */
-//    public Command(boolean canExecute) {
-//        this.canExecute = canExecute;
-//    }
-
-    /**
-     * Constructor that initialises canExecute to be true when super is not called in child class.
-     */
-//    public Command() {
-//        this.canExecute = true;
-//    }
-
     /**
      * Executes the command and returns the result message.
      *
      * @param model {@code Model} which the command should operate on.
      * @return feedback message of the operation result for display
      * @throws CommandException If an error occurs during command execution.
      */
-    public abstract CommandResult execute(Model model) throws CommandException;
+    CommandResult execute(Model model) throws CommandException;
 }

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

@@ -5,7 +5,6 @@
 import java.util.function.Predicate;
 
 import seedu.address.commons.core.Messages;
-import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 import seedu.address.model.person.NameContainsKeywordsPredicate;
 import seedu.address.model.person.Person;

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

@@ -5,7 +5,6 @@
 
 import java.util.function.Predicate;
 
-import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 import seedu.address.model.person.Person;
 

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

@@ -17,7 +17,7 @@ public class RedoCommand implements Command {
     @Override
     public CommandResult execute(Model model) throws CommandException {
         CommandHistory commandHistory = model.getCommandHistory();
-        Optional<Command> commandToRedo = commandHistory.getHistoryCommand(true);
+        Optional<UndoableCommand> commandToRedo = commandHistory.redo();
         if (commandToRedo.isPresent()) {
             CommandResult result = commandToRedo.get().execute(model);
             return new CommandResult(MESSAGE_UNDO_SUCCESS + result.getFeedbackToUser());

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

@@ -7,7 +7,6 @@
 import seedu.address.storage.CommandHistory;
 
 public class UndoCommand implements Command {
-
     public static final String COMMAND_WORD = "undo";
     public static final String MESSAGE_USAGE = COMMAND_WORD + ": Undoes the last executed command. \n"
             + "Example: " + COMMAND_WORD;
@@ -17,15 +16,10 @@ public class UndoCommand implements Command {
     @Override
     public CommandResult execute(Model model) throws CommandException {
         CommandHistory commandHistory = model.getCommandHistory();
-        Optional<Command> commandToUndo = commandHistory.getHistoryCommand(false);
+        Optional<UndoableCommand> commandToUndo = commandHistory.undo();
         if (commandToUndo.isPresent()) {
-            Command command = commandToUndo.get();
-            if (command instanceof UndoableCommand) {
-                CommandResult result = ((UndoableCommand) command).undo(model);
-                return new CommandResult(MESSAGE_UNDO_SUCCESS + result.getFeedbackToUser());
-            } else {
-                return new CommandResult("Undo not supported for this command, or cannot undo any further");
-            }
+            CommandResult result = commandToUndo.get().undo(model);
+            return new CommandResult(MESSAGE_UNDO_SUCCESS + result.getFeedbackToUser());
         } else {
             return new CommandResult(MESSAGE_NOT_UNDONE);
         }

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

@@ -12,9 +12,11 @@ public UndoableCommand() {
     }
 
     /**
-     * Checks if the current object state is able to call execute(). If it is able to,
-     * allow execution and toggle canExecute, else throw CommandException.
-     * @throws CommandException to prevent execution.
+     * Tries to execute the undoable command with the given model, using
+     * {@link UndoableCommand#executeDo}.
+     *
+     * If the command has been executed, a CommandException will be thrown.
+     * Otherwise, the command will be marked as executed, and can be undone.
      */
     @Override
     public CommandResult execute(Model model) throws CommandException {
@@ -27,6 +29,13 @@ public CommandResult execute(Model model) throws CommandException {
         return result;
     }
 
+    /**
+     * Tries to undo the undoable command with the given model, using
+     * {@link UndoableCommand#executeUndo}.
+     *
+     * If the command has been undone, a CommandException will be thrown.
+     * Otherwise, the command will be marked as undone, and can be executed again.
+     */
     public CommandResult undo(Model model) throws CommandException {
         if (this.canExecute) {
             throw new CommandException(Messages.MESSAGE_UNABLE_TO_UNDO);

src/main/java/seedu/address/logic/commands/task/AddTaskCommand.java

@@ -7,7 +7,6 @@
 
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.TaskCommand;
-import seedu.address.logic.commands.UndoableCommand;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 import seedu.address.model.task.Task;

src/main/java/seedu/address/logic/commands/task/SetTaskCommand.java

@@ -1,31 +1,39 @@
 package seedu.address.logic.commands.task;
 
+import static seedu.address.logic.commands.task.EditTaskCommand.MESSAGE_EDIT_TASK_SUCCESS;
+
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.UndoableCommand;
 import seedu.address.model.Model;
 import seedu.address.model.task.Task;
 
-import static seedu.address.logic.commands.task.EditTaskCommand.MESSAGE_EDIT_TASK_SUCCESS;
-
+/*
+ * Sets a given task in the task list with another given task.
+ */
 public class SetTaskCommand extends UndoableCommand {
-    private final Task oldTask;
-    private final Task newTask;
+    private Task oldTask;
+    private Task newTask;
 
+    /**
+     * Creates a SetTaskCommand with the given oldTask and newTask.
+     * @param oldTask The task to replace
+     * @param newTask The new task to replace the old task with
+     */
     public SetTaskCommand(Task oldTask, Task newTask) {
         this.oldTask = oldTask;
         this.newTask = newTask;
     }
 
     @Override
     protected CommandResult executeDo(Model model) {
-        model.setTask(oldTask, newTask);
+        newTask = model.setTask(oldTask, newTask);
         return new CommandResult(String.format(MESSAGE_EDIT_TASK_SUCCESS,
                 newTask));
     }
 
     @Override
     protected CommandResult executeUndo(Model model) {
-        model.setTask(newTask, oldTask);
+        oldTask = model.setTask(newTask, oldTask);
         return new CommandResult(String.format(MESSAGE_EDIT_TASK_SUCCESS,
                 newTask));
     }

src/main/java/seedu/address/model/Model.java

@@ -191,8 +191,9 @@ public interface Model {
      * Replaces the given task {@code target} with {@code editedTask}.
      * {@code target} must exist in the task list.
      * setTask uses targetIndex rather than target Person; This is because tasks may not be unique, unlike persons
+     * @return
      */
-    void setTask(Task target, Task editedTask);
+    Task setTask(Task target, Task editedTask);
 
     /**
      * Returns the command history instance.

src/main/java/seedu/address/model/ModelManager.java

@@ -319,11 +319,12 @@ public void deleteAllInFilteredTaskList() {
     }
 
     @Override
-    public void setTask(Task target, Task editedTask) {
+    public Task setTask(Task target, Task editedTask) {
         requireAllNonNull(target, editedTask);
         Task updatedEditedTask = updateTaskContacts(editedTask);
         taskList.setTask(target, updatedEditedTask);
         updateTaskFilters();
+        return updatedEditedTask;
     }
 
     /**

src/main/java/seedu/address/storage/CommandHistory.java

@@ -2,9 +2,7 @@
 
 import java.util.Optional;
 
-import seedu.address.logic.commands.Command;
-import seedu.address.logic.commands.RedoCommand;
-import seedu.address.logic.commands.UndoCommand;
+import seedu.address.logic.commands.UndoableCommand;
 
 /**
  * UserUndoStorage contains a stack of undo instructions to be executed when the user wants to
@@ -19,7 +17,7 @@ public class CommandHistory {
     private CommandHistoryNode current;
 
     private static class CommandHistoryNode {
-        private final Command command;
+        private final UndoableCommand command;
         private CommandHistoryNode previous;
         private CommandHistoryNode next;
 
@@ -28,7 +26,7 @@ private static class CommandHistoryNode {
          *
          * @param command The command to be encapsulated.
          */
-        public CommandHistoryNode(Command command) {
+        public CommandHistoryNode(UndoableCommand command) {
             this.command = command;
             this.previous = null;
             this.next = null;
@@ -60,7 +58,7 @@ public boolean replace(CommandHistoryNode nodeToReplaceWith) {
             return false;
         }
 
-        public Command getCommand() {
+        public UndoableCommand getCommand() {
             return this.command;
         }
 
@@ -90,10 +88,7 @@ public CommandHistory(int maxStackSize) {
      *
      * @param command Command to be inserted to the history stack
      */
-    public void pushCommand(Command command) {
-        if (command instanceof UndoCommand || command instanceof RedoCommand) {
-            return;
-        }
+    public void pushCommand(UndoableCommand command) {
         CommandHistoryNode newCommand = new CommandHistoryNode(command);
         if (this.isStackFull()) {
             this.start = this.start.next;
@@ -124,14 +119,23 @@ private boolean isStackFull() {
         return this.stackSize >= this.maxStackSize;
     }
 
+
+    public Optional<UndoableCommand> undo() {
+        return getHistoryCommand(false);
+    }
+
+    public Optional<UndoableCommand> redo() {
+        return getHistoryCommand(true);
+    }
+
     /**
      * Retrieves the requested command from the stack. Returns the next item in the stack if
      * isNext is true, and returns the current item in the stack is isNext is false.
      *
      * @return Optional of the Command. If the stack is empty, returns an empty Optional.
      */
 
-    public Optional<Command> getHistoryCommand(boolean isNext) {
+    private Optional<UndoableCommand> getHistoryCommand(boolean isNext) {
         CommandHistoryNode toReturn = this.current;
         if (this.current == null) {
             if (isNext) {

src/main/java/seedu/address/ui/TaskListViewCell.java

@@ -1,19 +1,13 @@
 package seedu.address.ui;
 
 import java.util.Optional;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 import javafx.scene.control.ListCell;
 import javafx.scene.input.KeyCode;
 import javafx.scene.input.KeyEvent;
-import seedu.address.commons.core.LogsCenter;
-import seedu.address.logic.commands.Command;
-import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.task.Task;
 
 public class TaskListViewCell extends ListCell<Task> {
-    private final Logger logger = LogsCenter.getLogger(TaskListViewCell.class);
     private final TaskListPanel.TaskEditor taskEditor;
 
     public TaskListViewCell(TaskListPanel.TaskEditor taskEditor) {

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

@@ -17,7 +17,6 @@
 import javafx.collections.ObservableList;
 import seedu.address.commons.core.GuiSettings;
 import seedu.address.logic.commands.exceptions.CommandException;
-import seedu.address.logic.guiactions.GuiAction;
 import seedu.address.model.AddressBook;
 import seedu.address.model.Model;
 import seedu.address.model.ReadOnlyAddressBook;
@@ -205,12 +204,7 @@ public ReadOnlyTaskList getTaskList() {
         }
 
         @Override
-        public void setTask(Task target, Task task) {
-            throw new AssertionError("This method should not be called.");
-        }
-
-        @Override
-        public void executeGuiAction(GuiAction action) {
+        public Task setTask(Task target, Task task) {
             throw new AssertionError("This method should not be called.");
         }