[Wave Collect] [Workflows] Display meaningful message when currency is not available for Direct Reimbursement #38318

lakchote · 2024-03-14T15:14:59Z

Details

Currently, there are no meaningful message displayed to the user when he has a currency not compatible with direct reimbursement, and he wants to use Make or track payments in Simplified Collect Workflows.

See #38235 and #38206
and latest updates from discussion here.

In addition to that, error messages were not handled for Delayed Submissions Add approvals.

Moreover, errors were not taken care of in Workspace > Profile for Currency, Description, and Workspace name.

This PR aims to solve these issues.

Fixed Issues

$ #38206
PROPOSAL:

Tests

Verify that no errors appear in the JS console

Prerequisites:

have collect policy enabled on NewDot (instructions)
Make or track payments in Workflows toggled to off.
No VBBA set up

Test 1

Go to Workspace > Profile, select an unsupport currency for direct reimbursement such as UGX
Go to Workspace > Workflows, toggle on Make or track payments
Click on Connect bank account
You should see this modal:

Cancel it
Click again on Connect bank account
The modal should appear again.
Click on Update to USD
You should see this in the RHP:

Test 2

Enable Make or track payments and ensure the toggle turns on
Tap Connect bank account and go through the flow to set up a VBBA account
Disable Make or track payments, ensure the toggle turns off
Change the workspace default currency to a non-USD currency
Enable Make or track payments, ensure the toggle is on
Tap Connect bank account and Update to USD in the modal
Dismiss the bank account modal and verify that you can toggle Make or track payments off

(Only for Expensify engineers)

In order to test Delayed Submission and Add approval errors, we'll need to force an error being thrown in the backend.

Throw an exception in PolicyAPI:update for Add approval, and do the same in PolicyAPI::setAutoHarvesting for Delayed Submission.

You should see something like this after trying to toggle them:

Offline tests

Complete the same steps as in the Tests section above. Whenever you come back online, the error messages should display.

QA Steps

Verify that no errors appear in the JS console
Same as in Tests section above (except for the section related to Expensify engineers).

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Screenshots are in `Tests` section above.

MacOS: Desktop

melvin-bot · 2024-03-14T15:15:30Z

@robertjchen Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

lakchote · 2024-03-14T15:15:43Z

Please keep in mind that I'll be OOO tomorrow.

lakchote · 2024-03-14T16:22:41Z

Putting this on HOLD.

We'll need to change the error location for direct reimbursement and prevent setting direct reimbursement as soon as Make or track payments is toggled.

See discussion here.

…ement choice

…set to yes

lakchote · 2024-03-18T14:33:57Z

@luacmartins @robertjchen @mountiny ready for review!

luacmartins · 2024-03-18T17:27:31Z

src/languages/en.ts

+        autoReportingErrorMessage: 'The delayed submission parameter could not be changed. Please try again or contact support.',
+        autoReportingFrequencyErrorMessage: 'The submission frequency could not be changed. Please try again or contact support.',
+        monthlyOffsetErrorMessage: 'The monthly frequency could not be changed. Please try again or contact support.',
+    },
+    workflowsApprovalPage: {
+        genericErrorMessage: 'The approver could not be changed. Please try again or contact support.',


Has marketing reviewed these copies?

Asked about it here

@lakchote This is usually done using Marketing label autoassigner on the linked issue so we get one dedicated person for this

luacmartins · 2024-03-18T17:29:05Z

src/libs/actions/Policy.ts

+const CURRENCY_AU = 'AUD';
+const CURRENCY_CA = 'CAD';
+const CURRENCY_GB = 'GBP';
+const CURRENCY_US = 'USD';
+const CURRENCY_EUR = 'EUR';
+
+const DIRECT_REIMBURSEMENT_CURRENCIES: string[] = [CURRENCY_AU, CURRENCY_CA, CURRENCY_EUR, CURRENCY_GB, CURRENCY_US];
+


We already have these defined here, it also seems like we support NZD, which is missing in your list above

+1 definitely lets reuse this from Const or add there what is missing there

We already have these defined here, it also seems like we support NZD, which is missing in your list above

NZD is not supported for direct reimbursement it seems?

See here.

I'm going to remove it in order to be ISO with PHP-Libs.

src/libs/actions/Policy.ts

src/pages/workspace/WorkspaceProfilePage.tsx

src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx

mountiny

Looking good, couple comments

mountiny · 2024-03-18T21:41:11Z

src/libs/actions/Policy.ts

+const CURRENCY_AU = 'AUD';
+const CURRENCY_CA = 'CAD';
+const CURRENCY_GB = 'GBP';
+const CURRENCY_US = 'USD';
+const CURRENCY_EUR = 'EUR';
+
+const DIRECT_REIMBURSEMENT_CURRENCIES: string[] = [CURRENCY_AU, CURRENCY_CA, CURRENCY_EUR, CURRENCY_GB, CURRENCY_US];
+


+1 definitely lets reuse this from Const or add there what is missing there

src/libs/actions/Policy.ts

src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx

luacmartins · 2024-03-18T22:36:01Z

@jjcoffee this would be an important PR to prioritize review.

src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx

lakchote · 2024-03-20T14:31:24Z

Changes look good to me but I think we should resolve @jjcoffee concerns with testing and clarify those

I've handled them, waiting for @jjcoffee review.

jjcoffee · 2024-03-20T15:56:56Z

src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx

@@ -219,10 +217,10 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses
                    </>
                ),
                isEndOptionRow: true,
-                isActive: policy?.reimbursementChoice !== CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_NO,
+                isActive: policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES,


This prevents the toggle getting toggled on if a workspace has a non-USD currency (as the toggle will set it to REIMBURSEMENT_MANUAL). I think the previous commit worked fine for this.

jjcoffee · 2024-03-20T16:04:45Z

src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx

-                    const effectiveReimbursementChoice = shouldUseDirectReimbursement ? newReimbursementChoice : CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_MANUAL;
+                    const effectiveReimbursementChoice = shouldUseDirectReimbursement
+                        ? CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES
+                        : CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_MANUAL;


Almost there! If I revert the change I commented on here, I can't toggle off, because we now never set REIMBURSEMENT_NO. The way I see it, we need to check if we are enabling the toggle (reimbursementChoice === REIMBURSEMENT_NO), then switch between REIMBURSEMENT_YES and REIMBURSEMENT_MANUAL based on shouldUseDirectReimbursement, otherwise we are turning the toggle off so should set to REIMBURSEMENT_NO.

Thank you @jjcoffee, I've pushed up a new commit here, let me know what you think!

jjcoffee · 2024-03-20T16:11:29Z

Since this slightly edge-case flow seems to be causing a bit of trouble, I'm pulling out some test steps here. It might be worth adding them to the PR test steps @lakchote so QA pick up on it too:

Enable Make or track payments and ensure the toggle turns on
Tap Connect bank account and go through the flow to set up a VBBA account
Disable Make or track payments, ensure the toggle turns off
Change the workspace default currency to a non-USD currency
Enable Make or track payments, ensure the toggle is on
Tap Connect bank account and Update to USD in the modal
Dismiss the bank account modal and verify that you can toggle Make or track payments off

lakchote · 2024-03-20T16:47:26Z

Since this slightly edge-case flow seems to be causing a bit of trouble, I'm pulling out some test steps here. It might be worth adding them to the PR test steps @lakchote so QA pick up on it too:

Enable Make or track payments and ensure the toggle turns on

Tap Connect bank account and go through the flow to set up a VBBA account

Disable Make or track payments, ensure the toggle turns off

Change the workspace default currency to a non-USD currency

Enable Make or track payments, ensure the toggle is on

Tap Connect bank account and Update to USD in the modal

Dismiss the bank account modal and verify that you can toggle Make or track payments off

Good point, test steps updated!

jjcoffee

LGTM and tests well!

melvin-bot · 2024-03-20T17:02:38Z

@rlinoz Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

luacmartins

LGTM!

addressed

OSBotify · 2024-03-20T17:43:45Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-03-22T16:20:54Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 1.4.56-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2024-03-27T04:50:03Z

🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.4.56-8 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

lakchote added 6 commits March 14, 2024 14:37

handle errors

16cae1b

add translations for workflows errors

cff3658

handle workflows errors

47c3c74

add errors for failure data

e515c8e

create method to check valid currency for reimbursement

a280421

handle errors for workspace name and description

ef1d156

lakchote self-assigned this Mar 14, 2024

lakchote requested a review from a team as a code owner March 14, 2024 15:15

lakchote requested review from luacmartins and mountiny and removed request for a team March 14, 2024 15:15

melvin-bot bot requested a review from robertjchen March 14, 2024 15:15

lakchote changed the title ~~[Wave Collect] [Workflows] Better errors handling~~ [HOLD] [Wave Collect] [Workflows] Better errors handling Mar 14, 2024

lakchote added 6 commits March 18, 2024 14:53

make method more specific

3aa3925

show currency modal switcher if vbba not set and fix default reimburs…

a7d48f2

…ement choice

fix style

b3d9939

remove unneeded translation key

20af98e

remove old logic for edge case when default reimbursement choice was …

be8fd34

…set to yes

Merge branch 'main' into lucien/fix-worklows-error-messages

e053abc

lakchote changed the title ~~[HOLD] [Wave Collect] [Workflows] Better errors handling~~ [Wave Collect] [Workflows] Display meaningful message when currency is not available for Direct Reimbursement Mar 18, 2024

fix typo

c1c65f6

luacmartins requested changes Mar 18, 2024

View reviewed changes

mountiny reviewed Mar 18, 2024

View reviewed changes

luacmartins requested a review from jjcoffee March 18, 2024 22:35

robertjchen previously requested changes Mar 19, 2024

View reviewed changes

src/pages/workspace/workflows/WorkspaceAutoReportingFrequencyPage.tsx Outdated Show resolved Hide resolved

lakchote added 4 commits March 20, 2024 14:58

use new naming for const collection keys

45c7923

fix toggle logic

c1d1aa6

improved toggle logic

172c8b3

Merge branch 'main' into lucien/fix-worklows-error-messages

1140772

lakchote added 2 commits March 20, 2024 15:56

fix style

a932d16

Merge branch 'main' into lucien/fix-worklows-error-messages

256049f

jjcoffee reviewed Mar 20, 2024

View reviewed changes

fix toggle logic

adb5d5e

lakchote force-pushed the lucien/fix-worklows-error-messages branch from dfc3e8b to adb5d5e Compare March 20, 2024 16:45

jjcoffee approved these changes Mar 20, 2024

View reviewed changes

melvin-bot bot requested a review from rlinoz March 20, 2024 17:02

luacmartins requested review from robertjchen and luacmartins March 20, 2024 17:33

luacmartins approved these changes Mar 20, 2024

View reviewed changes

luacmartins merged commit 399077f into main Mar 20, 2024
19 checks passed

luacmartins deleted the lucien/fix-worklows-error-messages branch March 20, 2024 17:37

github-actions bot mentioned this pull request Mar 22, 2024

Deploy Checklist: New Expensify 2024-03-22 #38811

Closed

lanitochka17 mentioned this pull request Mar 23, 2024

Workspace - Grammar error in "Let's get your company set up!" Concierge message #38872

Closed

6 tasks

izarutskaya mentioned this pull request Mar 25, 2024

[HOLD for payment 2024-04-05] [Simplified Collect][Workflows] "Make or track payments" section is no longer grayed out after updating to USD #38926

Closed

6 tasks

melvin-bot bot mentioned this pull request Mar 27, 2024

[HOLD for payment 2024-04-03] [Wave Collect] [Workflows] Unable to toggle Make or track payments #38206

Closed

6 tasks

lanitochka17 mentioned this pull request Apr 3, 2024

Workspace - The page scrolls to the top when Account Number field is tapped #39493

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wave Collect] [Workflows] Display meaningful message when currency is not available for Direct Reimbursement #38318

[Wave Collect] [Workflows] Display meaningful message when currency is not available for Direct Reimbursement #38318

lakchote commented Mar 14, 2024 •

edited

Loading

melvin-bot bot commented Mar 14, 2024

lakchote commented Mar 14, 2024

lakchote commented Mar 14, 2024 •

edited

Loading

lakchote commented Mar 18, 2024

luacmartins Mar 18, 2024

lakchote Mar 19, 2024

mountiny Mar 19, 2024

luacmartins Mar 18, 2024

mountiny Mar 18, 2024

lakchote Mar 19, 2024 •

edited

Loading

mountiny left a comment

mountiny Mar 18, 2024

luacmartins commented Mar 18, 2024

lakchote commented Mar 20, 2024

jjcoffee Mar 20, 2024

jjcoffee Mar 20, 2024

lakchote Mar 20, 2024

jjcoffee commented Mar 20, 2024 •

edited

Loading

lakchote commented Mar 20, 2024

jjcoffee left a comment

melvin-bot bot commented Mar 20, 2024

luacmartins left a comment

OSBotify commented Mar 20, 2024

OSBotify commented Mar 22, 2024

OSBotify commented Mar 27, 2024

[Wave Collect] [Workflows] Display meaningful message when currency is not available for Direct Reimbursement #38318

[Wave Collect] [Workflows] Display meaningful message when currency is not available for Direct Reimbursement #38318

Conversation

lakchote commented Mar 14, 2024 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Mar 14, 2024

lakchote commented Mar 14, 2024

lakchote commented Mar 14, 2024 • edited Loading

lakchote commented Mar 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lakchote Mar 19, 2024 • edited Loading

Choose a reason for hiding this comment

mountiny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luacmartins commented Mar 18, 2024

lakchote commented Mar 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjcoffee commented Mar 20, 2024 • edited Loading

lakchote commented Mar 20, 2024

jjcoffee left a comment

Choose a reason for hiding this comment

melvin-bot bot commented Mar 20, 2024

luacmartins left a comment

Choose a reason for hiding this comment

OSBotify commented Mar 20, 2024

OSBotify commented Mar 22, 2024

OSBotify commented Mar 27, 2024

lakchote commented Mar 14, 2024 •

edited

Loading

lakchote commented Mar 14, 2024 •

edited

Loading

lakchote Mar 19, 2024 •

edited

Loading

jjcoffee commented Mar 20, 2024 •

edited

Loading