Skip to content

Conversation

@iamgabrielma
Copy link
Contributor

@iamgabrielma iamgabrielma commented Jul 1, 2025

Continuation of WOOMOB-701

Description

This PR adds additional unit testing to the usage of created_via. No functional changes have been added.

On a related note, I've tried the suggestion on https://github.com/woocommerce/woocommerce-ios/pull/15847/files#r2177500395 about extending the Order model, but I faced some build troubles which I'm still wondering about. More context here: p1751433227323889-slack-CGPNUU63E

Testing information

No functional changes. CI should pass

@iamgabrielma iamgabrielma added type: task An internally driven task. feature: order details Related to order details. feature: order list Related to the order list. feature: POS labels Jul 1, 2025
@iamgabrielma iamgabrielma added this to the 22.8 milestone Jul 1, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 1, 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 Number30948
VersionPR #15848
Bundle IDcom.automattic.alpha.woocommerce
Commit1f93588
Installation URL5c6sia6c6trig
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Base automatically changed from task/WOOMOB-701-update-networking-model-to-use-createdvia-prop to trunk July 2, 2025 03:13
@iamgabrielma iamgabrielma requested a review from jaclync July 2, 2025 05:24
@iamgabrielma iamgabrielma marked this pull request as ready for review July 2, 2025 05:24
@jaclync
Copy link
Contributor

jaclync commented Jul 2, 2025

On a related note, I've tried the suggestion on https://github.com/woocommerce/woocommerce-ios/pull/15847/files#r2177500395 about extending the Order model, but I faced some build troubles which I'm still wondering about. More context here: p1751433227323889-slack-CGPNUU63E

I was testing p1751454825440359/1751433227.323889-slack-CGPNUU63E similar implementation to Product.prodcutType, and it seems to build at least: 3a11af9 - WDYT? The Yosemite/App layer tests likely need some adjustments from this change.

Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

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

Tests look good, feel free to consider the sales channel move to Networking separately.

let viewModel = OrderListCellViewModel(order: order, currencySettings: ServiceLocator.currencySettings)

// Then
XCTAssertEqual(viewModel.salesChannel, "POS")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: is "POS" directly shown in the UI? If so, might we consider making this string localizable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, it does show in the UI but I didn't consider it's a string we'd make localizable, ie: In Spanish the acronym is still POS despite the translation being "punto de venta" 🤔 . I see this is set as localizable string in POSTabViewController thought and definitely makes sense for it to be localized. Updated on 1f93588

@iamgabrielma
Copy link
Contributor Author

I was testing p1751454825440359/1751433227.323889-slack-CGPNUU63E similar implementation to Product.prodcutType, and it seems to build at least: 3a11af9 - WDYT? The Yosemite/App layer tests likely need some adjustments from this change.

Appreciated! Yeah, I'll look into these changes next 👏

@iamgabrielma iamgabrielma enabled auto-merge July 3, 2025 04:27
@iamgabrielma iamgabrielma merged commit 62e800e into trunk Jul 3, 2025
13 checks passed
@iamgabrielma iamgabrielma deleted the task/WOOMOB-701-continuation-add-more-unit-tests branch July 3, 2025 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: order details Related to order details. feature: order list Related to the order list. feature: POS type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants