LemonDrew / pe

0 stars 0 forks source link

Duplicates are allowed when editing a person #5

Open LemonDrew opened 3 days ago

LemonDrew commented 3 days ago

Steps to reproduce:

  1. Use the add command to add a person into the addressbook
  2. For all compulsory fields except for the name field, copy the inputs from an existing person in the addressbook
  3. For the newly added person, edit the person's name to the one which you have copied the inputs from, only that one of the characters in the name differs by capitalization (eg if the person's name which you are copying from has name "Alex Yeoh", then the name that you input in edit should be "Alex yeoh"

The edit command allows the name of the edited person to be essentially the same as the person which you have copied from. (eg "Alex Yeoh" should be the same as "Alex yeoh". Yet, the user guide claims that 2 person of the same name should be considered duplicates (it was mentioned that the constraints of the add command should be the same as the edit command) and that all name must be unique.

Edited a person to have name "Alex yeoh" when there is already a "Alex Yeoh" in the addressbook. image.png

Constraints stated in the "Add command" which was stated to apply for edit command as well

image.png

nus-se-bot commented 11 hours ago

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 explanation]


## :question: Issue type Team chose [`type.FeatureFlaw`] Originally [`type.FunctionalityBug`] - [x] I disagree **Reason for disagreement:** This should be considered as a functionality bug since the behavior of the edit command does not follow that stated in the documentation. In fact, even the add command treats same names with different capitalization as non-duplicates. ![image.png](https://raw.githubusercontent.com/LemonDrew/pe/main/files/16af95e9-2ef0-4d52-8235-eeafdb368c81.png) It was stated that the same names will be considered as duplicates. Since there was no specification on whether the names are case-insensitive, it should be clear to the user that the same names (regardless of whether they have the same capitalization or not) are treated as duplicates. Therefore, I feel that the edit command does not follow the behavior stated in the UG and hence this should be a functionality bug.
## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** Though the dev team stated that they will switch the unique identifier to another more concrete variable in the future such as phone number, the current implementation already poses a lot of issues for the user, especially since the only duplicate check now is the name field (and they have to be of the same capitalization), which is already very restrictive and narrow. This poses a lot of issues because users are unlikely to use the exact same capitalization whenever they are adding or editing a person's name in the address book (for instance, when I add/edit a person name to be "alex yeoh", it is unlikely that I will use the same exact capitalization such as "Alex Yeoh" which I have added/edited a few days ago). This results in a scenario where it is extremely easy for the user to add duplicates in the addressbook (eg. same name, same phone number, same email address and same address). Furthermore, with multiple duplicates now in the address book, this poses a hindrance to the user as the user now has to figure out which person is which. In fact, if the user were to add tags to a single person, he may instead add it to different person with the same name instead (which will hinder the user as he now has look through all the duplicates and figure out which is the original one) Therefore, since the error is easy to produce (we simply use a different capitalization to add duplicates) and it also hinders the user (the user now has to look through all the duplicates and perhaps take a lot of time and relabel or delete the duplicates), I feel that this bug should be a medium.