-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add edittask command #89
Add edittask command #89
Conversation
Codecov Report
@@ Coverage Diff @@
## master #89 +/- ##
============================================
+ Coverage 67.75% 67.85% +0.09%
- Complexity 635 654 +19
============================================
Files 110 112 +2
Lines 2199 2274 +75
Branches 274 291 +17
============================================
+ Hits 1490 1543 +53
- Misses 619 635 +16
- Partials 90 96 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! very comprehensive testcase. good QA!
@@ -65,6 +67,19 @@ public void parseCommand_editPatient() throws Exception { | |||
assertEquals(new EditPatientCommand(patient.getName().toString(), descriptor), command); | |||
} | |||
|
|||
@Test | |||
public void parseCommand_editTask() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be better to name the test parseCommand_editTask_failure()
or parseCommand_editTask_throwExceptions()
instead? Same for the tests at the bottom too!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. We should change the names of the tests here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
**Edit Room** | `editroom ROOM_NUMBER [r/NEW_ROOM_NUMBER] [p/PATIENT_NAME]` <br> e.g., `editroom 1 r/2 p/alex` | ||
**Add Task to Room** | `addtask d/DESCRIPTION r/ROOM_NUMBER [dd/DUE_DATE]` <br> | ||
**Delete Task from Room** | `deletetask r/ROOM_NUMBER t/TASK_NUMBER` <br> | ||
**Edit Task in Room** | `edittask r/ROOM_NUMBER t/TASK_NUMBER [d/DESCRIPTION] [dd/DUE_DATE]` <br> | ||
**Search Task** | `searchtask dd/DUE_DATE` <br> | ||
**Help** | `help` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We rearrange later in alphabetical order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... Should it be in alphabetical order? I was thinking maybe it should be sorted by the type. We can have Patient
, followed by Room
, then Task
.
3.2 Patient
3.2.1 Add Patient
3.2.2 Delete Patient
...
3.3 Room
3.3.1 Initialize Rooms
...
3.4 Task
3.4.1 Add Task
3.4.2 Delete Task
PREFIX_ROOM_NUMBER is used for both rooms and tasks. As such, it can be found in both TaskCliSyntax#PREFIX_ROOM_NUMBER and RoomCliSyntax#PREFIX_ROOM_NUMBER. Since PREFIX_ROOM_NUMBER is an attribute of room, it should be organized as such. Let's replace all usages of TaskCliSyntax#PREFIX_ROOM_NUMBER to RoomCliSyntax#PREFIX_ROOM_NUMBER.
e2bdbfe
to
89e0d26
Compare
Add
edittask
command. It takes a room number and a task number. Optionally, it takes in values corresponding to any of the task attributes such asDescription
andDateTimeDue
to replace the original. E.g.edit r/2 t/1 d/Hello World dd/-
would modify the first task from room number 2 to have the description "Hello World" and an empty due date.Update UG with instructions on how to use
edittask
.