fix(sync): propagate connection group membership changes to other devices#1477
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On one Mac, connections are organized into groups and iCloud sync is on. On a second Mac with sync enabled, the groups and connections both arrive, but the connections show up flat (ungrouped) and the groups report a count of 0. The group membership is lost across devices.
Root cause
Group membership lives in
connection.groupId. Three paths that change it persisted via the low-levelConnectionStorage.saveConnections(_:), which writes the file but does not mark connections dirty for sync (unlikeaddConnection/updateConnection/updateConnectionsand the form-save and reorder paths):WelcomeViewModel.moveConnections(_:toGroup:)WelcomeViewModel.removeFromGroup(_:)GroupStorage.deleteGroup(_:)So a
groupIdchange saved locally but never re-pushed. The connection record in iCloud kept its last-pushedgroupId(usually nil). Groups themselves synced fine (GroupStorage.saveGroupsmarks groups dirty), so the second device pulled correct groups but stale connections, leaving them ungrouped with empty group counts.This reproduces when grouping happens while sync is already on. When grouping happens before enabling sync,
enableSync()marks all data dirty and pushes everything correctly, which is why it sometimes works.Fix
Route the three membership mutations through the existing
updateConnections(_:)API, which persists once and marks each changed connection dirty (and skipslocalOnly/ sample connections). This keepssaveConnectionsas a pure write primitive, which the sync-apply path and load-time migration rely on.Tests
testDeleteGroupClearsMembershipAndMarksConnectionDirtyForSyncinGroupStorageTestsasserts that deleting a group clearsgroupIdon its connections and marks them dirty for sync.WelcomeViewModelpaths are covered transitively:updateConnections' dirty-marking is already tested inConnectionStoragePersistenceTests.Recovering already-affected devices
On the device that owns the correct grouping, toggle iCloud sync off then on.
enableSync()re-marks all local data dirty and re-pushes every connection with its currentgroupId, which the other device then pulls.