UdhayaShan1 / pe

0 stars 0 forks source link

`PHONE` should not be duplicate flagged #16

Open UdhayaShan1 opened 2 months ago

UdhayaShan1 commented 2 months ago

I believe we should not check for PHONE duplicates. As it may be possible for people to have the same number especially if someone was using a prepaid number which is then used by another.

image.png

A improvement is to check duplicates based on EMAIL where EMAIL makes sense to be duplicate flagged as its less likely to create the same email address.

nus-se-script commented 2 months ago

Team's Response

Refer to #3518.

The 'Original' Bug

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

Over-zealous rejection of phone input (disallowing users from having the same phone number)

Enter "add n/John Doe4 p/ 123456 e/qwertyuiopasdfghjklzxcvbmqwertyuiopasdfghjklzxcvbmqwertyuiopasdfghjklzxcvbmqwertyuiopasdfghjklzxcvbmqwertyuiopasdfghjklzxcvbmqwertyuiopasdfghjklzxcvbmqwertyuiopasdfghjklzxcvbmqwertyuiopasdfghj@example.com a/01/01/2024 a/02/01/2024 t/friends t/owesMoney", which is a person contact with the same phone number as "4. JohnDoe3", and the input is rejected.

However, 2 young volunteers from the same family might have registered under their parent's phone number. Blocking that input might not add any value but allowing it does. As this is a rare situation, the severity has been indicated as VeryLow.

A quick fix would be to revert back to the original AB3 which allows for duplicate phone numbers.

Screenshot 2024-04-19 at 4.30.51 PM.png


[original: nus-cs2103-AY2324S2/pe-interim#2920] [original labels: type.FeatureFlaw severity.VeryLow]

Their Response to the 'Original' Bug

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

Thanks for the catch.

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 response Team chose [`response.NotInScope`] - [x] I disagree **Reason for disagreement:** **The developers should have provided more justification as to why this was NotInScope in their response or UG.** I believe this is a inScope issue as this a overzealous parsing of the `PHONE` input as the original poster has correctly indicated. ![image.png](https://raw.githubusercontent.com/UdhayaShan1/pe/main/files/1d3aeb05-d616-4e66-b780-912e1c8ab2e8.png) To argue for NotInScope, ![image.png](https://raw.githubusercontent.com/UdhayaShan1/pe/main/files/0c3f1d1c-f4bd-44c8-b65a-783b7d6b989d.png) They should at least mention, in the UG or dev response, why this is the case, by justifying effort considerations or some real world context. Since they did not I do not see grounds for `NotInScope`. All they was mention we do not accept duplicates.
## :question: Issue severity Team chose [`severity.VeryLow`] Originally [`severity.Low`] - [x] I disagree **Reason for disagreement:** I disagree, this is not a cosmetic issue in any way. ![image.png](https://raw.githubusercontent.com/UdhayaShan1/pe/main/files/cd78940b-92f6-46e2-bb29-13687fd40e3e.png) This is at least a FeatureFlaw by ![image.png](https://raw.githubusercontent.com/UdhayaShan1/pe/main/files/455b05ab-ec1b-4ac5-af9f-d7a33a10469a.png) Hence, it should be at least a `Low`.