4648
This commit is contained in:
@@ -1,7 +1,16 @@
|
||||
{
|
||||
"permissions": {
|
||||
"allow": [
|
||||
"Bash(dotnet test:*)"
|
||||
"Bash(dotnet test:*)",
|
||||
"Bash(grep -c \"\\\\[Fact\\\\]\" /c/data/code/raven-test-integration/**/*.cs)",
|
||||
"Bash(grep -c \"Skip = \" /c/data/code/raven-test-integration/**/*.cs)",
|
||||
"Bash(grep -l \"public class.*Controller\" /c/data/code/raven/server/AyaNova/Controllers/*.cs)",
|
||||
"Bash(xargs -I {} basename {})",
|
||||
"Bash(sed 's/Controller.cs//')",
|
||||
"Bash(find /c/data/code/raven-test-integration -name \"*.cs\" -type f ! -path \"*/obj/*\" -exec grep -l \"public class.*Test\" {} ;)",
|
||||
"Bash(xargs grep \"public class\")",
|
||||
"Read(//c/data/code/raven-test-integration/**)",
|
||||
"Bash(sed 's/.cs$//')"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
157
todo.md
Normal file
157
todo.md
Normal file
@@ -0,0 +1,157 @@
|
||||
# Gap Analysis: Tests Needed Before Refactoring
|
||||
|
||||
## Context
|
||||
|
||||
The goal of adding tests before a refactor is **regression protection** — not completeness for its own sake. Every test added should answer the question: *"If I change how this code works internally, will a test catch it if I accidentally changed the behavior?"*
|
||||
|
||||
---
|
||||
|
||||
## What's Already Well-Covered (Don't Duplicate)
|
||||
|
||||
- User CRUD, concurrency, referential integrity, password change, inactive user login
|
||||
- Project CRUD and concurrency
|
||||
- WorkOrder + nested items/parts/labor/units CRUD
|
||||
- Attachments (upload/download/delete/authorization)
|
||||
- Search (phrase, wildcard, tags, serial, deletion cleanup)
|
||||
- Custom forms
|
||||
- Pick lists
|
||||
- Translations
|
||||
- Event log (object log, user log, pagination)
|
||||
- Tag bulk operations
|
||||
- Auth rights (unauthenticated 401, unauthorized 403)
|
||||
- Server health, metrics, log files
|
||||
|
||||
---
|
||||
|
||||
## Tier 1 — Critical Gaps (Block the Refactor)
|
||||
|
||||
These cover the most logic-dense areas where redundancies are most likely to exist. Without these, a refactor is risky.
|
||||
|
||||
### 1. DataList — Filtering, Sorting, Saved Filters
|
||||
|
||||
The single biggest gap. ~80 tests are commented out from case 4648. This is the cross-cutting query infrastructure used everywhere in the UI. A refactor of service/repository layers will almost certainly touch this.
|
||||
|
||||
**Add:**
|
||||
- Filter by each field type: string (contains/starts-with/ends-with/equals), date range, boolean, decimal/numeric range, null/not-null
|
||||
- Multi-condition AND filters
|
||||
- Sort ascending/descending on multiple fields
|
||||
- Pagination (limit/offset) correctness
|
||||
- Saved filter CRUD: create, list, update, delete, apply
|
||||
- Column view CRUD: create, update, delete, apply
|
||||
- Rights enforcement: user without list rights gets 403
|
||||
|
||||
Maps to: `DataListController`, `DataListSavedFilterController`, `DataListColumnViewController`
|
||||
|
||||
### 2. Quote — Full CRUD With Nested Hierarchy
|
||||
|
||||
Quote is structurally parallel to WorkOrder (header → items → labor/parts/expenses/tasks/travels/units → states) but has zero test coverage. If the refactor consolidates duplicated patterns between these two, tests on both are needed.
|
||||
|
||||
**Add:**
|
||||
- Quote header CRUD + concurrency violation
|
||||
- Quote item CRUD
|
||||
- At least one nested sub-type (labor or parts) CRUD
|
||||
- Quote state CRUD
|
||||
- Lookup by quote number (`/id-from-number/{number}`)
|
||||
|
||||
### 3. Customer — Core Relationships
|
||||
|
||||
Customer is the root of the customer hierarchy. Many other objects (WO, Quote, Contract, PM) belong to a Customer. A refactor could touch customer-related join logic.
|
||||
|
||||
**Add:**
|
||||
- Customer CRUD + concurrency violation
|
||||
- Customer alert retrieval
|
||||
- Referential integrity: customer with work orders should not be deletable (or verify delete cascades correctly per business rules)
|
||||
|
||||
---
|
||||
|
||||
## Tier 2 — Important Before a Thorough Refactor
|
||||
|
||||
These areas have enough complexity that refactoring without coverage is risky, but slightly less immediately critical than Tier 1.
|
||||
|
||||
### 4. Part — Inventory and Serials
|
||||
|
||||
Parts have a richer data model than simple CRUD (serials, stock levels per warehouse, cost tracking). If the refactor touches inventory or cost logic:
|
||||
|
||||
**Add:**
|
||||
- Part CRUD
|
||||
- Get/update serial numbers
|
||||
- Get/update stock levels
|
||||
- Get/update part cost
|
||||
|
||||
### 5. Contract CRUD
|
||||
|
||||
Contracts can have complex billing rules. Even basic CRUD coverage ensures the object graph survives the refactor.
|
||||
|
||||
**Add:**
|
||||
- Contract CRUD + concurrency violation
|
||||
|
||||
### 6. Preventive Maintenance (PM) CRUD
|
||||
|
||||
PM is structurally similar to WorkOrder (hierarchical items). If the refactor consolidates WorkOrder/PM patterns, both need coverage.
|
||||
|
||||
**Add:**
|
||||
- PM header CRUD
|
||||
- PM item CRUD
|
||||
- At least one PM item sub-type CRUD
|
||||
|
||||
### 7. Authorization Roles
|
||||
|
||||
The `AuthorizationRoles` business logic file is the largest in the codebase (~64KB). A refactor here is high risk.
|
||||
|
||||
**Add:**
|
||||
- List authorization roles
|
||||
- Verify role-based rights are enforced for at least 2-3 different role types
|
||||
- Verify that rights changes take effect (create a user with a role, test access, change role, re-test)
|
||||
|
||||
### 8. Schedule Reads
|
||||
|
||||
The schedule endpoint is used for the main dispatch board. Even basic read tests give a safety net.
|
||||
|
||||
**Add:**
|
||||
- Fetch service schedule for a date range
|
||||
- Fetch user schedule for a date range
|
||||
|
||||
---
|
||||
|
||||
## Tier 3 — Lower Priority (Nice to Have Before Refactor)
|
||||
|
||||
These are worth adding eventually but won't block a careful refactor if Tier 1 and 2 are covered.
|
||||
|
||||
| Area | What to Add |
|
||||
|---|---|
|
||||
| **Memo** | CRUD + concurrency |
|
||||
| **Unit / UnitModel** | CRUD |
|
||||
| **Vendor** | CRUD |
|
||||
| **ServiceRate / TravelRate / TaxCode** | CRUD (simple reference data) |
|
||||
| **Notification** | New count, fetch, delete one |
|
||||
| **EnumList** | Get by key, list keys |
|
||||
| **Name lookup** | Get name for a known object |
|
||||
| **Report** | Create, list, generate data (async job pattern) |
|
||||
| **GlobalBizSettings** | Fetch client settings |
|
||||
|
||||
---
|
||||
|
||||
## Cross-Cutting Concerns to Verify Everywhere
|
||||
|
||||
These should be confirmed on any *new* test entity, not just the ones already tested:
|
||||
|
||||
| Concern | Why It Matters for Refactor |
|
||||
|---|---|
|
||||
| **Concurrency (`ETag`/`rowVersion`)** | Optimistic locking is often in a shared base class — refactoring that class risks breaking all objects |
|
||||
| **Soft delete / referential integrity** | If a shared delete handler is consolidated, it must still block deletes where referenced |
|
||||
| **Custom field round-trip** | Custom fields are stored/retrieved through a shared mechanism; any refactor of that mechanism needs a test |
|
||||
| **Tags round-trip** | Tags use batch operations through shared infrastructure |
|
||||
|
||||
---
|
||||
|
||||
## Recommended Order of Work
|
||||
|
||||
1. **DataList filtering + sorting + saved filters** — biggest risk, most logic, rebuild from scratch
|
||||
2. **Quote CRUD** — parallels WorkOrder, needed to de-risk consolidation of the two
|
||||
3. **Customer CRUD + referential integrity** — root of object graph
|
||||
4. **Contract CRUD** — completes the main business objects
|
||||
5. **Auth roles / rights behavior** — largest single business logic file
|
||||
6. **Part CRUD** — inventory complexity
|
||||
7. **PM CRUD** — parallels WorkOrder
|
||||
8. **Schedule reads**
|
||||
9. Everything in Tier 3 as capacity allows
|
||||
Reference in New Issue
Block a user