carriezhengjr / pe

0 stars 0 forks source link

Incorrect warning message for editing that results in duplicate identity #9

Open carriezhengjr opened 1 year ago

carriezhengjr commented 1 year ago

After the command add -p n/Tan e/tan@example.com p/98765432 l/1 c/Meta to add a person, I tried to edit another person's details to be the same as the newly added person with the command edit -p 11 n/Tan e/tan@example.com p/98765432 l/1 c/Meta.

However, the error message provides no information that user is attempting to edit a person to an already existing person with exact same details. Screenshot 2022-11-11 at 17.04.36.png

The error message should be specific to warn user that editing a person that results in the duplicate identity is not allowed.

nus-pe-script commented 1 year ago

Team's Response

The command wasn't used as intended. edit -p does not take in the l/INTERNSHIP_INDEX parameter.
In this case, 98765432 l/1 is parsed as the phone number, hence the correct error message is shown (incorrect phone number). If l/1 is omitted, the correct error message is shown (there is a duplicate person in InterNUS).

Using an unknown prefix after a valid argument prefix will throw an error if the inclusion of that invalid prefix violates the parameter constraints of the preceding parameter.
Eg: commandX a/paramAAA b/paramBBB tells the user that AAA should only contain ___, if slashes or spaces are not allowed in paramAAA and commandX does not take in the b/paramBBB parameter.

We will take this bug report to be a duplicate of #4562, where error messages do not tell the user of incorrect (unknown) prefixes.

The 'Original' Bug

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

Incorrect warning message for adding a person

The command add -p n/Bob e/bob@example.com p/98765432 i/interest test shows an error message to the phone number, but the phone number is actually valid. Hence, it does not align with the actual error which is the non-existing i/ flag in the App. Screenshot 2022-11-11 at 16.43.15.png


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

Their Response to the 'Original' Bug

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

This error happens because the unknown prefix i/ occurs after a valid prefix + parameter p/98765432.

Thanks for pointing it out.

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]