6.0 KiB
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
- DataList filtering + sorting + saved filters — biggest risk, most logic, rebuild from scratch
- Quote CRUD — parallels WorkOrder, needed to de-risk consolidation of the two
- Customer CRUD + referential integrity — root of object graph
- Contract CRUD — completes the main business objects
- Auth roles / rights behavior — largest single business logic file
- Part CRUD — inventory complexity
- PM CRUD — parallels WorkOrder
- Schedule reads
- Everything in Tier 3 as capacity allows