Update OrdersRemote with async/throws network methods to parse orders response in a background thread
#15982
+87
−121
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e handling is not on the main thread.
… handling is not on the main thread.
Collaborator
|
|
jaclync
added
type: task
Collaborator
jaclync
changed the title
OrdersRemote with async/throws network methods to parse orders response in a background thread
iamgabrielma
approved these changes
Aug 7, 2025
Contributor
iamgabrielma
left a comment
iamgabrielma
left a comment
There was a problem hiding this comment.
Works great! Thanks for your work modernizing those.
I left a couple of nits, feel free to address them or not 👍
| switch result { | ||
| case .success(let orders): | ||
| self?.upsertStoredOrdersInBackground(readOnlyOrders: orders) { | ||
| Task { @MainActor in |
Contributor
There was a problem hiding this comment.
nit: The alignment of these props seems to be off!
| /// Verifies that loadAllOrders properly parses the `orders-load-all` sample response. | ||
| /// | ||
| func testLoadAllOrdersProperlyReturnsParsedOrders() throws { | ||
| func testLoadAllOrdersProperlyReturnsParsedOrders() async throws { |
Contributor
There was a problem hiding this comment.
nit: Since we're updating the test signature we can update the test name to use snake case as well 🐍 , ideally with when/then too.
jaclync
force-pushed
the
backlog/WOOMOB-981-orders-remote-async-await
branch
from
e9e1be6 to
aa00e4c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For WOOMOB-981
Just one review is required.
Description
Updates
OrdersRemote.loadAllOrdersandOrdersRemote.searchOrdersmethods to use modern async/await interface instead of completion handlers. This change improves response handling by ensuring it doesn't occur on the main thread and follows current Swift concurrency best practices. These two methods are the only ones inOrdersRemotethat fetch and parse a list of orders, which includemeta_datafield that can include any number of values resulting in a large response size.The updated methods now:
async throwsinstead of completion handlers which parse the response on the main threadAll call sites have been updated to maintain compatibility:
@MainActorfor thread safetySteps to reproduce
init(from:)Testing information
I tested on iPad A16 iOS 18.4 simulator.
RELEASE-NOTES.txtif necessary.