Skip to content

[NO QA] [Payment due @abzokhattab] Vendor matching CC - R1: App Phase 1 (foundations)#91235

Merged
Beamanator merged 24 commits into
mainfrom
beaman/vendor-matching-r1-app
May 27, 2026
Merged

[NO QA] [Payment due @abzokhattab] Vendor matching CC - R1: App Phase 1 (foundations)#91235
Beamanator merged 24 commits into
mainfrom
beaman/vendor-matching-r1-app

Conversation

@Beamanator
Copy link
Copy Markdown
Contributor

@Beamanator Beamanator commented May 20, 2026

Explanation of Change

Phase 1 of Vendor matching CC - R1: App — pure foundations, no UI changes yet. Adds the types, constants, actions, utilities, and violation-computation logic that the next two phases (workspace settings; expense vendor field + selector) hang off.

Five foundational chunks, one commit each:

  1. Types + constants (Add vendor-matching types + constants (Track D1)) — Comment.vendor (transaction NVP shape), QBOConnectionConfig.nonReimbursableCreditCardDefaultVendor, VIOLATIONS.INACTIVE_VENDOR, QUICKBOOKS_CONFIG.NON_REIMBURSABLE_CREDIT_CARD_DEFAULT_VENDOR. Plus the surface bumps the new types caused ('vendor' added to useViolations.validationFields + violationNameToField['inactiveVendor'], DebugUtils ObjectType entries). Policy.areVendorsEnabled and MORE_FEATURES.ARE_VENDORS_ENABLED were originally part of this chunk but have been deferred to Phase 2, where the locked Vendors More Features card actually consumes them — see follow-up below.
  2. QBO default-vendor action (Track D2a) — updateQuickbooksOnlineNonReimbursableCreditCardDefaultVendor, mirror of the existing …BillDefaultVendor one-liner.
  3. updateMoneyRequestVendor action (Track D2b) — standalone action (doesn't go through getUpdateMoneyRequestParams) that writes the user's manual vendor pick. Hard-coded isManuallySet=true because this is the user-driven path; the PHP fuzzy matcher writes auto-matches with isManuallySet=false directly. Passing vendorID='' clears the vendor. Also optimistically drops any existing inactiveVendor violation from TRANSACTION_VIOLATIONS: the vendor selector RHP only offers vendors from getQBOVendors(policy) so a pick is always valid (resolves the violation), and clearing the vendor likewise resolves it. A targeted drop is preferred over a full ViolationsUtils.getViolationsOnyxData recompute because vendor changes don't route through getUpdateMoneyRequestParams (vendor lives on comment.vendor, not a top-level Transaction field), and passing empty policyTagList / policyCategories would mis-fire other violation branches. failureData restores the original violation list on server rejection.
  4. PolicyUtils helpers (Track D3) — hasVendorFeature, getQBOVendors, getQBOVendorByID. Mirror QuickbooksOnline::hasVendorFeature on the PHP side so App and backend agree. Also exports Vendor from src/types/onyx/Policy.ts so callers can reference the return type. The hasVendorFeature(policy, isVendorMatchingBetaEnabled) signature gained the second boolean param in a later follow-up — see below.
  5. ViolationsUtils (Track D4) — client-side inactiveVendor violation logic in getViolationsOnyxData. Four branches: feature disabled → drop stale violation; vendor present but not in policy's vendor list → add violation; vendor present and matched → drop violation; vendor cleared while feature still active → drop violation. The two pushTransactionViolationsOnyxData callers the design doc calls out (after vendor-list sync; after export-type change) live in Phase 2 where the natural call sites already exist.

Plus a few additional pieces of work on the same branch:

  • inactiveVendor translation across all locales — case in getViolationTranslation plus matching strings in all 10 locale files (en, de, es, fr, it, ja, nl, pl, pt-BR, zh-hans), following the categoryOutOfPolicy / tagOutOfPolicy "X no longer valid" pattern. Without this, the rendered violation message would fall through to the raw 'inactiveVendor' constant once Phase 2 wires up the push.
  • BETAS.VENDOR_MATCHING beta gate — the inactive-vendor violation runs purely client-side off Onyx data, so once the Web-Expensify commands and event hooks PR ships, the PHP fuzzy matcher writes will arrive at the App via Onyx sync and trigger the violation immediately. Without an App-side gate, that couples merge ordering. hasVendorFeature(policy, isVendorMatchingBetaEnabled) now takes the beta state as a required boolean param (mirroring the existing canAccessSubmitWorkspaceFeatures(policy, isSubmit2026BetaEnabled) precedent in the same file). ViolationsUtils reads the beta from a module-level Onyx.connectWithoutView subscription so the existing getViolationsOnyxData signature is unchanged (already at the 10-param eslint ceiling). Net effect: the violation block is fully inert until the beta is flipped on, regardless of when the Web-Expensify PR ships.
  • Defer areVendorsEnabled / ARE_VENDORS_ENABLED to Phase 2 — both were originally added as Phase 1 type-contract additions but had no consumers in this PR. hasVendorFeature(policy) derives from the QBO config and never reads policy.areVendorsEnabled; no code path passed ARE_VENDORS_ENABLED to isPolicyFeatureEnabled. Phase 2 will introduce both with the locked Vendors More Features card that actually consumes them, plus an early-return in isPolicyFeatureEnabled mirroring IS_ATTENDEE_TRACKING_ENABLED.
  • Lint cleanup — JSDoc on the new Comment.vendor inner properties; removed two dead eslint-disable-next-line lines in ViolationsUtils.ts (the inactiveVendor switch case made the previously-suppressed @typescript-eslint/no-unnecessary-type-assertion rule a true positive on the as never cast, so the cast and its suppression both went away).

Cross-PR dependencies

  • Auth foundation PR (merged) — UpdateMoneyRequestVendor command + inactiveVendor violation type + ACTION_CONCIERGE_AUTO_MATCH_VENDOR. Auth ships the command and violation type but nothing dispatches them — Auth's PR confirms "this command is not connected to Web-E anywhere yet."
  • Web-Expensify commands and event hooks PR (open) — PHP commands and event hooks the App's actions hit. Can merge at any time independent of this PR thanks to the VENDOR_MATCHING beta gate above — even if PHP starts writing comment.vendor to live transactions, the App's inactiveVendor block stays inert until the beta is flipped on per workspace.

Both must ship before the App's vendor flows actually function end-to-end. This PR's compiled output is harmless without them — the new actions just fail at the API boundary, and the violation logic is gated off by the VENDOR_MATCHING beta.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/638653

PROPOSAL: N/A (internal feature, no external proposal)

Tests

No user-visible UI yet — the Vendor field, Vendors tab, Default vendor row, and Concierge auto-match rendering all land in Phase 2 / Phase 3. Phase 1 is foundations only (types, constants, actions, utilities, client-side violation logic, localized violation string, beta gate).

New unit-test coverage in this PR:

  • tests/unit/ViolationUtilsTest.ts — new inactiveVendor violation describe block inside getViolationsOnyxData, seven cases: adds when missing from vendor list, deduplicates when already present, removes on restore, removes on user-cleared while feature still active, removes when feature disabled via export-type change, no-op when no QBO connection, and no-op when the vendorMatching beta is disabled even with QBO + Credit/Debit-card export configured. Uses jest.spyOn(Permissions, 'isBetaEnabled') to default the beta on for the branch tests.
  • tests/unit/PolicyUtilsTest.ts — new Vendor matching helpers describe with 13 cases across three sub-describes: hasVendorFeature (7 — Credit Card / Debit Card true with beta enabled; beta disabled forces false even with Credit Card export; Vendor Bill / unset / no-connection / undefined-policy all false), getQBOVendors (3 — returns list / empty for missing / undefined-safe), and getQBOVendorByID (3 — returns match / undefined for the inactive-vendor case / undefined for no-connection).
  • tests/actions/IOUTest/UpdateMoneyRequestVendorTest.ts — new dedicated test file with 4 cases for the optimistic-violation fix: vendor picked clears the violation, vendor cleared clears the violation, failureData restores the original list, and no-op when there's no inactiveVendor violation to clear. Uses jest.spyOn(API, 'write') to inspect the optimisticData / failureData shape.
  • All 24 new tests pass; the combined ViolationUtils + PolicyUtils + UpdateMoneyRequestVendor suite is green.

CI gates: all 25+ required checks (typecheck, test x8, spellcheck, lint, validateSchemas, validateImmutableActionRefs, dryRun, verifySignedCommits, codecov, snyk, etc.) were green on the prior HEAD; CI is re-running on the latest commit.

  • Verify that no errors appear in the JS console

Offline tests

N/A — this PR adds types, constants, action stubs, utility functions, client-side violation-derivation logic (gated off behind a beta), and a localized violation string. None of the new code runs without Phase 2 / Phase 3 wiring it into UI, and none of the new actions are dispatched from any user flow yet. There is no behavior change to exercise online vs offline.

QA Steps

[No QA] — Phase 1 foundations only, no user-visible behavior. The Vendor field, Vendors tab, Default vendor row, and Concierge auto-match rendering all ship in subsequent App PRs (Phase 2 + Phase 3) and will carry their own QA steps.

  • 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
  • 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 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 used JaimeGPT to get English > Spanish translation. I then posted it 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.ts 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 new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • 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.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • 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

N/A — Phase 1 foundations only. No UI changes in this PR. Screenshots will accompany Phase 2 (workspace settings) and Phase 3 (expense flow) App PRs.

What's next

  • Phase 2 — workspace settings: Vendors tab in workspace nav, Vendors locked card on More features, Default vendor row in QBO export settings, plus the two pushTransactionViolationsOnyxData calls (export-type change + vendor-list sync listener). Also introduces Policy.areVendorsEnabled and MORE_FEATURES.ARE_VENDORS_ENABLED alongside the locked card that consumes them. The BETAS.VENDOR_MATCHING gate is already in place from Phase 1, so Phase 2 only needs to thread usePermissions().isBetaEnabled(BETAS.VENDOR_MATCHING) into the new UI affordances.
  • Phase 3 — expense flow: Vendor field on MoneyRequestView, vendor selector RHP (IOURequestStepVendor), ConciergeAutoMatchVendor action rendering with Explain link, inactive-vendor violation rendering.

Foundation for Vendor matching CC R1 App work. No UI changes; purely
type system + constant additions that the upcoming chunks (actions,
utils, pages) hang off.

- src/types/onyx/Policy.ts:
  - `QBOConnectionConfig.nonReimbursableCreditCardDefaultVendor?: string`
    — workspace fallback vendor for QBO Credit/Debit card export.
  - `Policy.areVendorsEnabled?: boolean` — required by the MORE_FEATURES
    plumbing (pendingFields / errorFields type machinery). The actual
    enablement is derived at read time from the QBO config (see
    `PolicyUtils.hasVendorFeature` in a follow-up chunk), so this field
    is never persisted independently — it just satisfies the type
    contract.

- src/types/onyx/Transaction.ts:
  - `Comment.vendor?: { externalID: string; isManuallySet: boolean }`
    — auto-match writes from PHP (`isManuallySet=false`) and user picks
    or merchant rules (`isManuallySet=true`). The flag is what stops a
    later auto-match from overwriting a deliberate selection.

- src/CONST/index.ts:
  - `QUICKBOOKS_CONFIG.NON_REIMBURSABLE_CREDIT_CARD_DEFAULT_VENDOR`
    sibling to the existing `NON_REIMBURSABLE_BILL_DEFAULT_VENDOR`.
  - `POLICY.MORE_FEATURES.ARE_VENDORS_ENABLED` — used for the locked
    Vendors card on the workspace More features page.
  - `VIOLATIONS.INACTIVE_VENDOR: 'inactiveVendor'` — string matches the
    Auth-side constant from PR #21725.

- src/hooks/useViolations.ts:
  - Add `vendor` to the validation field tuple + `inactiveVendor` to the
    violationNameToField map. Mirrors how `tagOutOfPolicy` -> 'tag'.

- src/libs/DebugUtils.ts:
  - Add `vendor` entries to the two ObjectType maps for the transaction
    `comment` field (pendingFields + comment types). Required by the
    debug type machinery now that `Comment.vendor` exists.

typecheck-tsgo clean against these changes (the 2 pre-existing MapView /
GPSPoint errors on main are untouched). Prettier clean.

Issue: Expensify/Expensify#638653
…export (Track D2a)

App-side wrapper for the new Web-Expensify command
`UpdateQuickbooksOnlineNonReimbursableCreditCardDefaultVendor`. Direct
mirror of the existing `…BillDefaultVendor` action — same Onyx config
update pattern, just keyed on the new
`NON_REIMBURSABLE_CREDIT_CARD_DEFAULT_VENDOR` constant introduced in
the previous chunk.

- src/libs/API/types.ts: WRITE_COMMANDS entry + Parameters type
  registration for the new write command.
- src/libs/actions/connections/QuickbooksOnline.ts:
  - `updateQuickbooksOnlineNonReimbursableCreditCardDefaultVendor`
    function (mirrors the bill variant).
  - Exported in the default export sibling list.

The standalone `updateMoneyRequestVendor` action lives in the next
chunk (D2b) — it has different optimistic-update semantics since the
vendor is a comment NVP rather than a workspace-config field.

typecheck clean against changed files (only pre-existing MapView /
GPSPoint errors remain on main). Prettier clean.

Issue: Expensify/Expensify#638653
Standalone App-side action for user-driven vendor selection on a
non-reimbursable expense. Writes the vendor as `{ externalID,
isManuallySet: true }` to the transaction's `comment.vendor` NVP via
the new Web-Expensify `UpdateMoneyRequestVendor` command from
[#53009](Expensify/Web-Expensify#53009).

`isManuallySet` is hard-coded `true` because this action only fires
from user-driven flows (the App vendor picker, etc.); the PHP fuzzy
matcher writes auto-matches directly via the same Web-E command with
`isManuallySet=false`. Auth's defense-in-depth then prevents auto-match
from overwriting a manual selection.

Passing `vendorID=''` clears the vendor (Auth erases the NVP key).

Doesn't go through `getUpdateMoneyRequestParams` — vendor lives on the
comment NVP, not a top-level Transaction field, and doesn't trigger the
violation/category/tag cascade the helper handles. The optimistic Onyx
update merges `transaction.comment.vendor` directly. failureData
restores the previous vendor (or null if none was set).

Adds:
- `UpdateMoneyRequestVendorParams` type
- `WRITE_COMMANDS.UPDATE_MONEY_REQUEST_VENDOR` entry
- `updateMoneyRequestVendor` function + export

typecheck clean (only pre-existing MapView/GPSPoint errors remain). Prettier clean.

Issue: Expensify/Expensify#638653
Three small read-only helpers that the upcoming UI chunks (workspace
Vendors tab, default vendor RHP, vendor selector, MoneyRequestView
field) all need. Read-only on Policy / OnyxEntry; no Onyx writes.

- `hasVendorFeature(policy)` — mirrors `QuickbooksOnline::hasVendorFeature`
  on the PHP side: true when QBO is connected AND the workspace's
  non-reimbursable export is one of credit_card / debit_card. Used to
  gate the Vendor field on the expense, the Vendors tab in workspace
  settings, the Vendors locked card on the More features page, and the
  Default Vendor row on the QBO export page.
- `getQBOVendors(policy)` — returns the imported vendor list. Source of
  truth for the Vendors tab + vendor selector RHP. Enable/disable
  filtering is post-R1 polish.
- `getQBOVendorByID(policy, vendorID)` — resolves a single vendor by
  external ID. Used to display the vendor name on the expense when
  only the ID is stored on the transaction NVP. Returns undefined when
  the ID isn't found (which is the inactive-vendor case the upcoming
  ViolationsUtils logic checks for).

Also exports `Vendor` from `src/types/onyx/Policy.ts` (it was already
defined but not exported, so the helpers can reference the return type).

typecheck clean against changed files. Prettier clean.

Issue: Expensify/Expensify#638653
Client-side computation of `inactiveVendor` violations on
non-reimbursable card expenses (Vendor matching CC R1, QBO). Mirrors
the `categoryOutOfPolicy` / `tagOutOfPolicy` pattern — entirely
client-derived from the policy's imported QBO vendor list, no server
roundtrip needed once the list is in Onyx.

`ViolationsUtils.getViolationsOnyxData()` now folds in three vendor-
related branches:

1. **Feature disabled** — if `hasVendorFeature(policy)` is false (admin
   switched the export type away from credit/debit card), strip any
   stale `INACTIVE_VENDOR` violation. The `vendor` object on the
   transaction is intentionally left alone — clearing it would lose
   the user's prior selection if they ever switch back.

2. **Vendor not in list** — if the transaction has a `vendor.externalID`
   that doesn't exist in `policy.connections.quickbooksOnline.data.vendors`
   (post-deletion in QBO, or QBO disconnected), push a new
   `INACTIVE_VENDOR` violation so the admin knows to re-pick.

3. **Vendor restored** — if the ID IS found in the list, remove any
   existing `INACTIVE_VENDOR` violation.

Uses the `hasVendorFeature` and `getQBOVendorByID` helpers added in
the D3 chunk.

The two `pushTransactionViolationsOnyxData` call sites the design doc
calls out (after vendor-list sync; after export-type change) live in
Phase 2 (workspace-settings PR) where the natural callers already
exist — keeping Phase 1 to pure foundations.

typecheck clean against changed files (only pre-existing MapView /
GPSPoint errors remain on main). Prettier clean.

Issue: Expensify/Expensify#638653
Two trivial lint fixes blocking the PR's ESLint check:

- src/types/onyx/Transaction.ts: add JSDoc comments on the two inner
  properties of the new Comment.vendor object (externalID, isManuallySet).
  jsdoc/require-jsdoc was firing because the outer property had docs but
  the inner properties didn't.

- src/libs/Violations/ViolationsUtils.ts: remove an unused
  eslint-disable-next-line for @typescript-eslint/no-unnecessary-type-assertion.
  The rule isn't firing on 'violation.name as never' anymore — adding
  INACTIVE_VENDOR to the switch made the type narrowing exhaustive so the
  cast is no longer flagged, and the suppression became dead.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

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.

Files with missing lines Coverage Δ
src/CONST/index.ts 93.84% <ø> (ø)
src/libs/API/types.ts 100.00% <ø> (ø)
src/libs/DebugUtils.ts 68.37% <ø> (ø)
src/libs/actions/IOU/UpdateMoneyRequest.ts 80.81% <100.00%> (+0.92%) ⬆️
src/hooks/useViolations.ts 54.66% <50.00%> (-0.74%) ⬇️
src/libs/PolicyUtils.ts 71.27% <92.85%> (+0.30%) ⬆️
src/libs/actions/connections/QuickbooksOnline.ts 28.77% <0.00%> (-0.86%) ⬇️
src/libs/Violations/ViolationsUtils.ts 77.65% <79.16%> (+0.42%) ⬆️
... and 209 files with indirect coverage changes

Codex caught that the switch in getViolationTranslation had no case for
inactiveVendor, so once Phase 2 wires up the violation push, every UI
that renders violation messages would show the raw 'inactiveVendor'
constant name (via the default-branch 'violation.name as never' cast).

Adds:
- ViolationsUtils.ts: case 'inactiveVendor' between 'futureDate' and
  'invoiceMarkup', mirroring the categoryOutOfPolicy / tagOutOfPolicy
  shape exactly.
- src/languages/en.ts (source of truth) + all 9 other locales:
  inactiveVendor key with pattern-matched translation, derived from the
  existing categoryOutOfPolicy / tagOutOfPolicy entries per language.

Translation strings match the 'X no longer valid' pattern already
established for the sibling out-of-policy violations.
@OSBotify
Copy link
Copy Markdown
Contributor

🦜 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 diff
diff --git a/src/languages/de.ts b/src/languages/de.ts
index 22ae6cfd..ffef46cc 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -8551,7 +8551,7 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc
         duplicatedTransaction: 'Möglicherweise dupliziert',
         fieldRequired: 'Berichtsfelder sind erforderlich',
         futureDate: 'Zukünftiges Datum nicht erlaubt',
-        inactiveVendor: 'Anbieter nicht mehr gültig',
+        inactiveVendor: 'Lieferant nicht mehr gültig',
         invoiceMarkup: (invoiceMarkup: number) => `Um ${invoiceMarkup}% erhöht`,
         maxAge: (maxAge: number) => `Datum ist älter als ${maxAge} Tage`,
         missingCategory: 'Fehlende Kategorie',
diff --git a/src/languages/es.ts b/src/languages/es.ts
index ff1d6b7a..1919ecc4 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -8731,7 +8731,7 @@ ${amount} para ${merchant} - ${date}`,
         duplicatedTransaction: 'Posible duplicado',
         fieldRequired: 'Los campos del informe son obligatorios',
         futureDate: 'Fecha futura no permitida',
-        inactiveVendor: 'El proveedor ya no es válido',
+        inactiveVendor: 'Proveedor ya no válido',
         invoiceMarkup: (invoiceMarkup) => `Incrementado un ${invoiceMarkup}%`,
         maxAge: (maxAge) => `Fecha de más de ${maxAge} días`,
         missingCategory: 'Falta categoría',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index e7acae34..fbf1be09 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -8575,7 +8575,7 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
         duplicatedTransaction: 'Doublon potentiel',
         fieldRequired: 'Les champs de note de frais sont obligatoires',
         futureDate: 'Date future non autorisée',
-        inactiveVendor: 'Fournisseur plus valide',
+        inactiveVendor: 'Fournisseur non valide',
         invoiceMarkup: (invoiceMarkup: number) => `Majoration de ${invoiceMarkup} %`,
         maxAge: (maxAge: number) => `Date antérieure de plus de ${maxAge} jours`,
         missingCategory: 'Catégorie manquante',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 200c8147..2c4fc800 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -8435,7 +8435,7 @@ ${reportName}
         duplicatedTransaction: '重複の可能性',
         fieldRequired: 'レポートの項目は必須です',
         futureDate: '将来の日付は使用できません',
-        inactiveVendor: 'ベンダーは無効です',
+        inactiveVendor: '取引先は無効になっています',
         invoiceMarkup: (invoiceMarkup: number) => `${invoiceMarkup}%値上げ済み`,
         maxAge: (maxAge: number) => `日付が${maxAge}日より前です`,
         missingCategory: 'カテゴリが未選択です',

Note

You can apply these changes to your branch by copying the patch to your clipboard, then running pbpaste | git apply 😉

View workflow run

Adding 'inactiveVendor' to the switch made it exhaustive again, so
TypeScript narrows violation.name to 'never' in the default branch
on its own — the explicit cast is no longer doing any work and is
now genuinely flagged by @typescript-eslint/no-unnecessary-type-assertion.
@Beamanator
Copy link
Copy Markdown
Contributor Author

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7fbbc469a

ℹ️ 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".

Comment thread src/libs/Violations/ViolationsUtils.ts Outdated
Addresses Codex P1 r3284834993: the inactive-vendor block had three
explicit state branches but only handled two — when the vendor feature
is still active and the user clears their vendor selection
(transactionVendorID becomes empty), the existing INACTIVE_VENDOR
violation was never removed, leaving a stale error on a transaction
with no vendor set.

Adds the missing else-if branch: when no vendor is selected but a
stale violation is present, drop it.
@Beamanator
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

ℹ️ 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".

Both were added in Phase 1 as paired type-contract additions, but
nothing in this PR actually consumes either:

- hasVendorFeature(policy) reads exclusively from the QBO connection
  config (nonReimbursableExpensesExportDestination), never from
  policy.areVendorsEnabled — so the field is genuinely dead.
- No code path in Phase 1 passes ARE_VENDORS_ENABLED to
  isPolicyFeatureEnabled or indexes pendingFields with it.

The "MORE_FEATURES plumbing requires it" justification was circular:
the field was needed because the constant was added, and the constant
was added because of MORE_FEATURES type machinery — but nothing else
was wired up.

Phase 2 will introduce both alongside the locked Vendors More Features
card that actually consumes them, plus an early-return branch in
isPolicyFeatureEnabled mirroring IS_ATTENDEE_TRACKING_ENABLED:

    if (featureName === CONST.POLICY.MORE_FEATURES.ARE_VENDORS_ENABLED) {
        return hasVendorFeature(policy);
    }
Covers the new logic added in Phase 1 so regressions get caught before
Phase 2 wires up the UI call sites.

tests/unit/ViolationUtilsTest.ts - new "inactiveVendor violation"
describe inside getViolationsOnyxData, six cases covering every branch
of the new logic:
- adds violation when transaction vendor is not in the policy vendor list
- does not duplicate when one is already present
- removes existing violation when vendor is restored in the policy list
- removes existing violation when user clears the vendor (Codex P1 case)
- removes existing violation when feature is disabled (export type changed)
- does not add violation when no QBO connection exists

tests/unit/PolicyUtilsTest.ts - new "Vendor matching helpers" describe
covering the three new pure-function helpers (12 cases):
- hasVendorFeature: true for CREDIT_CARD/DEBIT_CARD export, false for
  VENDOR_BILL, unset, no-connection, undefined-policy
- getQBOVendors: returns vendor list, [] when no connection, [] when
  policy is undefined
- getQBOVendorByID: returns matching vendor, undefined when ID missing
  (the inactive-vendor case), undefined with no connection

All 18 new tests pass; the 362-test ViolationUtils + PolicyUtils suites
remain green.
Code comments should describe what the code does, not which project or release introduced it. References like "(Vendor matching CC R1, QBO)" and "post-R1 polish" belong in the PR description and rot as the codebase evolves. Removes four such tags across PolicyUtils.ts (hasVendorFeature, getQBOVendors), ViolationsUtils.ts (inactive-vendor block comment), and Transaction.ts (Comment.vendor JSDoc). No logic changes; 362 ViolationUtils+PolicyUtils tests still pass.
The inactive-vendor violation runs purely client-side off Onyx data,
so once Web-Expensify #53009 ships the PHP fuzzy matcher writes will
arrive at the App via Onyx sync and trigger the violation immediately
- with no App-side change needed. Without an App-side gate, that
couples merge ordering: Web-E #53009 cannot ship until Phase 2
introduces the gate.

This change decouples the two PR streams by adding the gate now.

- src/CONST/index.ts: add BETAS.VENDOR_MATCHING_CC = 'vendorMatchingCC'
- src/libs/PolicyUtils.ts: hasVendorFeature now takes
  isVendorMatchingCCBetaEnabled as a required boolean param, returning
  false when the beta is off regardless of QBO config. Mirrors the
  existing canAccessSubmitWorkspaceFeatures(policy, isSubmit2026BetaEnabled)
  pattern.
- src/libs/Violations/ViolationsUtils.ts: add a module-level
  Onyx.connectWithoutView subscription to BETAS (matching
  TransactionInlineEdit.ts), compute the beta state inside
  getViolationsOnyxData and pass it to hasVendorFeature. No signature
  change to getViolationsOnyxData (already at the 10-param eslint
  ceiling).
- tests/unit/PolicyUtilsTest.ts: update the six existing hasVendorFeature
  cases to pass the beta param explicitly; add a new case proving
  beta-off forces false even with Credit Card export configured.
- tests/unit/ViolationUtilsTest.ts: spy on Permissions.isBetaEnabled to
  default-enable the beta for the existing six branch tests; add a new
  case proving the violation block stays inert when the beta is off.

All 364 ViolationUtils + PolicyUtils tests pass. ESLint, Prettier,
cspell clean. typecheck-tsgo unchanged (only the pre-existing main
errors and the inherited ADD_WORK_EMAIL duplicate from origin/main).

Web-Expensify #53009 can now merge at any time.
@Beamanator
Copy link
Copy Markdown
Contributor Author

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ebc4ea10e0

ℹ️ 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".

Comment thread src/libs/actions/IOU/UpdateMoneyRequest.ts
Copied verbatim from canAccessSubmitWorkspaceFeatures, but that function is called from React components via usePermissions(). hasVendorFeature is called from ViolationsUtils.getViolationsOnyxData, a non-component module that reads betas via Onyx.connectWithoutView - usePermissions() isn't available there. The advice told the actual caller to use a pattern it cannot use.
…endor

Addresses Codex P1 r3299495523: the action only mutated comment.vendor
but never touched TRANSACTION_VIOLATIONS, so a transaction with an
existing inactiveVendor violation would keep the stale violation in
Onyx until some unrelated recalculation triggered ViolationsUtils.
getViolationsOnyxData. Users would see the red brick road / RBR
indicator persist even after picking a valid vendor or clearing the
selection.

This is the user-driven write path: the vendor selector RHP only
offers vendors from getQBOVendors(policy), so a user pick is always a
valid vendor (which resolves the violation), and clearing the vendor
likewise resolves it (no vendor -> no inactive-vendor violation).
Both flows can optimistically drop the violation.

A targeted update (drop only inactiveVendor) is preferred over a full
ViolationsUtils.getViolationsOnyxData call because:
- Full recompute would require threading policy, policyTagList,
  policyCategories, iouReport, etc. into the action. Vendor isn't a
  top-level transaction field so it doesn't go through
  getUpdateMoneyRequestParams (which has all that plumbing).
- Passing empty policyTagList / policyCategories would mis-fire other
  violation branches (e.g. categoryOutOfPolicy if the transaction has
  a category but the policy is partially loaded).

failureData restores the original violation list so a server
rejection cleanly rolls back to the pre-optimistic state.

New tests in tests/actions/IOUTest/UpdateMoneyRequestVendorTest.ts
cover four cases: vendor picked clears the violation, vendor cleared
clears the violation, failureData restores the original list, and
no-op when there was no inactiveVendor violation to clear.

All 4 new tests pass. ESLint, Prettier, cspell clean.
@Beamanator Beamanator marked this pull request as ready for review May 25, 2026 19:50
@Beamanator Beamanator requested review from a team as code owners May 25, 2026 19:50
@melvin-bot melvin-bot Bot requested a review from abzokhattab May 25, 2026 19:50
@Beamanator
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ 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".

@Beamanator
Copy link
Copy Markdown
Contributor Author

@abzokhattab did you review as well? 🙏

Copy link
Copy Markdown
Contributor

@abzokhattab abzokhattab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did the review again ... it all looks good to me .. I didn't find any regressions or issues 👍

@abzokhattab
Copy link
Copy Markdown
Contributor

abzokhattab commented May 27, 2026

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible 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 checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified there are no new alerts related to the canBeMissing param for useOnyx
  • I verified proper code patterns were followed (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 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
    • 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 verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • 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.ts or at the top of the file that uses the constant) are defined as such
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • 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.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • 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.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp
Android: mWeb Chrome
iOS: HybridApp
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-05-27.at.03.37.58.mov

@melvin-bot melvin-bot Bot requested a review from amyevans May 27, 2026 00:02
@melvin-bot
Copy link
Copy Markdown

melvin-bot Bot commented May 27, 2026

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

@melvin-bot melvin-bot Bot changed the title Vendor matching CC - R1: App Phase 1 (foundations) [Payment due @abzokhattab] Vendor matching CC - R1: App Phase 1 (foundations) May 27, 2026
@melvin-bot
Copy link
Copy Markdown

melvin-bot Bot commented May 27, 2026

🎯 @abzokhattab, thanks for reviewing and testing this PR! 🎉

A payment issue will be created for your review once this PR is deployed to production.
E/E issue linked to the PR - https://www.github.com/Expensify/Expensify/issues/638653.

If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created.

@abzokhattab
Copy link
Copy Markdown
Contributor

Validated the error shows correctly in the transaction preview by hadcoding onyx values in the offline case .... updated the web screenshots

Copy link
Copy Markdown
Contributor

@amyevans amyevans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good, some of the comments are overly verbose IMO, but not a blocker if you want to merge without updating. Could you also add No QA to the title please?

Comment thread src/libs/Violations/ViolationsUtils.ts
@Beamanator Beamanator merged commit 73d8d1e into main May 27, 2026
40 checks passed
@Beamanator Beamanator deleted the beaman/vendor-matching-r1-app branch May 27, 2026 17:05
@Beamanator
Copy link
Copy Markdown
Contributor Author

merging b/c #91756 (the latest checklist) was JUST shipped and i want to get this into staging :D then I'll address your comment @amyevans !

@github-actions
Copy link
Copy Markdown
Contributor

🚧 @Beamanator has triggered a test Expensify/App build. You can view the workflow run here.

@Beamanator Beamanator changed the title [Payment due @abzokhattab] Vendor matching CC - R1: App Phase 1 (foundations) [NO QA] [Payment due @abzokhattab] Vendor matching CC - R1: App Phase 1 (foundations) May 27, 2026
@OSBotify
Copy link
Copy Markdown
Contributor

✋ 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
Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/Beamanator in version: 9.3.84-0 🚀

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

Bundle Size Analysis (Sentry):

@MelvinBot
Copy link
Copy Markdown
Contributor

No help site changes are required for this PR.

This is Phase 1 foundations only — it adds types, constants, actions, utility functions, client-side violation logic (gated behind a beta), and localized violation strings. There are no user-visible UI or behavior changes.

The user-facing features that would warrant help site updates (Vendor field on expenses, Vendors tab in workspace settings, Default vendor row in QBO export settings, Concierge auto-match rendering) are all deferred to Phase 2 and Phase 3 App PRs, which should each be reviewed for help site impact when they land.

@Beamanator, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review

@izarutskaya
Copy link
Copy Markdown

Hi @Beamanator Can I check it off the list if it's no QA? Thanks

@Beamanator
Copy link
Copy Markdown
Contributor Author

@izarutskaya done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants