Fallman2 / pe

0 stars 0 forks source link

Name field does not accept "/" key. #2

Open Fallman2 opened 11 months ago

Fallman2 commented 11 months ago

When entering a new tutor, an error is shown when the "/" key is attempted to be used in the name field. Considering that this software is to be used by tuition centres who employ tutors, the legal name is expected to be used as the name. Given that "s/o" is a relatively common term used in names, the "/" character should be accepted.

Steps to reproduce: Enter an add-t command with a / in the name field. Command input: add-t n/John s/o Doe p/98765432 e/johnd@example.com

image.png

soc-pe-bot commented 11 months ago

Team's Response

No details provided by team.

The 'Original' Bug

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

Name does not support s/o

For Indian names, it is possible that the legal names include the "/" character. As such, one should make a concession for this case where legal Indian names will be allowed

image.png


[original: nus-cs2103-AY2324S1/pe-interim#4794] [original labels: type.FunctionalityBug severity.VeryLow]

Their Response to the 'Original' Bug

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

In the app, there is no requirement for a legal name as the names are used purely to track and distinguish tutors.

In fact, enforcing the strict use of legal names would not allow people of the same name to be in the app (so users need a way to differentiate their names).

image.png

In the example raised about legal Indian names, users can easily solve the problem either by using so instead of s/o or not including the last part of the tutor's name.

Additionally, this was an intentional design choice. You may read more in the DG > Add tutor feature > Design Rationale > Aspect: Tutor name restrictions

Including too many special characters can lead to outcomes like erroneous parsing and it becomes more difficult for users to later retrieve this person using find commands.

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:** This bug should not be considered NotInScope for the following reasons: 1. This bug affects a function that is integral to the implementation of the feature of adding tutors as of v1.4. The adding of tutors is the very first feature that a user will interact with when using this app. Therefore, this bug is definitely relevant to the features presented in v1.4. 2. Even though the UG does state that having special characters including "/" is not deliberately not supported, considering the target audience of the app and the fact that it is intended for use in a professional environment where employment is involved, the legal name definitely should be supported as part of the app. This is despite the feature being a part of base AB3 because base AB3 was not intended for use in the same environment. Additionally, I will refer to the following two lines found under the bug triaging guidelines: ![image.png](https://raw.githubusercontent.com/Fallman2/pe/main/files/a8c6f8fe-0dca-48b0-9d1e-04f7fef2274a.png) 3. The flaw is still useful to a smaller extent without the bug being fixed. However, assuming that this app that is intended for use by tuition centre coordinators (the target audience) and intended for use in Singapore where the tuition industry is large, a significant percentage of people will be affected by the bug. Considering that even one entry being affected by this bug could cause a tuition centre to reconsider using the app (as legal names are extremely important when dealing with employment and business administration), I feel that fixing this flaw is to a large extent necessary for the app to be useful. 4. The feature could have been implemented to work in a better way without much effort. As stated by the developers themselves, the current implementation is intentional. However, it would not be difficult to change the regex used to validate names in order to make sure that this app works better. A quick search online would reveal several options to change the regex to in order to allow the use of specifically the "/" key, or other special characters that are relevant to names.
## :question: Issue type Team chose [`type.FunctionalityBug`] Originally [`type.FeatureFlaw`] - [x] I disagree **Reason for disagreement:** This bug is clearly a feature flaw as the restriction on special characters being used in names was deliberately implemented in the form of a regex in the base AB3 code. Classifying it as a functionality bug implies that excluding special characters in the name was unintentional which contradicts the statements made by the developers themselves as well as the evidence they have provided from the DG that claims that it was a deliberate choice.
## :question: Issue severity Team chose [`severity.VeryLow`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** Considering that 9% of Singapore's population is Indian, not being able to use the "/" key for names affects a non-insignificant number of people. Additionally, the bug can be considered serious contrary to the arguments provided by the developers. > In the app, there is no requirement for a legal name as the names are used purely to track and distinguish tutors. >In the example raised about legal Indian names, users can easily solve the problem either by using so instead of s/o or not including the last part of the tutor's name. The target audience for this app is **Tuition Centre Coordinators** as stated in the UG. Tuition centres employ their tutors as employees and thus, have to handle administrative matters concerned with employment such as paying their employees' salary and taxes or other related payments (CPF for Singaporeans). Additionally, tuition centres have to vet and check their tutors qualifications to determine the level at which they are able to teach their students, necessitating the need for the full legal name of tutors. Since this app is made for professional use for tuition centre coordinators, it is thus mandatory that this app is able to store the legal name of tutors. > In fact, enforcing the strict use of legal names would not allow people of the same name to be in the app (so users need a way to differentiate their names). While it is true that there are many people with the same legal name, there is more than one way to distinguish between two people with the same name. One of these ways could be via a tagging feature to tag to differentiate between tutors by other identifiers such as the subject they teach, or even making the subject a mandatory additional field. Additionally, each tutor's phone number and email should also be ways to differentiate between two tutors of the same name as it is highly improbable that both tutors share the same name, email and phone number. Therefore, not wanting to use a different method of differentiating two tutors of the same name should not be a reason to use non legal names, especially considering the importance of doing so in a professional setting where employment is involved. > Additionally, this was an intentional design choice. You may read more in the DG > Add tutor feature > Design Rationale > Aspect: Tutor name restrictions > Including too many special characters can lead to outcomes like erroneous parsing and it becomes more difficult for users to later retrieve this person using find commands. While this is correct, allowing the use of specific special character should not significantly affect the performance of the app, especially so if the app were developed to be used locally first. Support for more special characters used in names can be slowly added over time as the app sees wider adoption in the industry. Considering the above reasons, I believe that the severity of Medium is justified and that the severity chosen by the developers of Very Low is far too low as the bug is not merely a cosmetic one.