-
Notifications
You must be signed in to change notification settings - Fork 121
[Local catalog] Use cellular data setting for full syncs #16320
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
[Local catalog] Use cellular data setting for full syncs #16320
Conversation
|
|
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.
Works well! Tested on iPhone 14 by disabling the .pad constraint for the paginated endpoint approach. Could you invite me to some large store (largefuntesting.mystagingwebsite.com or other) so I can give it a go to the file-based catalog API?
🚢
| } | ||
| let request = URLRequest(url: url) | ||
| var request = URLRequest(url: url) | ||
| request.allowsCellularAccess = allowCellular |
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: allowsCellularAccess property in the URLRequest
|
Thanks for the review!
Sorry, should have checked you had that when I made this PR. Invited you now. |

Description
This PR updates our network requests for full syncs to respect the setting for whether to use cellular data or not.
The PR works for both the paginated endpoint approach, and the file download endpoint (which is feature flagged)
Test Steps
Testing is a little fiddly if you don't have an iPad with cellular (which I don't.) You can't test on a simulator
To handle this, I used my iPhone Pro Max, which can show POS if you change the
POSTabVisibilityChecker.checkVisibility()function to remove theguard userInterfaceIdiom == .padcheck.Paginated endpoint API
Refresh CatalogFile catalog API
When testing with the file-based catalog API, you'll need to enable the
pointOfSaleCatalogAPIfeature flag.It's also helpful to update
POSCatalogSyncCoordinator.performFullSyncIfApplicable(for:maxAge:regenerateCatalog:)to stop it constantly regenerating the catalog. This isn't essential, but can help speed up testing with large sites.If you use the larger test stores, you'll also need to update the
POSLocalCatalogEligibilityService.Constants.defaultCatalogSizeLimitto 100000 so that it's deemed eligible for local catalog.largefuntesting.mystagingwebsite.com)Refresh CatalogTests performed
Note that errors are not shown when the sync fails for this reason, generally speaking, because sync happens as a non-user process. We could show errors when it fails on tapping
Refresh catalog, since that's user invoked, but I see that as outside the scope of this PR.RELEASE-NOTES.txtif necessary.