optionalemon / pe

0 stars 0 forks source link

Unclear error message for validation on parameters in `add person` command #9

Open optionalemon opened 1 year ago

optionalemon commented 1 year ago

Testing the validation method on phone number:

Screenshot 2022-11-11 at 17.01.26.png

Seems like alphabets are not allowed, but not specified.

Testing the validation method on student name: Screenshot 2022-11-11 at 17.03.29.png

Seems like spacing in student names without a " " is not accepted, similarly there is no accurate error message to tell the user what is wrong with the input, and this is harder for the users to detect by themselves as compared to the phone number issue.

soc-se-bot commented 1 year ago

Team's Response

Hi, thanks for the bug report!

We agree that we should be accepting phone numbers with characters. However, this only causes a minor inconvenience to the user as they can still enter one phone number easily. Moreover, the application is meant to be used by student team leads in software engineering projects. It is highly likely that they will be working with their peers and classmates, and will only need to store one phone number - the mobile phone number - in any case.

With regards to the error message for the spacing in names, acknowledge this is a bug, but unfortunately we believe it is out of scope for v1.4 With reference to the CS2103T website:

image.png

In this case, detecting when it is an invalid command (e.g. invalid flag) vs a case of missing quotes requires extra parsing logic that would not be allowed in v1.4 (this is also explained in our user guide in the included screenshot below).

image.png

Based on the above 2 points, we're shifting the issue severity to low (as we accept the first part of the bug report regarding allowing characters in phone numbers, while we believe the latter is not in scope). Hope you understand our rationale.

Have a nice recess week ahead!

Items for the Tester to Verify

:question: Issue severity

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

Reason for disagreement: Hello, thanks for explaining! However I think there is a misunderstanding over what issue I am focusing on and therefore I do not think i will agree with your argument :(

I am referring to the error message provided being too general and therefore unclear what is the issue that makes the command unable to pass the validation, and not whether the validation itself is rational. Along the way, I guessed that maybe people cannot have spacing in between without a quote (I understand that it is written somewhere in UG but there are contradicting information in UG too, which is why I did not catch it then) and maybe the phone number cannot have characters. It is unclear what we are allowed to do and what we are not allowed to do given the general error message.

Medium severity has been chosen because in the worst scenario, user may not be able to use the edit person and similar command effectively as they are unsure what are the exact inputs they could key in.

This is especially the case when the edit person command has person name without double quote in UG (which is another error, but makes it even harder to figure out the bug, screenshot provided below) and there is nothing talking about the restriction on phone numbers in UG.

Screenshot 2022-11-15 at 12.15.38.png

I believe that user could still continue to use the product but this would cause occasional inconvenience to the users if they could not figure out what's wrong with the command and remember the restrictions themselves, therefore medium severity has been chosen.