search

harithh07 / pe

0 stars 0 forks source link

Using edit command with "leaves" as the only field results in an error #5

Open harithh07 opened 2 weeks ago

harithh07 commented 2 weeks ago

Steps to reproduce:

  1. Use edit command on a valid index with a valid input for leaves, with only "leaves" as the field to edit (e.g. edit 6 l/5)
  2. Error message appears indicating that at least one field must be provided.

UG does not state that this is intended behaviour.

Screenshot 2024-11-15 at 4.33.21 PM.png

Screenshot 2024-11-15 at 4.30.32 PM.png

soc-se-bot commented 1 week ago

Team's Response

Thank you for your feedback. Regardless, after reviewing the issue, we believe this behaviour of the app is working as intended and does not constitute a bug. Here’s why:

The user guide explicitly shows in the format that n/NAME p/PHONE_NUMBER e/EMAIL a/ADDRESS are required to edit. The command provided in the reproduction clearly violates the format.

The error message provided to the user clearly explains that required fields are missing.

Given the above points, we respectfully reject this issue as the reported behavior is not a bug but a deliberate and documented design choice.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

"edit" command unable to only edit "l/LEAVE" field

Steps to reproduce:

  1. Input "edit 1 l/11"

image.png

Result: Error given that at least one field to edit must be provided (as seen above).

Expected: Successful edit to change the 1st person's leaves to 11.

Elaboration: It is given in the User Guide that at least one of the optional fields must be provided. Since l/LEAVE is an optional field and provided, the functionality of the app does not work as specified. Additionally, it is very common as a manager to only change a person's leave count when they are taken, hence this functionality is quite commonly used. It can be worked around by editing another field to be the same.

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

Their Response to the 'Original' Bug

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

Thank you for your feedback. Regardless, after reviewing the issue, we believe this behaviour of the app is working as intended and does not constitute a bug. Here’s why:

The user guide explicitly shows in the format that n/NAME p/PHONE_NUMBER e/EMAIL a/ADDRESS are required to edit. The command provided in the reproduction clearly violates the format.

The error message provided to the user clearly explains that at least one required fields are missing.

Given the above points, we respectfully reject this issue as the reported behavior is not a bug but a deliberate and documented design choice.

Thank you for suggesting a future feature, which will make tracking leaves much easier. Hence the issue has been re-classified as a feature flaw with low severity as users are still able to use the app, while our response will be that this is not in scope.

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.NotInScope`] - [x] I disagree **Reason for disagreement:** While the User Guide does provide the format `edit INDEX n/NAME p/PHONE_NUMBER e/EMAIL a/ADDRESS [t/TAG]… [f/Boolean] [d/DEPARTMENT] [l/LEAVE]`, it seems that this bug is not intended behaviour. This format shows that `tag`, `favorite`, `department` and `leave` are all optional fields, and following your response, using only any of these mentioned fields as the only one in the `edit` command would not work, throwing the same error. However, this is not the case. The following commands : - `edit 1 d/Ops` - `edit 1 f/true` - `edit 1 t/Python` all work and do not throw an error although these are also optional fields. Furthermore, the error message in the app shows the wrong format for the parameters, indicating that ALL fields are optional with the square brackets. ![Screenshot 2024-11-19 at 1.07.09 PM.png](https://raw.githubusercontent.com/harithh07/pe/main/files/dfcc4c20-626c-429d-8ff8-b7fb23ba4feb.png) I would also like to add that in the User Guide, if name, phone number, email and address are all compulsory, that would mean that the `edit` command MUST have all these fields e.g. `edit 1 n/John Lim p/98827132 e/john@example.com a/Blk 30 Geylang Street 29, #06-40` would work, but `edit 1 n/John` would not as it does not have the compulsory fields. Hence, I disagree that this is not in scope, and I believe that this is a bug that is not intended behaviour.
## :question: Issue type Team chose [`type.FeatureFlaw`] Originally [`type.FunctionalityBug`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]
## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]