Merge pull request #61 from guanquann/branch-order-bug-fix

Bug fixes, enhance test cases for orders

docs/UserGuide.md

@@ -283,7 +283,7 @@ Adds an order to a supplier.
 Format: `addorder INDEX d/DATE r/REMARK`
 
 * Adds an order to the supplier at the specified `INDEX`. The index refers to the index number shown in the displayed supplier list. The index **must be a positive integer, starting from 1** (1, 2, 3, …​)
-* The date must be in the format `YYYY-MM-DD`. For example, `2020-12-31`.
+* The date must be in the format `YYYY-MM-DD`, where `YYYY` is the year (all the digits, i.e. 2012), `MM` is the month (01 to 12) and `DD` is the day (01 to 31). For example, `2020-12-31`.
 
 <box type="tip" seamless>
 
@@ -292,10 +292,7 @@ Format: `addorder INDEX d/DATE r/REMARK`
 
 Examples:
 * `addorder 1 d/2020-01-01 r/100 chicken wings`
-* `addorder 2 r/ 100 chicken wings d/ 2020-12-31`
-* `addorder 3 d/2020-01-01 r/100 chicken wings`
-* `addorder d/2020-01-01 r/100 chicken wings` returns an error as the index is not specified
-* `addorder r/` or `addorder d/` or `addorder r/ d/` returns an error message as the 'KEYWORD' field cannot be empty
+* `addorder 2 r/tomatoes d/2020-12-31`
 
 ### Listing orders : `listorder`
 

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

@@ -20,14 +20,15 @@ public class AddOrderCommand extends Command {
     public static final String COMMAND_WORD = "addorder";
 
     public static final String MESSAGE_SUCCESS = "New order added: %1$s";
+    public static final String MESSAGE_DUPLICATE_ORDER = "Order already exists for this person";
 
     public static final String MESSAGE_USAGE = COMMAND_WORD
             + ": Adds an order to the person identified "
             + "by the index number used in the last person listing.\n"
             + "Parameters: INDEX (must be a positive integer) "
             + "d/ [DATE] r/ [REMARK] \n"
             + "Example: " + COMMAND_WORD + " 1 "
-            + "d/ 2024-01-01 r/ 100 chicken wings.";
+            + "d/ 2024-01-01 r/ 100 chicken wings";
 
     private final Index index;
     private final Order order;
@@ -55,6 +56,10 @@ public CommandResult execute(Model model) throws CommandException {
 
         Person person = lastShownList.get(index.getZeroBased());
 
+        if (model.hasOrder(person, order)) {
+            throw new CommandException(MESSAGE_DUPLICATE_ORDER);
+        }
+
         model.addOrder(person, order);
 
         return new CommandResult(String.format(MESSAGE_SUCCESS, Messages.format(person)));

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

@@ -65,7 +65,7 @@ private String createOrderListString(List<Order> orders) {
         sb.append("Order(s) for the selected person:\n");
 
         if (orders.isEmpty()) {
-            sb.append("No orders found.");
+            sb.append("No orders found");
             return sb.toString();
         }
 

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

@@ -106,6 +106,14 @@ public void addOrder(Person target, Order order) {
         persons.setPerson(target, target);
     }
 
+    /**
+     * Checks if the person has the order.
+     * The person must exist in the address book.
+     */
+    public boolean hasOrder(Person target, Order order) {
+        return target.hasOrder(order);
+    }
+
     /**
      * Deletes an order from the person.
      * The person must exist in the address book.

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

@@ -95,6 +95,11 @@ public interface Model {
      */
     void addOrder(Person person, Order order);
 
+    /**
+     * Checks if the person has the order.
+     */
+    boolean hasOrder(Person person, Order order);
+
     /**
      * Returns the orders for a person.
      */

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

@@ -140,6 +140,13 @@ public void addOrder(Person target, Order order) {
         updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
     }
 
+    @Override
+    public boolean hasOrder(Person target, Order order) {
+        requireNonNull(target);
+        requireNonNull(order);
+        return addressBook.hasOrder(target, order);
+    }
+
     @Override
     public List<Order> getOrders(Person target) {
         requireNonNull(target);

src/main/java/seedu/address/model/order/Order.java

@@ -23,11 +23,16 @@ public Order(Date arrivalDate, Remark remark) {
         this.remark = remark;
     }
 
-
+    /**
+     * Returns the date of the order.
+     */
     public Date getDate() {
         return arrivalDate;
     }
 
+    /**
+     * Returns the remark of the order.
+     */
     public Remark getRemark() {
         return remark;
     }
@@ -52,9 +57,7 @@ public int hashCode() {
         return arrivalDate.hashCode() + remark.hashCode();
     }
 
-    /**
-     * Format state as text for viewing.
-     */
+    @Override
     public String toString() {
         return String.format("[%s (by: %s)]", remark, arrivalDate);
     }

src/main/java/seedu/address/model/person/Person.java

@@ -100,6 +100,21 @@ public void addOrder(Order order) {
         orders.add(order);
     }
 
+    /**
+     * Checks if the person has the order
+     * @param order the order to be checked
+     */
+    public boolean hasOrder(Order order) {
+        return orders.contains(order);
+    }
+
+    /**
+     * Returns the order list
+     */
+    public ArrayList<Order> getOrders() {
+        return orders;
+    }
+
     /**
      * Removes an order from the order list
      * @param order the order to be removed
@@ -126,10 +141,6 @@ public boolean getIsFavourite() {
         return this.isFavourite;
     }
 
-    public ArrayList<Order> getOrders() {
-        return orders;
-    }
-
     /**
      * Returns true if both persons have the same name.
      * This defines a weaker notion of equality between two persons.

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

@@ -50,7 +50,14 @@ public Order toModelType() throws IllegalValueException {
             throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, "Remark"));
         }
 
+        if (!Date.isValidDate(arrivalDate)) {
+            throw new IllegalValueException(Date.MESSAGE_CONSTRAINTS);
+        }
         final Date modelDate = new Date(arrivalDate);
+
+        if (!Remark.isValidRemark(remark)) {
+            throw new IllegalValueException(Remark.MESSAGE_CONSTRAINTS);
+        }
         final Remark modelRemark = new Remark(remark);
 
         return new Order(modelDate, modelRemark);

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

@@ -165,6 +165,11 @@ public void addOrder(Person target, Order order) {
             throw new AssertionError("This method should not be called.");
         }
 
+        @Override
+        public boolean hasOrder(Person person, Order order) {
+            throw new AssertionError("This method should not be called.");
+        }
+
         @Override
         public ArrayList<Order> getOrders(Person person) {
             throw new AssertionError("This method should not be called.");

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

@@ -55,6 +55,17 @@ public void execute_addOrder_success() {
         assertCommandSuccess(addOrderCommand, model, expectedMessage, expectedModel);
     }
 
+    @Test
+    public void execute_addDuplicateOrder_throwsCommandException() {
+        Person firstPerson = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());
+        Person editedPerson = new PersonBuilder(firstPerson).withOrders(ORDERS_STUB).build();
+        model.setPerson(firstPerson, editedPerson);
+
+        AddOrderCommand addOrderCommand = new AddOrderCommand(INDEX_FIRST_PERSON, ORDER_STUB);
+        assertThrows(CommandException.class, AddOrderCommand.MESSAGE_DUPLICATE_ORDER, () ->
+                addOrderCommand.execute(model));
+    }
+
     @Test
     public void execute_invalidIndex_throwsCommandException() {
         Index outOfBoundIndex = Index.fromOneBased(model.getFilteredPersonList().size() + 1);

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

@@ -34,7 +34,7 @@ public void execute_validIndexNoOrders_success() throws CommandException {
         ListOrderCommand listOrderCommand = new ListOrderCommand(Index.fromOneBased(2));
         CommandResult commandResult = listOrderCommand.execute(model);
 
-        String expectedMessage = "Order(s) for the selected person:\nNo orders found.";
+        String expectedMessage = "Order(s) for the selected person:\nNo orders found";
         assertEquals(expectedMessage, commandResult.getFeedbackToUser());
     }
 

src/test/java/seedu/address/model/AddressBookTest.java

@@ -6,10 +6,11 @@
 import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_BOB;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_HUSBAND;
 import static seedu.address.testutil.Assert.assertThrows;
+import static seedu.address.testutil.TypicalOrders.ORDER;
+import static seedu.address.testutil.TypicalOrders.getTypicalOrders;
 import static seedu.address.testutil.TypicalPersons.ALICE;
 import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -19,9 +20,7 @@
 
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
-import seedu.address.model.order.Date;
 import seedu.address.model.order.Order;
-import seedu.address.model.order.Remark;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.exceptions.DuplicatePersonException;
 import seedu.address.testutil.PersonBuilder;
@@ -87,32 +86,46 @@ public void getPersonList_modifyList_throwsUnsupportedOperationException() {
         assertThrows(UnsupportedOperationException.class, () -> addressBook.getPersonList().remove(0));
     }
 
+    @Test
+    public void addOrder_personExists_orderAdded() {
+        Person personWithOrder = new PersonBuilder().build();
+
+        addressBook.addPerson(personWithOrder);
+        addressBook.addOrder(personWithOrder, ORDER);
+
+        assertTrue(personWithOrder.getOrders().contains(ORDER));
+        assertTrue(addressBook.hasPerson(personWithOrder));
+    }
+
+    @Test
+    public void hasOrder_orderExists_returnsTrue() {
+        Person personWithOrder = new PersonBuilder().withOrders(getTypicalOrders()).build();
+        addressBook.addPerson(personWithOrder);
+        assertTrue(addressBook.hasOrder(personWithOrder, ORDER));
+    }
+
     @Test
     public void deleteOrder_orderExists_orderRemoved() {
-        Order order = new Order(new Date("2020-01-01"), new Remark("100 chicken wings"));
-        Person personWithOrder = new PersonBuilder().withOrders(new ArrayList<>(List.of(order))).build();
+        Person personWithOrder = new PersonBuilder().withOrders(getTypicalOrders()).build();
         addressBook.addPerson(personWithOrder);
 
-        addressBook.deleteOrder(personWithOrder, order);
+        addressBook.deleteOrder(personWithOrder, ORDER);
 
-        assertFalse(personWithOrder.getOrders().contains(order));
+        assertFalse(personWithOrder.getOrders().contains(ORDER));
         assertTrue(addressBook.hasPerson(personWithOrder));
     }
 
-
     @Test
     public void getOrders_personExists_returnsCorrectOrders() {
-        Order order = new Order(new Date("2020-01-01"), new Remark("100 chicken wings"));
-        Person personWithOrder = new PersonBuilder(ALICE).withOrders(new ArrayList<>(List.of(order))).build();
+        Person personWithOrder = new PersonBuilder(ALICE).withOrders(getTypicalOrders()).build();
         addressBook.addPerson(personWithOrder);
 
         List<Order> orders = addressBook.getOrders(personWithOrder);
 
         assertEquals(1, orders.size());
-        assertTrue(orders.contains(order));
+        assertTrue(orders.contains(ORDER));
     }
 
-
     @Test
     public void toStringMethod() {
         String expected = AddressBook.class.getCanonicalName() + "{persons=" + addressBook.getPersonList() + "}";

src/test/java/seedu/address/model/ModelManagerTest.java

@@ -5,33 +5,24 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 import static seedu.address.testutil.Assert.assertThrows;
+import static seedu.address.testutil.TypicalOrders.ORDER;
+import static seedu.address.testutil.TypicalOrders.getTypicalOrders;
 import static seedu.address.testutil.TypicalPersons.ALICE;
 import static seedu.address.testutil.TypicalPersons.BENSON;
 
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.List;
 
 import org.junit.jupiter.api.Test;
 
 import seedu.address.commons.core.GuiSettings;
-import seedu.address.model.order.Date;
-import seedu.address.model.order.Order;
-import seedu.address.model.order.Remark;
 import seedu.address.model.person.NameContainsKeywordsPredicate;
 import seedu.address.model.person.Person;
 import seedu.address.testutil.AddressBookBuilder;
 import seedu.address.testutil.PersonBuilder;
 
 public class ModelManagerTest {
-
-    private static final Date DATE_STUB = new Date("2020-01-01");
-    private static final Remark REMARK_STUB = new Remark("100 chicken wings");
-    private static final Order ORDER_STUB = new Order(DATE_STUB, REMARK_STUB);
-    private static final ArrayList<Order> ORDERS_STUB = new ArrayList<>(List.of(ORDER_STUB));
-
     private ModelManager modelManager = new ModelManager();
 
     @Test
@@ -105,29 +96,42 @@ public void getFilteredPersonList_modifyList_throwsUnsupportedOperationException
         assertThrows(UnsupportedOperationException.class, () -> modelManager.getFilteredPersonList().remove(0));
     }
 
+    @Test
+    public void addOrder_validOrder_orderAdded() {
+        Person personWithOrder = new PersonBuilder().build();
+        modelManager.addPerson(personWithOrder);
+        modelManager.addOrder(personWithOrder, ORDER);
+        assertTrue(personWithOrder.getOrders().contains(ORDER));
+    }
+
+    @Test
+    public void hasOrder_validPersonAndOrder_returnsTrue() {
+        Person personWithOrder = new PersonBuilder().build();
+        modelManager.addPerson(personWithOrder);
+        modelManager.addOrder(personWithOrder, ORDER);
+        assertTrue(modelManager.hasOrder(personWithOrder, ORDER));
+    }
+
     @Test
     public void deleteOrder_removeExistingOrder_orderRemoved() {
-        Person personWithOrder = new PersonBuilder().withOrders(ORDERS_STUB).build();
+        Person personWithOrder = new PersonBuilder().withOrders(getTypicalOrders()).build();
         AddressBook addressBookWithOrder = new AddressBookBuilder().withPerson(personWithOrder).build();
         ModelManager modelManager = new ModelManager(addressBookWithOrder, new UserPrefs());
 
-        Order orderToRemove = ORDERS_STUB.get(0); // Assuming ORDERS_STUB is not empty
-        modelManager.deleteOrder(personWithOrder, orderToRemove);
+        modelManager.deleteOrder(personWithOrder, ORDER);
 
-        assertFalse(modelManager.getOrders(personWithOrder).contains(orderToRemove));
+        assertFalse(modelManager.getOrders(personWithOrder).contains(ORDER));
     }
 
     @Test
     public void getOrders_personWithOrders_returnsCorrectOrderList() {
-        Person personWithOrder = new PersonBuilder().withOrders(ORDERS_STUB).build();
+        Person personWithOrder = new PersonBuilder().withOrders(getTypicalOrders()).build();
         AddressBook addressBookWithOrder = new AddressBookBuilder().withPerson(personWithOrder).build();
         ModelManager modelManager = new ModelManager(addressBookWithOrder, new UserPrefs());
 
-        assertEquals(ORDERS_STUB, modelManager.getOrders(personWithOrder));
+        assertEquals(getTypicalOrders(), modelManager.getOrders(personWithOrder));
     }
 
-
-
     @Test
     public void equals() {
         AddressBook addressBook = new AddressBookBuilder().withPerson(ALICE).withPerson(BENSON).build();