Add billing currency

[narefyev91]

Details

Add new Currency select screens for payment card and subscriptions

Fixed Issues

$ #38619, #38630, #44118, #38633
PROPOSAL:

Tests

Add payment card flow:

1. Go to settings/subscription/add-payment-card
2. Click on currency field
3. Verify that you see just currency field and text below
4. Change currency -> Save
5. Verify that currency is changed in add payment card form

Change subscription currency:

1. Either have saved card in subscription and click on 3 dots to change card subscription currency or just go to route settings/subscription/change-billing-currency
2. Verify that you see 2 fields - currency and security code, and text below
3. Change currency
4. Click save
5. Verify that error for security code is shown
6. Fill security code -> Save
7. Verify that API call UpdateBillingCardCurrency executed and currency is changed for existed card

Edit payment card:

1. You need to have saved card in subscription and click on 3 dots to change a card
2. Verify that you see payment card fields. Not all fields should be filled - based on the API type structure we should see filed fields - card number, zip, expiration date, currency.
3. Change some fields, fill rest of them
4. Click save
5. Verify that API call AddPaymentCard executed

• Verify that no errors appear in the JS console

Offline tests

Add payment card flow:

1. Go to settings/subscription/add-payment-card
2. Click on currency field
3. Verify that you see just currency field and text below
4. Change currency -> Save
5. Verify that currency is changed in add payment card form

Change subscription currency:

1. Either have saved card in subscription and click on 3 dots to change card subscription currency or just go to route settings/subscription/change-billing-currency
2. Verify that you see 2 fields - currency and security code, and text below
3. Change currency
4. Click save
5. Verify that error for security code is shown
6. Fill security code -> Verify that errors disappear

Edit payment card:

1. You need to have saved card in subscription and click on 3 dots to change a card
2. Verify that you see payment card fields. Not all fields should be filled - based on the API type structure we should see filed fields - card number, zip, expiration date, currency.

QA Steps

Add payment card flow:

1. Go to settings/subscription/add-payment-card
2. Click on currency field
3. Verify that you see just currency field and text below
4. Change currency -> Save
5. Verify that currency is changed in add payment card form

Change subscription currency:

1. Either have saved card in subscription and click on 3 dots to change card subscription currency or just go to route settings/subscription/change-billing-currency
2. Verify that you see 2 fields - currency and security code, and text below
3. Change currency
4. Click save
5. Verify that error for security code is shown
6. Fill security code -> Save
7. Verify that API call UpdateBillingCardCurrency executed and currency is changed for existed card

Edit payment card:

1. You need to have saved card in subscription and click on 3 dots to change a card
2. Verify that you see payment card fields. Not all fields should be filled - based on the API type structure we should see filed fields - card number, zip, expiration date, currency.
3. Change some fields, fill rest of them
4. Click save
5. Verify that API call AddPaymentCard executed

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

android-web.mov

android-web1.mov

android-web2.mov

iOS: Native

iOS: mWeb Safari

iso-web1.mov

ios-web3.mov

ios-web2.mov

MacOS: Chrome / Safari

web1.mov

web2.mov

Screen.Recording.2024-06-21.at.14.09.31.mov

MacOS: Desktop

[melvin-bot]

@situchan 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]

[narefyev91]

@Expensify/design please take a look on 2 small screens. Thanks!

[shawnborton]

What exactly is security code?

For the hint text, I would think that should always show under the currency input, as in, it's part of that component. So when we ask for a security code, I would think the security code would be below the currency input + hint text.

[narefyev91]

What exactly is security code?

For the hint text, I would think that should always show under the currency input, as in, it's part of that component. So when we ask for a security code, I would think the security code would be below the currency input + hint text.

What exactly is security code?

For the hint text, I would think that should always show under the currency input, as in, it's part of that component. So when we ask for a security code, I would think the security code would be below the currency input + hint text.

I think that security code is CVV - ok i will add hint right under currency input. Thanks!

[shawnborton]

How do we ask for that in the actual credit card form itself? Do we say security code there too? Just want to make sure they are consistent. We might consider adding some hint text below that too like "This is the CVV code found on the card you are using to pay for your Expensify subscription" or something like that.

[narefyev91]

How do we ask for that in the actual credit card form itself? Do we say security code there too? Just want to make sure they are consistent. We might consider adding some hint text below that too like "This is the CVV code found on the card you are using to pay for your Expensify subscription" or something like that.

In credit card we just show without security code

[shawnborton]

Can you show me a screenshot please?

[narefyev91]

Can you show me a screenshot please?

i send both of them in earlier message #43572 (comment)

[dannymcclain]

@shawnborton do you mean in this context?

[shawnborton]

Sorry, I mean a screenshot of the form we use to ask for someone's credit card. I want to make sure "Security code" is the same terminology we use there.

In our "Add debit card" flow, we don't say security code, we call it CVV:

[shawnborton]

@dannymcclain or @MitchExpensify any ideas there?

[dannymcclain]

Jinx. I think we need to make sure we're calling that the same thing everywhere. Personally CVV is more understandable to me, but I don't know if that's just me or not.

[narefyev91]

@shawnborton like this one:

[shawnborton]

Jinx indeed! Yeah, that's exactly what I was getting at. I think we're doing some work to harmonize these card forms everywhere, does that sound right @trjExpensify? I forget who is working on that. But yeah, I agree, I mostly just want to make sure we are consistent. Security code almost feels like it could be mistaken for our magic codes, whereas CVV feels more explicitly understood (as you noted, Danny!).

[narefyev91]

Jinx indeed! Yeah, that's exactly what I was getting at. I think we're doing some work to harmonize these card forms everywhere, does that sound right @trjExpensify? I forget who is working on that. But yeah, I agree, I mostly just want to make sure we are consistent. Security code almost feels like it could be mistaken for our magic codes, whereas CVV feels more explicitly understood (as you noted, Danny!).

Are good to rename to CVV?

[shawnborton]

That looks better to me, thanks!

[dannymcclain]

I like renaming to CVV. I'll let @MitchExpensify weigh in there too though.

@shawnborton Also, in this flow, should the change currency screen look more like this screenshot instead of what we're seeing in the video?

I guess I don't understand why we need the double push input. And I also don't understand why sometimes you see the CVV field to change the currency and sometimes you don't? Do you only see the CVV field if you already have a saved card on file?

CleanShot.2024-06-12.at.09.16.09.mp4

[shawnborton]

Interesting, that's a great point. Also, why is currency not displayed like a push row if it pushes to a new page? It has a weird custom style to look like some kind of select menu, when it's not.

I don't how the CVV stuff works, but I think I would prefer not to put the CVV input on the same page as this:

So that might mean we need to do the "double" push pattern when the CVV field is present.

[dannymcclain]

Yes I think you're right. When the CVV is present, we would need to do the double push. Going to mock some things up real quick to hopefully clarify this.

[dannymcclain]

Ok can you all take a look at this and double-check that it makes sense?

I suppose if we wanted to keep things simple, we could just implement the bottom flow with and without the CVV (and not worry about the double push input). @shawnborton thoughts?

[shawnborton]

Cool, that looks much better to me. I agree, it might just be easier to implement the bottom flow but would definitely love to understand just exactly when we decide to also ask for the CVV to change the billing currency.

[narefyev91]

@dannymcclain @shawnborton hey guys! Let me know if we get to agreement to change design for screens above :-)

[narefyev91]

Also, there's console error: "VirtualizedLists should never be nested..."

Yeah good point - fix that as well!

[blimpich]

Looks good, few questions/comments. Thank you for all the work on this! Happy that this is moving along and that we are close to getting it done.

[OSBotify]

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

[kavimuru]

@blimpich @situchan Which test data should we put for payment card details? The one in 1password credit card does not work

[blimpich]

@kavimuru there are a few issues unrelated to this PR that might be causing you issues. If there is an issue to be checked off can you link it to me and I can QA this PR? If not, can you show a screenshot of what exactly isn't working, with the network tab open in the chrome dev tools?

[kavimuru]

@blimpich since the added card does not show we can't validate

Recording.1988.mp4

Recording.1987.mp4

[OSBotify]

🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.3-7 🚀

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

[OSBotify]

🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.5-13 🚀

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

[ikevin127]

Adding fallback to CONST.CURRENCY.USD here led to the following issue:

• [HOLD for payment 2025-02-07] [$250] Subscription: Currency selected incorrectly after page reload #55569

because on first mount useOnyx returns undefined until the value actually loads, by having the fallback, when the actual currency value is loaded - the component won't rerender, displaying previously cached value.

We fixed the issue by exposing useOnyx ResultsMetadata status and adding additional check to above early return such that when the status becomes "loaded" the component will rerender, actualizing the value to the correct one (non-cached).