Skip to content

fix(payment-instalments): move zero-allocation guard before allocateForPayment#209

Merged
RJK134 merged 2 commits into
mainfrom
fix/payment-instalment-zero-allocation-guard
May 11, 2026
Merged

fix(payment-instalments): move zero-allocation guard before allocateForPayment#209
RJK134 merged 2 commits into
mainfrom
fix/payment-instalment-zero-allocation-guard

Conversation

@claude
Copy link
Copy Markdown

@claude claude Bot commented May 11, 2026

Summary

recordPayment in the Phase 18D bridge called paymentService.allocateForPayment (with persist: true) before checking whether the account had open charges. When the post-hoc guard fired (allocation.totalAllocated === 0), the 18C transaction had already committed: StudentAccount.balance decremented, totalCredits incremented, ChargeLine/Invoice updates written. Throwing ValidationError at that point left the ledger mutated but the instalment stuck in PENDING, requiring manual reversal.

Root cause

The guard at payment-instalments.service.ts:223 (allocation.totalAllocated === 0) ran after allocateForPayment had already committed all side-effects. There was no way to undo those writes without a rollback mechanism that does not exist in this bridge.

Fix

Move the guard before allocateForPayment. Use chargeLineRepo.findOpenForAccount to check whether the account has any open charges. If the set is empty, throw ValidationError immediately — no DB mutations have occurred. force: true bypasses the check (administrative settlement path).

Tests

  • "rejects the COMPLETED flip when there are no open charges on the account (guard fires before allocateForPayment)" — verifies allocateForPayment is NOT called when the guard fires.
  • "allows the COMPLETED flip when no open charges exist but force:true is supplied (guard bypassed)" — verifies force: true bypasses the check.
  • Updated beforeEach to mock chargeLineRepo.findOpenForAccount with a non-empty result by default, and reset mocks properly between tests.

Acceptance criteria

  • npx tsc --noEmit → 0 errors
  • Server Vitest: 25/25 payment-instalments tests passing
  • No DB side-effects committed when the guard fires (verified by expect(mockedPaymentsService.allocateForPayment).not.toHaveBeenCalled())
  • force: true correctly bypasses the guard
  • KI-P18D-001 logged and closed in docs/KNOWN_ISSUES.md

Known Issues resolved

  • KI-P18D-001: recordPayment guard fires post-allocation, leaving ledger in inconsistent state

🤖 Generated with Claude Code

…orPayment persist call\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 11, 2026

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

Project Deployment Actions Updated (UTC)
sjms-2-5-client Ready Ready Preview, Comment May 11, 2026 2:58pm

Request Review

@github-actions
Copy link
Copy Markdown

🤖 SJMS-Agent dispatched — action: review, agent id: bc-51fa631c-5f97-4b99-8837-0d78afca4721. Track progress: https://cursor.com/agents/bc-51fa631c-5f97-4b99-8837-0d78afca4721

@RJK134
Copy link
Copy Markdown
Owner

RJK134 commented May 11, 2026

@copilot resolve the merge conflicts in this pull request

…t-zero-allocation-guard

# Conflicts:
#	docs/KNOWN_ISSUES.md
#	server/src/__tests__/unit/payment-instalments.service.test.ts
#	server/src/api/payment-instalments/payment-instalments.service.ts

Co-authored-by: RJK134 <167345619+RJK134@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 11, 2026

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in b30bbec. There were three conflicted files, all minor differences between the branch and the two upstream commits that landed on main after my original commit:

  • docs/KNOWN_ISSUES.md — kept the KI-P18D-001 entry from this branch and accepted the KI-P19-001 closure note from main
  • server/src/__tests__/unit/payment-instalments.service.test.ts — accepted a clarifying comment on mockReset from main
  • server/src/api/payment-instalments/payment-instalments.service.ts — accepted improved comment wording from main

No logic changes were required.

Copilot AI requested a review from RJK134 May 11, 2026 14:59
Copilot finished work on behalf of RJK134 May 11, 2026 14:59
@github-actions
Copy link
Copy Markdown

🤖 SJMS-Agent dispatched — action: review, agent id: bc-208b39a7-5f1b-452e-935a-9e025362120c. Track progress: https://cursor.com/agents/bc-208b39a7-5f1b-452e-935a-9e025362120c

@RJK134 RJK134 merged commit a228706 into main May 11, 2026
19 of 20 checks passed
@RJK134 RJK134 deleted the fix/payment-instalment-zero-allocation-guard branch May 11, 2026 15:20
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.

2 participants