From df6bc8d502cbddea0dcd8304b52aae268281b2bd Mon Sep 17 00:00:00 2001 From: Yiqun Sun Date: Sat, 13 Apr 2019 14:00:46 +0800 Subject: [PATCH 1/2] Fix bug related to #169 select-display-select bug The bug is when select 1, display, and select 1, the browser panel will not change. I fix this in this commit. I add more tests to deal with it aslo. --- .../equipment/logic/commands/DisplayCommand.java | 1 + .../equipment/logic/commands/RouteCommand.java | 1 + src/main/java/seedu/equipment/model/Model.java | 5 +++++ .../java/seedu/equipment/model/ModelManager.java | 5 +++++ src/main/java/seedu/equipment/ui/BrowserPanel.java | 4 ++-- .../logic/commands/AddClientCommandTest.java | 5 +++++ .../equipment/logic/commands/AddCommandTest.java | 5 +++++ .../logic/commands/AddWorkListCommandTest.java | 5 +++++ .../java/systemtests/DisplayCommandSystemTest.java | 13 +++++++++++++ 9 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/main/java/seedu/equipment/logic/commands/DisplayCommand.java b/src/main/java/seedu/equipment/logic/commands/DisplayCommand.java index bb59887f8eb9..cc2b89029df3 100644 --- a/src/main/java/seedu/equipment/logic/commands/DisplayCommand.java +++ b/src/main/java/seedu/equipment/logic/commands/DisplayCommand.java @@ -21,6 +21,7 @@ public DisplayCommand() { @Override public CommandResult execute(Model model, CommandHistory history) { + model.unsetSelectedEquipment(); return new CommandResult( Messages.MESSAGE_EQUIPMENT_DISPLAYED_OVERVIEW, false, false, true); } diff --git a/src/main/java/seedu/equipment/logic/commands/RouteCommand.java b/src/main/java/seedu/equipment/logic/commands/RouteCommand.java index 6a57f30c49c0..ed139bbe3fee 100644 --- a/src/main/java/seedu/equipment/logic/commands/RouteCommand.java +++ b/src/main/java/seedu/equipment/logic/commands/RouteCommand.java @@ -50,6 +50,7 @@ public CommandResult execute(Model model, CommandHistory history) throws Command throw new CommandException(MESSAGE_TOO_MANY_EQUIPMENTS_TO_ROUTE); } + model.unsetSelectedEquipment(); return new CommandResult(MESSAGE_ROUTE_EQUIPMENT_SUCCESS, false, false, false, true, startendAddress); } diff --git a/src/main/java/seedu/equipment/model/Model.java b/src/main/java/seedu/equipment/model/Model.java index 720ca13cc36b..b15845d87bb3 100644 --- a/src/main/java/seedu/equipment/model/Model.java +++ b/src/main/java/seedu/equipment/model/Model.java @@ -225,6 +225,11 @@ public interface Model { */ void setSelectedEquipment(Equipment equipment); + /** + * Unset the selected equipment in the filtered equipment list. + */ + void unsetSelectedEquipment(); + /** * Sets the selected WorkList in the filtered WorkList list. */ diff --git a/src/main/java/seedu/equipment/model/ModelManager.java b/src/main/java/seedu/equipment/model/ModelManager.java index 0e1ae94b071a..3c993032f336 100644 --- a/src/main/java/seedu/equipment/model/ModelManager.java +++ b/src/main/java/seedu/equipment/model/ModelManager.java @@ -313,6 +313,11 @@ public void setSelectedEquipment(Equipment equipment) { selectedEquipment.setValue(equipment); } + @Override + public void unsetSelectedEquipment() { + selectedEquipment.setValue(null); + } + @Override public void deleteTag(Tag tag) { versionedEquipmentManager.removeTag(tag); diff --git a/src/main/java/seedu/equipment/ui/BrowserPanel.java b/src/main/java/seedu/equipment/ui/BrowserPanel.java index 909e765e8d91..92626689b3c5 100644 --- a/src/main/java/seedu/equipment/ui/BrowserPanel.java +++ b/src/main/java/seedu/equipment/ui/BrowserPanel.java @@ -32,14 +32,14 @@ public class BrowserPanel extends UiPart { @FXML private WebView browser; - public BrowserPanel(ObservableValue selectedPerson) { + public BrowserPanel(ObservableValue selectedEquipment) { super(FXML); // To prevent triggering events for typing inside the loaded Web page. getRoot().setOnKeyPressed(Event::consume); // Load equipment page when selected equipment changes. - selectedPerson.addListener((observable, oldValue, newValue) -> { + selectedEquipment.addListener((observable, oldValue, newValue) -> { if (newValue == null) { loadDefaultPage(); return; diff --git a/src/test/java/seedu/equipment/logic/commands/AddClientCommandTest.java b/src/test/java/seedu/equipment/logic/commands/AddClientCommandTest.java index d3f8c9004874..096b26fa9fff 100644 --- a/src/test/java/seedu/equipment/logic/commands/AddClientCommandTest.java +++ b/src/test/java/seedu/equipment/logic/commands/AddClientCommandTest.java @@ -293,6 +293,11 @@ public void setSelectedEquipment(Equipment equipment) { throw new AssertionError("This method should not be called."); } + @Override + public void unsetSelectedEquipment() { + throw new AssertionError("This method should not be called."); + } + @Override public void setSelectedWorkList(WorkList workList) { throw new AssertionError("This method should not be called."); diff --git a/src/test/java/seedu/equipment/logic/commands/AddCommandTest.java b/src/test/java/seedu/equipment/logic/commands/AddCommandTest.java index e228d6c8517a..8b7911840968 100644 --- a/src/test/java/seedu/equipment/logic/commands/AddCommandTest.java +++ b/src/test/java/seedu/equipment/logic/commands/AddCommandTest.java @@ -300,6 +300,11 @@ public void setSelectedEquipment(Equipment equipment) { throw new AssertionError("This method should not be called."); } + @Override + public void unsetSelectedEquipment() { + throw new AssertionError("This method should not be called."); + } + @Override public void setSelectedWorkList(WorkList workList) { throw new AssertionError("This method should not be called."); diff --git a/src/test/java/seedu/equipment/logic/commands/AddWorkListCommandTest.java b/src/test/java/seedu/equipment/logic/commands/AddWorkListCommandTest.java index 4d1048afcb5d..aff19879edab 100644 --- a/src/test/java/seedu/equipment/logic/commands/AddWorkListCommandTest.java +++ b/src/test/java/seedu/equipment/logic/commands/AddWorkListCommandTest.java @@ -318,6 +318,11 @@ public void setSelectedEquipment(Equipment equipment) { throw new AssertionError("This method should not be called."); } + @Override + public void unsetSelectedEquipment() { + throw new AssertionError("This method should not be called."); + } + @Override public void setSelectedWorkList(WorkList workList) { throw new AssertionError("This method should not be called."); diff --git a/src/test/java/systemtests/DisplayCommandSystemTest.java b/src/test/java/systemtests/DisplayCommandSystemTest.java index c7ce05a24b8f..ae3f5cc26d07 100644 --- a/src/test/java/systemtests/DisplayCommandSystemTest.java +++ b/src/test/java/systemtests/DisplayCommandSystemTest.java @@ -8,6 +8,7 @@ import seedu.equipment.commons.core.Messages; import seedu.equipment.logic.commands.DisplayCommand; +import seedu.equipment.logic.commands.SelectCommand; import seedu.equipment.model.Model; import seedu.equipment.ui.BrowserPanel; @@ -22,6 +23,18 @@ public void display() { Model expectedModel = getModel(); assertCommandSuccess(command, expectedModel); + /* ----------------------- Perform select operations and then display and select ---------------------------- */ + + /* Case: Select some equipment, display, and then select the same equipment. The browser panel should change. + */ + String selectCommand = "" + SelectCommand.COMMAND_WORD + " 1"; + URL oldUrl = getBrowserPanel().getLoadedUrl(); + executeCommand(selectCommand); + assertNotEquals(oldUrl.toString(), getBrowserPanel().getLoadedUrl().toString()); + assertCommandSuccess(command, expectedModel); + oldUrl = getBrowserPanel().getLoadedUrl(); + executeCommand(selectCommand); + assertNotEquals(oldUrl.toString(), getBrowserPanel().getLoadedUrl().toString()); } /** From 7a22307d15bd09e69ba3be2af68ed7e10165e1fc Mon Sep 17 00:00:00 2001 From: Yiqun Sun Date: Sat, 13 Apr 2019 14:07:54 +0800 Subject: [PATCH 2/2] Add more test to route related to the #169 issue --- .../java/systemtests/RouteCommandSystemTest.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/test/java/systemtests/RouteCommandSystemTest.java b/src/test/java/systemtests/RouteCommandSystemTest.java index 2074632c89fc..b362a2e5f98c 100644 --- a/src/test/java/systemtests/RouteCommandSystemTest.java +++ b/src/test/java/systemtests/RouteCommandSystemTest.java @@ -7,6 +7,7 @@ import org.junit.Test; import seedu.equipment.logic.commands.RouteCommand; +import seedu.equipment.logic.commands.SelectCommand; import seedu.equipment.model.Model; import seedu.equipment.ui.BrowserPanel; @@ -21,6 +22,18 @@ public void route() { Model expectedModel = getModel(); assertCommandSuccess(command, expectedModel); + /* ----------------------- Perform select operations and then route and select ---------------------------- */ + + /* Case: Select some equipment, route, and then select the same equipment. The browser panel should change. + */ + String selectCommand = "" + SelectCommand.COMMAND_WORD + " 1"; + URL oldUrl = getBrowserPanel().getLoadedUrl(); + executeCommand(selectCommand); + assertNotEquals(oldUrl.toString(), getBrowserPanel().getLoadedUrl().toString()); + assertCommandSuccess(command, expectedModel); + oldUrl = getBrowserPanel().getLoadedUrl(); + executeCommand(selectCommand); + assertNotEquals(oldUrl.toString(), getBrowserPanel().getLoadedUrl().toString()); } /** @@ -40,7 +53,6 @@ private void assertCommandSuccess(String command, Model expectedModel) { assertApplicationDisplaysExpected("", expectedResultMessage, expectedModel); assertCommandBoxShowsDefaultStyle(); assertStatusBarUnchanged(); - assertSelectedPersonCardUnchanged(); assertNotEquals(oldUrl.toString(), getBrowserPanel().getLoadedUrl().toString()); assertNotEquals(BrowserPanel.DEFAULT_PAGE, getBrowserPanel().getLoadedUrl().toString()); assertNotEquals(BrowserPanel.MAP_MULTIPLE_POINT_BASE_URL, getBrowserPanel().getLoadedUrl().toString());