ararchch / pe

0 stars 0 forks source link

Incorrect error message when wrong command entered for fitadd #7

Open ararchch opened 4 months ago

ararchch commented 4 months ago

Overview App does not recognise the true issue, and prompts user incorrectly as seen below in images.

Steps to reproduce

Expected behaviour

Actual behaviour

Potential solution

Screenshots

image.png

nus-pe-script commented 4 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]

Incorrect note parameter leads to fitbook ignoring the note rather than prompting the user of incorrect command format

Overview When the tag for note is incorrectly typed, fitbook does not seem to recognise that as the issue, but rather goes ahead to execute the command and ignoring the note. This is because the note is considered part of the address ( i think)

Steps to reproduce

  • Start up fitbook
  • Enter add n/Johffn Doe p/98765432 e/johnd@gmail.com a/John street, block 123,#01-01 ni/john from school

Expected behaviour

  • Error message saying that command format is wrong

Actual behaviour

  • Client is added to book

Potential solution Check for note or possible wrong formats before assuming it is the address

Screenshots

image.png


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

Their Response to the 'Original' Bug

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

We understand where the tester is coming from. Indeed it would be useful if users are corrected for minor typos, to the command prefix.

However, we are rejecting this for the following reasons:

  1. It's not a bug.

image.png

According to the UG, any text is allowed, so this input would be considered valid.

  1. This is intended behaviour.

Unless a new prefix is specified, all text is assumed to belong to the address.

Therefore, we chose to reject this bug.

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: This bug concerns the error message displayed while the other one is about the user being added to fitbook. I think that while they are similar, to consider them duplicates is a stretch.

image.png

based on the CS2103T website definition of a duplicate bug, the two in question here are neither the same bug, and would require independent fixes, hence I do not believe that they are duplicates.


## :question: Issue response Team chose [`response.Rejected`] - [x] I disagree **Reason for disagreement:** This issue is similar to the one reported for the phone number problem, hence the justification for why I disagree with the rejection is similar. I understand where the team is coming from - the prefixes after the squats are optional, so if the prefix is incorrect, fitbook will treat it as a squat number and throw the error message. While I can understand this sentiment as a developer, as a user I do not think that I will be able to justify this. The fact remains that I typed in a command with a valid number of sets and an invalid prefix but the system was unable to identify this error accurately, and misled me by saying that the issue was in the number of sets. As mentioned in the big report, this message does not highlight the true issue, and the user would spend time trying to figure out what is wrong with the number of sets, rather than looking at the prefix. It is not reasonable to assume that the user will be able to dissect the true error from a completely incorrect error message, and I believe that as developers it is our responsibility to consider all such edge cases and make sure they are accounted for. The CS2103T website also notes that error messages that are not specific to the problem are considered feature flaws. I do feel that this error message completely missed the exact problem so should be considered a featureflaw. ![image.png](https://raw.githubusercontent.com/ararchch/pe/main/files/40138e10-f14b-4ae3-939e-d41ddaf49908.png) While it is arguable that this is a feature to be implemented down the line rather than 1.4 (depending on other features implemented in v1.4), I do think that this is an improvement of the product and should not be outright rejected. ![image.png](https://raw.githubusercontent.com/ararchch/pe/main/files/c3f558bf-7605-4857-aafc-81a8ba68d519.png) Additionally, the 2103T website highlights the the system should detect easy to detect incorrect flags, and I would argue that `ra/` instead or `r/` fits this bill. ![image.png](https://raw.githubusercontent.com/ararchch/pe/main/files/775ad021-f3da-4565-a379-6d087fe52b16.png) In any case, I do feel that this is a valid improvement that the team can consider, and Hence I disagree with the teams position to outright reject the bug