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

feat: sync push #2413

Merged
merged 51 commits into from
Nov 12, 2024
Merged

feat: sync push #2413

merged 51 commits into from
Nov 12, 2024

Conversation

tm-ruxandra
Copy link
Contributor

@tm-ruxandra tm-ruxandra commented Nov 5, 2024

Description

Integrate change-tracking, local soil data logic, and soil data push endpoint into full push system. This is comprised of the following notable pieces:

  • The PushDispatcher component listens for unsynced soil data and, when the app is online, dispatches a push to the server. It will also initialize a retry cycle on failure, which will end the next time the soil sync state changes (i.e., if a push succeed, the user makes new changes, or offline status changes.)
  • soilIdSlice has been updated to ingest push results using the applySyncActionResults method, which handles updating internal recordkeeping based on push operation results. In particular, it only ingests results that are "up-to-date" (the result is for the same revision ID as the record's current revision ID).
  • The sync model has been updated to include support for sync errors. Records which had an error on the last sync are still considered 'synced' until their next change and retain the last-known 'successful' data.
  • remoteSoilDataActions contains logic for turning the ChangeRecords representing a user's un-synced changes into input for the Push mutation.
  • soilDataDiff contains the bare-minimum logic needed to compare soil data versions, so that we can indicate to the server when a depth interval has been removed. (To be expanded upon in [Offline] Field-level change tracking for mobile client push #2283)
  • The sync file has been split into revisions, records, and results for better separation of concerns and readability.

Checklist

  • Corresponding issue has been opened
  • New tests added

Related Issues

#2279

Verification steps

  1. Open the app in offline mode, make changes, then go online; observe that changes made are synced when app goes online.
  2. Open the app in online mode, make changes; observe that changes are periodically synced to the server.
  3. Simulate server errors on the push endpoint (e.g., randomly fail). Observe that changes are still synced to server eventually once operation succeeds.
  4. Verify that making additional changes while a retry is active aborts that retry and pushes the new changes.
  5. Verify that toggling online status while a retry is active aborts that retry, and pushes the new changes when online.

dev-client/package.json Outdated Show resolved Hide resolved
Copy link
Member

@shrouxm shrouxm left a comment

Choose a reason for hiding this comment

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

ok, did a first pass. i would say the only comment that feels kinda blocking-ish is the ask for tests on the PushDispatcher

separately, i feel like it would be really nice to have some good documentation of how the sync system works, i spent a while staring blankly at sync.ts which i think is partly because i just have a hard time keeping the high-level view of the system straight in my head while going through the weeds of the code, which would be alleviated by a proper write-up

dev-client/src/model/sync/sync.ts Outdated Show resolved Hide resolved
dev-client/src/model/soilId/soilIdSelectors.ts Outdated Show resolved Hide resolved
dev-client/src/model/sync/sync.ts Outdated Show resolved Hide resolved
dev-client/src/model/soilId/soilIdSlice.ts Show resolved Hide resolved
dev-client/src/store/components/PushDispatcher.tsx Outdated Show resolved Hide resolved
dev-client/src/store/components/PushDispatcher.tsx Outdated Show resolved Hide resolved
dev-client/src/store/components/PushDispatcher.tsx Outdated Show resolved Hide resolved
@tm-ruxandra tm-ruxandra merged commit efe3b0d into main Nov 12, 2024
14 checks passed
@tm-ruxandra tm-ruxandra deleted the feat/sync-push branch November 12, 2024 15:14
tm-ruxandra added a commit that referenced this pull request Nov 13, 2024
Description
Enhance the basic soilDataDiff system introduced in #2279 to support tracking changes to all fields. The new methods plug into the remoteSoilDataActions methods for generating push input and populate the push input with the diff of the last-synced data from the server.

Note: this PR must be reviewed after #2413, as it builds on that changeset.
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