-
Notifications
You must be signed in to change notification settings - Fork 121
[Woo POS][Local Catalog] Trigger sync periodically in the system background #16244
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
[Woo POS][Local Catalog] Trigger sync periodically in the system background #16244
Conversation
Simplify posCatalogSync tasks by merging into one and then checking within the task whether full or incremental sync needs to be done. Since we have no guarantee when the sync runs, it's more convenient to call a refresh task every hour and then decide whether we do incremental or full.
BackgroundTaskSchedule uses a simple logic to provide the next task to schedule. - When app goes to background, sets the preferred refresh date based on the predefined periods (30 min for order sync and 60 for pos catalog sync) - Provides the next task based on the earliest time - The preferred time is only reset when app goes into background ensuring that when the app is longer in the background, eventually all tasks will get executed
… to select the next task to dispatch
|
|
iamgabrielma
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
Sorry it took me a bit longer than usual, a bunch of stuff to unpack here.
Merge posCatalogFullSync and posCatalogIncrementalSync tasks
I don't have the full context of the project for this change, but it sounds like a good idea to me for these tasks, specially when we cannot guarantee precision and is handled by the system.
Minor suggestions for the logs, just from a debugging perspective when we need them:
- We should add timezone for easier debugging later on, ie to add an
UTChere
🔄 Starting incremental catalog sync for site ID: 215063064, modifiedAfter: 2025-10-16 03:33:35 +0000
- This one was a bit confusing regarding what's happening with variations, but unrelated to this PR:
💾 Persisting incremental catalog with 0 products and 0 variations
✅ Incremental catalog persistence complete
Total after incremental update: 86 products, 47 product images, 21 product attributes, 106 variations, 95 variation images, 258 variation attributes
- When there is no updates to the local catalog (ie "Persisted 0 products and 0 variations to database for siteID xyz") perhaps we should add the reason.
| @Test func performSmartSync_performs_full_sync_when_last_full_sync_older_than_threshold() async throws { | ||
| // Given - last full sync was 25 hours ago (older than 24 hour threshold) | ||
| let twentyFiveHoursAgo = Date().addingTimeInterval(-25 * 60 * 60) | ||
| try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: twentyFiveHoursAgo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL about PersistedSite, just for context this seems to be the table that holds each site's ID and associated catalog and sync details, in GRDB?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Persisted prefix is used for GRDB entities. I'm not sure if that's great naming. We discussed that we may move all the GRDB stuff into a separate module, maybe we could ditch the prefixes.
| /// | ||
| func scheduleAppRefresh() { | ||
| scheduleTask(type: .ordersAndDashboardSync, earliestBeginDate: Date(timeIntervalSinceNow: 30 * 60)) | ||
| BGTaskScheduler.shared.cancelAllTaskRequests() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could "cancelling pending tasks" each time we enter in background, introduce any problem with either POS or the dashboard? ie: A long-planned task, let's say a couple of days, is never triggered since we background the app more often than that. My understanding that this is currently handled in setDefaultPreferredTaskDates(), so just raising it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't foresee any problems. What I want to ensure is that we only end up scheduling a single thing we're intending to schedule. Right now the mechanism is made naive (setDefaultPreferredTaskDates). Maybe in the future we want to synchronize dates between both foreground and background sync, so that should be a source of truth.
|
Thanks for testing, @iamgabrielma!
Understandable. It took me time to develop and test as well. One of the reasons I stayed with the current dispatcher class is that I know that it works. So I only integrated within it.
I'll try to get feedback from @jaclync on that as well.
Good suggestion! I'll do it.
It means that there were 0 new products and 0 new variations. If you make a change in between the updates, this should reflect the number of new updates. However, I will update the logs to be clear about that. |
… only with updated products
…nated and relaunched in the background

WOOMOB-1095
Continuation of #16205
Description
The goal of this PR is to implement the logic of the dispatching system background tasks to make sure:
Not in scope:
Exploration
I began my explorations by looking for a universal way of scheduling foreground and background tasks for POS Catalog (and OrderDashboard sync) with clever prioritization. I've done such a solution locally, but it felt too large and complicated with no clear benefits. Theoretically, we could have one scheduler that:
However, given
BGAppRefreshTaskalready doesn't guarantee when and if it's going to run, the precision is not that important. Therefore, I decided to keep the existingBackgroundTaskRefreshDispatcherand implement a foreground runner to run separately, independent ofBackgroundTaskRefreshDispatcherand specifically for POS Catalog. I will share that in another PR.Solution
posCatalogFullSyncandposCatalogIncrementalSynctasks created in [Local Catalog] Refactor background task dispatcher to support POS catalog sync #16205 into a singleposCatalogSynctask that runs every hour. The motivation is that the system already doesn't guarantee if and when this is going to run. Therefore,posCatalogSynccan check if a full sync is available right before running a task, otherwise, run incremental sync.BackgroundTaskScheduleto implement a couple of simple methods to pick the next task based on the run period and the last schedule date. Asked AI to generate comprehensive test cases based on my scenarios.BackgroundTaskSchedulewithinBackgroundTaskRefreshDispatcherSteps to reproduce
Prerequisites:
BackgroundTaskScheduleand make periods shorter, e.g 30 and 60 secondsScheduled orderDashboardSyncwith time is loggedScheduled posCatalogSyncTesting information
Tested on iPad Air M2 device
Screenshots
Running session
testing.flow.tasks.mov
RELEASE-NOTES.txtif necessary.