fix: Local state wrong on multiple set operations on same property

[mortenmo]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

Closes: #2115
Closes: #1451

Approach

When local state has multiple set/unset ops on the same base property, use the result of the previous instead of always going back to the server version for every Op to allow them to chain.

The test included fails until the fix is in place.

Tasks

• Add tests

[parse-github-assistant]

I will reformat the title to use the proper commit message syntax.

[parse-github-assistant]

Thanks for opening this pull request!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (72bc9ac) to head (da3121c).
Report is 15 commits behind head on alpha.

Additional details and impacted files

@@             Coverage Diff             @@
##            alpha     #2116      +/-   ##
===========================================
+ Coverage   99.98%   100.00%   +0.01%     
===========================================
  Files          61        64       +3     
  Lines        6185      6208      +23     
  Branches     1499      1504       +5     
===========================================
+ Hits         6184      6208      +24     
+ Misses          1         0       -1     

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

[dplewis]

@mtrezza @mortenmo A PR for this has already been opened. Try copying the test over from that PR. I'll review.

#1451

[mortenmo]

I really need to check existing PRs better :) . The fixes looks basically identical, so feel free to commit that one and I can close this one. I added the #1450 test case here just to be sure and it does also pass.

[mstniy]

Hey. I made some changes to #1451 . The patch also handles doubly-nested objects now, it used to be still broken in that case. I also modified the regression test to also include this scenario.

[mtrezza]

Is this PR made obsolete by #1451? If not, which one should be merged first, in case order matters?

[mstniy]

I believe so. I copied the new regression test in this branch over to my branch, and it does pass there also.

I would suggest #1451 to be merged in favor of this one, as that one implements a more robust deep cloning logic after today's changes, evidenced by the regression test here passing there, but not vica versa.

[mtrezza]

Superseded by #1451