search

kohkaijie / pe

0 stars 0 forks source link

Misleading error message when entering wrong index for edit command #3

Open kohkaijie opened 10 months ago

kohkaijie commented 10 months ago

Steps to reproduce: edit 0 n/Jackson

Expected result: Invalid index provided

Actual result: Invalid command format! edit: Edits the details of the person identified by the index number used in the displayed person list. Existing values will be overwritten by the input values. Parameters: INDEX (must be a positive integer) [n/NAME] [p/PHONE] [e/EMAIL] [a/ADDRESS] [th/TELEHANDLE] [t/TAG]... [c/ORIGINAL_COURSE-NEW_COURSE]... Example: edit 1 p/91234567 e/johndoe@example.com c/add-MA1521 c/del-MA1521 c/MA2001-MA1521

The error message returned should have been about a wrong index entered as the command format used was correct.

image.png

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

Error message for invalid indexes is misleading

Index 0 and negative indexes should be recognised as an invalid index error and not give an invalid command error because the command used was edit, which is a valid command. This can cause confusion to users as the error message is misleading.

Screenshot 2023-11-17 at 4.31.38 PM.png

Screenshot 2023-11-17 at 4.31.21 PM.png Screenshot 2023-11-17 at 4.31.04 PM.png

[original: nus-cs2103-AY2324S1/pe-interim#5262] [original labels: severity.Low type.FeatureFlaw]

Their Response to the 'Original' Bug

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

The error messages are all correct.

For INDEX values 0 and -1, the index provided is not a positive integer and thus does not follow the command format specified. The command format clearly specifies that

INDEX (must be a positive integer)

and the exact command format is also seen in your screenshot.

With reference to the course website,

if a date input is required to be in YYYY-MM-DD format, 2021-13-28 is a format error (reason: MM should be in 1..12)

Similarly, for the delete command, it has to be in delete INDEX format. delete 0 is a format error (reason: INDEX should be 1...MAX_INT).

Please also refer to the UG, where we stated that INDEX has to be an unsigned positive integer.

Screenshot 2023-11-20 at 11.02.41 AM.png

These INDEX values clearly fail the constraints specified in the command format. Therefore, the commands have invalid formats.

For INDEX 10, while 10 is a positive integer and fulfils the command format, there is no student with an index 10 in a list of 8 students. Therefore, it is an invalid index.

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 response Team chose [`response.Rejected`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]
## :question: Issue type Team chose [`type.FeatureFlaw`] Originally [`type.FunctionalityBug`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]