search

AY1920S1-CS2103-F09-4 / main

Insurelytics
https://ay1920s1-cs2103-f09-4.github.io/main/
MIT License
1 stars 5 forks source link

Branch input validation #75

Closed chowyiyin closed 5 years ago

chowyiyin commented 5 years ago

Added policy class and new fields to person. Added addpolicycommand. UI can now display new person fields. Tests are incomplete (6 still failing).

chowyiyin commented 5 years ago

actually im not sure how json works so idk how to the policies are supposed to be converted to the jsonadaptedpolicy🤔

On Sun, Oct 6, 2019 at 6:14 PM Chaitanya Baranwal notifications@github.com wrote:

@chaitanyabaranwal commented on this pull request.

In src/test/data/JsonSerializableAddressBookTest/typicalPersonsAddressBook.json https://github.com/AY1920S1-CS2103-F09-4/main/pull/75#discussion_r331780955 :

 "phone" : "87652533",

"email" : "cornelia@example.com", "address" : "10th street",

  • "tagged" : [ "friends" ]
  • "date of birth" : "14.2.2019",
  • "policies" : [ ["Infant Insurance", "Infant Insurance", "years/2", "$1500"] ],

Why is this wrapped under an extra set of square brackets?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AY1920S1-CS2103-F09-4/main/pull/75?email_source=notifications&email_token=AM4VMSZIU5B7URVJJ2RPV23QNG3APA5CNFSM4I5WGAL2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCHAILSQ#pullrequestreview-297829834, or mute the thread https://github.com/notifications/unsubscribe-auth/AM4VMSYCXTLBSZRQI564EK3QNG3APANCNFSM4I5WGALQ .

larrylawl commented 5 years ago

wow this is alot of work! thanks so much yi yin! I didn't have much time to look through the code in detail, but I did smoke tests and they work fine (other than input validation for phone number).

i'd also suggest to add // TODO: ... in places which the team needs to look into further in the future (such as the tests which are failing), so that we don't forget about what exactly needs to be fixed.