From 92dc3ebd0862681aa3a079164b690ed8b788133a Mon Sep 17 00:00:00 2001 From: Scott Pouliot Date: Mon, 27 Apr 2026 19:35:16 -0400 Subject: [PATCH] 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 --- CLAUDE.md | 43 ++++ docs/DATA_ACCESS_ARCHITECTURE.md | 336 +++++++++++++++++++++++++++++++ 2 files changed, 379 insertions(+) create mode 100644 docs/DATA_ACCESS_ARCHITECTURE.md diff --git a/CLAUDE.md b/CLAUDE.md index d36c20b..27ed08f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -122,6 +122,49 @@ Company Admin (seed): demo@powdercoatinglogix.com / CompanyAdmin123! - Global query filters enforce company isolation at database level - Users have both system role (SuperAdmin) and company role (CompanyAdmin, Manager, Worker, Viewer) +## Data Access Rules (ENFORCE THESE) + +> **`ApplicationDbContext` is NEVER injected into a controller.** +> All data access in controllers goes through `IUnitOfWork`. No exceptions outside the list below. +> Full rationale and migration roadmap: `docs/DATA_ACCESS_ARCHITECTURE.md` + +### Three tiers — use the right one: + +**Tier 1 — Simple CRUD** → `IUnitOfWork.EntityName` (generic `IRepository`) +```csharp +var items = await _unitOfWork.CatalogItems.GetAllAsync(); +await _unitOfWork.Announcements.AddAsync(entity); +await _unitOfWork.CompleteAsync(); +``` + +**Tier 2 — Complex domain queries** → typed repositories on `IUnitOfWork` +```csharp +// Include chains and domain-specific queries belong 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); +``` +Typed repositories: `IJobRepository`, `IInvoiceRepository`, `IQuoteRepository`, +`ICustomerRepository`, `IBillRepository`, `IPurchaseOrderRepository` +— defined in `Core/Interfaces/Repositories/`, implemented in `Infrastructure/Repositories/` + +**Tier 3 — Aggregate/reporting queries** → injected read services +```csharp +// P&L, AR aging, cycle time, powder usage — shaped DTOs, never tracked entities +var aging = await _financialReports.GetArAgingAsync(companyId); +``` +Services: `IFinancialReportService`, `IOperationalReportService` +— defined in `Core/Interfaces/Services/`, implemented in `Infrastructure/Services/` + +### Permanent exceptions (ApplicationDbContext allowed — intentional, documented): +`StripeWebhookController`, `WebhooksController`, `PaymentController`, `RegistrationController`, +`DataExportController`, `AccountDataExportController`, `DataPurgeController`, +`SystemInfoController`, `SystemLogsController`, `CompanyHealthController` + +If you think you need a new exception, you almost certainly don't. Check the spec first. + +--- + ## Data Access Patterns ### Common Controller Pattern diff --git a/docs/DATA_ACCESS_ARCHITECTURE.md b/docs/DATA_ACCESS_ARCHITECTURE.md new file mode 100644 index 0000000..3d97f98 --- /dev/null +++ b/docs/DATA_ACCESS_ARCHITECTURE.md @@ -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` 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` cannot express live in typed repositories or +> read services — both accessible through `IUnitOfWork`. + +--- + +## Target Architecture + +``` +Controllers (Presentation Layer) + │ + ├── IUnitOfWork.EntityName → IRepository 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 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` 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.