Icesiolz / pe

0 stars 0 forks source link

Undoing the change in body status #5

Open Icesiolz opened 4 years ago

Icesiolz commented 4 years ago

image.png The undo function undone the changes in status when it should undo my addition of the body.

(The change in status was automatic, not typed by me!)

nus-pe-bot commented 4 years ago

Team's Response

Hi there, this is intended behaviour. This specific behaviour is stated in the User Guide, as seen here:

image.png

In the context of Mortago in v1.4, I don't think this is a feature flaw. In fact, I put in extra effort to make it undoable.

During our testing, we found the UX very jarring for users when the automated update was not undoable. In the example you gave, when you undid the addition of the body, the automated update would be undone together if it were not undoable. I didn't want users to wonder why their add command now affects another Body, or why their update of one field is changing so many other things. This is especially so for the PE, because notifications are set to execute after only 10 seconds and testers will be running many commands in quick succession.

Additionally, ignoring the changes caused by the notification can cause desynchronisation issues. This means that history will be inconsistent, which increases the chances of bugs in undo/redo, could causes bugs in other commands, and is also confusing for users. Imagine an example where the automated update happens, and the user undoes and redoes a previous update command. The change made by the notification would not be preserved when redone, because the notification uses an update command to make its changes.

The other option was to propagate the automated status update back to the initial add command that triggered the automated command, but I thought it was not a scaleable or elegant solution. This would require going through every command in history in the worst case to propagate the change made by the notification. It's very inefficient.

Rest assured that I have put a lot of thought into this design choice. You may not agree with my choice, but I don't think I should be penalised for this.

Thank you for the bug report. I hope this has made things clearer.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: Perhaps it would be clearer for the user to know what was being undone instead. I might be expecting to undo my commands when suddenly an automatic update was undone instead, especially when automatic updates can happen without the user knowing.

I believe this is a feature flaw because it would be clearer if the user knows what is being undone instead of simply showing the ID that was undone.