AngPengXuan / pe

0 stars 0 forks source link

Able to add names that are not unique #2

Open AngPengXuan opened 1 week ago

AngPengXuan commented 1 week ago

Screenshot from 2024-11-15 16-12-58.png

Above can be replicated with these commands: add n/John Doe p/98765432 e/johnd@example.com a/John street, block 123,#01-01 add n/john Doe p/98765432 e/johnd@example.com a/John street, block 123,#01-01

John Doe and john Doe should be considered the same person. The documentation specifies only unique names can be added without specifying if upper case or lower case affects uniqueness. Based on normal use case, I would assume that John Doe and john Doe are the same person and be considered unique, and should not be allowed to be added.

nus-se-script commented 5 days ago

[IMPORTANT!: Please do not edit or reply to this comment using the GitHub UI. You can respond to it using CATcher during the next phase of the PE]

Team's Response

This bug is linked to the unique identifier being case sensitive name. The intended fix has been elaborated in that issue.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

Person with different Captialisation is not flagged as duplicate

Inputs: First command: add n/John Doe p/98765432 e/johnd@example.com a/John street, block 123, #01-01 Then add n/JOHN DOE p/98765432 e/johnd@example.com a/John street, block 123, #01-01 (note the capitalised name)

Expected Output: This person already exists in the address book

Actual Output: New person added...

As real-world names are usually not case sensitive, this may cause the user to accidentally create a new person when a duplicate already exists (e.g. they accidentally hit the caps lock button midway through typing the name)

UG might also be misleading as it has the following disclaimer: image.png


[original: nus-cs2103-AY2425S1/pe-interim#749] [original labels: severity.Low type.FunctionalityBug]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

Decision for Response

Thank you for the feedback, our current implementation of Bizbook uses the name as the identifier and thus we wanted to allow as much flexibility in setting the name, thus the decision for case sensitive names.

However, we do agree that this is a feature flaw, and thus in a future iteration (stated in our planned enhancement), we intend to fix this, by switching the unique identifier to another more concrete variable, such as phone number. This change would then allow contacts of same name, as it is quite common for people of the same name irl.

image.png

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: [replace this with your reason]