-
Notifications
You must be signed in to change notification settings - Fork 121
Shipping Labels: Add new Networking type for WooShipping shipments #15885
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
Shipping Labels: Add new Networking type for WooShipping shipments #15885
Conversation
|
|
|
Converting the PR back to draft for a better solution. |
RafaelKayumov
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. Works as described. Left couple questions.
| currency: currency) | ||
| shipmentContents.append(content) | ||
| let purchasedLabel: ShippingLabel? = { | ||
| guard let label = shipment.shippingLabel, label.refund == nil else { |
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.
Should we also check for status here?
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.
@RafaelKayumov - status is already checked when populating labels in the shipments when decoding the config response. Could you share why it's necessary here?
| public let orderID: Int64 | ||
|
|
||
| /// Index of the shipment | ||
| public let index: String |
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 you please add a comment clarifying the index format?
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.
Done in cc35a03.
RafaelKayumov
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.
Thx for the updates ![]()

Part of WOOMOB-754
Description
To prepare for the update of the order details screen to show shipments, this PR adds a new type for WooShipping shipments and change the type of
shipmentsin the config fetched from the remote.The reasoning behind this change is that the shipments need to be saved to the local storage with a site ID and order ID for a better UX on the order details screen. The current dictionary cannot be persisted the same way.
The new
WooShippingShipmenttype also comes with an optionalshippingLabelproperty mapped from the data in the config response. The logic for managing shipping labels in the purchase flow has been simplified accordingly.In subsequent PRs, I'll add a new Core Data entity for the shipments and support syncing shipments from the order details form.
Testing steps
Testing information
Screenshots
No UI changes.
RELEASE-NOTES.txtif necessary.