[Order editing] Non-default currency order edit notice #14345

joshheald · 2024-11-07T10:58:42Z

Description

This PR disables order editing for orders with a different currency compared to the site's default.

This is a workaround. If we allowed this, items on these orders would be shown in the wrong currency. See peaMlT-XF-p2 for context.

Multi-currency is not supported in the app at present, but when we add support properly, this workaround should be removed and the edit screen fixed.

Steps to reproduce

Launch the app and switch to a multi currency store
Go to the orders tab and select an order in a different currency from the store's default
Tap edit
Observe that you're shown a notice explaining that you can't edit in the app

Repeat for an order in your store's default currency – observe that you can edit it as normal.

Testing information

I added some unit tests for this, but the reliance on whether the order is synced or not was tricky. I effectively exposed that property to be mockable in tests, which is a little strange but it would have made for very brittle tests to actually allow the syncEverything function to work via lots of mocking.

I tested on an iPhone running iOS 18.1, and an iPad running iOS 17.7

Tracks event registered.

Note that I also see issues when creating an order – the order creation flow shows my site's default currency (GBP), but the order is then created in USD. I'm not really sure why that is, and it seems more drastic to disable order creation than order editing... so I've treated it as out of scope for this ticket, but will find out why and create another issue if needed.

Screenshots

edit-order-notice.mp4

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

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

As a workaround, we disable editing orders with a non-default currency. If we allowed this, items on these orders would be shown in the wrong currency. See peaMlT-XF-p2 for context. Multi-currency is not supported in the app at present, but when we add support properly, this workaround should be removed and the edit screen fixed.

dangermattic · 2024-11-07T10:59:20Z

	1 Message
📖	This PR contains changes to Tracks-related logic. Please ensure (author and reviewer) the following are completed: The tracks events must be validated in the Tracks system. Verify the internal Tracks spreadsheet has also been updated. Please consider registering any new events. The PR must be assigned the category: tracks label.

Generated by 🚫 Danger

wpmobilebot · 2024-11-07T11:09:35Z

📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS
	Build Number	pr14345-d7b3ac0
	Version	21.1
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	`d7b3ac0`
	App Center Build	WooCommerce - Prototype Builds #11509

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

…g-non-default-currency

jaclync · 2024-11-08T08:55:33Z

👋 Sorry I saw the PR late, I will review it within a few hours.

wpmobilebot · 2024-11-08T10:19:04Z

Version 21.1 has now entered code-freeze, so the milestone of this PR has been updated to 21.2.

jaclync

LGTM! Works as expected

Note that I also see issues when creating an order – the order creation flow shows my site's default currency (GBP), but the order is then created in USD. I'm not really sure why that is, and it seems more drastic to disable order creation than order editing... so I've treated it as out of scope for this ticket, but will find out why and create another issue if needed.

Is this happening with some multi-currency plugin or setting? I was testing with a typical store with previous orders created with a different currency and I can't reproduce this.

jaclync · 2024-11-08T10:21:38Z

WooCommerce/Classes/Analytics/WooAnalyticsEvent.swift

@@ -654,6 +654,10 @@ extension WooAnalyticsEvent {
            ])
        }

+        static func orderEditButtonTappedWhileDisabledForCurrencyConflict() -> WooAnalyticsEvent {
+            WooAnalyticsEvent(statName: .orderEditButtonTappedWhileDisabledForCurrencyConflict, properties: [:])


Thanks for adding a Tracks event to see how many merchants are encountering this to have a better idea about the priority 👍

joshheald · 2024-11-08T10:39:29Z

Is this happening with some multi-currency plugin or setting? I was testing with a typical store with previous orders created with a different currency and I can't reproduce this.

No, it happens because order creation uses the logged in user's default currency as the one for the order when it's not specified in the request.

To change that from the site default, you have to go to /my-account/edit-account/, which I had done and forgotten about on this test store/user.

I'm just checking whether there are implications on that for editing as well; this PR may need to change or be followed up if it does. It's been bumped to 21.2 now anyway so it's fine.

joshheald · 2024-11-08T10:39:52Z

Oh and thanks for the review @jaclync 😄

joshheald · 2024-11-08T10:45:06Z

Is this happening with some multi-currency plugin or setting?

Actually yes... I doubt the user-level currency setting is available without a plugin.

My test site uses the multi-currency features built in to WooPayments.

…g-non-default-currency

joshheald added type: bug A confirmed bug. feature: order creation All tasks related to creating an order labels Nov 7, 2024

joshheald added this to the 21.1 milestone Nov 7, 2024

14304 Add order edit notice to 21.1 release notes

8f6359e

Merge branch 'trunk' into issue/14304-disable-editing-for-orders-usin…

3ccc174

…g-non-default-currency

joshheald marked this pull request as ready for review November 7, 2024 11:25

joshheald requested a review from jaclync November 7, 2024 11:26

joshheald added the category: tracks Related to analytics, including Tracks Events. label Nov 7, 2024

14304 update test to ensure order action works

d7d69c5

wpmobilebot modified the milestones: 21.1, 21.2 Nov 8, 2024

14304 Order editing currency limitation to 21.2

2ef7eeb

jaclync approved these changes Nov 8, 2024

View reviewed changes

Merge branch 'trunk' into issue/14304-disable-editing-for-orders-usin…

d7b3ac0

…g-non-default-currency

joshheald enabled auto-merge November 8, 2024 17:17

joshheald merged commit cf1658f into trunk Nov 8, 2024
14 checks passed

joshheald deleted the issue/14304-disable-editing-for-orders-using-non-default-currency branch November 8, 2024 17:42

malinajirka mentioned this pull request Jan 20, 2025

Editing Order Details - Incorrect Currency Displayed When Order Currency Differs from Store Default #14304

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Order editing] Non-default currency order edit notice #14345

[Order editing] Non-default currency order edit notice #14345

joshheald commented Nov 7, 2024 •

edited

Loading

dangermattic commented Nov 7, 2024 •

edited

Loading

wpmobilebot commented Nov 7, 2024 •

edited

Loading

jaclync commented Nov 8, 2024

wpmobilebot commented Nov 8, 2024

jaclync left a comment

jaclync Nov 8, 2024

joshheald commented Nov 8, 2024

joshheald commented Nov 8, 2024

joshheald commented Nov 8, 2024

[Order editing] Non-default currency order edit notice #14345

[Order editing] Non-default currency order edit notice #14345

Conversation

joshheald commented Nov 7, 2024 • edited Loading

Description

Steps to reproduce

Testing information

Screenshots

Reviewer (or Author, in the case of optional code reviews):

dangermattic commented Nov 7, 2024 • edited Loading

wpmobilebot commented Nov 7, 2024 • edited Loading

jaclync commented Nov 8, 2024

wpmobilebot commented Nov 8, 2024

jaclync left a comment

Choose a reason for hiding this comment

jaclync Nov 8, 2024

Choose a reason for hiding this comment

joshheald commented Nov 8, 2024

joshheald commented Nov 8, 2024

joshheald commented Nov 8, 2024

joshheald commented Nov 7, 2024 •

edited

Loading

dangermattic commented Nov 7, 2024 •

edited

Loading

wpmobilebot commented Nov 7, 2024 •

edited

Loading