shaunlxw / pe

0 stars 0 forks source link

delete command: inaccurate error message #8

Open shaunlxw opened 4 months ago

shaunlxw commented 4 months ago

Input:

  1. delete 0
  2. delete a
  3. delete 1 2

Error message says invalid command format although format for 1 and 2 is correct, just that validation of index is wrong.

image.png

In potential errors of the UG, it says ensure that all required fields are provided. Fields are provided, but the value is wrong.

nus-se-script commented 4 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]

marka, unmarka: inaccurate error message

Similar to #11,

Input:

  1. marka/unmarka 1 a
  2. marka/unmarka 1 0

image.png


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

Their Response to the 'Original' Bug

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

The input provided is clearly invalid and does not follow the correct command format that is described in the UG. Furthermore, the error message shown also displays the correct command format that should be provided. In this case, since a character was provided for the WEEK_NUMBER parameter instead of an integer, it is indeed an incorrect command format.

Severity changed to Low since it does not affect normal operations of our product, and the error would not be encountered at all if the user followed our UG closely.

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: Similar bug, but not a duplicate. The feature in the "original" bug uses ParserUtil::parseIndexes(...) while the delete command in this bug uses ParserUtil::parseIndex(...). However, the underlying cause of the issue can be fixed independently.

image.png

Referring to screenshot below, the bug is due to catching the ParseException thrown from ParseUtil::parseIndex(...), which can be fixed independently for each command's parser. i.e. this bug can exist here, while not existing in the "original" bug.

image.png


## :question: Issue response Team chose [`response.Rejected`] - [x] I disagree **Reason for disagreement:** Similar to the "original" bug. Replicated below (with tweaks to fit this bug): Command format refers to the number and order of parameters, while input validation refers to whether the given value of input is within the valid constraints. This issue introduces invalid inputs instead of invalid command format. >The input provided is clearly invalid and does not follow the correct command format that is described in the UG. In the example inputs given above, the input is invalid, but the command format is in fact correct. The command format for `delete` is given as `delete INDEX` in which the example inputs given above satisfy. Hence the error message should not say invalid command format, but should instead say "The person index provided is invalid", similar to when `delete 7` is given if the index 7 does not exist. ![image.png](https://raw.githubusercontent.com/shaunlxw/pe/main/files/dcb147c5-7cf5-4bd0-a75a-f81142f404b0.png)