Add vendorID action field to merchant coding rules (vendor matching R5, Phase B) - #97811
Add vendorID action field to merchant coding rules (vendor matching R5, Phase B)#97811MelvinBot wants to merge 5 commits into
Conversation
Co-authored-by: Alex Beaman <Beamanator@users.noreply.github.com>
🦜 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 d0991c549c7..75af3475e5d 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -7696,6 +7696,8 @@ Fordern Sie Spesendetails wie Belege und Beschreibungen an, legen Sie Limits und
expenseDefaultsSubtitle: 'Felder aktualisieren, ohne dass die einreichende Person etwas tun muss',
ifAnyExpenseMatches: 'Wenn eine Ausgabe übereinstimmt:',
thenApplyFollowingDefaults: 'Wenden Sie dann die folgenden Standardwerte an:',
+ vendorUnavailable: 'Anbieter nicht verfügbar',
+ supplierUnavailable: 'Lieferant nicht verfügbar',
},
categoryRules: {
title: 'Kategorienregeln',
diff --git a/src/languages/el.ts b/src/languages/el.ts
index 4391e63ad2c..5176e22b090 100644
--- a/src/languages/el.ts
+++ b/src/languages/el.ts
@@ -8027,6 +8027,8 @@ ${reportName}`,
importColumnUpdatedCategory: 'Ενημερωμένη κατηγορία',
importColumnUpdatedTag: 'Ενημερωμένη ετικέτα',
importColumnUpdatedDescription: 'Ενημερωμένη περιγραφή',
+ vendorUnavailable: 'Ο προμηθευτής δεν είναι διαθέσιμος',
+ supplierUnavailable: 'Ο προμηθευτής δεν είναι διαθέσιμος',
},
newRule: {
title: 'Νέος κανόνας',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index e27e999b89d..9c4069c444e 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -7723,6 +7723,8 @@ Rendez obligatoires des informations de dépense comme les reçus et les descrip
expenseDefaultsSubtitle: 'Mettre à jour les champs sans que le déclarant ne fasse quoi que ce soit',
ifAnyExpenseMatches: 'Si une dépense correspond :',
thenApplyFollowingDefaults: 'Appliquez ensuite les valeurs par défaut suivantes :',
+ vendorUnavailable: 'Fournisseur indisponible',
+ supplierUnavailable: 'Fournisseur indisponible',
},
categoryRules: {
title: 'Règles de catégorie',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index cf6c4b48a40..0b7ac73f1bf 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -7668,6 +7668,8 @@ Richiedi dettagli sulle spese come ricevute e descrizioni, imposta limiti e valo
expenseDefaultsSubtitle: 'Aggiorna i campi senza che chi invia debba fare nulla',
ifAnyExpenseMatches: 'Se una qualsiasi spesa corrisponde a:',
thenApplyFollowingDefaults: 'Quindi applica le seguenti impostazioni predefinite:',
+ vendorUnavailable: 'Fornitore non disponibile',
+ supplierUnavailable: 'Fornitore non disponibile',
},
categoryRules: {
title: 'Regole di categoria',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index b5cb7e25463..87a6d7d2bab 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -7575,6 +7575,8 @@ ${reportName}`,
expenseDefaultsSubtitle: '申請者が何も操作しなくてもフィールドを更新する',
ifAnyExpenseMatches: 'いずれかの経費が次の条件に一致する場合：',
thenApplyFollowingDefaults: '次に、以下のデフォルトを適用します。',
+ vendorUnavailable: 'ベンダーを利用できません',
+ supplierUnavailable: 'サプライヤーを利用できません',
},
categoryRules: {
title: 'カテゴリルール',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 0ef89e0a32c..137d17ce334 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -7651,6 +7651,8 @@ Vereis onkostendetails zoals bonnen en beschrijvingen, stel limieten en standaar
expenseDefaultsSubtitle: 'Velden bijwerken zonder dat de indiener iets hoeft te doen',
ifAnyExpenseMatches: 'Als een uitgave overeenkomt:',
thenApplyFollowingDefaults: 'Pas vervolgens de volgende standaardinstellingen toe:',
+ vendorUnavailable: 'Leverancier niet beschikbaar',
+ supplierUnavailable: 'Leverancier niet beschikbaar',
},
categoryRules: {
title: 'Categorisatieregels',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index cbe6f9a6372..fbe2e673251 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -7630,6 +7630,8 @@ Wymagaj szczegółów wydatków, takich jak paragony i opisy, ustawiaj limity i
expenseDefaultsSubtitle: 'Aktualizuj pola bez wymagania działania od osoby zgłaszającej',
ifAnyExpenseMatches: 'Jeśli którykolwiek wydatek pasuje:',
thenApplyFollowingDefaults: 'Następnie zastosuj następujące domyślne ustawienia:',
+ vendorUnavailable: 'Dostawca niedostępny',
+ supplierUnavailable: 'Dostawca niedostępny',
},
categoryRules: {
title: 'Reguły kategorii',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index d755a20e994..9d92516656a 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -7646,6 +7646,8 @@ Exija dados de despesas como recibos e descrições, defina limites e padrões e
expenseDefaultsSubtitle: 'Atualizar campos sem o responsável pelo envio fazer nada',
ifAnyExpenseMatches: 'Se alguma despesa corresponder:',
thenApplyFollowingDefaults: 'Em seguida, aplica os seguintes padrões:',
+ vendorUnavailable: 'Fornecedor indisponível',
+ supplierUnavailable: 'Fornecedor indisponível',
},
categoryRules: {
title: 'Regras de categoria',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index ee4798c1282..5452f7f8265 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -7399,6 +7399,8 @@ ${reportName}`,
expenseDefaultsSubtitle: '在提交人无须执行任何操作的情况下更新字段',
ifAnyExpenseMatches: '如果任一报销符合以下条件：',
thenApplyFollowingDefaults: '然后应用以下默认设置：',
+ vendorUnavailable: '供应商不可用',
+ supplierUnavailable: '供应商不可用',
},
categoryRules: {
title: '类别规则',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@MelvinBot fix the polyglot changes by running the correct script Then in a new commit fix Oxfmt and typescript checks |
Co-authored-by: Alex Beaman <Beamanator@users.noreply.github.com>
Co-authored-by: Alex Beaman <Beamanator@users.noreply.github.com>
|
Done — pushed two commits to 1. Polyglot translations ( 2. Oxfmt (
|
|
@MelvinBot fix the formatting of the PR template text, then consider what kind of tests we need to write for #97811 (comment) |
|
1. PR template formatting — fixed. The body had a stray reviewer-checklist block and a 2. Tests for the Codecov drop The Codecov report flags two genuine gaps and a lot of noise. The existing a)
b)
c) The "Set vendor to" row in Skip (noise): the deltas on Reply |
|
from Alex's Claude agent I looked at each proposal against the current tree. Disagree with (a), agree with (b) + (c), agree skipping the nav wiring. (a) PolicyUtils vendor helpers — skipAlready covered on
These landed with R2 / R4. The Codecov "gap" on them in this PR is measurement noise from the impacted-files view, not a real coverage hole — nothing here would exercise those branches better than the existing R2/R4 suite. Adding a duplicate matrix in this PR would be pure scope creep. (b) AddVendorPage.tsx — agreeNew file, 53 lines, 0% coverage. RTL test file Fixtures — QBO policy (with vendor list), Xero policy (with contacts), empty-list policy. Seed Cases to assert:
(c) "Set vendor to" row in
|
Co-authored-by: Alex Beaman <Beamanator@users.noreply.github.com>
|
Done — pushed What landed
Deviation: pure-logic unit tests instead of RTL renders — and why
What that leaves uncovered: the parts of your plan that genuinely need a render — If you'd rather have the full RTL suite exactly as scoped, say the word and I'll write it — with the caveat that I can't run it locally, so it'll rely on CI to validate.
|
|
@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: 8b91c1e25c
ℹ️ 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".
| key: 'vendorID', | ||
| description: vendorFieldLabel, | ||
| title: vendorDisplayName, |
There was a problem hiding this comment.
When VENDOR_MATCHING is enabled but RULES_REVAMP is not, this new row lets users create or edit merchant rules whose only update is vendorID, but the legacy rules list still renders descriptions via src/pages/workspace/rules/MerchantRulesSection.tsx#getRuleDescription, which never reads rule.vendorID. After saving a vendor-only rule in that legacy flow, the rule card returns with a blank action summary, making it look like the rule does nothing; the legacy summary renderer needs the same vendor/supplier handling added here for the revamp table.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@MelvinBot analyze this comment and fix if needed
There was a problem hiding this comment.
Confirmed — valid P2, and fixed in cded6ec.
Why it's real: the "Set vendor to" row is gated only by isVendorFeatureEnabled (VENDOR_MATCHING beta + an active QBO/Intacct/Xero integration) — MerchantRulePageBase.tsx:218 — with no RULES_REVAMP dependency. The add/edit pages that render it are shared by both flows, so a user on the legacy experience can save a vendor-only rule. The legacy summary renderer getRuleDescription handled merchant/category/tag/comment/tax/reimbursable/billable but had no vendorID branch, so such a rule came back with a blank action summary.
Fix: added a vendorID branch to getRuleDescription mirroring the revamp table (MerchantTypeRulesUtils.ts:159-170) — resolve the name via findVendorByID, fall back to the Xero-aware supplierUnavailable/vendorUnavailable string when the vendor list is loaded, else the raw ID — plus a Xero-aware vendor/supplier field label (MerchantRulesSection.tsx:72-83). Both summary paths now render vendor-only rules identically.
⚠️ tsc/oxfmtcan't run in this environment; I matched the config by hand (import order per.oxfmtrc.json, types against thePolicyUtilssignatures). CI is the final validator.getRuleDescriptionis a local (non-exported) function, so a targeted unit test would need it exported or an RTL render — happy to add one if you want it; the logic it now mirrors is already covered for the revamp path.
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
3 participants
Explanation of Change
Implements Phase B of vendor matching for company card expenses (R5) — adds
vendorIDas an action field on merchant coding rules so admins can create a rule that automatically sets a specific vendor on all expenses from a given merchant. This is the App side of the project; the Auth and Web-Expensify pieces are handled separately.Everything is gated behind
hasVendorFeature(policy, isBetaEnabled(CONST.BETAS.VENDOR_MATCHING)), so nothing is visible off-beta or on a workspace without a connected vendor-matching integration (QBO / Sage Intacct / Xero). Per the issue's resolved field-naming decision, the stored value is the integration's external vendor ID (vendorID), and only the ID is stored — the display name is always resolved at render time viagetMatchingVendors/findVendorByID, so renamed vendors stay in sync with the backend.What's included (Milestones 1–3, one PR):
VENDOR_IDinput onMerchantRuleForm,vendorID?: stringon theCodingRuleOnyx type, andvendorIDwired through both rule mappers inPolicy/Rules.ts(serialized asnullwhen unset for Onyx merge, omitted entirely for the API — mirroring howtagis handled).RULES_MERCHANT_VENDORscreen/route wired throughSCREENS,ROUTES,ModalStackNavigators,linkingConfig,WORKSPACE_TO_RHP, andNavigation/types, plus a newAddVendorPage.tsx(modeled onAddCategoryPage.tsx) that sources the list fromgetMatchingVendors(policy)and flips the header between Vendor and Supplier for Xero workspaces viaisXeroActiveMatchingSource.MerchantRulePageBase's Apply Updates section (gated as above, Xero-aware label), and a vendor line in the rules-list summary (getMerchantCodingRulesTableData). The summary resolves the name viafindVendorByID; if the ID is unresolved and the vendor list is loaded it shows "Vendor/Supplier unavailable", otherwise (list not yet synced) it falls back to the raw external ID to avoid flashing "unavailable" during sync.Explicitly out of scope (per the issue): CSV importer changes (
ImportedMerchantRulesPage/PreviewMatchesPageuntouched — follow-up), any Concierge action UI, and any cached vendor name.Sequencing note: the beta must not be enabled on any real workspace until the corresponding Web-Expensify change lands and deploys.
Automated tests: Added
tests/unit/VendorMatchingMerchantRulesTest.tscovering the two rule mappers (vendorID set/unset), the rule-summary resolved / unavailable-but-loaded / list-not-loaded branches, the Xero supplier-wording flip, and the beta+integration gating predicate that governs the editor row's visibility.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/638612
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Screenshots/Videosundefined