mmaimer33 / pe

0 stars 0 forks source link

Mismatched handling of index 0 in `status` command #6

Open mmaimer33 opened 1 year ago

mmaimer33 commented 1 year ago

If a command such as status 0 s/U is run with 0 as the index the error message is as such:

"Invalid command format! status: Handles lead statuses, allows for setting of statuses. Parameters: INDEX s/STATUS_TYPE Example: status 1 s/Qualified"

This does not highlight the issue with the command and does not match the error message when the index is some other invalid number (e.g. more than the number of items in the list)

image.png

nus-se-script commented 1 year ago

Team's Response

While it is acknowledged that the error could've been caught with an alternative message tailored just for out-of-bounds index, this is a minor detail. Moreover, current error message provided is clear about the syntax used, and the usage of the rest of the app and commands are clear that the index is a positive integer.

Reassigned as 'VeryLow' because this hardly impacts user experience. Besides the error message, the function works exactly as expected. It didn't execute for an OOB error as shown by the highlighting of red of the text.

Response assigned as NotInScope. This minor mismatch in error message is far from the top priority of things to implement for v1.4.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: [replace this with your explanation]


## :question: Issue severity Team chose [`severity.VeryLow`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** Not a purely cosmetic problem: ![image.png](https://raw.githubusercontent.com/mmaimer33/pe/master/files/7b77914b-9859-47d0-a608-77dfc813241a.png) `severity.Low` might be appropriate. I chose medium because it doesn't give an indication of what is going wrong, so a user might not realise what they are doing wrong (e.g. if they think that the list is 0-indexed).