Add a Time sensitive to-do for employees with missing deposit accounts - #96623
Add a Time sensitive to-do for employees with missing deposit accounts#96623sharabai wants to merge 10 commits into
Time sensitive to-do for employees with missing deposit accounts#96623Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89c755aa46
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89c755aa46
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Object.values(allReports ?? {}) | ||
| .filter((report): report is Report => !!report?.reportID && report.isWaitingOnBankAccount === true && report.ownerAccountID === accountID) |
There was a problem hiding this comment.
should we add !isSettled(report) to this filter @sharabai
There was a problem hiding this comment.
@grgia Great question! As far as I can tell, we don't need it. REIMBURSEMENT_QUEUED remains in the action history even after the report transitions to REIMBURSED. That's why getIndicatedMissingPaymentMethod() uses !isSettled() - to avoid acting on that historical action once the report has transitioned to REIMBURSED.
Our selector already requires isWaitingOnBankAccount === true, so adding !isSettled() would only guard against an invalid REIMBURSED + isWaitingOnBankAccount state. I tested the full flow, and the backend cleared the flag when the report transitioned to REIMBURSED, and that persisted after a refresh.
|
@grgia Could you run the workflow to make sure the translations match?
|
This comment has been minimized.
This comment has been minimized.
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index cd7a0ad4132..f6cb6092ebf 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -964,9 +964,7 @@ const translations: TranslationDeepObject<typeof en> = {
title: 'Zeitkritisch',
addShippingAddress: {title: 'Wir benötigen deine Versandadresse', subtitle: 'Geben Sie eine Adresse an, um Ihre Expensify Karte zu erhalten.', cta: 'Adresse hinzufügen'},
addPaymentCard: {title: 'Fügen Sie eine Zahlungskarte hinzu, um Expensify weiter zu nutzen', subtitle: 'Konto > Abonnement', cta: 'Hinzufügen'},
- addBankAccount: {
- title: 'Fügen Sie ein Bankkonto hinzu, um Rückerstattungen zu erhalten',
- },
+ addBankAccount: {title: 'Fügen Sie ein Bankkonto hinzu, um eine Erstattung zu erhalten'},
activateCard: {title: 'Aktivieren Sie Ihre Expensify Karte', subtitle: 'Validieren Sie Ihre Karte und beginnen Sie mit dem Ausgeben.', cta: 'Aktivieren'},
reviewCardFraud: {
title: 'Möglichen Betrug mit Ihrer Expensify Karte überprüfen',
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 1b0bc747878..8c29a3f5ad1 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -929,9 +929,7 @@ const translations: TranslationDeepObject<typeof en> = {
subtitle: 'Cuenta > Suscripción',
cta: 'Añadir',
},
- addBankAccount: {
- title: 'Añade una cuenta bancaria para recibir tus reembolsos',
- },
+ addBankAccount: {title: 'Añade una cuenta bancaria para recibir el reembolso'},
activateCard: {
title: 'Activa tu Tarjeta Expensify',
subtitle: 'Valida tu tarjeta y empieza a gastar.',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 50252f59ad7..16bdc7fd84b 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -967,9 +967,7 @@ const translations: TranslationDeepObject<typeof en> = {
title: 'Urgent',
addShippingAddress: {title: 'Nous avons besoin de votre adresse de livraison', subtitle: 'Indiquez une adresse pour recevoir votre Carte Expensify.', cta: 'Ajouter une adresse'},
addPaymentCard: {title: 'Ajoutez une carte de paiement pour continuer à utiliser Expensify', subtitle: 'Compte > Abonnement', cta: 'Ajouter'},
- addBankAccount: {
- title: 'Ajoutez un compte bancaire pour recevoir vos remboursements',
- },
+ addBankAccount: {title: 'Ajoutez un compte bancaire pour être remboursé'},
activateCard: {title: 'Activer votre Carte Expensify', subtitle: 'Validez votre carte et commencez à dépenser.', cta: 'Activer'},
reviewCardFraud: {
title: 'Examiner une éventuelle fraude sur votre Carte Expensify',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index a97e1ee6851..63a2e9165bc 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -965,9 +965,7 @@ const translations: TranslationDeepObject<typeof en> = {
title: 'Sensibile al tempo',
addShippingAddress: {title: 'Ci serve il tuo indirizzo di spedizione', subtitle: 'Fornisci un indirizzo per ricevere la tua Carta Expensify.', cta: 'Aggiungi indirizzo'},
addPaymentCard: {title: 'Aggiungi una carta di pagamento per continuare a usare Expensify', subtitle: 'Account > Abbonamento', cta: 'Aggiungi'},
- addBankAccount: {
- title: 'Aggiungi un conto bancario per ricevere i rimborsi',
- },
+ addBankAccount: {title: 'Aggiungi un conto bancario per ricevere il rimborso'},
activateCard: {title: 'Attiva la tua Carta Expensify', subtitle: 'Convalida la tua carta e inizia a spendere.', cta: 'Attiva'},
reviewCardFraud: {
title: 'Esamina una possibile frode sulla tua Carta Expensify',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 2543093b7de..aa94a25afb6 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -954,9 +954,7 @@ const translations: TranslationDeepObject<typeof en> = {
title: '時間に敏感',
addShippingAddress: {title: '配送先住所が必要です', subtitle: 'Expensify カードを受け取る住所を入力してください。', cta: '住所を追加'},
addPaymentCard: {title: 'Expensify を引き続きご利用いただくには、支払いカードを追加してください', subtitle: 'アカウント ＞ サブスクリプション', cta: '追加'},
- addBankAccount: {
- title: '払い戻しを受け取るために銀行口座を追加してください',
- },
+ addBankAccount: {title: '銀行口座を追加して払い戻しを受け取りましょう'},
activateCard: {title: 'Expensify カードを有効化する', subtitle: 'カードを認証して支出を始めましょう。', cta: '有効化'},
reviewCardFraud: {
title: 'Expensify カードの不正利用の可能性を確認する',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 804a5f07729..b4c2e9b14ff 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -963,9 +963,7 @@ const translations: TranslationDeepObject<typeof en> = {
title: 'Tijdgevoelig',
addShippingAddress: {title: 'We hebben je verzendadres nodig', subtitle: 'Geef een adres op om je Expensify Kaart te ontvangen.', cta: 'Adres toevoegen'},
addPaymentCard: {title: 'Voeg een betaalkaart toe om Expensify te blijven gebruiken', subtitle: 'Account > Abonnement', cta: 'Toevoegen'},
- addBankAccount: {
- title: 'Voeg een bankrekening toe om vergoedingen te ontvangen',
- },
+ addBankAccount: {title: 'Voeg een bankrekening toe om je terugbetaling te ontvangen'},
activateCard: {title: 'Activeer je Expensify Kaart', subtitle: 'Valideer je kaart en begin met uitgeven.', cta: 'Activeren'},
reviewCardFraud: {
title: 'Controleer mogelijk misbruik van je Expensify Kaart',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 1123d54abb8..dd237b53364 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -965,9 +965,7 @@ const translations: TranslationDeepObject<typeof en> = {
title: 'Wymaga szybkiej reakcji',
addShippingAddress: {title: 'Potrzebujemy Twojego adresu wysyłki', subtitle: 'Podaj adres, na który mamy wysłać twoją Kartę Expensify.', cta: 'Dodaj adres'},
addPaymentCard: {title: 'Dodaj kartę płatniczą, żeby dalej korzystać z Expensify', subtitle: 'Konto > Subskrypcja', cta: 'Dodaj'},
- addBankAccount: {
- title: 'Dodaj konto bankowe, aby otrzymywać zwroty',
- },
+ addBankAccount: {title: 'Dodaj konto bankowe, aby otrzymać zwrot'},
activateCard: {title: 'Aktywuj swoją Kartę Expensify', subtitle: 'Zatwierdź swoją kartę i zacznij wydawać.', cta: 'Aktywuj'},
reviewCardFraud: {
title: 'Sprawdź potencjalne oszustwo na swojej Karcie Expensify',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 815e16930eb..f170d55a27f 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -963,9 +963,7 @@ const translations: TranslationDeepObject<typeof en> = {
title: 'Urgente',
addShippingAddress: {title: 'Precisamos do seu endereço de entrega', subtitle: 'Informe um endereço para receber seu Cartão Expensify.', cta: 'Adicionar endereço'},
addPaymentCard: {title: 'Adicione um cartão de pagamento para continuar usando o Expensify', subtitle: 'Conta > Assinatura', cta: 'Adicionar'},
- addBankAccount: {
- title: 'Adicione uma conta bancária para receber reembolsos',
- },
+ addBankAccount: {title: 'Adicione uma conta bancária para ser reembolsado'},
activateCard: {title: 'Ative seu Cartão Expensify', subtitle: 'Valide seu cartão e comece a gastar.', cta: 'Ativar'},
reviewCardFraud: {
title: 'Analisar possível fraude no seu Cartão Expensify',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 06ebf221ab8..04bf2d21e13 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -937,9 +937,7 @@ const translations: TranslationDeepObject<typeof en> = {
title: '时间敏感',
addShippingAddress: {title: '我们需要您的收货地址', subtitle: '请提供一个地址以接收您的 Expensify 卡。', cta: '添加地址'},
addPaymentCard: {title: '添加支付卡以继续使用 Expensify', subtitle: '账户 ＞ 订阅', cta: '添加'},
- addBankAccount: {
- title: '添加银行账户以便接收报销款',
- },
+ addBankAccount: {title: '添加银行账户以接收报销'},
activateCard: {title: '激活你的 Expensify 卡', subtitle: '验证您的银行卡并开始消费。', cta: '启用'},
reviewCardFraud: {
title: '审查您 Expensify 卡上的潜在欺诈交易',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
|
Applied |
|
@JmillsExpensify's issue, re-assigning. |
| switch (activeTab) { | ||
| case NAVIGATORS.SETTINGS_SPLIT_NAVIGATOR: | ||
| Navigation.goBack(ROUTES.SETTINGS_WALLET); | ||
| break; | ||
| case SCREENS.HOME: |
There was a problem hiding this comment.
NAB. should we add a test covering the Home exit path in goBack, asserting Navigation.closeRHPFlow fires when the flow starts from SCREENS.HOME?
There was a problem hiding this comment.
Good idea, I'll add one.
|
@ZhenjaHorbach the autoassigner didn't add ya |
| cta: 'Add', | ||
| }, | ||
| addBankAccount: { | ||
| // @context Home reminder shown to a payment recipient when a reimbursement is waiting for them to add a personal deposit account. |
There was a problem hiding this comment.
Not sure why @context is used to provide context for the strings 🤔, but that's the convention I've seen in this file. There are lots of examples that do the same thing.
|
🚧 grgia has triggered a test Expensify/App build. You can view the workflow run here. |
| const waitingReportIDsSelector = (allReports: OnyxCollection<Report>): string[] => | ||
| Object.values(allReports ?? {}) | ||
| .filter((report): report is Report => !!report?.reportID && report.isWaitingOnBankAccount === true && report.ownerAccountID === accountID) | ||
| .map((report) => report.reportID); |
There was a problem hiding this comment.
Can we use reduce to decrease the number of operations?
There was a problem hiding this comment.
Good suggestion, thanks! In this case, though, .map() only runs on the filtered result, which is at most one report in practice, so there's nothing meaningful to save here. I'd rather keep filter().map() for readability.
| const [userWalletTierName] = useOnyx(ONYXKEYS.USER_WALLET, {selector: tierNameSelector}); | ||
| const canShowAddBankAccount = accountID !== undefined && !isLoadingOnyxValue(bankAccountListMetadata) && !hasCreditBankAccount(bankAccountList); | ||
|
|
||
| const shouldShowAddBankAccountSelector = (allReportActions: OnyxCollection<ReportActions>): boolean => { |
There was a problem hiding this comment.
Let's move this to the selectors folder for consistency
And let's add tests
There was a problem hiding this comment.
@ZhenjaHorbach Hmm, I had a look before replying, and I'm not sure we have a consistent convention here:
src/selectors/has 136 exported selectors across 26 files.- There are also 199 named selectors defined in the same file that uses them, across 164 files, plus 45 inline selectors at the call site.
So moving selectors into src/selectors/ doesn't really seem to be the dominant pattern. It also doesn't seem to provide much benefit: 56 of those 136 selectors are referenced from exactly one file, and 2 aren't referenced outside the selectors folder at all. If they'd stayed next to the code that used them, they would have been removed along with it instead of being left behind.
With that in mind, I'd rather keep this one next to the hook, especially since it isn't just projecting a single Onyx value. It would need canShowAddBankAccount, waitingReportIDs, userWalletTierName, and bankAccountList passed in, three of which are themselves the results of useOnyx calls.
When you mentioned consistency, did you have something more specific in mind? Happy to move it if I'm missing the pattern you're pointing out.
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@grgia could you rerun the build? The iOS build failed because this branch wasn't up to date. I've synced it with the latest
|
5 participants
Explanation of Change
This PR adds a Time sensitive Home item when an ACH reimbursement is queued because the current payee has no personal deposit account. It uses the queued action's
paymentTypeto exclude Wallet/KYC waits, opens the existing bank-account setup flow, and adds the required route, copy, documentation, and unit coverage.Fixed Issues
$ #88187
PROPOSAL: #88187 (comment)
Tests
ACH reimbursement waiting for a personal bank account
Preconditions:
Wallet payment exclusion (not manually verified)
Note
I couldn't manually verify this case with the available test setup. The payer has a test Wallet, while an unactivated recipient is treated as a non-test Wallet, and the backend rejects that pairing with error code 666: “Cannot mix test and non-test wallets in a payment.” Therefore, the queued Wallet state can't currently be reached with these test accounts.
Preconditions:
Offline tests
N/A — this change only derives UI from existing Onyx state and opens the existing bank-account setup flow. It adds no API calls, writes, or offline-specific behavior.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
time.card.mp4
fix.success.navigation.mp4