Skip to content

Fix sales journal edit committing a void when repost fails - #35

Draft
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/critical-bug-investigation-9d98
Draft

Fix sales journal edit committing a void when repost fails#35
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/critical-bug-investigation-9d98

Conversation

@cursor

@cursor cursor Bot commented Aug 20, 2026

Copy link
Copy Markdown

Bug and impact

After PR #31 allowed sales_journals.status = 'voided', editing a posted sale that cannot be reposted still voids the original journal and restores outlet stock, while the UI reports failure.

Concrete trigger:

  1. Outlet sold 5 of a lot and has 2 remaining.
  2. Admin opens the posted journal and saves quantity 8 (or mistypes the lot/SKU).
  3. replace_sales_journal voids first (stock restored to 7 available), then post_sales_journal returns insufficient_stock.
  4. PostgREST commits that jsonb error return. The original sale leaves the posted list; stock is restored; no new journal is posted.
  5. The operator sees “Failed to update sale” and still believes the original sale is live.

Impact: silent loss of a posted sale and unexpected stock restore. A follow-up stock take or re-entry can double-count.

This is distinct from open PR #30 (a successful qty-only replace consuming leftover BATCH lots).

Root cause

replace_sales_journal (migration 043) calls void_sales_journal then post_sales_journal. Both report failure with RETURN success:false rather than RAISE. PostgREST treats that as a successful RPC and commits.

Before #31, voiding hit sales_journals_status_check and the exception rolled back the whole replace. After #31 the void succeeds, so a failed edit persists.

Fix

Migration 20260820110000_replace_sales_journal_atomic.sql:

  • RAISE EXCEPTION if void or post returns success:false, so the transaction rolls back and the original posted journal stays intact.
  • Require is_profiles_admin() on replace, matching void_sales_journal.

Validation

  • npm test — 27 passed
  • npm run lint — no errors (3 pre-existing warnings)
  • npm run typecheck — clean
  • npm run build — clean
  • Local Postgres is not available here; the SQL was reviewed against 043 / 057b and the PostgREST commit-on-RETURN behavior already documented for this repo.

Apply this migration on the hosted project after merge.

Open in Web View Automation 

After voided status was allowed, replace_sales_journal could persist the
void (stock restored, original sale off the posted list) when post_sales_journal
returned success:false. Raise on inner failure so the transaction rolls back.

Co-authored-by: ezzademir <ezzademir@users.noreply.github.com>
@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
project Ready Ready Preview Aug 20, 2026 11:07am

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.

1 participant