woojiahao / pe

1 stars 0 forks source link

converttolead under view will edit the wrong user #16

Open woojiahao opened 12 months ago

woojiahao commented 12 months ago

Prerequisite: second person is a client

  1. view 2
  2. converttolead 1
  3. Convert command works and the second person becomes a lead

This command can cause the user to not be able to reliably edit their users and cause confusion

nus-pe-bot commented 11 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]

delete at person view page

For a list with at least one person

view 2
delete 1

would delete the person being viewed.

This is unexpected as I expected to delete the first person of the previous list or not delete any one at all since the UG said "filtered list shown in the address book" when no filtered list is shown


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

Their Response to the 'Original' Bug

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

Thanks for bringing this up! We feel that while this is may be severe when the user deletes a wrong client/lead as it deletes all their corresponding information, however it does not warrant a severity of "Medium". We feel that this should be of severity "Low" due to the following reasons: 1) It is very rare for the user to delete a different person after viewing a person (e.g. view 1 and delete 2) 2) Based on the intended sequence of steps the user takes to delete a person, after viewing a person (e.g. view 1 which views Person A), IF the user wishes to delete a person, it makes sense that the person the user wishes to delete is Person A (the one the user is viewing), which is what happens (although we agree that having the INDEX after the delete should be checked for). It is definitely unexpected that the user views person A and proceeds to delete person B.

We could have additionally mentioned this in our UG that "delete" deletes from the filtered list and the "view" restricts that filtered list to be of the person the user is viewing, to prevent causing such an unintended behaviour.

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 severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** > It is very rare for the user to delete a different person after viewing a person (e.g. view 1 and delete 2) 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. > Based on the intended sequence of steps the user takes to delete a person, after viewing a person (e.g. view 1 which views Person A), IF the user wishes to delete a person, it makes sense that the person the user wishes to delete is Person A (the one the user is viewing), which is what happens (although we agree that having the INDEX after the delete should be checked for). It is definitely unexpected that the user views person A and proceeds to delete person B. While this may be true, it does not stop it from behaving out of the realm of expectation. The user's expectation with the `converttolead` 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 `converttolead` 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.