TY1Fan / pe

0 stars 0 forks source link

Feature Flaw: Phone number restriction at 16 #4

Open TY1Fan opened 2 weeks ago

TY1Fan commented 2 weeks ago

image.png

Steps to reproduce:

  1. Launch the app
  2. Enter this command:
    add_student n/Betsy Crowe t/friend e/betsycrowe@example.com p/12345678901234567 t/likesMath
  3. Error saying that max length of 16 digits is thrown.

Expected: According to the World Population Review, the longest phone number in the world is 17 digits (North Korea).

Actual: Error is thrown.

Suggestion/ Why it is problematic: According to your value proposition, you did not mention that your app is not available to North Koreans. Hence, by restricting your phone number to less than 16 digits would mean that North Koreans are unable to use your app. While most user are still able to use the app , I feel that the restrictions would still affect the usability for users with contacts from North korea.

nus-pe-bot commented 1 week ago

Team's Response

While it is true that the longest North Korean numbers have at most 17 digits, the rest of the world should follow the international guidelines on phone numbers.

International specifications provide guidelines that [international phone numbers are 15 digits long.](https://en.wikipedia.org/wiki/Telephone_numbering_plan#:~:text=The%20International%20Telecommunication%20Union%20(ITU,15%20digits%20to%20telephone%20numbers.)

In addition, North Koreans are unlikely to be users given that sanctions/international trade are not permitted with users of the country, hence the chances of a North Korean using our app and encountering this problem is very little and very rare. Hence the severity is reduced to low.

However, a future iteration may improve on this to allow 17 digits.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: Thank you for agreeing with my view that this feature is indeed flawed. However, I feel that the response to this issue should be Accepted instead because of the following:

public static final int MAXIMUM_LENGTH = 16; public static final String VALIDATION_REGEX = "\d{3,}";

  • The changes required to ease the phone number restriction does not require much effort as the dev team only needs to update the validation regex, help message and documentation, which should not take more than 7 lines of codes.

Based on the course website (2nd point of the screenshot), image.png

I feel that this issue should be Accepted, since the dev team has agreed that this feature could have been made better (as evident from their choice of NotInScope) and the changes could be implemented without much additional effort (as justified above).


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** >In addition, North Koreans are unlikely to be users given that sanctions/international trade are not permitted with users of the country, hence the chances of a North Korean using our app and encountering this problem is very little and very rare. In response to this point, while North Korea is sanctioned from international trade, North Koreans can still study abroad and may require tuition (based on this [article](https://www.youngpioneertours.com/can-north-koreans-travel/#:~:text=There%20is%20a%20large%20number,massive%20North%20Korean%20student%20populations)). Therefore, the user in this case might be non North Korean tutor with North Korean students. Justification for `Medium`: 1. As SWE, while it is important to keep up with foreign affairs, but I feel that the software built should still meet the requirements as specified. Since the dev team specified their target user as `Tech-savvy Tuition Teachers` which **did not exclude North Korean users or users with North Korea contact**, I feel that the phone number restriction is too restrictive and did not meet the requirements / expectations of target users as specified in the DG. 1. While it is true that this restriction won't affect much users, but I feel that it should still be classified as `Medium` because such a restriction would deter some users from using this app. For instance, imagine if you are a tuition teacher with 1 - 2 North Korean students, and the dev team notify you of such a restriction, would you still want to use the app? I guess the answer would be `no` since I probably won't even be able to save my students' contact, and I definitely won't want to use a separate app to record the contacts of my North Korean students. 1. Furthermore, `Low` severity are for > A flaw that is unlikely to affect normal operations of the product. Appears only in very rare situations and causes a minor inconvenience only. I feel that the overly restrictive phone number restriction would mean that users can't save the contact of some students. Hence, this would affect the normal operations of the product and cause inconvenience to **some user** where he/she would have to use another app to record the contacts of student's whose phone numbers are rejected. However, since this flaw would not be applicable to **most users**, Ive decided to classify it as `Medium` instead of `High`. Based on the reasons stated above, I would like to insist on my view that the severity should be `Medium` instead of `Low`.