Skip to content

fix(recipe): apply remote edits to recipes already synced locally - #533

Open
plusmobileapps wants to merge 1 commit into
mainfrom
claude/recipe-update-sync-bug-dbca67
Open

fix(recipe): apply remote edits to recipes already synced locally#533
plusmobileapps wants to merge 1 commit into
mainfrom
claude/recipe-update-sync-bug-dbca67

Conversation

@plusmobileapps

Copy link
Copy Markdown
Collaborator

What

The pull half of RecipeRepositoryImpl.syncWithRemote skipped every recipe it already had a local row for:

val existing = db.getByRemoteId(remoteId).executeAsOneOrNull()
if (existing != null) continue

So the pull only ever created rows. A recipe edited on one device reached Supabase but never came back down to any other device that already had it — the local copy only changed when that same device edited it.

The pull now reconciles remote edits into the existing row via a new updateFromRemote query.

Why the push side is not the problem

Traced it: Recipe.sq's update sets isDirty = 1, updateRecipe calls pushUpdateToRemote immediately, and a push that fails is retried from getDirty() on the next full sync. The data was reaching the remote fine.

Notes for review

  • Follows the pattern GroceryRepositoryImpl.syncWithRemote already uses (queries.updateFromRemote guarded by !existing.isDirty).
  • updateFromRemote updates the content columns plus isPublic, ownerId and updatedAt, and deliberately does not touch isDirty, remoteId or clientId.
  • Rows with unpushed local changes are skipped: their dirty push runs earlier in the same pass, and overwriting here would drop an edit made while we were fetching.
  • Tombstoned rows are skipped too, so they keep their tombstone until the remote delete lands (covered by the existing syncWithRemote_pull_does_not_resurrect_tombstoned_recipe test).

Testing

Two new tests in RecipeRepositoryImplTest:

  • syncWithRemote_pull_applies_remote_edits_to_an_already_synced_recipe — verified it fails against the old code and passes with the fix.
  • syncWithRemote_pull_does_not_clobber_a_recipe_with_unpushed_local_edits — makes the fake remote's upsert throw so the row is still dirty when the pull runs.

./gradlew :client:recipe:data:impl:jvmTest → 27/27 passing.

Known related gaps, not in this PR

  • RecipeBookRepositoryImpl and MealPlanRepositoryImpl have the identical if (existing != null) continue in their pulls, so e.g. a book renamed on one device never renames on another.
  • Recipes are never pruned locally when deleted remotely (grocery does prune). Left alone deliberately: fetchAccessibleRecipes also covers shared-book recipes, so a naive prune would delete local copies whenever a book share is revoked — that needs its own thinking about ownership.

🤖 Generated with Claude Code

The pull half of RecipeRepositoryImpl.syncWithRemote skipped every
recipe it already had a local row for (`if (existing != null) continue`),
so it only ever created rows. An edit made on another device reached
Supabase but never came back down — the local copy only changed when
that same device edited it.

The push side was already correct: `update` sets isDirty = 1,
updateRecipe pushes immediately, and a failed push is retried from
getDirty() on the next sync.

Reconcile remote edits into the existing row instead, following the
pattern GroceryRepositoryImpl already uses. The new updateFromRemote
query touches content columns plus isPublic/ownerId/updatedAt and
deliberately leaves isDirty, remoteId and clientId alone. Rows with
unpushed local changes are skipped — their dirty push runs earlier in
the same pass, and overwriting here would drop an edit made while we
were fetching — as are tombstoned rows, which keep their tombstone
until the remote delete lands.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant