[Woo POS][Historical Orders] Order Details - Load product images #16073

staskus · 2025-09-02T14:04:35Z

Description

Added image to OrderItem and POSOrderItem
Setting image.src to POSItemImageView

Steps to reproduce

Open POS -> Menu -> Orders
When order appears, confirm that order item products that contain the image, loads the image succesfuly
Close and reopen the view
Confirm the image is loaded immediatelly

Testing information

Tested on iPad Air 18.5, reopening orders, quickly selecting different orders, products with and without variations

Screenshots

Order.Images.mov

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

wpmobilebot · 2025-09-02T14:16:09Z

📲 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 Name	WooCommerce iOS Prototype
	Build Number	`pr16073-4a438c6`
	Version	`23.1`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`4a438c6`
	Installation URL	6jsoqi1mf8fi0

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

staskus · 2025-09-02T15:58:04Z

Modules/Sources/Yosemite/PointOfSale/OrderList/PointOfSaleOrdersProductImageResolver.swift

+    private func fetchAndCacheProductImages(for productIDs: [Int64]) async {
+        guard !productIDs.isEmpty else { return }
+
+        let products = try? await productsRemote.loadProducts(for: siteID, by: productIDs)


@joshheald I wonder what you think as well.

Short context: In order to get imageURLs for order items, I need to get products first. This one is a naive implementation for POS, which relies on making API calls for selected orders to get the images. Is it realistic to rely on GRBD product storage in the near future and only make API calls in case the product doesn't exist (for large catalogs)? Thanks!

GRDB storage will include image URLs (and other image properties), so yes...

But... we'll only populate the catalog for smaller stores which have the new variation API endpoint. So there will be other stores which don't get the image that way.

I'd be tempted to say that images are a nice-to-have here, and that trade off is OK, to the point of not fetching products/images for non-catalog stores.

However... are you sure you need to?

When I request /wc/v3/orders/, without specifying _fields, the response includes image details:

{ "id": 2898, "parent_id": 0, "status": "completed", "currency": "USD", "version": "10.1.2", [...] ], "line_items": [ { "id": 8601, "name": "A different beanie", "product_id": 1523, [...] "image": { "id": "103", "src": "https:\/\/jheverydaypay.mystagingwebsite.com\/wp-content\/uploads\/2023\/08\/beanie-2.jpg" }, [...] } ], [...]

Now I see you're using those below. Why not map from the productID in the line_item instead of fetching the whole product?

When I request /wc/v3/orders/, without specifying _fields, the response includes image details:

Okay.. I missed it by looking at how it looks on the Woo app

Good that I asked, it will be a big simplification. 🙇

iamgabrielma

Works well! 🚀

iamgabrielma · 2025-09-03T04:34:28Z

Modules/Sources/Yosemite/PointOfSale/OrderList/POSOrderItem.swift

    public let subtotal: String
    public let total: String
    public let attributes: [OrderItemAttribute]
+    public let imageURL: String?


Should this be renamed to imageURLString or similar instead?

Well, its type is String, so maybe it's fine not appending that to the name?

Yeah, that's my point, the property seems to indicate that will hold an URL type when it's a string. What about imageSource? I might be over reading it, so feel free to ignore :D

iamgabrielma · 2025-09-03T04:37:40Z

Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift

 import Foundation
 import WooFoundation
 import struct Alamofire.JSONEncoding
+import struct NetworkingCore.JetpackSite


👀 I'm guessing this was automatically generated on rake generate, but I'm not sure why. Perhaps some other PR missed it? Since builds without the import, we could remove it from this PR.

Yes, it was added by rake generate.

Since builds without the import, we could remove it from this PR.

Well.. it would re-appear with another rake generate so probably not much value in removing it here.

I just noticed that the test target won't run locally unless we add this import via rake generate

Undefined symbol: NetworkingCore.AlamofireNetwork.__allocating_init(credentials: NetworkingCore.Credentials?, selectedSite: Combine.AnyPublisher<NetworkingCore.JetpackSite?, Swift.Never>?, sessionManager: Alamofire.Session?) -> NetworkingCore.AlamofireNetwork

Must have sneaked through some other changes, still odd that CI has been passing.

iamgabrielma · 2025-09-03T04:42:34Z

WooCommerce/Classes/POS/Controllers/PointOfSaleOrderListController.swift

+        Task { @MainActor in
+            selectedOrder = await imageResolver.resolveImagesFromCache(for: order)
+
+            let orderWithFreshImages = await imageResolver.resolveImages(for: order)
+            if selectedOrder?.id == order.id {
+                selectedOrder = orderWithFreshImages
+            }
+        }


Should we mark the function as async and remove the Task block ?

I want the selection to happen synchronously, and then later update to the selected order to happen asynchronously. So I think it's appropriate to keep this code in a task block. wdyt?

Oh sure, that sounds better than doing everything async 👍

iamgabrielma · 2025-09-03T04:48:14Z

Modules/Sources/Yosemite/PointOfSale/OrderList/PointOfSaleOrdersProductImageResolver.swift

+
+    /// Fetches product data and updates the cache.
+    private func fetchAndCacheProductImages(for productIDs: [Int64]) async {
+        guard !productIDs.isEmpty else { return }


Is this needed? It's only used in resolveImages, where we already perform the empty check.

Suggested change

guard !productIDs.isEmpty else { return }

iamgabrielma · 2025-09-03T04:49:22Z

Modules/Sources/Yosemite/PointOfSale/OrderList/PointOfSaleOrdersProductImageResolver.swift

+    private func fetchAndCacheProductImages(for productIDs: [Int64]) async {
+        guard !productIDs.isEmpty else { return }
+
+        let products = try? await productsRemote.loadProducts(for: siteID, by: productIDs)


Should we wrap this in a do/catch block and log/bubble-up if there's an error? Otherwise might fail silently from throwing on the remote.

I wasn't sure if there was value in even logging some network/server failure 🤔 Of course, we could add it to help out with troubleshooting some issues where users don't see photos for their order products.

dangermattic · 2025-09-04T11:26:18Z

	1 Warning
⚠️	This PR is assigned to the milestone 23.2. 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 added this to the 23.2 milestone Sep 2, 2025

staskus added the feature: POS label Sep 2, 2025

staskus marked this pull request as ready for review September 2, 2025 15:55

staskus requested a review from iamgabrielma September 2, 2025 15:55

staskus commented Sep 2, 2025

View reviewed changes

iamgabrielma self-assigned this Sep 3, 2025

iamgabrielma approved these changes Sep 3, 2025

View reviewed changes

Base automatically changed from woomob-1138-woo-poshistorical-orders-order-details-wireframe-ui to trunk September 4, 2025 11:23

staskus force-pushed the woomob-1138-woo-poshistorical-orders-order-details-image-loading branch from 9d0135d to bc3655a Compare September 4, 2025 11:25

staskus added 3 commits September 4, 2025 14:26

Include image in OrderItem object

8a5a2bf

Use imageSrc for PointOfSaleOrderDetailsView

cc2a704

Remove unused code

8c6ae37

staskus force-pushed the woomob-1138-woo-poshistorical-orders-order-details-image-loading branch from bc3655a to 8c6ae37 Compare September 4, 2025 11:26

staskus added 2 commits September 4, 2025 14:38

Remove unused initializer

a802238

Make OrderItemProductImage src optional to avoid decoding issues

4a438c6

staskus enabled auto-merge September 4, 2025 21:01

staskus merged commit ad10bf7 into trunk Sep 4, 2025
14 checks passed

staskus deleted the woomob-1138-woo-poshistorical-orders-order-details-image-loading branch September 4, 2025 21:24

[Woo POS][Historical Orders] Order Details - Load product images #16073

[Woo POS][Historical Orders] Order Details - Load product images #16073

Uh oh!

Conversation

staskus commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Steps to reproduce

Testing information

Screenshots

Uh oh!

wpmobilebot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

However... are you sure you need to?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamgabrielma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dangermattic commented Sep 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

staskus commented Sep 2, 2025 •

edited

Loading

wpmobilebot commented Sep 2, 2025 •

edited

Loading