QinHaichen12 / pe

0 stars 0 forks source link

Wrong display message when fields are missing in add command #3

Open QinHaichen12 opened 1 week ago

QinHaichen12 commented 1 week ago

Description

As per the user guide, a missing required field error message should be displayed when a required field is not given, however an invalid command format message is displayed instead

Command:

add n/John Doe p/9875432 e/johnd@example.com a/311, Clementi Ave 2, #02-25 t/backPai role/patient

image.png

image.png

nus-pe-script commented 1 week 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: Add Command] Error Message not specific

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


[original: nus-cs2103-AY2425S1/pe-interim#827] [original labels: severity.Medium type.FeatureFlaw]

Their Response to the 'Original' Bug

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

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 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 type Team chose [`type.DocumentationBug`] Originally [`type.FunctionalityBug`] - [x] I disagree **Reason for disagreement:** Thank you for reviewing the bug and providing your insights. According to the PE guidelines, the distinction between a functionality bug and a documentation bug depends on what needs to be corrected. From the perspective of an end-user, having an error message that clearly notifies them of missing fields is far more intuitive. As a result, users are inclined to believe that the UG is written correctly. Additionally, I have tested the behavior described in the updated UG by intentionally omitting required fields or leaving them blank. The behavior I observed was the system providing specific error messages for each field. For instance, when a blank name was entered, the following error message was displayed: > "Names should only contain alphanumeric characters and spaces, and it should not be blank." This behavior suggests that specific feedback for invalid or missing fields is the intended functionality, not a generic error message like "missing fields required." From the user's perspective, this generic error message would instead seem relevant only when an **entire command parameter** is missing. Therefore, the mismatch here is in the **functionality** rather than the documentation. While it might be easier to amend the UG to remove references to the generic error message, users would still find the generic message less useful and inconsistent with the specific feedback they receive elsewhere. The end-user would have no way of knowing the UG is at fault; instead, they would assume the system's behavior is incorrect. ![image.png](https://raw.githubusercontent.com/QinHaichen12/pe/main/files/9e835734-c2ec-4b4f-bf93-2d1c039d61f4.png) Furthermore, as per PE guidelines, in cases where a bug could reasonably fit into more than one category, the category that aligns most closely with the tester's perspective should be retained. In this case, the inconsistency in the system's behavior suggests that this is a **functionality bug**, not a documentation bug. ![image.png](https://raw.githubusercontent.com/QinHaichen12/pe/main/files/507928e9-842b-4290-a51b-8314f9d0af89.png)