Skip to content
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

Morphing ID to public identity #168

Merged
merged 25 commits into from
Oct 19, 2020
Merged

Morphing ID to public identity #168

merged 25 commits into from
Oct 19, 2020

Conversation

hopinxian
Copy link

@hopinxian hopinxian commented Oct 18, 2020

This pull request aims to resolve #167
Users are now required to input a unique identifier for all locations and persons inputted into the system.

The following features come attached with the morphing of ID to a public unique identifier:

  1. Add person and location commands now require an additional input of a unique identifier.
  2. Delete location and person methods have been overloaded, allowing the user to either delete using index or unique identifier
  3. Visits can be created by either referencing the index or unique identifier of the person and location involved. A mix of both is not allowed.
  4. Usage of IDs by other info handling functions have been checked for any breaking.
  5. Adding via CSV has been morphed to ensure ids are required inside the CSV files.

This is currently a draft pull request.

@hopinxian hopinxian added priority.High Needs to be completed soon Implementation Coding required Prerequisite Must complete before issues referenced labels Oct 18, 2020
@hopinxian hopinxian added this to the v1.3 milestone Oct 18, 2020
@hopinxian hopinxian self-assigned this Oct 18, 2020
@codecov-io
Copy link

Codecov Report

Merging #168 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #168   +/-   ##
=========================================
  Coverage     74.70%   74.70%           
  Complexity      801      801           
=========================================
  Files           113      113           
  Lines          2479     2479           
  Branches        308      308           
=========================================
  Hits           1852     1852           
  Misses          538      538           
  Partials         89       89           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9dd634...708bccc. Read the comment docs.

hopinxian and others added 23 commits October 18, 2020 11:22
AddLocationCommandParser can parse and detect a string id which
it passes to the location for storage as a string id.
Tests have not been updated.
AddPersonCommandParser can parse and detect a string id which
it passes to the person for storage as a string id.
Tests have not been updated.
Add location and add person parsers can parse ids.
Location and Person objects now store an id instead of string
for the unique identifier.
Fix bug in generate commands due to misuse of id as index when
checking if person is infected.
VirusTracker can now be built, but there are failing tests.
More tests still failing but half cleared.
IDs used in tests are two characters with the first being a letter
and the second being a digit. Location IDs use L for the letter
(e.g. L1) while person IDs use S for the letter (e.g. S1).
Delete person command is now overloaded to take in either index
or id to identify person to be deleted.
Delete location command is now overloaded to take in either index
or id to identify person to be deleted.
…into v1.3/publicID

# Conflicts:
#	src/main/java/seedu/address/commons/core/Messages.java
#	src/test/data/JsonSerializableVisitBookTest/typicalVisitVisitBook.json
#	src/test/java/seedu/address/testutil/TypicalVisits.java
Add visit command can now take in either Id or index as input.
@hopinxian hopinxian marked this pull request as ready for review October 19, 2020 10:57
@hopinxian
Copy link
Author

This pull request is ready for merging.

@hopinxian
Copy link
Author

While all effort has been made not to regress VirusTracker in this PR, because ID is used widely across all aspects of the VirusTracker, it is recommended that all teammates check through the tests and functionality in detail for any breaking of code or bugs.

Copy link

@siangernlow siangernlow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@siangernlow siangernlow merged commit 66c5359 into AY2021S1-CS2103T-T13-1:master Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation Coding required Prerequisite Must complete before issues referenced priority.High Needs to be completed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Morphing of ID to public identity number
3 participants