Repository navigation
OUT-4262: Disable QBO bank reconciliation beta and remove setting from UI - #291
aschwartz91 wants to merge 2 commits into
Conversation
- Add hard-off flag in isPortalInBankDepositABTest to disable the feature for all portals regardless of AB_FEATURE_TESTING_PORTALS env var - Remove bank deposit toggle and bank account picker from Invoice Details UI - Remove bank deposit warning modal and related state from useInvoiceDetailSettings - Remove unused props from InvoiceDetail component and SettingAccordion - Update unit tests to reflect the hard-off behavior The feature code is preserved and can be re-enabled by setting BANK_DEPOSIT_FEATURE_ENABLED to true in src/utils/abTesting.ts. This changes the semantics of AB_FEATURE_TESTING_PORTALS: previously empty/unset meant enabled for all portals; now the hard-off flag takes precedence, so empty/unset means disabled for everyone. Co-authored-by: Adam Schwartz <aschwartz91@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
 [Medium impact] Disables bank reconciliation feature across the product. Finish or explicitly clean up existing batched invoices before merging the hard-off. FindingsSummaryThis PR disables the bank deposit beta for every portal and removes its controls from Invoice Details.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Existing invoice: isBatchedDeposit = true"] --> B["Payment goes to Undeposited Funds"]
A --> C["Individual Stripe fee is deferred"]
B --> D["Payout arrives"]
C --> D
D --> E["Hard-off gate returns no deposit"]
E --> F["Webhook records SUCCESS"]
F --> G["Payment remains in Undeposited Funds; fee is missing"]
Reviews (1) · Last reviewed commit: "feat: disable QBO bank reconciliation be..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| * listed portals get it. | ||
| */ | ||
| export function isPortalInBankDepositABTest(_portalId: string): boolean { | ||
| if (!BANK_DEPOSIT_FEATURE_ENABLED) return false |
There was a problem hiding this comment.
Existing invoices with isBatchedDeposit = true still send payments to Undeposited Funds and defer their Stripe fees until the payout. This hard-off makes PayoutService.reconcile skip their deposit, but the webhook still records SUCCESS. Their payments remain in Undeposited Funds and their fees are never booked.
Disable batching for new invoices, but finish existing batched invoices or provide an explicit cleanup path before disabling payout work.
| }) | ||
|
|
||
| it('saves the flag and the bank account ref for an AB-test portal', async () => { | ||
| it('drops the flag even when gate mock returns true (defensive test for future re-enablement)', async () => { |
There was a problem hiding this comment.
The new name says the flag is dropped when the gate returns true, but the assertions require bankDepositFeeFlag to be saved as true and the bank account to be written. This gives maintainers the wrong description of what the test protects. Rename it to describe saving the flag and account when the gate allows the portal.
| it('drops the flag even when gate mock returns true (defensive test for future re-enablement)', async () => { | |
| it('saves the flag and bank account when the gate returns true', async () => { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…262) - Remove AB gate from PayoutService#reconcile() so existing invoices with isBatchedDeposit=true can still have their deposits created and fees booked - Keep the hard-off at the invoice freeze point (InvoiceService#readBankDepositFeeFlag) so new invoices freeze as non-batched - Update payout service unit tests to reflect the removal of AB gate - Rename misnamed test to 'saves the flag and bank account when the gate returns true' This ensures payments already in Undeposited Funds get properly deposited and Stripe fees are booked, while preventing new invoices from being batched. Co-authored-by: Adam Schwartz <aschwartz91@users.noreply.github.com>
Context for @priosshrsth
Marlon approved shutting down the QBO automatic bank reconciliation beta. It still has bugs reported from customers, and further work is backlogged. As of today, all 8 beta workspaces have been emailed that it's turning off this week, with success@assembly.com cc'd.
This PR is the code side of OUT-4262 removes the setting from the UI and hard-disables deposit creation for every portal. Regular invoice and payment sync isn't affected.
After merging: clear the beta workspaces from
AB_FEATURE_TESTING_PORTALS(rZSajb5cp, s2L9uTbcp, us-west-2_RGylW5Iu6, O2LKFe_9p, t4gOgqxzp, ASNx0Cznp, mWBjPk09p, us-east-1_FwKKN1cgl). The hard-off flag already blocks the feature, but this keeps the env tidy. I'm out after Friday, so please reach out to Marlon with any questions.Context for @priosshrsth
Marlon approved shutting down the QBO automatic bank reconciliation beta. It still has bugs reported from customers, and further work is backlogged. As of today, all 8 beta workspaces have been emailed that it's turning off this week, with success@assembly.com cc'd.
This PR is the code side of OUT-4262 removes the setting from the UI and hard-disables deposit creation for every portal. Regular invoice and payment sync isn't affected.
After merging: clear the beta workspaces from
AB_FEATURE_TESTING_PORTALS(rZSajb5cp, s2L9uTbcp, us-west-2_RGylW5Iu6, O2LKFe_9p, t4gOgqxzp, ASNx0Cznp, mWBjPk09p, us-east-1_FwKKN1cgl). The hard-off flag already blocks the feature, but this keeps the env tidy. I'm out after Friday, so please reach out to Marlon with any questions.Changes
BANK_DEPOSIT_FEATURE_ENABLED = false) inisPortalInBankDepositABTest()to block new invoices from being marked as batchedPayoutService#reconcile()so existing invoices withisBatchedDeposit=truecan still have their deposits created and Stripe fees bookeduseInvoiceDetailSettingshookInvoiceDetailcomponent andSettingAccordionResolves OUT-4262
Requesting review from @priosshrsth
How Existing Batched Invoices Are Handled
The hard-off works at two different points:
InvoiceService#readBankDepositFeeFlag)false→ new invoices freeze as non-batchedPayoutService#reconcile)isBatchedDeposit=trueinvoices get their deposits created and fees bookedThis ensures payments already in Undeposited Funds are properly deposited and Stripe fees are booked, while preventing new invoices from being batched.
Env Var Semantics Change
Important: This changes the semantics of
AB_FEATURE_TESTING_PORTALS:To re-enable the feature in the future, set
BANK_DEPOSIT_FEATURE_ENABLED = trueinsrc/utils/abTesting.ts.Testing Criteria
npx vitest run --project unit)isPortalInBankDepositABTestdirectly)Note: Integration tests could not be run in the CI environment due to Docker unavailability, but they mock the AB gate function directly and will continue to work.
Notes
qb_settings.bank_deposit_fee_flagvalues in the DB are ignored at runtime since the hard-off gate returnsfalsebefore checking themImpact & Surface Area of Change
InvoiceService#readBankDepositFeeFlagreturnsfalsefor all portals, so new invoices freeze withisBatchedDeposit = falsePayoutService#reconcilecontinues to process all payouts normally, so existing batched invoices get their deposits created