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

Fix errors when editing Tree due to missing insPrevID in CRDTTree #756

Merged
merged 7 commits into from
Feb 26, 2024

Conversation

raararaara
Copy link
Contributor

@raararaara raararaara commented Feb 26, 2024

What this PR does / why we need it?

This PR aims to resolve the issue of incorrect index calculation during tree edit when editing copy nodes, due to the missing insPrevID information in the deepcopy() process. By adding this information, the problem can be fixed.

Any background context you want to provide?

The process of creating a document from a snapshot works fine(link). However, the issue arises when editing copy nodes during the editing process, as the insPrevID information is not copied during the deepcopy() process. This omission leads to incorrect index calculation. To address this, relevant modifications have been implemented in the code.

What are the relevant tickets?

Address #754

Checklist

  • Added relevant tests or not required
  • Didn't break anything

Copy link

codecov bot commented Feb 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.67%. Comparing base (0bc7ce1) to head (daea4b4).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #756      +/-   ##
==========================================
+ Coverage   81.50%   81.67%   +0.16%     
==========================================
  Files          59       59              
  Lines        4348     4350       +2     
  Branches      853      853              
==========================================
+ Hits         3544     3553       +9     
+ Misses        543      539       -4     
+ Partials      261      258       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@raararaara raararaara marked this pull request as ready for review February 26, 2024 04:51
Copy link
Contributor

@chacha912 chacha912 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!
It looks good :)

@raararaara raararaara merged commit 8b6ae97 into main Feb 26, 2024
2 checks passed
@raararaara raararaara deleted the fix-missing-insprevid branch February 26, 2024 05:42
hackerwins pushed a commit that referenced this pull request Mar 5, 2024
When performing `CRDTTree.edit()`, the edits are reflected by
deepcopying the CRDTTreeNodes for the given range. This commit adds
information about `insPrevID` and `insNextID` during the `deepcopy()`
process to ensure that the correct location is returned from the
correct path.

---------

Co-authored-by: Yourim Cha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants