-
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
Fix bug to prevent /ec when editing doctor #225
Fix bug to prevent /ec when editing doctor #225
Conversation
Codecov Report
@@ Coverage Diff @@
## master #225 +/- ##
============================================
+ Coverage 75.37% 75.41% +0.03%
- Complexity 756 758 +2
============================================
Files 112 112
Lines 2530 2538 +8
Branches 272 271 -1
============================================
+ Hits 1907 1914 +7
Misses 530 530
- Partials 93 94 +1
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
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.
Looks great. May be you can also try to cover the test cases for the newly added code.
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. Good job on solving the issue, now just to ensure code coverage is good!
throw new CommandException("Doctors cannot have Condition or BloodType fields."); | ||
} | ||
if (editPersonDescriptor.getTags().isPresent() | ||
&& !editPersonDescriptor.isValidDoctorTagList(editPersonDescriptor.getTags().get())) { | ||
logger.warning(editPersonDescriptor.getTags().toString()); | ||
throw new CommandException("Please enter valid Doctor tags."); | ||
} | ||
editedPerson = createEditedDoctor((Doctor) personToEdit, editPersonDescriptor); | ||
Doctor doctorToEdit = (Doctor) personToEdit; | ||
validateDoctor(); | ||
editedPerson = createEditedDoctor(doctorToEdit, editPersonDescriptor); | ||
} | ||
if (!personToEdit.isSamePerson(editedPerson) && model.hasPerson(editedPerson)) { | ||
logger.warning("Edited Person and orignal person are the same"); | ||
throw new CommandException(MESSAGE_DUPLICATE_PERSON); | ||
} | ||
return editedPerson; | ||
} | ||
private void validatePatient() throws CommandException { | ||
if (editPersonDescriptor.getTags().isPresent() | ||
&& !editPersonDescriptor.isValidPatientTagList(editPersonDescriptor.getTags().get())) { | ||
logger.warning("Invalid tag for patient"); | ||
throw new CommandException("Please enter a valid patient tag."); | ||
} | ||
} | ||
private void validateDoctor() throws CommandException { | ||
if (editPersonDescriptor.getCondition().isPresent() || editPersonDescriptor.getBloodType().isPresent() | ||
|| editPersonDescriptor.getEmergencyContact().isPresent()) { | ||
logger.warning("Error thrown - tried to edit condition/blood type/emergency contact of doctor"); | ||
throw new CommandException("Doctors cannot have Condition, BloodType or Emergency Contact fields."); | ||
} | ||
if (editPersonDescriptor.getTags().isPresent() | ||
&& !editPersonDescriptor.isValidDoctorTagList(editPersonDescriptor.getTags().get())) { | ||
logger.warning(editPersonDescriptor.getTags().toString()); | ||
throw new CommandException("Please enter valid Doctor tags."); | ||
} | ||
} |
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.
gotta write new test cases for all these!
ec3d7ef
into
AY2324S1-CS2103T-T09-3:master
fixes #221