Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Oct 15, 2025

WOOMOB-1517
Related PR: #16244

Description

In #16244 I implemented a functionality to run hourly POS sync in the background.

In this PR, I'm implementing a similar functionality that should run when the app is in the foreground.

Outside the scope:

  • Implementing specific criteria when sync happens or doesn't happen (cellular network, 30-day POS open window, etc.) we have a task for this

Solution

  1. Created ForegroundPOSCatalogSyncDispatcher to be next to the background dispatcher for easier discoverability
  2. ForegroundPOSCatalogSyncDispatcher uses DispatchSourceTimer, which is an efficient way to run a timer in the background. It has parameters such as leeway, which allows the system to prioritize other ongoing tasks and perform the work without precision, since precision is not that important for us.
  3. ForegroundPOSCatalogSyncDispatcher observes the app going to and from the background to stop and restart the timer
  4. When ForegroundPOSCatalogSyncDispatcher starts, the first sync is executed almost immediately, the next after 1 hour
  5. POSCatalogSyncCoordinator decides whether to do incremental or full sync (if 24h have passed)
  6. ForegroundPOSCatalogSyncDispatcher is started when the POS tab becomes visible and stopped if the POS tab becomes invisible, or if the app no longer has a coordinator or a siteID (logged out)

Steps to reproduce

It should be much easier to test compared to system background tasks! Important to verify that timer starts in expected times when logging in, launching app, going into foreground, switching sites...

Prerequisites:

  • Set lower syncInterval in ForegroundPOSCatalogSyncDispatcher, for example 30 seconds
  1. Launch the app
  2. Observe the logs, you can filter ForegroundPOSCatalogSyncDispatcher for DDLog.. calls
  3. When the POS tab becomes visible, the syncing should happen for the first time
  4. After syncInterval it should repeat
  5. Go to the background, timer should stop
  6. Come back to the foreground, the timer should begin again, and sync should happen
  7. Log out
  8. The timer doesn't stop immediately; it should be stopped after the next syncInterval when the system notices that the app is no longer logged in
  9. Log in
  10. The timer should start again
  11. Change to the site which is eligible for POS
  12. The timer should restart
  13. Change to the site which is ineligible for POS
  14. The timer should be stopped

Testing information

Tested using iPad Air M2 26.0 simulator


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@staskus staskus added this to the 23.5 milestone Oct 15, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 15, 2025

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16246-2dd0372
Version23.4
Bundle IDcom.automattic.alpha.woocommerce
Commit2dd0372
Installation URL3s465p9o2hmgg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@iamgabrielma iamgabrielma self-assigned this Oct 16, 2025
Copy link
Contributor

@iamgabrielma iamgabrielma left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

func stop() {
guard isRunning else { return }

DDLogInfo("🛑 ForegroundPOSCatalogSyncDispatcher: Stopping foreground sync dispatcher")
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider if passing the siteID in the DDLog here would be useful: When I switched from a POS-eligible store to a non-eligible one the only notice logged was that the timer stopped. Perhaps not necessary as we would see the siteID changing from context of other events.


DDLogInfo("🔄 ForegroundPOSCatalogSyncDispatcher: Starting sync for site \(siteID)")

Task.detached(priority: .utility) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good use of detached and priority queue!

private var posTabVisibilityChecker: POSTabVisibilityCheckerProtocol?
private var posEligibilityCheckTask: Task<Void, Never>?
private var posCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol?
private lazy var posSyncDispatcher = ForegroundPOSCatalogSyncDispatcher()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this needs to be lazy because the POS tab visibility and eligibility? Otherwise we would be paying for initializing the sync dispatcher despite the merchant not being able to use POS.

Base automatically changed from woomob-1095-woo-poslocal-catalog-bg-app-refresh-task-dispatching to trunk October 16, 2025 10:35
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ This PR is assigned to the milestone 23.5. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@staskus staskus enabled auto-merge October 16, 2025 12:42
@staskus staskus merged commit 4194f0f into trunk Oct 16, 2025
13 checks passed
@staskus staskus deleted the woomob-1517-woo-poslocal-catalog-add-a-periodic-sync-when-the-app-is-in-foreground branch October 16, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants