From 8fb9bec1ad56a9c0dbb3a682a51c034e79936ed2 Mon Sep 17 00:00:00 2001 From: Wang Zihan Date: Sat, 4 Nov 2023 01:18:10 +0800 Subject: [PATCH 1/3] Not allowing null in EventTime and fix the bug of deleting contact lead to ghost events --- .../logic/commands/ListEventCommand.java | 4 ++-- .../logic/parser/AddEventCommandParser.java | 6 ++++-- .../logic/parser/ListEventCommandParser.java | 17 +++++++++++------ .../java/seedu/address/model/AddressBook.java | 1 + .../seedu/address/model/event/EventTime.java | 9 +++------ .../logic/commands/ListEventCommandTest.java | 4 ++-- .../address/logic/parser/ParserUtilTest.java | 2 -- 7 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/ListEventCommand.java b/src/main/java/seedu/address/logic/commands/ListEventCommand.java index d4b21a25ff2..87a68fb9f5e 100644 --- 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 index 40ef42aea51..8a064193119 100644 --- 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 index 2ad50e284da..097d197feea 100644 --- 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 index ce15788e788..d7121d5e733 100644 --- 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/EventTime.java b/src/main/java/seedu/address/model/event/EventTime.java index 7b594bb564d..694ef924d3b 100644 --- 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/test/java/seedu/address/logic/commands/ListEventCommandTest.java b/src/test/java/seedu/address/logic/commands/ListEventCommandTest.java index 961bb95bf3f..1d4fd2162d4 100644 --- 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 index 7ca15d92b6c..f70ed4d1b40 100644 --- 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 From a97009331554b5397f934aeb74b2b376b75fbb53 Mon Sep 17 00:00:00 2001 From: Wang Zihan Date: Sat, 4 Nov 2023 01:24:05 +0800 Subject: [PATCH 2/3] Add more coverage --- src/test/java/seedu/address/model/event/EventTimeTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/java/seedu/address/model/event/EventTimeTest.java b/src/test/java/seedu/address/model/event/EventTimeTest.java index 7c127fcf686..5a86cf6b3bb 100644 --- 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"), From 690fcfdfc40406b302408a34694d3ff097598e8d Mon Sep 17 00:00:00 2001 From: Wang Zihan Date: Sat, 4 Nov 2023 01:47:32 +0800 Subject: [PATCH 3/3] Changes in time related logic for events Remove the unnecessary logic in `UniqueEventList::getOverlappingEvent` since in AddEventCommand, end time is set to start time if user does not specify an end time Change the logic in `Event::getUiText` so that when start time equals to end time, end time will be hidden (also add this information to UG) --- docs/UserGuide.md | 2 ++ src/main/java/seedu/address/model/event/Event.java | 2 +- .../seedu/address/model/event/UniqueEventList.java | 10 ++-------- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/docs/UserGuide.md b/docs/UserGuide.md index 96d5b208e77..07ae845b301 100644 --- 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/model/event/Event.java b/src/main/java/seedu/address/model/event/Event.java index e8450c80447..214e37556f9 100644 --- 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/UniqueEventList.java b/src/main/java/seedu/address/model/event/UniqueEventList.java index 1646a8b068f..a2964c91fdf 100644 --- 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;