Remove Date arg from class To Field of Meeting; Update Calendar to Schedule #211

MerlinLim · 2020-11-04T14:31:42Z

Fixes #163
Fixes #137

Add Time class
To field now only takes HH:mm
Remove Index from Calendar panel and rename to schedule

Class Time takes in args and parses as hh:mm.

To only takes in args HH:mm. Minor changes to error message.

Minor changes to styling and scaling.

codecov-io · 2020-11-04T14:33:14Z

Codecov Report

Merging #211 into master will increase coverage by 0.19%.
The diff coverage is 61.53%.

@@             Coverage Diff              @@
##             master     #211      +/-   ##
============================================
+ Coverage     53.72%   53.91%   +0.19%     
- Complexity      751      758       +7     
============================================
  Files           166      167       +1     
  Lines          3131     3144      +13     
  Branches        347      344       -3     
============================================
+ Hits           1682     1695      +13     
  Misses         1341     1341              
  Partials        108      108

Impacted Files	Coverage Δ	Complexity Δ
...edu/address/logic/commands/meeting/AddCommand.java	`100.00% <ø> (ø)`	`8.00 <0.00> (ø)`
...c/main/java/seedu/address/model/util/DateTime.java	`86.95% <ø> (ø)`	`11.00 <0.00> (ø)`
.../java/seedu/address/model/util/SampleDataUtil.java	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...java/seedu/address/ui/CalendarDeliverableCard.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../main/java/seedu/address/ui/CalendarListPanel.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ain/java/seedu/address/ui/CalendarMeetingCard.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
src/main/java/seedu/address/model/util/Time.java	`81.25% <81.25%> (ø)`	`7.00 <7.00> (?)`
...a/seedu/address/model/meeting/meeting/Meeting.java	`88.67% <100.00%> (ø)`	`23.00 <2.00> (ø)`
...n/java/seedu/address/model/meeting/meeting/To.java	`80.00% <100.00%> (ø)`	`5.00 <1.00> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 849c30a...4a8f7ce. Read the comment docs.

Include labels for schedule

shadowezz · 2020-11-04T14:35:47Z

src/main/java/seedu/address/model/meeting/meeting/Meeting.java

-        LocalDateTime dateFrom = from.getLocalDateTime();
-        LocalDateTime dateTo = to.getLocalDateTime();
+        LocalTime dateFrom = from.getLocalDateTime().toLocalTime();
+        LocalTime dateTo = to.getLocalTime();

        return dateFrom.compareTo(dateTo) <= 0;


I think its better to make it strictly less than.

shadowezz · 2020-11-04T14:45:44Z

src/main/java/seedu/address/model/util/Time.java

+    public static final String TIME_REGEX = "(([0-1]\\d)|(2[0-3])):([0-5]\\d)";
+    public static final String MESSAGE_CONSTRAINTS =
+            "Time should be in the format of HH:mm, "
+                    + "minute must start with a leading zero.";


Perhaps its clearer to state that single digit hour and minute must start with a leading zero

shadowezz · 2020-11-04T14:46:13Z

src/main/java/seedu/address/model/util/DateTime.java

@@ -20,7 +19,7 @@
                    + "Note: Single digit month, day, and minute must start with a leading zero.";


Could you include hour here as well

shadowezz · 2020-11-04T14:48:53Z

src/main/java/seedu/address/model/util/Time.java

+            "Time should be in the format of HH:mm, "
+                    + "minute must start with a leading zero.";
+
+    public static final String VALIDATION_REGEX = String.format("%s", TIME_REGEX);


Is this the same as TIME_REGEX?

shadowezz · 2020-11-04T14:55:44Z

src/test/java/seedu/address/model/meeting/meeting/MeetingTest.java

        editedMeeting = new MeetingBuilder(MEETING_A)
-                .withTo(VALID_TO_B).build();


I think VALID_TO_B should still work since its updated to 12:00

nope it will throw false, MEETING_A has a time of 14:00.

shadowezz · 2020-11-04T14:57:22Z

src/test/java/seedu/address/model/meeting/meeting/MeetingTest.java

@@ -68,10 +63,10 @@ public void isValidFromAndTo() {
        assertTrue(Meeting.isValidFromAndTo(new From(VALID_FROM_A), new To(VALID_TO_A)));

        // FROM = TO -> returns true
-        assertTrue(Meeting.isValidFromAndTo(new From(VALID_FROM_A), new To(VALID_FROM_A)));
+        assertTrue(Meeting.isValidFromAndTo(new From(VALID_FROM_B), new To(VALID_TO_B)));


If we change the part in front this will now return false.

shadowezz · 2020-11-04T14:59:42Z

src/test/java/seedu/address/model/util/TimeTest.java

+    }
+
+    @Test
+    void parseFail() {


Might need to change the comments here

shadowezz · 2020-11-04T15:22:41Z

src/main/java/seedu/address/model/util/DateTime.java

@@ -11,7 +11,6 @@
 import java.util.Date;

 public class DateTime implements Comparable<DateTime> {
-    public static final String TIME_REGEX = "(([0-1]\\d)|(2[0-3])):([0-5]\\d)";
    public static final String DATE_REGEX = "(([0-2]\\d)|(3[0-1]))-((0[1-9])|(1[0-2]))-(\\d{4})";
    public static final String EARLIEST_DATE_STRING = "01-01-2019 00:00";
    public static final String MESSAGE_CONSTRAINTS =


Perhaps we should shift this message to the deadline and from class and state explicitly that deadline/from should be in the format ...

shadowezz · 2020-11-04T15:24:49Z

src/main/java/seedu/address/model/util/Time.java

+
+public class Time implements Comparable<Time> {
+    public static final String TIME_REGEX = "(([0-1]\\d)|(2[0-3])):([0-5]\\d)";
+    public static final String MESSAGE_CONSTRAINTS =


Same as above I think its better to shift this message into to class and say To should be in the format ...

shadowezz · 2020-11-04T15:32:46Z

src/main/java/seedu/address/model/meeting/meeting/Meeting.java

@@ -49,8 +50,8 @@ public Meeting(Title title, OptionalDescription description, From from, To to,
     * Returns true if From is earlier than To chronologically.
     */
    public static boolean isValidFromAndTo (From from, To to) {


The error message for this prob needs to change also cos "to" is no longer a date.

Update Bug fix

shadowezz

LGTM

MerlinLim added 6 commits November 4, 2020 17:57

Add util class Time and test cases to model

1a7fc01

Class Time takes in args and parses as hh:mm.

To field extends from Time

55d2236

To only takes in args HH:mm. Minor changes to error message.

Change UI for Dashboard and Meeting

470d1de

Minor changes to styling and scaling.

Merge branch 'master' into meeting-to-field

a72ef14

Fix Checkstyle and fail test cases.

b280661

Merge branch 'master' into meeting-to-field

c027db6

MerlinLim added this to the mid-v1.4 milestone Nov 4, 2020

MerlinLim requested a review from shadowezz November 4, 2020 14:31

MerlinLim self-assigned this Nov 4, 2020

MerlinLim added this to PRs Pending Approvals in Productiv via automation Nov 4, 2020

Fix wrapping issues and schedule UI

260c0ab

Include labels for schedule

shadowezz reviewed Nov 4, 2020

View reviewed changes

MerlinLim added 3 commits November 5, 2020 00:31

Merge branch 'master' into meeting-to-field

4a8f7ce

Update Bug fix

Bug fixes

d87399c

Revert deliverable scaling and UI

ae85754

shadowezz approved these changes Nov 4, 2020

View reviewed changes

Productiv automation moved this from PRs Pending Approvals to PRs approved / Ready for Testing Nov 4, 2020

MerlinLim merged commit dc34bdf into AY2021S1-CS2103T-F11-2:master Nov 4, 2020

Productiv automation moved this from PRs approved / Ready for Testing to Issues Done / PRs Merged (After Testing) Nov 4, 2020

MerlinLim changed the title ~~Remove Date arg from class To Field of Meeting~~ Remove Date arg from class To Field of Meeting; Update Calendar to Schedule Nov 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Date arg from class To Field of Meeting; Update Calendar to Schedule #211

Remove Date arg from class To Field of Meeting; Update Calendar to Schedule #211

MerlinLim commented Nov 4, 2020 •

edited

codecov-io commented Nov 4, 2020 •

edited

shadowezz Nov 4, 2020

shadowezz Nov 4, 2020

shadowezz Nov 4, 2020

shadowezz Nov 4, 2020

shadowezz Nov 4, 2020

MerlinLim Nov 4, 2020

shadowezz Nov 4, 2020

shadowezz Nov 4, 2020

shadowezz Nov 4, 2020

shadowezz Nov 4, 2020

shadowezz Nov 4, 2020

shadowezz left a comment

		@@ -20,7 +19,7 @@
		+ "Note: Single digit month, day, and minute must start with a leading zero.";

		editedMeeting = new MeetingBuilder(MEETING_A)
		.withTo(VALID_TO_B).build();

Remove Date arg from class To Field of Meeting; Update Calendar to Schedule #211

Remove Date arg from class To Field of Meeting; Update Calendar to Schedule #211

Conversation

MerlinLim commented Nov 4, 2020 • edited

codecov-io commented Nov 4, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shadowezz left a comment

Choose a reason for hiding this comment

MerlinLim commented Nov 4, 2020 •

edited

codecov-io commented Nov 4, 2020 •

edited