acekhoon / pe

0 stars 0 forks source link

Overzealous name validation for add method #6

Open acekhoon opened 2 months ago

acekhoon commented 2 months ago

Description: Person containing "s/o" in the name cannot be added into the list of CCA members (or legal name such as James Choi Junior. with peroid cannot be added as well). Regarding this feature, it might be the case where the person's "legal" name contains such delimiters, slashes or other special characters. You can refer to the paragraph in PE website regarding feature flow:

"Use of symbols in input values: It is acceptable to disallow certain characters in input values if there is a justification (e.g., because using those symbols in an input value makes the command harder to parse), but they can still be considered FeatureFlaw bugs if they cause inconvenience to the user. For example, disallowing s/o in a person name because / is used as a command delimiter can cause a major problem if the input is expected to match the legal name of the person."

Stepts to reproduce: Typed in command "add n/John s/o Doe p/98765432 e/johnd@example.com a/6 Sin Ming #01-01 c/NUS Cycling r/Treasurer r/Logistics d/Manages money"

Expected: New person added (with description)

Actual: Names should only contain alphabetical characters and spaces, and it should not be blank

Screenshot 2024-04-19 at 4.49.40 PM.png

nus-se-bot commented 2 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 restrictions are incorrect

The application should allow name field to include S/O and D/O. The purpose of setting the name to be proper case is to allow the user to key in student's full name.

Screenshot 2024-04-19 at 4.41.43 PM.png

Does not allow for names like: "John S/O Seetham" which is not uncommon.

This restriction is also not stated in the user guide.


[original: nus-cs2103-AY2324S2/pe-interim#1124] [original labels: type.FunctionalityBug severity.Low]

Their Response to the 'Original' Bug

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

For this project, we did not want to allow special characters as it will lead to a tendency where users will add insincere names such as "J@son M^" or "Josh&Drake!Mi@". If you wish to include s/o, you can always just say son of instead.

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.FunctionalityBug`] Originally [`type.FeatureFlaw`] - [x] I disagree **Reason for disagreement:** Rather than functionality flaw, I believe that this is feature flaw since it is incomplete feature that does not handle valid Name with special characters. One can refer to the course website for the PE to verify this claim: "Use of symbols in input values: It is acceptable to disallow certain characters in input values if there is a justification (e.g., because using those symbols in an input value makes the command harder to parse), but they can still be considered **FeatureFlaw bugs** if they cause inconvenience to the user. For example, disallowing s/o in a person name because / is used as a command delimiter can cause a major problem if the input is expected to match the legal name of the person." Moreover, the team's response for this bug is that they did not want to allow special characters, but the problem is that some legal names are written with some special characters, which would indeed potentially cause inconvenience to the users. For instance, Daniel s/o James, James Junior. Scarlette , Josè, Jôse, are all valid legal names (from other countries) but they are not supported by this product. Thus, I believe that this bug is feature flaw with possibly low severity.
## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.VeryLow`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]