# QuickBooks Integration v2 — Post-Launch Readiness Plan **Status:** Active **Created:** 2026-05-06 **Owner:** rvanvuren **Goal:** Bring Spec 048 to "complete and trustworthy" before any external user touches it. This is a financial integration — slow rollout is intentional. ## Background This document captures the plan to close out Spec 048 (QuickBooks Integration v2) before external beta. It was created after a session of E2E testing on store `pc00` revealed several latent bugs (UI rendering, reconciliation math, JE staging direction) and surfaced a list of code paths we've never actually exercised. What was already fixed in this session is in `git log` and the implementation summary below. This document is forward-looking — what still needs to happen. ## What got fixed in the discovery session (2026-05-06) For context — these are *done*, not action items: - **Reconciliation `qboTotal=0` bug** — QBO `JournalEntry.TotalAmt` is always 0 for JEs. Fixed `ReconciliationService::computeQboTotal()` to prefer debit-line sum. - **Reconciliation `posTotal` over-counting** — was summing every column of `drsDailySFileData` (~30× over-inflation). Now reads canonical `totalDebits` from `qb_staged_entries`. - **Auto-balance variance handling** — uses `MAX(staged.totalDebits, staged.totalCredits)` to match the post-balance QBO debit sum, so reconciliation correctly shows variance=0 for legitimately auto-balanced entries. - **Returns sign-flip mapping** — `newReturns`/`usedReturns` mappings updated from `entryType=debit` → `credit` (migration `048_006_qb_returns_entry_type_fix.json`). Returns now post as DEBITS to contra-revenue accounts. - **Bootstrap 5 modal backdrop stacking** — all 5 approval-queue modals (and modals on sync-log + audit-log) now move to `` on init to escape the `#wrapper > #page-wrapper` stacking context. `getOrCreateInstance` used everywhere to prevent duplicate-backdrop bugs. - **Stale-row 404 handler** — `loadEntryDetails` catches 404 and refreshes the queue with a toast instead of showing the raw error. - **Syncfusion Grid hidden-tab issue** — documented in skill, no code change needed (rAF override is testing-only). ## Phase 1 — Data correctness (must close before any external user) **Exit gate:** any pc00 day in the last 60 days re-stages with `gap = real POS variance only`. No buggy returns flip, no missing offset accounts. ### 1.1 `storeCreditsIssued` offset mapping — RESOLVED (no change needed) - **Status:** WITHDRAWN after deeper analysis on 2026-05-07. - **Original concern:** `storeCreditsIssued` mapping has `offsetQbAccountId = NULL`, appearing to leave a $19.94/day uncovered credit. - **Why it's not a bug:** `cashTendered` is a NET number (cash in from sales minus cash refunds out — verified against drawer math: `openingCash + cashTendered − buyPaidCash + cashOverShort = cashActual`). When a customer is issued a store credit instead of a cash refund, `cashTendered` is implicitly $X higher than it would have been with a cash refund; the matching CREDIT to `storeCreditsIssued` liability balances against this implicit DEBIT increase. Adding an explicit offset would double-count. - **Optional sanity check:** Confirm with the accountant that `cashTendered` is expected to be net of cash refunds. If they say "it should be gross," then the POS-side data convention needs to change, not the JE mapping. 30-second conversation, not a blocker for Phase 1. ### 1.2 Clean up May 1–5 pending entries - **Problem:** Entries staged before the returns mapping fix still have the buggy direction. They will auto-balance through Cash Over/Short on approval, inflating it by `2 × (newReturns + usedReturns)`. - **Action:** For each of May 1, May 2, May 3, May 4, May 5: 1. Reject via Approval Queue UI with reason "Re-staging with corrected mapping" 2. `DELETE FROM qb_staged_entries WHERE id = X` (or via Phase 1.3 below if it lands first) 3. `POST /api/quickbooks/pc00/sync/manual` with `date=YYYY-MM-DD` 4. Verify new entry's `totalDebits` and `totalCredits` are closer to balanced - **Acceptance:** All 5 dates re-staged. New entries' imbalance is consistent with legitimate POS variance only (typically <$300/day for this store). - **Est:** 30 min. ### 1.3 Allow re-stage of rejected entries — DECISION: allow with audit preservation - **Decision (2026-05-07):** Yes, allow re-stage. Preserve the rejected row as an audit record by allowing multiple rows per date. - **Problem:** `JournalEntryService::stageEntry()` refuses to re-stage when ANY entry exists for the date, even if the existing entry is rejected. Beta users who reject by mistake have no UI recovery — only DBA-level DELETE. - **Why allow:** (1) misclick recovery — a beta user shouldn't need a support ticket to undo a wrong reject; (2) data corrections — POS may re-issue corrected close data; (3) mapping-bug recovery — exactly what we hit with 048_006; entries rejected because their numbers looked wrong should be re-stageable after the fix. - **Implementation (preserve approach, NOT replace):** 1. **Migration:** Drop `UNIQUE KEY uk_sync_date` on `qb_staged_entries`. Replace with non-unique `INDEX idx_sync_date` (still needed for query performance). 2. **stageEntry duplicate-date check:** only block if a non-rejected row exists for that date. SQL: `SELECT id FROM qb_staged_entries WHERE syncDate = :date AND status <> 'rejected' LIMIT 1` 3. **Lookup queries:** every read of "the staged entry for this date" needs `WHERE status <> 'rejected' ORDER BY id DESC LIMIT 1`. Already in place for `getStagedTotalDebits` (from reconciliation fix). Spread to all callers. Files to audit: `JournalEntryService.php`, `ApprovalService.php`, `ReconciliationService.php`, `QBApiController.php`. 4. **New audit event:** `'sync_restaged'` with `priorEntryId` in the details JSON so the audit trail clearly shows "this re-staging supersedes prior entry #N". 5. **UI:** "Re-sync this date" button on rejected rows in the Approval Queue when filter is set to `status=rejected`. Calls `POST /sync/manual` with the date. - **Race-condition note:** dropping the unique constraint means two simultaneous manual-sync calls for the same date could insert two pending rows. Mitigate with a `SELECT ... FOR UPDATE` inside a transaction in `stageEntry` (lock the date-row range during the duplicate check), OR application-level mutex via Redis. For Phase 1, transaction-level locking is sufficient. - **Acceptance:** (a) Reject an entry via UI; (b) click "Re-sync this date"; (c) confirm a new staged entry is created with `pending_approval` status and the rejected entry stays in the table; (d) audit log shows: original sync_staged → reject → sync_restaged events; (e) approve the new entry; (f) verify the JE in QBO references the NEW entry's id, not the rejected one. - **Est:** 1.5–2 hr including the constraint migration, query audit, locking, and unit tests. ### 1.4 Verify migration `048_006` deploys to real (non-dev) stores - **Problem:** `getAllStoresData(0, 0)` filters `WHERE dev = 0`, so the migration runner never applied `048_006` locally (pc00 is `dev=1`). The migration JSON was tested only via direct SQL on the dev store. - **Action:** Either (a) temporarily flip pc00 `dev=0`, run `php conductor run`, verify migration_log entry exists, flip back; or (b) test against a known production store on a staging environment if one exists. - **Acceptance:** `migration_log` shows `048_006_qb_returns_entry_type_fix_` with `status='success'` for at least one non-dev store. - **Est:** 15 min. ### 1.5 Unit tests for this session's fixes - **Problem:** The reconciliation fixes have new tests in `ReconciliationServiceTest.php` but the JE staging side (the actual sign-flip behavior fix) doesn't have a regression test. Future maintainer could revert the mapping or change the sign-flip logic without test catching it. - **Action:** Add tests in `JournalEntryServiceTest.php` covering: - Negative-valued field with `entryType=credit` mapping → posts as DEBIT - Negative-valued field with `entryType=debit` mapping → posts as CREDIT (current behavior, codified so the trap is visible) - The `cashOverShort` adjustment-style use case (sign IS directional) — should still work - **Acceptance:** Tests pass; intentionally breaking the sign-flip in `buildJournalLines` fails the new tests. - **Est:** 1 hr. ### 1.6 Sign-flip code clarification (lightweight) - **Problem:** The mapping fix in 048_006 is *counter-intuitive* — `newReturns` is marked `entryType=credit` but actually posts as DEBIT due to the sign-flip. A future maintainer could "fix" this back to `debit` and reintroduce the bug. - **Action:** Add an inline code comment at `JournalEntryService.php:494-547` (around `buildJournalLines`) explaining the convention: "Negative-valued fields flip their entry type. For fields with consistently-negative source data (returns, refunds), the mapping `entryType` is set to the opposite of the desired direction so the flip lands correctly. Do not 'fix' this without changing both the data convention and the flip logic." - **Acceptance:** Comment in source explaining the trap. - **Est:** 5 min. ## Phase 2 — Untested critical surfaces (must close before external beta) **Exit gate:** every UI surface has been clicked at least twice (happy path + error path) and each error shows a clear user-facing message. Pattern for each: drive via Claude-in-Chrome MCP or manual browser → verify API call matches expectation → verify QBO sandbox state updated correctly → verify our local DB reflects the result. ### 2.1 Edit flow **Test scenarios:** - (a) Edit a line amount by $0.50 (no reason required), save, verify staged payload reflects the change but still says `pending_approval`. - (b) Edit a line by $5.00 (reason required), verify save is blocked without reason ≥ 10 chars. - (c) Edit `taxCollected` to $0, verify save is blocked (Rule 6 — tax cannot be zeroed). - (d) Add 21 adjustment lines, verify the 21st is rejected (Rule 7 cap = 20). - (e) After edit, approve and verify the QBO JE has the EDITED values, not the original. **Watch out for:** front-end posts `editedPayload` but backend approval reads `originalPayload`. This is a class of bug that's invisible in unit tests but lethal in production. **Acceptance:** All 5 scenarios pass. Audit log shows the diff between original and edited for any approved-after-edit entry. ### 2.2 Void flow **Test scenarios:** - Approve an entry first (creates JE in sandbox) - Click Void with reason - Verify: (a) local entry status = `voided`; (b) the JE in QBO sandbox is actually voided (re-query and check); (c) audit log captures the void with reason and user. **Watch out for:** Local state updates but QBO call fails silently (the JE is still posted in QBO but our system thinks it's voided — book inconsistency). **Acceptance:** State consistency between our DB and QBO sandbox after void. ### 2.3 Bulk Approve **Test scenarios:** - Select 3 pending entries, click Bulk Approve - Verify chronological order is enforced server-side - Then deliberately break one (zero out tax to violate Rule 6 first), bulk approve again - Verify processing stops at the broken one and the front-end shows "succeeded: X, failed: Y" clearly **Watch out for:** Rule 4 says "stop on first failure" — verify partial state is consistent (no half-posted JEs in QBO that aren't reflected locally). **Acceptance:** Partial-failure scenario produces a clean state — succeeded entries are `posted`, the failed one is still `pending_approval`, the un-attempted ones are also still `pending_approval`. ### 2.4 Settings page **Test scenarios:** - (a) Toggle environment sandbox ↔ production: does the next sync use the new environment? - (b) Toggle sync mode auto ↔ manual: does an "auto" mode skip the staging step entirely? - (c) Force OAuth re-auth: does it work? Does an in-flight token survive? **Watch out for:** If "auto" mode is broken, JEs post without review. **Acceptance:** All 3 toggles work, environment indicator is visible site-wide so users always know which environment they're in. ### 2.5 Account Mapping page **Test scenarios:** - (a) Edit a mapping row and save. Confirm DB persists and next stage uses the new mapping. - (b) Try to edit mapping mid-sync: is there a guard, or does it cause a half-mapped JE? - (c) "Refresh accounts from QBO" if such a button exists: does it surface deleted accounts? **Watch out for:** If mappings can be edited mid-period without warning, books get inconsistent. **Acceptance:** Mappings persist, mid-sync edit is either blocked or queued. ### 2.6 Reconciliation CSV export **Test scenarios:** - Click Export CSV with a 30-day range - Verify CSV has correct columns - Verify totals match what's on screen - Open in Excel — no encoding issues, no formula injection from dollar signs **Acceptance:** CSV opens cleanly in Excel and the totals match the on-screen reconciliation report. ### 2.7 Reconciliation day drill-down **Decision required:** Does the UI expose `getDayDetail`? If yes, verify per-field POS↔QBO comparison renders correctly. If no UI yet, decide: build for Phase 2 or defer to Phase 3. ### 2.8 QBO error surfacing **Test scenarios:** - Deliberately approve a JE for a date inside a closed QBO period (you can close a period in sandbox) - Verify the user sees a clear error like "QuickBooks rejected: this period is closed" and the staged entry stays in `pending_approval` (not stuck in approved-but-no-JE-id state) **Watch out for:** The Intuit SDK wraps errors awkwardly; surface the human-readable message, not the raw response body. **Acceptance:** Closed-period rejection produces a user-readable error and the entry can be re-attempted later when the period is reopened. ## Phase 3 — Operational ergonomics (Phase 1 of beta = polish, not blockers) Build these as Phase 1 customer feedback rolls in. Don't pre-build all of them — let the first 1–2 customers tell you which ones matter most. ### 3.1 Variance threshold alerts A daily Cash Over/Short > $X should be flagged on the Approval Queue list view ("⚠ $384 variance — review source data"), not buried inside an expanded row. Configurable per store. Make the threshold configurable in Settings. ### 3.2 Mapping-health check A weekly background check (or on Settings page load) that re-queries QBO chart of accounts and warns if any active mapping points at a deleted/renamed account. Surface in Settings page or as a yellow banner site-wide. ### 3.3 Period-completeness view "April 2026 Status: 28 of 30 days posted, 1 pending, 1 rejected, 0 missing." A simple month-by-month dashboard so accountants can answer "did we get everything?". ### 3.4 Auto-balance line help text When the JE has a "Daily Close Variance (auto-balanced)" line, tooltip/info on the approval modal: "This line balances debits and credits when your POS reports a discrepancy. The amount is the unexplained variance — review your S-file source data if this seems high." ### 3.5 Sign-flip code refactor (replaces 1.6's lightweight comment) Replace the implicit sign-flip with an explicit `signSemantics: 'directional' | 'absolute'` column on `qb_account_mapping`. Migration sets this for existing mappings. Removes the "mapping says credit but means debit" cognitive trap that 1.6 only papers over with a comment. ## Recommended sequencing | When | What | |------|------| | **This week** | Phase 1 (~3.5 hrs of dev + an accountant call about 1.1). Closes data-corruption paths. | | **Next 1–2 weeks** | Phase 2 (~1 day of focused E2E + bug fixes). Each surface will reveal at least one bug — budget 2–3 minor fixes per surface. | | **Phase 1 of customer feedback (post-launch)** | Phase 3 items as customers ask. | ## Tracking - Items 1.1, 1.3 require decisions (accountant for 1.1, product call for 1.3 semantics). - Items 1.2, 1.4, 1.5, 1.6 can be done autonomously by Claude. - Phase 2 items are best done with a human in the loop (clicking through the UI) but Claude can write the test scripts and verify state. - Phase 3 items are sized for separate dev tickets; don't bundle into Phase 2 work. ## Definition of "done" for this plan The plan is complete when: - [ ] Every Phase 1 item has a checkmark - [ ] Every Phase 2 item has been exercised on at least one real-data store - [ ] An external beta tester has approved at least 5 days of real data without a support ticket related to a known issue from this list When all three are met, Spec 048 is "complete" in the sense the user asked: foundation is trustworthy enough to start adding new features on top of, instead of finding more foundation bugs. ## Skills referenced Future sessions hitting any of these symptoms should auto-surface: - `qbo-journalentry-totalamt-zero` — JE TotalAmt=0 bug - `je-staging-sign-flip-imbalance` — sign-flip semantic confusion - `bootstrap5-modal-backdrop-stacking` — endemic modal backdrop trap (now also in CLAUDE.md) - `syncfusion-grid-hidden-tab-raf-defer` — Grid renders 0 rows on hidden tabs (testing artifact)