search

woojiahao / pe

1 stars 0 forks source link

converttoclient under a view will cause the currently viewed person to be altered even if the index is not correct #7

Open woojiahao opened 10 months ago

woojiahao commented 10 months ago

Prerequisite: second person is a lead

  1. view 2
  2. converttoclient 1
  3. Convert command works and the second person becomes a client
nus-pe-script commented 10 months ago

Team's Response

Thank you for pointing this out. This is essentially the same issue as #5848 as after view command, the current list will only contain the current person, so convertclienttolead 1 will change the person viewed.

The 'Original' Bug

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

Message from the app is not showing correctly.

Here is the the list.

image.png

image.png

I view the person with index 7. After I view the person, I got this.

image.png

After that I try to view person 1, the app says that I am able to view it successfully but I am still viewing person 7 according to the list in the first screenshot.

image.png

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

Their Response to the 'Original' Bug

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

Thanks for pointing this out. While we do agree that this is an issue for the user, we disagree with the severity being "Medium". However, we feel that the severity is "Low" instead based on the following points: 1) Firstly, the user may not know what previous list that the user was viewing might be, thus not being able to even view the person based on the index of the previously viewed list 2) This happens very rarely as most users will go back to the list first before viewing the specific person. 3) It causes a very minor inconvenience to the user as no data is manipulated and the user can clearly tell that he/she is viewing the same user as before.

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.FeatureFlaw`] Originally [`type.FunctionalityBug`] - [x] I disagree **Reason for disagreement:** This is a **FunctionalityBug** because the `converttoclient` command's intended usage is as such: ![Screenshot 2023-11-22 at 22.10.15.png](https://raw.githubusercontent.com/woojiahao/pe/main/files/424f4870-c7dc-4f8c-badd-bae3ef942389.png) In the UG, it specifies that the index should be from `[1, last index on filtered address book list]`. However, they do not specify nor clarify that after applying view, the list will be filtered and as such, only have 1 remaining entry. It is this ambiguity that makes it NOT work as specified (by definition of a **FunctionalityBug**) since without clarification, users will be under the impression that the previous address book list is still available, hence why they would try `converttoclient n` where `n != previously viewed index` and the outcome (i.e. converting the viewed entry rather than converting the intended entry) goes against the expected output given the expectation set in the UG. A user who tries to convert person at index 1 to a client while viewing the person at index 2 should not accidentally change their current user (2) to a client.
## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** > Firstly, the user may not know what previous list that the user was viewing might be, thus not being able to even view the person based on the index of the previously viewed list I fail to see how this is an issue on the user side. The user knows what the previous list was since they would have been the one issuing commands. > This happens very rarely as most users will go back to the list first before viewing the specific person. Just because it may be a rare occurrence, does not make it impossible. If I was a power user who wants to use the application (and this was properly supported), I would not want to backtrack so often and instead, remember a few entries and perform my operations immediately without returning to the original people list. > It causes a very minor inconvenience to the user as no data is manipulated and the user can clearly tell that he/she is viewing the same user as before. While this may be true, it does not stop it from behaving out of the realm of expectation. As I have laid out in my reasons for why this is a **FunctionalityBug**, the user's expectation with the `converttoclient` command is to be change the details of the persons given the "last index of the filtered address book list". Given that there is no clarification that a filter is applied after view, the users would be under the impression that the underlying address book list would contain more than just one entry and as such, continue to apply the `converttoclient` command and fail at updating the right user. There is clearly data manipulated and while it is reversible, it is does cause inconvenience to the user who is trying to use the command as they would have to figure out why the issue is occurring (some users would not even know where to start to debug) and then backtrack (which can be quite tedious if they have issued the command wrongly several times already). It also should not be up to the user to debug an erroneous path in the system since they may not be technically inclined. Hence, I don't think this is a **Low** severity issue.