lwlshawn / pe

0 stars 0 forks source link

Language allows any string #1

Open lwlshawn opened 3 years ago

lwlshawn commented 3 years ago

Although this is explicitly stated in the UG, perhaps restricting it to alphanumeric, or even strictly only alphabets might be better, given the form we expect a language to come in.

image.png

nus-pe-bot commented 2 years ago

Team's Response

In reality, it is possible that the language spoken by an elderly is not one that is well-documented. Furthermore, we want to give the users flexibility to customise the language options and possibly add in side notes on language.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: Unfortunately, their explanation is unconvincing to me. I don't know what they mean by "language spoken by an elderly is not one that is well-documented". Even dialects or rarer languages have a simple spelling in english (Hokkien, Urdu, etc.). Giving the users flexibility to customise the language option and add in notes still wouldn't require anything other then alphabetical characters, or at most possibly numeric characters as well.

My perspective remains that if a special character is used in the language field, it is more likely an error on the part of the user, rather then a intentional choice. Thus I believe that guarding against it would likely provide a net benefit for users, albeit only in rarer circumstances.

In fact, on reflection I felt like I might have classified this as "verylow" only to be nicer to the group. This isn't a cosmetic issue, and is a case where I think reasonable input validation was lacking. In the same way the original AB3 bothers to have reasonable validation like ensuring the phone number field consists only of digits, and is at least 3 digits long, or how it checks that the "Name" field of a "Person" class contains only alphanumeric characters, I think input validation here is a reasonable expectation, and should even be a "low" bug.

If this is allowed, I want to also argue for reclassifying this bug as "low" and not just "verylow".