Merge pull request #85 from larrywang0701/bug-fix-milestone-1.4

Not allowing null in EventTime and fix the bug of deleting contact lead to ghost events
AY2324S1-CS2103T-W16-1 · Nov 4, 2023 · b7b9c00 · b7b9c00
2 parents 0abcaa5 + 690fcfd
commit b7b9c00
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 29 deletions.
diff --git a/docs/UserGuide.md b/docs/UserGuide.md
@@ -200,6 +200,8 @@ Adds an event to a contact. The added event should not have clashes in timing wi
 
 Format: `add event -id CONTACT_ID -en EVENT_NAME -st START_TIME [-et END_TIME] [-loc LOCATION] [-info INFORMATION]`
 
+(If start time is exactly equals to end time, the end time for the event will not be displayed when outputting the event to text-based UI)
+
 Date-Time Format:
  - You can use one of the following formats for `START_TIME` and `END_TIME`:
     - Both date and time: `yyyy-MM-dd HH:mm[:ss]`

diff --git a/src/main/java/seedu/address/logic/commands/ListEventCommand.java b/src/main/java/seedu/address/logic/commands/ListEventCommand.java
@@ -38,8 +38,8 @@ public class ListEventCommand extends ListCommand {
      *     {@code false} in descending order.
      */
     public ListEventCommand(EventTime filterStartTime, EventTime filterEndTime, boolean sortAscending) {
-        this.filterStartTime = filterStartTime.getTime();
-        this.filterEndTime = filterEndTime.getTime();
+        this.filterStartTime = filterStartTime != null ? filterStartTime.getTime() : null;
+        this.filterEndTime = filterEndTime != null ? filterEndTime.getTime() : null;
         this.sortAscending = sortAscending;
     }
 

diff --git a/src/main/java/seedu/address/logic/parser/AddEventCommandParser.java b/src/main/java/seedu/address/logic/parser/AddEventCommandParser.java
@@ -40,8 +40,10 @@ public AddEventCommand parse(String args) throws ParseException {
                 PREFIX_EVENT_INFORMATION);
 
         EventName eventName = ParserUtil.parseEventName(argMultimap.getValue(PREFIX_EVENT_NAME).get());
-        EventTime startTime = ParserUtil.parseEventTime(argMultimap.getValue(PREFIX_START_TIME).get());
-        EventTime endTime = ParserUtil.parseEventTime(argMultimap.getValue(PREFIX_END_TIME).orElseGet(()->null));
+        String startTimeStr = argMultimap.getValue(PREFIX_START_TIME).get();
+        EventTime startTime = ParserUtil.parseEventTime(startTimeStr);
+        EventTime endTime = ParserUtil.parseEventTime(argMultimap.getValue(PREFIX_END_TIME)
+                .orElseGet(()->startTimeStr));
         EventLocation location =
                 ParserUtil.parseEventLocation(argMultimap.getValue(PREFIX_EVENT_LOCATION).orElseGet(()->null));
         EventInformation information =

diff --git a/src/main/java/seedu/address/logic/parser/ListEventCommandParser.java b/src/main/java/seedu/address/logic/parser/ListEventCommandParser.java
@@ -33,14 +33,19 @@ public ListEventCommand parse(String args) throws ParseException {
 
         argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_START_TIME, PREFIX_END_TIME, PREFIX_SORT_DESCENDING);
 
-        EventTime filterStartTime = ParserUtil.parseEventTime(argMultimap.getValue(PREFIX_START_TIME)
-                .orElseGet(()->null));
-        EventTime filterEndTime = ParserUtil.parseEventTime(argMultimap.getValue(PREFIX_END_TIME).orElseGet(()->null));
+        EventTime filterStartTime = ParserUtil.arePrefixesPresent(argMultimap, PREFIX_START_TIME)
+                ? ParserUtil.parseEventTime(argMultimap.getValue(PREFIX_START_TIME).get())
+                : null;
+        EventTime filterEndTime = ParserUtil.arePrefixesPresent(argMultimap, PREFIX_END_TIME)
+                ? ParserUtil.parseEventTime(argMultimap.getValue(PREFIX_END_TIME).get())
+                : null;
 
         boolean useAscendingOrder = !ParserUtil.arePrefixesPresent(argMultimap, PREFIX_SORT_DESCENDING);
-
-        if (filterStartTime.isAfter(filterEndTime)) {
-            throw new ParseException(String.format(MESSAGE_START_TIME_AFTER_END_TIME, filterStartTime, filterEndTime));
+        if (filterStartTime != null) {
+            if (filterStartTime.isAfter(filterEndTime)) {
+                throw new ParseException(String.format(MESSAGE_START_TIME_AFTER_END_TIME,
+                        filterStartTime, filterEndTime));
+            }
         }
         return new ListEventCommand(filterStartTime, filterEndTime, useAscendingOrder);
     }

diff --git a/src/main/java/seedu/address/model/AddressBook.java b/src/main/java/seedu/address/model/AddressBook.java
@@ -127,6 +127,7 @@ public void setPerson(Person target, Person editedPerson) {
      * {@code key} must exist in the address book.
      */
     public void removePerson(Person key) {
+        key.getEvents().forEach(this.allEvents::remove);
         persons.remove(key);
     }
 

diff --git a/src/main/java/seedu/address/model/event/Event.java b/src/main/java/seedu/address/model/event/Event.java
@@ -111,7 +111,7 @@ public String getInformationStr() {
      */
     public String getUiText() {
         String result = this.getName() + "\nStarts at: " + this.start;
-        if (!this.end.toString().isEmpty()) {
+        if (!this.end.equals(this.start)) {
             result += "\nEnds at: " + this.end;
         }
         if (!this.location.toString().isEmpty()) {

diff --git a/src/main/java/seedu/address/model/event/EventTime.java b/src/main/java/seedu/address/model/event/EventTime.java
@@ -14,9 +14,6 @@ public class EventTime {
 
     private final LocalDateTime time;
 
-    private EventTime() {
-        this.time = null;
-    }
     private EventTime(String time) throws DateTimeParseException {
         this.time = DateTimeUtil.parseString(time);
     }
@@ -27,7 +24,7 @@ private EventTime(String time) throws DateTimeParseException {
      * @return The {@code EventTime} object
      */
     public static EventTime fromString(String timeStr) throws DateTimeParseException {
-        return timeStr.isEmpty() ? new EventTime() : new EventTime(timeStr);
+        return new EventTime(timeStr);
     }
 
     /**
@@ -58,8 +55,8 @@ public boolean equals(Object other) {
             return false;
         }
 
-        EventTime otherName = (EventTime) other;
-        return time.equals(otherName.time);
+        EventTime otherTime = (EventTime) other;
+        return time.equals(otherTime.time);
     }
 
     /**

diff --git a/src/main/java/seedu/address/model/event/UniqueEventList.java b/src/main/java/seedu/address/model/event/UniqueEventList.java
@@ -165,20 +165,14 @@ private Event getOverlappingEvent(Event newEvent) {
         LocalDateTime newEventEndTime = newEvent.getEndTime();
 
         assert newEventStartTime != null : "Start time should not be null";
-
-        if (newEventEndTime == null) {
-            newEventEndTime = newEventStartTime;
-        }
+        assert newEventEndTime != null : "End time should not be null";
 
         for (Event e : this.internalList) {
             LocalDateTime startTime = e.getStartTime();
             LocalDateTime endTime = e.getEndTime();
 
             assert startTime != null : "Start time should not be null";
-
-            if (endTime == null) {
-                endTime = startTime;
-            }
+            assert endTime != null : "End time should not be null";
 
             if (DateTimeUtil.timeIntervalsOverlap(newEventStartTime, newEventEndTime, startTime, endTime)) {
                 return e;

diff --git a/src/test/java/seedu/address/logic/commands/ListEventCommandTest.java b/src/test/java/seedu/address/logic/commands/ListEventCommandTest.java
@@ -33,8 +33,8 @@ public void execute_correctCommandFullList_success() {
         model.findPersonByUserFriendlyId(ContactID.fromInt(1)).addEvent(VALID_EVENT_0);
         model.findPersonByUserFriendlyId(ContactID.fromInt(2)).addEvent(VALID_EVENT_1);
         model.findPersonByUserFriendlyId(ContactID.fromInt(2)).addEvent(VALID_EVENT_2);
-        assertCommandSuccess(() -> new ListEventCommand(EventTime.fromString(""),
-                EventTime.fromString(""), true).execute(model));
+        assertCommandSuccess(() -> new ListEventCommand(null,
+                null, true).execute(model));
     }
 
     @Test

diff --git a/src/test/java/seedu/address/logic/parser/ParserUtilTest.java b/src/test/java/seedu/address/logic/parser/ParserUtilTest.java
@@ -337,8 +337,6 @@ public void parseEventTime_success() throws Exception {
                 ParserUtil.parseEventTime("2023-12-01").toString());
         assertEquals("2023-12-01 10:02:03",
                 ParserUtil.parseEventTime("2023-12-01 10:02:03").toString());
-        assertEquals("",
-                ParserUtil.parseEventTime(null).toString());
     }
 
     @Test

diff --git a/src/test/java/seedu/address/model/event/EventTimeTest.java b/src/test/java/seedu/address/model/event/EventTimeTest.java
@@ -1,7 +1,9 @@
 package seedu.address.model.event;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
@@ -11,6 +13,9 @@
 public class EventTimeTest {
     @Test
     public void test_eventTimeEquals_pass() {
+        EventTime instance = EventTime.fromString("2023-11-03 00:30:00");
+        assertTrue(instance.equals(instance));
+        assertFalse(instance.equals(new Object()));
         assertEquals(EventTime.fromString("2023-11-02 00:30:00"),
                 EventTime.fromString("2023-11-02 00:30:00"));
         assertEquals(EventTime.fromString("00:30:00"),