Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whiteboard undo redo bug #11260

Merged
merged 28 commits into from
Aug 4, 2022

Conversation

daesun-park
Copy link
Contributor

@daesun-park daesun-park commented Jul 23, 2022

Description

When an undo/redo of an empty trait is done, it can lead to a insert/detach of a malformed trait. This issue occurs when the following is done.

  1. Detach/delete an empty trait
  2. Undo: inserts empty traits
  3. Redo: detaches empty traits, which is malformed

This PR aims to prevent this issue by skipping the change rather than performing an insertion/detachment of an empty trait. The undo/redo logic of the revert( ) method in HistoryEditFactory.ts is updated to achieve this. Additionally, an optional telemetry logger is passed through the function to track these events when they occur. More details regarding this issue can be found in Bug 635

@github-actions github-actions bot added area: dds Issues related to distributed data structures base: main PRs targeted against main branch labels Jul 23, 2022
@daesun-park daesun-park marked this pull request as ready for review July 25, 2022 06:26
@daesun-park daesun-park requested a review from a team as a code owner July 25, 2022 06:26
@daesun-park daesun-park requested a review from noencke July 26, 2022 23:59
@noencke noencke requested a review from jenn-le July 27, 2022 00:02
@daesun-park daesun-park requested a review from taylorsw04 August 1, 2022 15:24
@daesun-park daesun-park merged commit 0804151 into microsoft:main Aug 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

This commit is queued for merging with the next branch! Please ignore this PR for now. Contact @microsoft/fluid-cr-infra for help.

seanimam pushed a commit to seanimam/FluidFramework that referenced this pull request Aug 9, 2022
* Return undefined during empty detach trait

* return undefined when inserting empty traits

* Fix previous commit for inserting empty trait

* update test case

* fix lint errors

* fix lint error

* revert previous commit

* fix logic for inserting empty trait

* Add test case for empty detach

* fix lint errors

* Updated test titles, changed test case logic.

* Changed test case formatting

* fix linting error

* Updated traitlabel, and fixed testcase

* prevent residual builtNodes after insert

* replace undefined with continue for empty traits

* build but skips insert/detach of empty traits

* update test cases for skip instead of undefined

* Simplified test case

* Add use of helper function

* Added expectDefined to test case

* test to check if built/detached nodes are cleared

* Add test case for single empty insert

* fix lint errors

* added whitespace

* added logging to revert function

* Comment addressing input/output array size

* cleaned up code for checking if logger is defined
daesun-park added a commit to daesun-park/FluidFramework that referenced this pull request Nov 11, 2022
* Return undefined during empty detach trait

* return undefined when inserting empty traits

* Fix previous commit for inserting empty trait

* update test case

* fix lint errors

* fix lint error

* revert previous commit

* fix logic for inserting empty trait

* Add test case for empty detach

* fix lint errors

* Updated test titles, changed test case logic.

* Changed test case formatting

* fix linting error

* Updated traitlabel, and fixed testcase

* prevent residual builtNodes after insert

* replace undefined with continue for empty traits

* build but skips insert/detach of empty traits

* update test cases for skip instead of undefined

* Simplified test case

* Add use of helper function

* Added expectDefined to test case

* test to check if built/detached nodes are cleared

* Add test case for single empty insert

* fix lint errors

* added whitespace

* added logging to revert function

* Comment addressing input/output array size

* cleaned up code for checking if logger is defined
jenn-le pushed a commit that referenced this pull request Nov 11, 2022
Cherry pick of PR #11260 from main to release branch.
tylerbutler added a commit to tylerbutler/FluidFramework that referenced this pull request Mar 26, 2024
tylerbutler added a commit that referenced this pull request Mar 28, 2024
Cherry pick of PR #11260 from main to lts branch.

This change was ported from main to the 1.3 release branch in #12894,
but the change was never ported to lts (so wouldn't be in 1.4 if/when we
release it).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds Issues related to distributed data structures base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants