Add data access architecture spec and enforce rules in CLAUDE.md

Defines the target architecture for eliminating direct ApplicationDbContext injection from controllers. Documents the three-tier model (generic repo, typed domain repos, read services), the 6 typed repository interfaces to build, the 2 reporting service interfaces to build, permanent exceptions, and the 4-phase migration roadmap with per-controller checklist. CLAUDE.md updated with the hard rule and tier quick-reference so every session and every team member sees the constraint immediately. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-27 19:35:16 -04:00
parent 5631d1d57a
commit 92dc3ebd08
2 changed files with 379 additions and 0 deletions
@@ -0,0 +1,336 @@
+# Data Access Architecture
+
+## Status: Migration In Progress
+
+This document defines the target data access architecture for Powder Coating Logix and tracks
+the migration from the current mixed pattern to the clean layered pattern.
+
+---
+
+## The Problem
+
+The codebase currently has ~50 controllers injecting `ApplicationDbContext` directly alongside
+`IUnitOfWork`. This happened organically: the generic `Repository<T>` could not express complex
+multi-level include queries, so `_context` became the escape hatch. Once injected for one complex
+query, it was used for everything else in that controller too. The inconsistency compounds with
+every new controller a developer writes.
+
+For a solo developer this is manageable. For a team it creates a daily decision tax — "which
+pattern do I follow?" — with no clear answer. New developers copy the nearest example, which is
+usually `_context`, so the problem grows.
+
+---
+
+## The Rule (Short Version)
+
+> **`ApplicationDbContext` is never injected into a controller. Ever.**
+>
+> All data access in controllers goes through `IUnitOfWork`.
+> Complex queries that the generic `Repository<T>` cannot express live in typed repositories or
+> read services — both accessible through `IUnitOfWork`.
+
+---
+
+## Target Architecture
+
+```
+Controllers (Presentation Layer)
+    │
+    ├── IUnitOfWork.EntityName          → IRepository<T>          Simple CRUD
+    ├── IUnitOfWork.Jobs                → IJobRepository           Complex domain queries
+    ├── IUnitOfWork.Invoices            → IInvoiceRepository       Complex domain queries
+    ├── IUnitOfWork.Quotes              → IQuoteRepository         Complex domain queries
+    ├── IUnitOfWork.Customers           → ICustomerRepository      Complex domain queries
+    ├── IUnitOfWork.Bills               → IBillRepository          Complex domain queries
+    │
+    ├── IFinancialReportService         Aggregate/reporting reads
+    └── IOperationalReportService       Aggregate/reporting reads
+
+Infrastructure Layer (the only layer that knows about ApplicationDbContext)
+    │
+    ├── Repository<T>                   Generic implementation
+    ├── JobRepository : IJobRepository  Typed implementations
+    ├── InvoiceRepository              ...
+    ├── QuoteRepository                ...
+    ├── CustomerRepository             ...
+    ├── BillRepository                 ...
+    │
+    ├── FinancialReportService          DbContext used directly (read-only, no tracking)
+    └── OperationalReportService        DbContext used directly (read-only, no tracking)
+```
+
+`ApplicationDbContext` never crosses into the Presentation layer. It lives in Infrastructure and
+only Infrastructure.
+
+---
+
+## Three Tiers of Data Access
+
+### Tier 1 — Simple CRUD → Generic `IRepository<T>` via `IUnitOfWork`
+
+Use for: single-entity lookups, lists, adds, soft deletes, simple filtered queries.
+
+```csharp
+// Good
+var items = await _unitOfWork.CatalogItems.GetAllAsync();
+var item  = await _unitOfWork.CatalogItems.GetByIdAsync(id);
+await _unitOfWork.Announcements.AddAsync(entity);
+await _unitOfWork.CompleteAsync();
+```
+
+Entities in this tier (generic repo is sufficient):
+- Announcements, BugReports, CatalogItems, CatalogCategories, CatalogPriceCheckReports
+- CompanyBlastSetups, CompanyOperatingCosts, CompanyPreferences
+- ContactSubmissions, CreditMemos, CreditMemoApplications
+- DashboardTips, Deposits, Equipment
+- GiftCertificates, GiftCertificateRedemptions
+- InventoryItems, InventoryTransactions
+- JobChangeHistories, JobDailyPriorities, JobItemCoats, JobItems, JobNotes, JobPhotos
+- JobStatusHistory, JobTemplates, JobTemplateItems, JobTemplateItemCoats, JobTemplateItemPrepServices
+- JobTimeEntries, MaintenanceRecords, ManufacturerLookupPatterns
+- NotificationLogs, NotificationTemplates
+- OvenBatches, OvenBatchItems, OvenCosts
+- Payments, PrepServices, PricingTiers
+- PowderUsageLogs, PurchaseOrderItems
+- QuoteChangeHistories, QuoteItemCoats, QuoteItems, QuoteItemPrepServices, QuotePhotos
+- Refunds, ReworkRecords
+- ShopWorkers, ShopWorkerRoleCosts, SubscriptionPlanConfigs
+- Vendors
+
+### Tier 2 — Complex Domain Queries → Typed Repositories
+
+Use for: multi-level include chains, domain-specific filtered loads, queries that require
+`IgnoreQueryFilters`, queries that span multiple related entities in non-trivial ways.
+
+The typed repository interface lives in `Core/Interfaces/Repositories/`.
+The implementation lives in `Infrastructure/Repositories/`.
+The property is on `IUnitOfWork` — same access point as Tier 1.
+
+```csharp
+// Good — the complex include chain lives in the repository, not the controller
+var job = await _unitOfWork.Jobs.LoadForDetailsAsync(id);
+var invoice = await _unitOfWork.Invoices.LoadForViewAsync(id);
+var quote = await _unitOfWork.Quotes.GetByApprovalTokenAsync(token);
+```
+
+#### `IJobRepository`
+| Method | Purpose |
+|--------|---------|
+| `LoadForDetailsAsync(int id)` | Full include chain for Job Details view |
+| `LoadForEditAsync(int id)` | Includes needed for Job Edit form |
+| `LoadForBoardAsync(int companyId, ...)` | Jobs for the kanban board with status/priority filters |
+| `GetByStatusAsync(int companyId, int statusId)` | Filtered by status with customer include |
+| `GetAssignedToWorkerAsync(int workerId)` | All active jobs for a worker |
+| `GetOverdueAsync(int companyId)` | Jobs past due date |
+
+#### `IInvoiceRepository`
+| Method | Purpose |
+|--------|---------|
+| `LoadForViewAsync(int id)` | Full 8-table include chain (current `LoadInvoiceForViewAsync`) |
+| `GetOverdueAsync(int companyId)` | Invoices past due date with customer info |
+| `GetByPaymentTokenAsync(string token)` | Online payment portal lookup |
+| `GetForJobAsync(int jobId, bool includeDeleted)` | Invoice for a given job (1:1 check) |
+
+#### `IQuoteRepository`
+| Method | Purpose |
+|--------|---------|
+| `LoadForViewAsync(int id)` | Full include chain for Quote Details |
+| `LoadForEditAsync(int id)` | Includes needed for wizard edit |
+| `GetByApprovalTokenAsync(string token)` | Customer approval portal lookup |
+| `GetPendingApprovalsAsync(int companyId)` | Quotes awaiting customer approval |
+
+#### `ICustomerRepository`
+| Method | Purpose |
+|--------|---------|
+| `LoadForDetailsAsync(int id)` | Customer with jobs, quotes, invoices, notes summary |
+| `GetWithOutstandingBalancesAsync(int companyId)` | AR summary data |
+| `FindByEmailAsync(string email, int companyId)` | Duplicate check on create/edit |
+
+#### `IBillRepository`
+| Method | Purpose |
+|--------|---------|
+| `LoadForViewAsync(int id)` | Bill with line items, payments, vendor |
+| `GetApPayablesAsync(int companyId)` | Open AP ledger with aging |
+
+#### `IPurchaseOrderRepository`
+| Method | Purpose |
+|--------|---------|
+| `LoadForViewAsync(int id)` | PO with line items and vendor |
+| `GetByStatusAsync(int companyId, string status)` | Filtered PO list |
+
+### Tier 3 — Aggregate/Reporting Queries → Read Services
+
+Use for: P&L calculations, AR aging, powder usage aggregates, job cycle time, any query that
+uses `GROUP BY`, window functions, or multi-table joins that return shaped result DTOs rather
+than tracked entities.
+
+These services are injected directly into controllers alongside `IUnitOfWork`. They use
+`ApplicationDbContext` internally (with `.AsNoTracking()`) — that is correct and intentional,
+because they live in Infrastructure.
+
+```csharp
+// Controller constructor
+public ReportsController(IUnitOfWork unitOfWork, IFinancialReportService financialReports, ...)
+
+// Usage
+var aging = await _financialReports.GetArAgingAsync(companyId);
+var pl    = await _financialReports.GetProfitLossAsync(companyId, startDate, endDate);
+```
+
+#### `IFinancialReportService`
+- `GetArAgingAsync(int companyId)` → AR aging buckets (current, 30, 60, 90+ days)
+- `GetProfitLossAsync(int companyId, DateTime start, DateTime end)` → P&L summary
+- `GetMonthlyRevenueAsync(int companyId, int months)` → monthly invoiced vs collected
+- `GetTopOutstandingCustomersAsync(int companyId, int count)` → largest open balances
+- `GetCashFlowProjectionAsync(int companyId, int days)` → forward-looking cash position
+- `GetAnomaliesAsync(int companyId, int lookbackDays)` → bill/expense anomaly detection
+- `GetRecentPaymentsAsync(int companyId, int count)` → recent payment activity
+
+#### `IOperationalReportService`
+- `GetJobCycleTimeAsync(int companyId, DateTime start, DateTime end)` → avg days per stage
+- `GetPowderUsageAsync(int companyId, DateTime start, DateTime end)` → usage by color/vendor
+- `GetWorkerProductivityAsync(int companyId, DateTime start, DateTime end)` → jobs per worker
+- `GetOvenUtilizationAsync(int companyId, DateTime start, DateTime end)` → oven throughput
+- `GetReworkRateAsync(int companyId, DateTime start, DateTime end)` → defect/rework trends
+- `GetStatusFlowAsync(int companyId, DateTime start, DateTime end)` → job status transitions
+
+---
+
+## Permanent Exceptions
+
+The following controllers are **intentionally allowed** to inject `ApplicationDbContext` directly.
+This is not a smell — it is correct for their use cases. Each file has a comment explaining why.
+
+| Controller | Reason |
+|------------|--------|
+| `StripeWebhookController` | Idempotency key lookup must bypass soft-delete and tenant filters |
+| `WebhooksController` | Twilio raw event handling; same reasoning as Stripe |
+| `PaymentController` | Stripe Connect embedded payment flow; raw session state needed |
+| `RegistrationController` | PendingRegistrationSession queries bypass normal tenant scoping |
+| `DataExportController` | Bulk streaming export; repository pattern adds unnecessary overhead |
+| `AccountDataExportController` | Same as above |
+| `DataPurgeController` | Destructive bulk operations; needs direct transaction control |
+| `SystemInfoController` | Infrastructure diagnostics; queries metadata, not business data |
+| `SystemLogsController` | Log table queries; not a business entity |
+| `CompanyHealthController` | Cross-tenant health checks for SuperAdmin; ignores all filters |
+
+If you think you need to add a controller to this list, you almost certainly don't. Ask first.
+
+---
+
+## Migration Roadmap
+
+### Phase 1 — Foundation (no behavior change)
+- [ ] Create `Core/Interfaces/Repositories/` directory
+- [ ] Define `IJobRepository`, `IInvoiceRepository`, `IQuoteRepository`, `ICustomerRepository`, `IBillRepository`, `IPurchaseOrderRepository`
+- [ ] Define `IFinancialReportService`, `IOperationalReportService` in `Core/Interfaces/Services/`
+- [ ] Create `Infrastructure/Repositories/` directory
+- [ ] Implement all typed repositories (move include chains from controllers)
+- [ ] Implement `FinancialReportService` (move aggregate queries from `ReportsController`)
+- [ ] Implement `OperationalReportService`
+- [ ] Extend `IUnitOfWork` with typed repository properties
+- [ ] Register all new types in `Program.cs`
+- [ ] Build passes, all tests green — no controller has changed yet
+
+### Phase 2 — Complex controller migration
+- [ ] `InvoicesController` → `IInvoiceRepository`
+- [ ] `JobsController` → `IJobRepository`
+- [ ] `QuotesController` → `IQuoteRepository`
+- [ ] `CustomersController` → `ICustomerRepository`
+- [ ] `BillsController` → `IBillRepository`
+- [ ] `PurchaseOrdersController` → `IPurchaseOrderRepository`
+- [ ] `ReportsController` → `IFinancialReportService` + `IOperationalReportService`
+
+### Phase 3 — Simple controller sweep
+Remove `ApplicationDbContext` injection from all controllers not in the permanent exceptions list,
+replacing with existing `IUnitOfWork` generic repository calls.
+
+- [ ] `AnnouncementsController`
+- [ ] `AiQuickQuoteController`
+- [ ] `AiUsageReportController`
+- [ ] `AuditLogController`
+- [ ] `BannedIpsController`
+- [ ] `BugReportController`
+- [ ] `CompaniesController`
+- [ ] `CompanySettingsController`
+- [ ] `CompanyUsersController`
+- [ ] `DashboardController`
+- [ ] `DashboardTipsController`
+- [ ] `DepositsController`
+- [ ] `EmailBroadcastController`
+- [ ] `ExpensesController`
+- [ ] `InAppNotificationsController`
+- [ ] `InventoryController`
+- [ ] `JobsPriorityController`
+- [ ] `JobTemplatesController`
+- [ ] `NotificationLogsController`
+- [ ] `PasskeyController`
+- [ ] `PlatformNotificationsController`
+- [ ] `QuoteApprovalController`
+- [ ] `ReleaseNotesController`
+- [ ] `RevenueController`
+- [ ] `SetupWizardController`
+- [ ] `SmsConsentAuditController`
+- [ ] `StripeEventsController`
+- [ ] `SubscriptionManagementController`
+- [ ] `UnsubscribeController`
+- [ ] `UsageQuotaController`
+- [ ] `UserActivityController`
+- [ ] `VendorsController`
+
+### Phase 4 — Enforcement
+- [ ] Remove `ApplicationDbContext` from controller DI scope in `Program.cs`
+        (controllers that still need it will get a compile error — the compiler enforces the rule)
+- [ ] Update `CLAUDE.md` to mark migration complete
+- [ ] Update this document status from "Migration In Progress" to "Complete"
+
+---
+
+## File Locations Reference
+
+```
+src/
+  PowderCoating.Core/
+    Interfaces/
+      IRepository.cs                        existing
+      IUnitOfWork.cs                        existing — extended in Phase 1
+      Repositories/                         NEW in Phase 1
+        IJobRepository.cs
+        IInvoiceRepository.cs
+        IQuoteRepository.cs
+        ICustomerRepository.cs
+        IBillRepository.cs
+        IPurchaseOrderRepository.cs
+      Services/                             NEW in Phase 1
+        IFinancialReportService.cs
+        IOperationalReportService.cs
+
+  PowderCoating.Infrastructure/
+    Repositories/                           NEW in Phase 1
+      UnitOfWork.cs                         existing — extended
+      Repository.cs                         existing
+      JobRepository.cs
+      InvoiceRepository.cs
+      QuoteRepository.cs
+      CustomerRepository.cs
+      BillRepository.cs
+      PurchaseOrderRepository.cs
+    Services/
+      FinancialReportService.cs             NEW in Phase 1
+      OperationalReportService.cs           NEW in Phase 1
+      NotificationService.cs               existing — correct as-is
+      PdfService.cs                        existing — correct as-is
+```
+
+---
+
+## Code Review Checklist
+
+When reviewing a PR that touches data access:
+
+1. Does the controller inject `ApplicationDbContext`? If yes and it's not in the permanent
+   exceptions list → request changes.
+2. Is a complex include chain written inline in a controller action? → move to typed repository.
+3. Is a GROUP BY / aggregate query inline in a controller action? → move to report service.
+4. Does a new typed repository method duplicate logic already in another repository? → consolidate.
+5. Are all DbContext calls in report services using `.AsNoTracking()`? → required for read services.