Chronoxy / pe

0 stars 0 forks source link

[Error Message: Add Command] Error Message not specific #8

Open Chronoxy opened 4 days ago

Chronoxy commented 4 days ago

Background

Based on UG, mentioned that there should be 3 kinds of error messages. The last of the 3 is the specific error message. image.png

Test Case

add n/John Lee nric/S9369777D p/91234567 a/123 Medical D role/PATIENT t/Chicken

Expected Behaviour

Error message: Missing required field: Email

Encountered Behaviour

Invalid command format!

image.png

Suggestion

Best would be to specify the missing fields, but minimal changes would be to change your UG

nus-pe-script commented 13 hours ago

Team's Response

I agree there's a discrepancy between the documentation and actual behaviour, but I would classify this as a type.DocumentationBug with severity.Low rather than a feature flaw. This is a documentation bug because:

The current error handling behavior (showing 'Invalid command format!') is following standard CLI conventions and provides clear feedback to users The UG needs to be updated to accurately reflect the different error scenarios and their corresponding messages The actual implementation is working as intended - it's the documentation that needs to match the implementation

I would classify this as severity.Low because:

  1. Users still receive helpful error messages that guide them to the solution
  2. The command format example is shown, making it clear what's missing
  3. The documentation inconsistency doesn't prevent users from using the application
  4. Users can still understand and correct their mistakes
  5. It's primarily a cosmetic issue with the documentation rather than a functional problem

The fix would be to update the UG to accurately document both cases:

  1. When command parameters are missing entirely
  2. When required fields are present but empty

    Items for the Tester to Verify

    :question: Issue type

Team chose [type.DocumentationBug] Originally [type.FeatureFlaw]

Reason for disagreement: I believe the User Guide was updated with the intention to include specific error messages but it may have lapsed somewhere along the way. Otherwise, it would be added to the User Guide. Furthermore, as mentioned in the Bug Triaging advises, "Error Messages can be correct but not specific enough", these cases can be considered Type.FeatureFlaw. Therefore, I firmly believe this should remain as a type.FeatureFlaw bug report.


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** Thank you for your response. Quote: "The current error handling behavior (showing 'Invalid command format!') is following standard CLI conventions and provides clear feedback to users" I believe the current error handling behavior (showing 'Invalid command format!') is following standard CLI conventions but I disagree with it providing clear feedback to users. Providing clear feedback would be telling the user what went wrong instead of the entire thing being wrong. By just showing 'Invalid command format!' does not direct the user to the issue. Especially using the `add` command, with many fields required like `name`, `NRIC`, `Phone number`, `email`, `address`, `role`, and `tag`. Requiring the user to trace through to find out what is missing can cause a lot of inconvenience. Quote: "The command format example is shown, making it clear what's missing" I agree that with the command format example shown, it may help slightly for the user to find out what is missing. However in the event that the user decide to use different sequences when adding the contact, it will cause a lot of issues. For example: 1. `add n/ p/ e/ a/ role/ NRIC/ t/` 2. `add NRIC/ n/ role/ a/ e/` 3. and many more just these examples have shown that the command format example shown of `add n/ nric/ p/ e/ a/ role/ t/` is very different. Requiring a "matching game" to get the commands right. With these reasons, i firmly state my stance to keep the severity at `Medium`.