Distribution_GL Initialization
This commit is contained in:
@@ -0,0 +1,324 @@
|
||||
# CLAUDE.md — Working Rules for This Project
|
||||
|
||||
This file governs how Claude Code should work in the **General-Ledgar-Service** repository. Read this before touching any code. It works together with:
|
||||
|
||||
- `PROJECT_SPECIFICATION.md` — what already exists in this repo today, and what it's for
|
||||
- `Docs/01_Full_Development_Plan.md` — the full scope of what's being built
|
||||
- `Docs/02_Phase_By_Phase_Development_Plan.md` — the exact order of work, one API endpoint per phase
|
||||
- `Docs/03_Progress_Tracker.md` — what has actually been done so far, checked before any work starts (Rule 0, below)
|
||||
- `Docs/06_Enums_Reference.md` — every `enum` in `Enums/`, what backs it, and exactly how its value is bound on the API surface (Rule 15, below)
|
||||
|
||||
---
|
||||
|
||||
## 0. Check `Docs/03_Progress_Tracker.md` before doing anything
|
||||
|
||||
This is the first step of every session, before reading code, before proposing a change, before anything else:
|
||||
|
||||
1. Open `Docs/03_Progress_Tracker.md` and check the status of the current/next phase.
|
||||
2. If the tracker says a phase is `☑ Done`, treat it as done — do not re-implement it or "improve" it as a side effect of a later phase without asking first (Rule 3).
|
||||
3. If the tracker says a phase is `◐ In progress` or `⚠ Blocked`, that's the phase to resume — say so explicitly and confirm with the user before continuing, rather than assuming and jumping ahead.
|
||||
4. If the tracker disagrees with what's actually in the codebase (e.g. it says Phase 5 is done but the code isn't there, or vice versa), stop and flag the mismatch instead of trusting either source blindly — the tracker is only useful if it's kept honest.
|
||||
5. Only after this check is the next phase's work (per `Docs/02_Phase_By_Phase_Development_Plan.md`) proposed — following Rule 3 below, still asking before making changes.
|
||||
|
||||
**After a phase is confirmed working** (its acceptance test passes), `Docs/03_Progress_Tracker.md` is updated immediately — status flipped to `☑ Done`, completion date filled in — before moving to the next phase. This update itself is a file edit and follows Rule 3 (ask first) like any other change.
|
||||
|
||||
---
|
||||
|
||||
## 1. Always use the existing foundation — do not rebuild it
|
||||
|
||||
Four pieces of the codebase are **already implemented and considered final**. Every phase's code must build on top of them, never duplicate or route around them:
|
||||
|
||||
| Component | Location | Use it for |
|
||||
|---|---|---|
|
||||
| `CustomException` types | `Exceptions/CustomExceptions.cs` | `NotFoundException`, `BadRequestException`, `UnauthorizedException`, `ForbiddenException` — thrown from the **Service** layer whenever a domain rule is violated. Never throw a raw `Exception` for something that maps cleanly to one of these four. |
|
||||
| `Helpers` | `Helpers/DataExtractor.cs`, `DeserializePayload.cs`, `EncryptionHelper.cs` | Use `DataExtractor` whenever an endpoint has to parse a loosely-typed JSON payload instead of a clean DTO; use `DeserializePayload` to get that payload into the `Dictionary<string, JsonElement>` shape `DataExtractor` needs. Use `EncryptionHelper` only if a phase explicitly calls for encrypting a stored value. **Parsing happens in the Service, not the Controller** — the Controller accepts `[FromBody] Dictionary<string, JsonElement>` and passes it straight through; the Service calls `DataExtractor` internally. This keeps the Controller a pure pass-through with zero parsing logic, consistent with Rule 7. |
|
||||
| Unit of Work | `Infra/UoW/IUnitOfWork.cs`, `EFUnitOfWork.cs` | Every Service method that writes data wraps its work in `BeginAsync()` → repository calls → `SaveChangesAsync()` → `CommitAsync()`, with `RollbackAsync()` on failure. |
|
||||
| `ApiResponse` | `DTOs/Common/ApiResponse.cs` | The standard response envelope (`StatusCode`, `Success`, `Message`, `Data`) for **every** controller endpoint in this service, success or failure — not just auth-related calls. Built in the **Service** layer for the success path, not the Controller — see Rule 7. |
|
||||
|
||||
**Do not modify these four components.** If a phase seems to need a change to one of them (a new exception type, a new helper method, a change to the UoW contract, a new field on `ApiResponse`), stop and ask first — see Rule 3.
|
||||
|
||||
### 1.1 Two more foundation pieces, built in Phase 1, then treated the same way
|
||||
|
||||
Once Phase 1 is complete, these two are **also** foundation — built once, then not modified per-phase without asking first:
|
||||
|
||||
| Component | Location | What it does |
|
||||
|---|---|---|
|
||||
| `ApiKeyMiddleware` | `Middleware/ApiKeyMiddleware.cs` | Validates the `X-Api-Key` header on every incoming request against `ApiKey:Value` in configuration, before the request reaches any controller. Rejects with a hand-built `401` in the `ApiResponse` shape if missing/wrong. `/health` and `/swagger` are excluded. This is a pipeline-level check — individual controllers/endpoints never implement their own API key check. |
|
||||
| API versioning | every controller | Every controller uses `Asp.Versioning.Mvc`: `[ApiVersion("1.0")]` + `[Route("api/v{version:apiVersion}/[controller]")]`. All routes in this service live under `api/v1/...` unless a documented breaking change justifies a `v2` controller. |
|
||||
|
||||
Every manual test of every endpoint, from Phase 1 onward, must include a valid `X-Api-Key` header — a request without one is rejected by the middleware before it ever reaches the code being tested.
|
||||
|
||||
---
|
||||
|
||||
## 2. Keep the code simple
|
||||
|
||||
- No premature abstraction. No generic repository base classes, no reflection-driven mapping, no design patterns introduced "for flexibility" unless a phase specifically calls for it — the one standing exception is Rule 11's shared recursive-tree/depth-guard helpers, which exist because the *same* CTE/config-check logic was found copy-pasted across entities, not because it might be needed someday.
|
||||
- **Repository granularity is one-per-table by default, but a small, tightly-coupled table cluster within one domain can share a single repository file when the user directs it.** Decided 2026-07-19: `Repositories/TaxRepository.cs` covers both `TaxType` and `TaxCode`; `Repositories/FixedAssetRepository.cs` covers `FixedAsset`, `DepreciationSchedule`, and `AssetDisposal`; `Repositories/BankRepository.cs` covers `BankAccount` and `BankStatementLine`; `Repositories/BudgetRepository.cs` covers `Budget` and `BudgetLine` — in each case the tables are only ever touched together, by that domain's own single Service. Ask before consolidating a new cluster rather than assuming the pattern applies. **Reversed 2026-07-19 (post-completion refactor, see Rule 12):** `JournalEntry`/`JournalEntryLine`/`JournalCorrection` were originally called out here as an example of tables that *stay separate* — that reasoning didn't survive scrutiny (they pass the exact same "only ever touched together, by one Service" test used for Tax/FixedAsset/Bank/Budget, since `JournalEntryService` is their sole consumer) and they are merged into one `JournalEntryRepository.cs` as part of Rule 12's refactor. Likewise `FiscalYearRepository`/`AccountingPeriodRepository`/`PeriodLockLogRepository` merge into one `FiscalCalendarRepository.cs`, mirroring `FiscalCalendarService`'s existing scope.
|
||||
- A Service method should be readable top-to-bottom as a sequence of plain steps: validate → check business rules → call repository/repositories → save.
|
||||
- A Repository method does one specific data-access job and nothing else — no business logic, no validation, no exception-throwing beyond what EF Core itself does.
|
||||
- Prefer explicit, boring code over clever code. If there are two ways to write something and one is simpler to read, use the simpler one even if it's a few more lines.
|
||||
- Only implement what the current phase's endpoint needs. Do not add extra CRUD methods, extra fields, or extra validation "while already in the file" — that belongs to whichever later phase actually needs it (see `Docs/02_Phase_By_Phase_Development_Plan.md`'s phase template note on this).
|
||||
|
||||
---
|
||||
|
||||
## 3. Ask before making changes — every time
|
||||
|
||||
Before writing or editing any code:
|
||||
1. State what you're about to do (which files, which methods, what the change is).
|
||||
2. Wait for explicit approval.
|
||||
3. Only then make the change.
|
||||
|
||||
This applies to every phase, every file, and every edit — including small ones, including anything inside `Models/`, `Services/`, `Repositories/`, `Controllers/`, and especially anything inside `Program.cs`, `AppDBContext.cs`, or the four foundation components listed in Rule 1. **Never make a change and then explain it afterward — approval comes first.**
|
||||
|
||||
If a phase's work reveals that something from an earlier, already-approved phase needs to change, that's treated as a new change request for the current phase — propose it, explain why, and wait for approval, rather than silently going back and editing old code.
|
||||
|
||||
---
|
||||
|
||||
## 4. Interfaces live in `Interfaces/Repositories/` and `Interfaces/Services/`
|
||||
|
||||
**Reversed 2026-07-19 (post-completion refactor, see Rule 12).** This rule originally removed the `Interfaces/` folder entirely (as the first action of Phase 1) in favor of co-locating each interface with its one implementation, on the reasoning that a separate interface file adds indirection without adding value when there's exactly one implementation. That reasoning held for a 36-phase build with everything still fresh in view; once the project was fully built out, the user redirected: interfaces now live apart from their implementations, so the whole project's contract surface is scannable from two folders instead of file-hopping through every Repository/Service file.
|
||||
|
||||
```text
|
||||
Interfaces/
|
||||
Repositories/
|
||||
IAccountRepository.cs
|
||||
IFiscalCalendarRepository.cs
|
||||
IJournalEntryRepository.cs
|
||||
...
|
||||
Services/
|
||||
IAccountService.cs
|
||||
IFiscalCalendarService.cs
|
||||
...
|
||||
```
|
||||
|
||||
```csharp
|
||||
// Interfaces/Repositories/IAccountRepository.cs
|
||||
namespace General_Ledger_Service.Interfaces.Repositories
|
||||
{
|
||||
public interface IAccountRepository
|
||||
{
|
||||
Task<Account?> GetByIdAsync(long accountId);
|
||||
}
|
||||
}
|
||||
|
||||
// Repositories/AccountRepository.cs
|
||||
using General_Ledger_Service.Interfaces.Repositories;
|
||||
|
||||
namespace General_Ledger_Service.Repositories
|
||||
{
|
||||
public class AccountRepository : IAccountRepository
|
||||
{
|
||||
// implementation
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- **One interface file per repository/service implementation file, not per table/entity.** Since Rule 2 allows a tightly-coupled table cluster to share one repository/service file (Tax, FixedAsset, Bank, Budget, and — as of Rule 12's refactor — FiscalCalendar and JournalEntry), that file's interface covers every method for the whole cluster in one interface file too. There is still exactly one implementation per interface in this codebase — this reversal is about discoverability, not about supporting multiple implementations or mocking.
|
||||
- **Namespace matches the folder path**: `General_Ledger_Service.Interfaces.Repositories` / `General_Ledger_Service.Interfaces.Services` — not the old `General_Ledger_Service.Repositories`/`.Services` namespace the interfaces used to share with their implementation. Every consumer needs the new `using` — the implementing class itself, any Service that depends on a Repository, `Program.cs`'s DI registrations, and the Controller's constructor dependencies.
|
||||
|
||||
---
|
||||
|
||||
## 5. Unit of Work belongs in the Service layer only
|
||||
|
||||
Repositories never call `BeginAsync`, `CommitAsync`, or `RollbackAsync`. Repositories are thin data-access classes that operate against `AppDBContext` and return entities/results — nothing more.
|
||||
|
||||
Services own the transaction boundary, because only a Service knows the full scope of what needs to succeed or fail together — sometimes that's one repository call, sometimes it's several (e.g. posting a journal entry that also needs to update a fixed asset's schedule). The standard shape inside a Service write method is:
|
||||
|
||||
```csharp
|
||||
await _unitOfWork.BeginAsync();
|
||||
try
|
||||
{
|
||||
// one or more repository calls
|
||||
await _unitOfWork.SaveChangesAsync();
|
||||
await _unitOfWork.CommitAsync();
|
||||
}
|
||||
catch
|
||||
{
|
||||
await _unitOfWork.RollbackAsync();
|
||||
throw;
|
||||
}
|
||||
```
|
||||
|
||||
Read-only Service methods (reports, `GET` lookups) do not need to open a Unit of Work at all.
|
||||
|
||||
---
|
||||
|
||||
## 6. Working through the phases
|
||||
|
||||
- Check `Docs/03_Progress_Tracker.md` first, every time (Rule 0).
|
||||
- Follow `Docs/02_Phase_By_Phase_Development_Plan.md` in order. Do not skip ahead or combine phases.
|
||||
- At the start of each phase, restate: the endpoint being built, the Service/Repository methods being added, and which of the four foundation components (Rule 1) will be used and how.
|
||||
- A phase is not "done" until its one endpoint has been manually tested (via Swagger or the `.http` file) against a real local database and confirmed working.
|
||||
- Each phase should result in a small, reviewable set of changes — not a sprawling diff touching many unrelated files.
|
||||
- Once a phase is confirmed working, update `Docs/03_Progress_Tracker.md` for that phase before starting the next one.
|
||||
|
||||
---
|
||||
|
||||
## 7. One controller; `ApiResponse` is built in the Service layer
|
||||
|
||||
Two rules, layered on top of Rule 1's `ApiResponse` row:
|
||||
|
||||
- **Single controller.** Every endpoint in this service — every phase, every module (Accounts, Journal Entries, Reporting, etc.) — is an action method on `Controllers/GeneralLedgerController.cs`. No new controller classes are created per module (no `AccountsController`, `JournalEntriesController`, `ReportsController`, and so on). The controller-level route carries **only** the version prefix — `[Route("api/v{version:apiVersion}")]`, with no `[controller]` token — so each action's own route template supplies its full path relative to `api/v1/` directly, with no per-action prefix repetition and no absolute-override slashes needed:
|
||||
|
||||
```csharp
|
||||
[ApiVersion("1.0")]
|
||||
[ApiController]
|
||||
[Route("api/v{version:apiVersion}")]
|
||||
public class GeneralLedgerController(IAccountService accountService) : ControllerBase
|
||||
{
|
||||
[HttpPost("accounts")]
|
||||
public async Task<IActionResult> CreateAccount([FromBody] CreateAccountRequest request)
|
||||
{
|
||||
var response = await _accountService.CreateAccountAsync(request);
|
||||
return StatusCode(response.StatusCode, response);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This lands the route exactly on `api/v1/accounts` — there is no `[controller]` segment (and so no `GeneralLedger` in the URL) to override in the first place.
|
||||
|
||||
- **`ApiResponse` is constructed in the Service layer, not the Controller.** A Service method's success path returns an already-built `ApiResponse` (`StatusCode`/`Success`/`Message`/`Data` all set there) — not the raw entity/DTO. The Controller action's only job is to call the one Service method and return its result via `StatusCode(response.StatusCode, response)`; it never writes `new ApiResponse { ... }` itself. Error paths are unaffected by this rule — a Service still throws a `CustomException` for domain-rule violations, and `ExceptionHandlingMiddleware` (Rule 1.1-equivalent foundation, added in Phase 2) builds the `ApiResponse` for those failure cases. This rule is about the success-path envelope only.
|
||||
|
||||
---
|
||||
|
||||
## 8. Look up by unique code, not by numeric id
|
||||
|
||||
Wherever an entity has a unique business-key column — `Account.AccountCode`, `CostCenter.Code`, `TaxCode.Code`, `Currency.CurrencyCode`, `JournalEntry.JournalNo`, and so on — a **path parameter that looks up a single resource uses that code, not the internal numeric id**. E.g. `GET /api/v1/accounts/{accountCode}`, `PUT /api/v1/accounts/{accountCode}`, not `.../{id}`.
|
||||
|
||||
Scope of this rule, decided explicitly to avoid over-applying it:
|
||||
|
||||
- **Applies to:** path/route parameters on single-resource lookups (`GET`/`PUT`/etc. on one specific record) — the "get information about this thing" case.
|
||||
- **Does not apply to:** foreign-key reference fields inside request bodies (e.g. `parentAccountId` in `CreateAccountRequest`/`UpdateAccountRequest`) or any internal repository/recursive-CTE logic. The recursive tree/cycle/depth-check machinery (`CLAUDE.md` is downstream of `Docs/Accounting_System_Design_LKAS.md` here) is built entirely on the numeric self-referencing FK (`parent_account_id`, etc.) throughout the design doc's SQL — rewriting that to join on codes would fight the reference schema, not follow it. Those fields stay numeric.
|
||||
- Internally, a Service resolves the code to the entity (and its numeric id) first via a `GetByCodeAsync`-style repository method, then uses the numeric id for anything that touches the recursive tree logic.
|
||||
|
||||
Applies retroactively to Phase 3 (retrofitted from `{id}` to `{accountCode}`) and to every phase from Phase 4 onward wherever that phase's entity has its own unique code column.
|
||||
|
||||
### 8.1 Unique codes are system-generated and immutable — never accepted from a request body
|
||||
|
||||
**Retrofitted 2026-07-20.** `Account.AccountCode`, `CostCenter.Code`, `FixedAsset.AssetCode`, and `TaxCode.Code` were originally accepted as plain fields on their `Create*Request` DTOs (and, for `Account`, editable via `PUT /accounts/{accountCode}`). That was wrong: a unique business-key code must never be user-suppliable or user-editable — the Service generates it at creation time, and it never changes afterward.
|
||||
|
||||
- **The Service builds the code inside `Create*Async`, before insert; the corresponding `Create*Request` DTO has no code field at all.** The client never sends one and can't influence it.
|
||||
- **No update endpoint may accept or mutate a code field, ever** — `AccountService.UpdateAccountAsync` no longer reads or writes `AccountCode`. If a future phase needs a "rename this code" capability, that is a new, explicit feature request (Rule 3), not something bolted onto an existing update endpoint.
|
||||
- **Generation scheme per entity** (no scheme was specified anywhere in `Docs/Accounting_System_Design_LKAS.md` beyond `VARCHAR UNIQUE NOT NULL`, so these were decided fresh, following standard LKAS chart-of-accounts numbering conventions where applicable):
|
||||
- **`Account.AccountCode`** — root accounts (`ParentAccountId == null`) get the next available multiple of 100 within their account type's standard LKAS block: Asset(1)=1000s, Liability(2)=2000s, Equity(3)=3000s, Income(4)=4000s, Expense(5)=5000s (e.g. `1000`, `1100`, `1200`...; capped at 10 roots per type before `BadRequestException`). Child accounts append a 2-digit sequential suffix to the parent's code (e.g. a child of `1100` becomes `1100-01`, its own child becomes `1100-01-01`), which supports arbitrary depth without a fixed digit-width per level. Generation throws `BadRequestException` if the resulting code would exceed the column's 20-character limit (relevant only if `COA_ENFORCE_MAX_DEPTH=FALSE` and nesting goes unusually deep).
|
||||
- **`CostCenter.Code`** — same shape as Account minus the type blocks (cost centers have no type classification): roots get `CC-001`, `CC-002`...; children append `-01`, `-02` to the parent's code.
|
||||
- **`FixedAsset.AssetCode`** — flat entity, no recursion or natural grouping: a plain global sequence, `FA-000001`, `FA-000002`...
|
||||
- **`TaxCode.Code`** — prefixed with its parent `TaxType.Code` (existing seed data: `VAT`, `WHT`, `NBT`, `SSCL`, `INCOME_TAX`), sequential per tax type: `VAT-001`, `WHT-001`, `WHT-002`... Keeps the code human-recognizable while still being system-generated.
|
||||
- **`JournalEntry.JournalNo`** was already compliant before this retrofit — `JV-{year}-{seq:D6}`, generated in `JournalEntryService.GenerateJournalNoAsync`, never user-supplied or editable. It's the precedent this rule generalizes from.
|
||||
- Repository layer gained small counting methods to support this (`CountRootAccountsByTypeAsync`/`CountChildAccountsAsync`, `CountRootCostCentersAsync`/`CountChildCostCentersAsync`, `CountAssetsAsync`, `CountTaxCodesByTypeAsync`) — plain `COUNT` queries, no new shared abstraction (Rule 2 still applies; this isn't the same shape as Rule 11's tree/depth-guard duplication).
|
||||
- The now-unused `CodeExistsAsync`/`TaxCodeExistsAsync` uniqueness-conflict checks (there was no longer any user-supplied value to conflict with) were deleted from the four affected repositories/interfaces rather than left dead.
|
||||
- Not affected: `TaxType.Code` and `Currency.CurrencyCode` are seed/reference data with no create endpoint (out of scope); `BankAccount.AccountNumber` is a real external bank account number, not a system-assigned code (out of scope).
|
||||
|
||||
### 8.2 Reversed 2026-07-22 — request-body/query-string FK references also use codes, not just path parameters
|
||||
|
||||
Rule 8's original scope note said this rule "does not apply to" foreign-key reference fields inside request bodies, and that those fields "stay numeric." **That carve-out is reversed for leaf references** — every request-body or query-string field that references another entity by foreign key now uses that entity's own unique code wherever one exists, the same as a path parameter would. The client (and any core backend consuming this API) never sends or receives a raw numeric id for a coded entity; the Service resolves the code to its numeric id internally via `GetByCodeAsync`, exactly as it already did for path parameters.
|
||||
|
||||
**What changed, concretely** — every one of these fields was a `long`/`long?`/`int` FK id before this retrofit, and is now a `string`/`string?` code:
|
||||
|
||||
| DTO | Field(s) | Resolves via |
|
||||
|---|---|---|
|
||||
| `JournalEntryLineRequest` | `accountCode` (was `accountId`), `costCenterCode` (was `costCenterId`) | `IAccountRepository.GetByCodeAsync`, `ICostCenterRepository.GetByCodeAsync` |
|
||||
| `JournalEntryCorrectionRequest` | `originalJournalNo` (was `originalJournalId`) | `IJournalEntryRepository.GetJournalEntryByJournalNoAsync` |
|
||||
| `BudgetLineRequest` | `accountCode` (was `accountId`), `costCenterCode` (was `costCenterId`) | same as above |
|
||||
| `CreateTaxCodeRequest` | `taxTypeCode` (was `taxTypeId`), `outputTaxAccountCode`/`inputTaxAccountCode` (was `...AccountId`) | `ITaxRepository.GetTaxTypeByCodeAsync`, `IAccountRepository.GetByCodeAsync` |
|
||||
| `CalculateTaxRequest` | `taxCode` (was `taxCodeId`) | `ITaxRepository.GetEffectiveTaxCodeByCodeAsync` |
|
||||
| `CreateFixedAssetRequest` | `assetAccountCode`/`accumDeprAccountCode`/`deprExpenseAccountCode`/`gainLossAccountCode` (were `...AccountId`) | `IAccountRepository.GetByCodeAsync` |
|
||||
| `CreateFixedAssetTransactionRequest` | `proceedsAccountCode` (was `proceedsAccountId`) | same |
|
||||
| `RecurringTemplateLineRequest` | `accountCode` (was `accountId`) | same |
|
||||
| `CreateBankAccountRequest` / `CreateCashAccountRequest` | `glAccountCode` (was `glAccountId`) | same |
|
||||
| `ReconcileStatementLineRequest` | `matchedJournalNo` (was `matchedJournalId`) | `IJournalEntryRepository.GetJournalEntryByJournalNoAsync` |
|
||||
| `ListReportsRequest` (query string) | `accountCode` (was `accountId`, `GeneralLedger` report only) | `IAccountRepository.GetByCodeAsync` |
|
||||
|
||||
**What did NOT change — Rule 8's original exception still stands for parent/recursive-tree fields.** `parentAccountId` (`CreateAccountRequest`/`UpdateAccountRequest`) and `parentCostCenterId` (`CreateCostCenterRequest`) stay numeric, explicitly reconfirmed 2026-07-22 rather than silently swept into this retrofit — the recursive CTE/cycle/depth-check machinery is still built entirely on the numeric self-referencing FK, and rewriting that to join on codes would still fight the reference schema. Likewise, fields referencing an entity with **no** unique code column (`periodId`, `fiscalYearId`, `budgetId`) stay numeric — there's nothing to convert them to.
|
||||
|
||||
**Internal storage is unaffected.** Persisted model columns (`JournalEntryLine.AccountId`, `FixedAsset.AssetAccountId`, `RecurringJournalTemplateLine.AccountId`, `BankStatementLine.MatchedJournalId`, ...) all stay numeric FKs — this rule is about the **API request surface**, not the database schema. A Service that already holds a numeric id internally (e.g. `FixedAssetService` building a depreciation journal entry from `asset.DeprExpenseAccountId`) resolves that id back to a code via `IAccountRepository.GetByIdAsync(id).AccountCode` before constructing the request DTO for another Service's `CreateAsync` call — see `FixedAssetService.ResolveAccountCodeAsync` for the pattern.
|
||||
|
||||
---
|
||||
|
||||
## 9. Fixed sets of discriminator values get an enum — but not when a real lookup table already exists
|
||||
|
||||
Whenever a column holds one of a small, fixed set of literal values — a status, an action name, a mode flag — and that set is **not already backed by its own reference table**, don't scatter string/int literals or an ad hoc `HashSet<string>` through the code. Define an enum once. Enums live in `Enums/`, one file per set.
|
||||
|
||||
- **Skip the enum if a real table already is the enumeration.** `TaxType`, `AccountType`, `Currency`, and similar are proper reference tables with FK relationships from other entities (`TaxCode.TaxTypeId` → `tax_type`, `Account.AccountTypeId` → `account_type`). The table row already is the source of truth; a parallel C# enum would just duplicate it and risk drifting out of sync with whatever rows actually exist/get edited. Use the FK relationship as normal — load the referenced row, or compare ids.
|
||||
- **Use an enum for inline discriminator columns that have no such table** — `AccountingPeriod.Status`, `PeriodLockLog.Action`, and future cases like `JournalEntry.EntryStatus`, `FixedAsset.DepreciationMethod`, `TaxCode.CalculationBasis`.
|
||||
|
||||
Before picking a representation for one of these, check the underlying DB column type on the relevant Model:
|
||||
|
||||
- If the column is (or should be) numeric, use a real C# `enum` with **explicit values assigned to every member** — never left as implicit 0/1/2/3 auto-numbering — e.g. `public enum PeriodStatus { Open = 1, SoftClose = 2, Locked = 3, Closed = 4 }`.
|
||||
- C# enums cannot be string-backed — there is no such thing as `enum { OPEN = "OPEN" }` in C#. If the existing column is a `VARCHAR` (as most status/type columns are in `Docs/Accounting_System_Design_LKAS.md`), converting it to an integer column is a real trade-off against Rule 8's whole rationale (staying aligned with the design doc's schema) — **ask before converting a VARCHAR column to integer; don't default to either answer silently.**
|
||||
|
||||
First applied in `FiscalCalendarService` (Phase 8): `AccountingPeriod.Status` and `PeriodLockLog.Action` were converted from `VARCHAR` to `integer` (explicitly approved by the user, migration `ConvertPeriodStatusAndActionToInt`), backed by `Enums/PeriodStatus.cs` and `Enums/PeriodLockAction.cs`. The API surface didn't change — clients still send/receive human-readable action names as JSON strings (`"LOCK"`, `"CLOSE"`, etc.), parsed via `Enum.TryParse(raw, ignoreCase: true, out var value)`; only the internal representation and DB storage became integers.
|
||||
|
||||
---
|
||||
|
||||
## 10. Audit logging — every write phase logs one `audit_log` row, categorized `LEDGER` or `ADMINISTRATIVE`
|
||||
|
||||
A generic `audit_log` table (`Models/AuditLog.cs`, one row per write, no user/role FK — `performed_by` is free text, same pattern as `JournalEntry.PreparedBy`) exists alongside the two purpose-specific audit tables that already covered their own areas (`journal_correction`, `period_lock_log` — those keep existing exactly as they are; `audit_log` fills in everywhere else that had zero write tracking).
|
||||
|
||||
**Every Service method that performs a write** inserts one `audit_log` row, inside the same Unit-of-Work transaction as the write itself — the same shape as any other repository call in that transaction, not a separate mechanism. `GET` endpoints never write an `audit_log` row; there's nothing to log on a read.
|
||||
|
||||
**The `category` column is the required discriminator: `LEDGER` or `ADMINISTRATIVE`.** The full per-phase classification table lives in `Docs/01_Full_Development_Plan.md` Section 4.3 — check it before writing any Service method's audit insert rather than guessing. The short version:
|
||||
- **`LEDGER`** — the write touches recorded balances, the double-entry ledger itself, or the structural integrity of something reports roll up through (the COA tree, tax rates applied to future postings, the fiscal calendar, journal entries).
|
||||
- **`ADMINISTRATIVE`** — everything else (cost centers, recurring templates, bank account master data, budget planning figures).
|
||||
- Two phases are field-dependent, not fixed: **Phase 4** (`PUT /accounts/{accountCode}`) is `LEDGER` only if `parentAccountId` changed, `ADMINISTRATIVE` otherwise; **Phase 28** (`PUT /system-config/{key}`) is classified per config key, not fixed for the endpoint as a whole (both current keys happen to be `LEDGER`).
|
||||
|
||||
**Representation of `category` and `action`:** `integer` (EF Core's plain enum-to-int default — no `.HasConversion<short>()` applied) with an explicit-valued C# enum (`Enums/AuditCategory.cs`: `Ledger = 1, Administrative = 2`; `Enums/AuditAction.cs`: `Create = 1, Update = 2, Post = 3, Reverse = 4, Adjust = 5, Lock = 6, Unlock = 7, SoftClose = 8, Close = 9`) — decided 2026-07-18, since this is a brand-new table with no legacy `VARCHAR` data to migrate away from, avoiding the exact retrofit-migration `PeriodStatus`/`PeriodLockAction` needed. (Corrected 2026-07-18: this row originally said `smallint`, but the `AddAuditLog` migration actually mapped both columns as plain `integer`; the wording is now aligned with the real schema rather than the column being migrated to match the wording.)
|
||||
|
||||
**Retrofit required.** Because Phases 1, 2, 4, 6, 7, and 8 are already built and marked `☑ Done`, adding `audit_log` means going back into already-completed, working code — this is exactly the situation Rule 3's "ask before changes, including to already-approved work" and Rule 0's "check the tracker, flag mismatches" both anticipate. Don't silently bundle the retrofit into unrelated future-phase work; treat it as its own explicit, approved change, same as the `{accountCode}` retrofit to Phase 3 was.
|
||||
|
||||
---
|
||||
|
||||
## 11. Don't copy-paste recursive-tree or depth-guard logic across entities — share it
|
||||
|
||||
`Account` and `CostCenter` are the two structurally recursive entities in this schema (`Docs/01_Full_Development_Plan.md` Section 4.2), and their repositories/services were originally written independently, which produced two verbatim-shaped copies of the same recursive-CTE SQL and the same max-depth config check, differing only in table/column names and error messages. That was found and fixed 2026-07-18. Going forward:
|
||||
|
||||
- **Repository layer.** Any self-referencing entity's ancestor-depth, ancestor-breadcrumb, descendant-tree, or would-create-a-cycle logic goes through `Helpers/RecursiveTreeQueries.cs`, not hand-written per repository. **Updated 2026-10-06 (PostgreSQL → MySQL migration):** there is no raw SQL anywhere in the codebase any more — LINQ cannot express recursive CTEs, so each repository loads its entity's rows once as the helper's plain `TreeRow` (`Id`/`Code`/`Name`/`ParentId`/`Depth`) via an EF Core projection, and the helper's pure static methods (`GetAncestorDepth`, `GetAncestorBreadcrumb`, `GetDescendantTree`, `GetDescendantIds`, `WouldCreateCycle`) walk that in-memory list. Each entity's repository stays the only thing that knows its own public interface, method signatures, and result DTO (`AccountTreeNode`, `CostCenterTreeNode`, ...) — it just maps `TreeRow` into that DTO. Report queries follow the same rule: sums/joins/filters are EF Core LINQ (run in the database), and tree-inherited tags (`cash_flow_category`, `income_statement_section`, `balance_sheet_classification`) and running balances are computed in C#. Do not reintroduce `Database.SqlQuery`/`FromSql`/`ExecuteSql` without asking first (Rule 3). This keeps the JSON response shape of every existing endpoint unchanged.
|
||||
- **Service layer.** Any Service's "look up a max-depth `system_config` value, optionally gated by an enforce-flag config, compare against a resulting depth, throw `BadRequestException` if exceeded" check goes through `Services/DepthGuard.EnsureWithinMaxDepthAsync(...)`, passing the config keys and the entity-specific exceeded-message — not repeated inline in each Service method (including twice in the same Service, e.g. `AccountService`'s create and re-parent branches).
|
||||
- **This is not a green light for a generic Repository/Service base class or an `IRecursiveEntity` abstraction.** It's two narrow, parameterized helpers for one specific, already-duplicated pattern. If a future phase's duplication doesn't fit this shape, don't force it in here — raise it as its own case (per this rule's spirit: find it, name it, share it, document it) rather than stretching `RecursiveTreeQueries`/`DepthGuard` to cover something they weren't built for.
|
||||
- Before writing a new Repository or Service method, check whether it's the same shape as something that already exists elsewhere in `Repositories/` or `Services/` (not just within the current file) — that check is what was skipped the first time around.
|
||||
|
||||
---
|
||||
|
||||
## 12. Post-completion architecture refactor (2026-07-19)
|
||||
|
||||
With all 36 original phase slots resolved (Rule 0 / `Docs/03_Progress_Tracker.md`), the user directed a project-wide consolidation pass across `Models/`, `Repositories/`, `Services/`, and `Controllers/GeneralLedgerController.cs` — fewer files per layer, shared helpers for duplicated CRUD/response-building boilerplate, and interfaces moved out to a dedicated `Interfaces/` folder (Rule 4, reversed). Full design, the file-by-file grouping tables, and the ordered execution phases (R1–R6) live in `Docs/01_Full_Development_Plan.md` Section 10 and `Docs/02_Phase_By_Phase_Development_Plan.md`'s Refactor Phases section — this rule is the short pointer + the standing conventions that apply going forward.
|
||||
|
||||
- **Models/** group by domain (mirroring the Repository clusters below), not one class per file — e.g. `Models/FixedAsset.cs` holds `FixedAsset`, `DepreciationSchedule`, and `AssetDisposal`. Pure reorganization; no property changes.
|
||||
- **Repositories/** consolidate two more clusters beyond the four Rule 2 already lists: `FiscalYearRepository`+`AccountingPeriodRepository`+`PeriodLockLogRepository` → `FiscalCalendarRepository.cs`; `JournalEntryRepository`+`JournalEntryLineRepository`+`JournalCorrectionRepository` → `JournalEntryRepository.cs` (Rule 2's reversal, above). `AccountRepository` and `CostCenterRepository` stay standalone — too large/central to fold into anything.
|
||||
- **`Repositories/RepositoryHelpers.cs`** — a new shared static helper (no base class, no inheritance — same shape as `RecursiveTreeQueries.cs`) for the `GetByIdAsync`/`ExistsAsync`/`AddAsync` one-liners duplicated across most repositories. Each repository still declares its own public interface/method signatures and calls the shared helper internally, exactly the pattern Rule 11 already established for tree queries — this is that same pattern applied to a second, independently-discovered duplication, not a new exception to Rule 2's "no generic repository base classes."
|
||||
- **`Services/ResponseHelpers.cs`** — a new shared static helper with `ApiResponse`-building methods (e.g. `Ok(message, data)`) to remove the repeated `new ApiResponse { StatusCode = ..., Success = true, ... }` block duplicated in nearly every Service method. This does **not** modify `ApiResponse` itself (Rule 1 foundation, still off-limits without asking) — it's a separate builder file that constructs one.
|
||||
- **Controller** gains one private helper, `Respond(ApiResponse r) => StatusCode(r.StatusCode, r)`, replacing the repeated two-line `var response = ...; return StatusCode(response.StatusCode, response);` pattern in every action.
|
||||
- **Interfaces/Repositories/** and **Interfaces/Services/** — see Rule 4's rewritten text above for the full convention.
|
||||
- This refactor changes **no public API contract, no request/response DTO shape, and no database schema** — it is a pure internal reorganization. Every endpoint must behave identically before and after; the Refactor Phases' acceptance checks are "solution builds" + "every endpoint still returns the same result it did before," not new business-rule tests.
|
||||
|
||||
---
|
||||
|
||||
## 13. Amounts are stored and returned at full precision — rounding to 2 decimals happens in exactly one place: PDF rendering
|
||||
|
||||
**Added 2026-07-20.** `Docs/Accounting_System_Design_LKAS.md` specifies every monetary/decimal column as `NUMERIC(18,2)` (or `NUMERIC(18,6)` for the two FX rate fields) — the user directed a deliberate deviation from that: no column in this database truncates or rounds a decimal value on storage, and no API response rounds one either. A value is only ever rounded to 2 decimal places at the moment it's rendered into a PDF.
|
||||
|
||||
- **Database:** every `decimal`/`decimal?` property in `AppDBContext.cs` — both true monetary amounts (`DebitAmount`, `CreditAmount`, `BaseDebit`, `BaseCredit`, `AcquisitionCost`, `ResidualValue`, `DepreciationAmount`, `AccumulatedDepreciation`, `NetBookValue`, `Proceeds`, `NetBookValueAtDisposal`, `GainLoss`, `BankStatementLine.Amount`, `BudgetedAmount`) and the rate/multiplier fields (`ExchangeRate`, `TaxCode.RatePercent`, `FixedAsset.DepreciationRatePercent`, `ExchangeRate.RateToBase`) — was, on PostgreSQL, a plain unbounded `numeric` (migration `WidenAmountAndRateColumnsToUnboundedNumeric`, 2026-07-20). **Updated 2026-10-06 (PostgreSQL → MySQL migration):** MySQL has no unbounded numeric (maximum `DECIMAL(65,30)`), and Pomelo's default for an unconfigured `decimal` is `decimal(18,2)`, which would silently round. So `AppDBContext.ApplyDecimalPrecision` sets every `decimal`/`decimal?` property to **`DECIMAL(38,10)`** — 10 decimal places, the same scale the old report SQL rounded to. Nothing in the 2-decimal range is ever rounded by the database; only values with more than 10 decimal places would be, which no current input or calculation produces.
|
||||
- **API responses:** nothing in `Services/` or `Repositories/` calls `Math.Round`/`decimal.Round` on a value before persisting or returning it (verified — there were zero such calls anywhere in the codebase even before this change). A value comes back from `GET`/`POST`/`PUT` exactly as stored, to whatever decimal length the client originally sent or a calculation produced (e.g. `TaxService.CalculateTaxAsync`'s pure in-memory `amount * (ratePercent / 100)` was already unrounded, and remains so).
|
||||
- **PDF generation — the one exception, by design:** `ReportService.RenderPdf`'s per-cell formatter (`FormatCellValue`) special-cases `decimal`/`decimal?` property values: `Math.Round(d, 2, MidpointRounding.AwayFromZero).ToString("F2")`. Every other value type (`string`, `DateOnly`, `int`, ...) still just calls `.ToString()`. This is the **only** rounding call in the entire codebase.
|
||||
- **Rounding rule: round-half-up (away from zero), not .NET's default round-half-to-even.** `Math.Round`'s default `MidpointRounding.ToEven` would round `1000.005` down to `1000.00` (since `.00`'s last digit is even) — wrong for this project's requirement. `MidpointRounding.AwayFromZero` rounds any value in `[x.xx5, x.xx5 + 0.01)` up to the next cent, e.g. `1000.005` → `1000.01`, `1000.0149999` → `1000.01`, `1000.015` → `1000.02`, `1000.0574536` → `1000.06`. Verified against exactly these boundary values in a throwaway scratch console app before wiring it into `ReportService` — all six matched expected output. This works correctly *because* `decimal` (unlike `double`) represents values like `1000.005` exactly in base 10; the same logic applied to a `double` could give the wrong answer due to binary floating-point representation error.
|
||||
- **Scope check before extending this pattern:** if a future report type or endpoint needs 2-decimal display formatting outside a PDF (e.g. a hypothetical CSV export), reuse `FormatCellValue`'s rounding expression rather than reintroducing `HasPrecision` on a column or rounding at the Service layer — the whole point of this rule is that only the final render step ever loses precision. (Superseded in practice by the `05_LKAS_Report_PDF_Templates.md` Section 10 CSV export added 2026-07-22, which does exactly this — reuses the same round-half-up rounding, no new logic.)
|
||||
|
||||
---
|
||||
|
||||
## 14. Journal entries have no client-facing DRAFT state — `POST /journal-entries` always creates and posts in one call
|
||||
|
||||
**Retrofit 2026-07-22.** The original design (Phases 11–13) had a three-step lifecycle: create a `DRAFT` (optionally with inline lines), separately `PUT .../lines` to set/replace lines, then separately `PUT .../post` to validate and post. The client (and any core backend integrating with this service) always sends a complete, ready-to-post journal entry in one call — there is no workflow that needs an entry to sit in `DRAFT` waiting for a follow-up call. Both `PUT` endpoints are **removed**; `POST /journal-entries` is now the only write endpoint for journal entries, and `GET /journal-entries/{journalNo}` is the only read.
|
||||
|
||||
- **`lines` is required**, not optional, for a plain create and for a `PartialAdjustment` correction — both previously deferred lines to a separate call; both now take them inline in the same `POST` body. `FullReversal` corrections still auto-build their lines by mirroring the original (unaffected).
|
||||
- **`JournalEntryService.CreateAsync` does everything in one Unit-of-Work transaction**: validate period exists → resolve/validate account codes and build lines → insert entry + lines → balance check (`SUM(debit) == SUM(credit)`) → period-lock check → flip to `Posted` → if the correction is a `FullReversal`, flip the original to `Reversed` in the same transaction → audit rows for both the create action (`Create`/`Reverse`/`Adjust` depending on the case) and the post action (plus a second `Reverse` audit row against the original, for `FullReversal`). A balance or period-lock failure throws inside this same transaction, so nothing is left half-inserted — the whole call is atomic (create-and-post-or-neither), not two separable steps anymore.
|
||||
- **`SetLinesAsync`/`PostAsync` are gone from `IJournalEntryService`** — do not re-add them for a future phase without asking first (Rule 3); if a genuine need for a separate draft/review step resurfaces, that is a new, explicit feature request, not a reversion to the old shape by default.
|
||||
- Every other Service that generates a journal entry internally (`FixedAssetService`'s `Depreciate`/`Dispose` transactions, `RecurringTemplateService.GenerateAsync`) calls `_journalEntryService.CreateAsync(...)` once and uses the returned (already-posted) entry directly — there is no longer a separate `PostAsync` call to make afterward. `FixedAssetService` resolves its own numeric account-id fields (`asset.DeprExpenseAccountId`, etc.) back into codes via `ResolveAccountCodeAsync` before building the request, per Rule 8.2.
|
||||
|
||||
---
|
||||
|
||||
## 15. `Docs/06_Enums_Reference.md` is updated in the same change as any enum
|
||||
|
||||
**Added 2026-08-05.** Every `enum` in `Enums/` (Rule 9) is catalogued in `Docs/06_Enums_Reference.md` — its values, whether it backs a real DB column or is a pure in-memory dispatch discriminator, and (the part most worth getting right) exactly how a caller supplies it: a JSON-body field declared `string` and parsed via `Enum.TryParse` in the Service, a query-string field declared as the real (optionally nullable) enum type, or an action name pulled straight out of a raw `Dictionary<string, JsonElement>` payload with no named DTO field at all. This project has no global `JsonStringEnumConverter`, so picking the wrong one of these three for a new JSON-body field silently fails to bind — a real bug hit twice already (`ChequePayeeType`/`ReceivedChequeFromType`, Phase 37) before this file existed to check against.
|
||||
|
||||
- **Creating a new enum file, adding/renaming/renumbering a member, or changing which DB column/DTO field an enum backs** — all of these require updating the relevant table in `Docs/06_Enums_Reference.md` in the same change, the same way `Docs/03_Progress_Tracker.md` is updated before a phase counts as done (Rule 0). Treat "did I update the enums doc" as part of the same checklist as "did I ask before changing code" (Rule 3), not a separate follow-up task.
|
||||
- **Before adding a new enum-backed request field**, check `Docs/06_Enums_Reference.md`'s "How values reach the API" table for the closest existing precedent (is this a JSON body? a query-string filter? a `PUT .../status` action?) rather than guessing at the binding shape from scratch.
|
||||
Reference in New Issue
Block a user