Skip to content

Conversation

@iamgabrielma
Copy link
Contributor

@iamgabrielma iamgabrielma commented Aug 8, 2025

Part of WOOMOB-103

Description

This PR addresses the Locale.isoCurrencyCodes deprecation warning, and to use Locale.Currency.isoCurrencies instead.

The return type of the built-in function is updated from [String] to [Locale.Currency], so we use the identifier property to extract the currency value. Applying uppercased() to the iso currency before comparing the wrapped value is redundant, so I left it out.

Testing information

  • CI should pass
  • Processing a transaction via card reader and refunding it should work normally. We only seem to use this property wrapper in these two cases, for matching the currency and returning it in lowercase when we pass it to PaymentIntentParametersBuilder and such.

You can add a breakpoint to return value.lowercased() to observe that nothing is off when performing currency comparison and returning the lowercase version of the iso identifier.


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

@iamgabrielma iamgabrielma added this to the 23.1 milestone Aug 8, 2025
@iamgabrielma iamgabrielma added type: task An internally driven task. type: technical debt Represents or solves tech debt of the project. labels Aug 8, 2025
@iamgabrielma iamgabrielma marked this pull request as ready for review August 8, 2025 04:41
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Aug 8, 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 Numberpr15992-2b799a1
Version22.9
Bundle IDcom.automattic.alpha.woocommerce
Commit2b799a1
Installation URL6ced2c7qiq3d0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

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

Works as expected, one suggestion inline.

public var wrappedValue: String {
get {
guard Locale.isoCurrencyCodes.map({ $0.uppercased() }).contains(value.uppercased()) else {
guard Locale.Currency.isoCurrencies.map({ $0.identifier }).contains(value.uppercased()) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest keeping the .uppercased() for both parts of the comparison, just for good practice.

While the enum definitions are all upper-case, there doesn't appear to be anything in ISO-4217 saying they have to be. Stripe use lower case, which is the reason for this wrapper even existing.

Since we're converting one side of the comparison, it's probably safest/clearest to mirror that on the other side.

@iamgabrielma iamgabrielma merged commit 5b2b28a into trunk Aug 11, 2025
14 checks passed
@iamgabrielma iamgabrielma deleted the task/ios17-currencycode branch August 11, 2025 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: task An internally driven task. type: technical debt Represents or solves tech debt of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants