search

ARPspoofing / pe

0 stars 0 forks source link

Undo state #16

Open ARPspoofing opened 1 year ago

ARPspoofing commented 1 year ago

Undo state has not been specified that it will be the latest state.

  1. add n/hello
  2. undo
  3. add n/bye
nus-pe-bot commented 1 year ago

Team's Response

Details are slightly unclear though.

The 'Original' Bug

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

DG does not explain how diverging branches in the undo state trees are handled.

Note from the teaching team: This bug was reported during the Part II (Evaluating Documents) stage of the PE. You may reject this bug if it is not related to the quality of documentation.

In the details for the Undo/Redo feature, the DG discusses how the current state is handled when redo or undo command is executed.

However, it is possible for the state tree to be non linear. For example, lets say there are three linear states a -> b -> c. undoing from c and then making a change creates another branch a->b>d.

The user guide does not explain what happens to the state tree in such a case and what option would be chosen if someone called redo from b.

The expected behaviour in such a case would be to go to state d but then state c would be lost (which should be mentioned, at least).

[original: nus-cs2103-AY2223S2/pe-interim#4508] [original labels: severity.Medium type.DocumentationBug]

Their Response to the 'Original' Bug

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

image.png

This has been mentioned in the DG further into the explanation of the undo/redo implementation

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.Rejected`] - [x] I disagree **Reason for disagreement:** Agree that is has been mentioned in the DG, but it should at least been mentioned in the UG that the states will be purged. By that logic, we can remove every restriction and explanation from the UG, and say that it has been mentioned in the DG thus we do not have to mention it in the UG. Just a sentence of "Warning: executing commands from undo state purges all future states" will be good enough. Also, a user will not need to refer to the DG. So, if the User is not proficient in technology, he will be unaware on how to read the activity diagrams, and thus be unaware of the purged states feature. Therefore, it is still a bug and with duplicates, it should not be rejected.
## :question: Issue type Team chose [`type.DocumentationBug`] Originally [`type.FeatureFlaw`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]
## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.High`] - [x] I disagree **Reason for disagreement:** After thinking about it, it should be medium, because what if the user undo a lot of important CRUD executions before executing a new command? Everything would have been unknowingly lost. Also, a user is not required to read the developer guide, because as the name suggests, that is for developers. Since this might affect some critical CRUD features, the severity is changed to Medium.