This repository has been archived by the owner on Jan 11, 2021. It is now read-only.
forked from nus-cs2103-AY2021S1/tp
-
Notifications
You must be signed in to change notification settings - Fork 4
Create and modify fields for Person #119
Merged
vanGoghhh
merged 21 commits into
AY2021S1-CS2103T-T17-3:master
from
LeeJoonJie:branch-new-person-fields
Oct 16, 2020
Merged
Create and modify fields for Person #119
vanGoghhh
merged 21 commits into
AY2021S1-CS2103T-T17-3:master
from
LeeJoonJie:branch-new-person-fields
Oct 16, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Create Experience class. The new experience field in Person represents the number of years of experience the person has in that job. It has to be a non-negative number to be valid. Updates were also made to the relevant tests.
Create ExperienceTest class
The UrlLink class is used to represent a Person's personal url page. It is an optional field for Person. Tests were also updated.
The updated isValidLink method uses the UrlValidator class from the apache commons library to validate the format of the link. The apache commons validator (v1.7) dependency was also added to the build.gradle file.
Test UrlLink in new class. Also, update old tests with the UrlLink field.
Previously, UrlLink field in Person cannot be reset/cleared after adding a valid url link. Now the command "edit can {index} link/" without any arguments will clear the url link. The urlLink field will be changed back to null in personaddressbook.json.
When adding candidate with "link/" empty link prefix, there is a null pointer exception. This was fixed by changing Optional.of method to Optional.ofNullable method that is able to accept null as parameter.
The Salary class is a data class used to represent expected salary of a Person. The new Salary field was added to Person and is optional.
Create SalaryTest class to test new Salary field. Also update old tests with the Salary.
Originally, the Address field for candidates was a compulsory field. It is not optional.
Previously, the usage message for Edit Job shows that all fields except Tag were compulsory. This is incorrect. All fields should be optional for Edit Job command (at least 1 field present). This has been changed to all fields optional.
Logic for the resetting of optional fields such as "link/" and "sal/" in Person has been shifted from ParserUtil.java to EditPersonCommand.java. This is so that other commands (apart from "edit can") will not be affected by it as this might cause bugs. These optional fields can only be reset in "edit can" commands so the logic should be localised in the EditPersonCommand class, not in the ParserUtil class used by multiple commands.
Codecov Report
@@ Coverage Diff @@
## master #119 +/- ##
============================================
+ Coverage 75.26% 75.83% +0.56%
- Complexity 576 626 +50
============================================
Files 91 94 +3
Lines 1767 1924 +157
Branches 207 235 +28
============================================
+ Hits 1330 1459 +129
- Misses 390 407 +17
- Partials 47 58 +11
Continue to review full report at Codecov.
|
vanGoghhh
approved these changes
Oct 16, 2020
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!
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Created Experience, Salary and UrlLink classes for use in Person. Modified Address field to be optional for Person.
Closes #105
Closes #111
Closes #114
Closes #117
Closes #57
Closes #58