search

RichDom2185 / nus-ay2223s1-cs2103t-pe

This repository was solely used as part of the CS2103T course curriculum at National University of Singapore.
0 stars 0 forks source link

Overzealous input validation #3

Open RichDom2185 opened 2 years ago

RichDom2185 commented 2 years ago

image.png

The restriction of phone number format seems to be overly restrictive, as the User Guide did not mention where exactly the target users may be staying. A lot of other countries have phone numbers that exceed (or are shorter than) 8 digits. Even in Singapore alone, since the app is supposed to manage contacts of students; a number of students may only have international numbers (e.g. if they just came to Singapore and have not gotten a local number yet).

Let alone the NOKs of these international students which may not even reside in Singapore at all.

soc-se-bot commented 2 years ago

Team's Response

They both point out the restrictive nature of our validation for the phone number.

The 'Original' Bug

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

Overzealous input validation of phone numbers

Better to warn instead of block any numbers that do not follow the 8 digits starting with 6,8, or 9

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

Their Response to the 'Original' Bug

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

We intend to only allow Singapore phone numbers but forget to include this point in UG. Thank you for your suggestion!

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 severity

Team chose [severity.VeryLow] Originally [severity.Medium]

Reason for disagreement: Team did not explain why severity should be downgraded to very low. As this is not a purely cosmetic bug, per the module website, this definitely would not be very low severity.

As it was not mentioned in the User Guide, this could potentially exclude a lot of potential users from being able to use the app meaningfully. Even in the Singapore context, restricting to only Singapore phone numbers for both NOK and student phone numbers will quite significantly restrict the types of students that can be logged in the data as international students/students with international NOK make up a non-negligible proportion of the Singapore's student population, let alone those going for private tuition. Hence, I stand by my original severity of medium.