firms and collections

cheapnud commented

2026-08-09 16:32:29 +00:00

Owner

Adds firm legal entities with a default-firm invariant, a collection registry that routes orders to firms, and real firm identity on invoice, credit note and purchase order documents. Includes a firms admin page and a draft-editable firm select on orders.

cheapnud added 9 commits

2026-08-09 16:32:29 +00:00

rework multi-user plan around a dedicated server host 9abc2592b5

merge master 94751542da

add firm and collection masters with default-firm invariant 4950d6438a

route orders to a firm via collection with draft-time override d99d5370ad

issue invoices and credit notes under the order's firm 4c9467244d

stamp purchase order documents with the default firm 8a1ac6dee7

add firms admin page with collections editor da2faeec42

expose the order's firm as a draft-editable select 4a08705f8c

guard blank firm addresses and rework test bank codes

Build & Test / Build and Test (pull_request) Successful in 6m46s

Details

Claude PR Review / AI Code Review (pull_request) Successful in 24s

Details

55d1f23292

cheapnud merged commit 73eab82d80 into master

2026-08-09 16:40:04 +00:00

cheapnud referenced this pull request from a commit

2026-08-09 16:40:05 +00:00

Merge pull request 'firms and collections' (#1) from d/fi1-firms into master

forgejo-actions commented

2026-08-09 22:11:56 +00:00

🤖 Claude AI Code Review

1. Summary

Adds firm legal entities with a default-firm invariant, collection→firm routing, and firm identity on invoice/credit-note/PO documents, plus an admin page and order firm-select. Well-structured with thorough test coverage.

2. Code Quality

⭐⭐⭐⭐☆

Excellent test coverage across services, PDF/XML export, and components, with clear intent comments explaining routing and fallback rules.

3. Issues

🟡 FirmService.cs — default-firm invariant broken on delete. DeleteFirmAsync allows deleting a non-default firm freely, but nothing promotes a remaining firm to default when the default firm is deleted. The guard only blocks deleting the default while others exist, and permits deleting the default when it's the sole firm. That's fine — but if you delete all non-default firms then the default, you end with zero firms (acceptable). The real gap: there's no path that ever leaves multiple firms with none defaulted, so this is actually safe. No action needed — but worth confirming SetDefaultAsync/add flows are the only default mutators (they are).

🟡 FirmService.SetDefaultAsync / AddFirmAsync — TOCTOU on default-firm invariant under concurrency. The "exactly one default" and unique-code checks are read-then-write without a transaction or unique constraint on IsDefault. Two concurrent AddFirmAsync calls on an empty table could both set IsDefault = true. This matches the existing single-user "Mode B" IOUs documented in TODO.md, so it's consistent with the codebase's stated posture — flagging only for awareness.

🟢 CollectionsDialog.razor — rename UX is confusing. RenameAsync reuses the shared _newName add field and no-ops with a hint if blank, per the inline comment. Not a bug, but the coupling between the Add form and per-row rename will surprise users. (Skip if intentional.)

4. Verdict

APPROVE

No blocking bugs, resource leaks, or breaking changes. The concurrency gaps are pre-existing, documented tradeoffs consistent with the single-user model. Migration, model, service, and export changes are internally consistent and well-tested.

📊 Tokens: 72594 input + 764 output | 💰 Cost: ~$0.3821
Generated by Claude Opus 4.8 via Gitea Actions

## 🤖 Claude AI Code Review ## 1. Summary Adds firm legal entities with a default-firm invariant, collection→firm routing, and firm identity on invoice/credit-note/PO documents, plus an admin page and order firm-select. Well-structured with thorough test coverage. ## 2. Code Quality ⭐⭐⭐⭐☆ Excellent test coverage across services, PDF/XML export, and components, with clear intent comments explaining routing and fallback rules. ## 3. Issues 🟡 **FirmService.cs — default-firm invariant broken on delete.** `DeleteFirmAsync` allows deleting a non-default firm freely, but nothing promotes a remaining firm to default when the *default* firm is deleted. The guard only blocks deleting the default while others exist, and permits deleting the default when it's the sole firm. That's fine — but if you delete all non-default firms then the default, you end with zero firms (acceptable). The real gap: there's no path that ever leaves multiple firms with none defaulted, so this is actually safe. No action needed — but worth confirming `SetDefaultAsync`/add flows are the only default mutators (they are). 🟡 **FirmService.SetDefaultAsync / AddFirmAsync — TOCTOU on default-firm invariant under concurrency.** The "exactly one default" and unique-code checks are read-then-write without a transaction or unique constraint on `IsDefault`. Two concurrent `AddFirmAsync` calls on an empty table could both set `IsDefault = true`. This matches the existing single-user "Mode B" IOUs documented in TODO.md, so it's consistent with the codebase's stated posture — flagging only for awareness. 🟢 **CollectionsDialog.razor — rename UX is confusing.** `RenameAsync` reuses the shared `_newName` add field and no-ops with a hint if blank, per the inline comment. Not a bug, but the coupling between the Add form and per-row rename will surprise users. (Skip if intentional.) ## 4. Verdict **APPROVE** No blocking bugs, resource leaks, or breaking changes. The concurrency gaps are pre-existing, documented tradeoffs consistent with the single-user model. Migration, model, service, and export changes are internally consistent and well-tested. --- 📊 **Tokens**: 72594 input + 764 output | 💰 **Cost**: ~$0.3821 *Generated by Claude Opus 4.8 via Gitea Actions*

Rows
Columns

firms and collections #1

🤖 Claude AI Code Review

1. Summary

2. Code Quality

3. Issues

4. Verdict