Change Assign to Append #209
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #209 +/- ##
============================================
- Coverage 66.25% 65.33% -0.93%
+ Complexity 566 564 -2
============================================
Files 95 95
Lines 2199 2204 +5
Branches 222 223 +1
============================================
- Hits 1457 1440 -17
- Misses 655 680 +25
+ Partials 87 84 -3 ☔ View full report in Codecov by Sentry. |
Previous iteration did not prevent adding of roles correctly. Could still add a role when there was not a CCA. Additionally, there would be many strange errors after.
| @@ -76,7 +78,7 @@ public CommandResult execute(Model model) throws CommandException { | |||
|
|
|||
| Person personToAssign = lastShownList.get(index.getZeroBased()); | |||
|
|
|||
| if (personToAssign.getCcas().isEmpty() && !personToAssign.getRoles().isEmpty()) { | |||
There was a problem hiding this comment.
In the case where there is no CCA and no roles assigned yet this will fail to trigger.
!personToAssign.getRoles().isEmpty() will be false and no error will be thrown even though there is no CCA and no role should be assigned.
The end result is that a role will be assigned to a person even though they do not have CCA.
There was a problem hiding this comment.
In either case, there is no need to check for the role portion for the assign command as if there is no CCA, the assign command should already fall through.
This test does not comply with current version of assign
chuahjiajie
merged commit 3fb0a9a
into
AY2324S2-CS2103T-W11-2:master
Fixes #145