Merged PR 2246: refactor reps first slice
Related work items: #22570, #22576, #22577, #22583, #22586, #22587, #22588, #22590, #22591
This commit is contained in:
Robert Bond
@@ -0,0 +1,354 @@
|
||||
# New Appeal Refactor — Current State of Play
|
||||
|
||||
## Purpose
|
||||
|
||||
This note updates the earlier **pre-refactor analysis** with the **current state after the Slice 1–8 refactor stream**.
|
||||
|
||||
Scope remains the new appeal flow areas:
|
||||
|
||||
- `pages/newappeal/**`
|
||||
- `components/newappeal/**`
|
||||
|
||||
---
|
||||
|
||||
## Executive summary
|
||||
|
||||
The planned refactor stream has been completed through **Slice 8** on the `refactor` branch integration line.
|
||||
|
||||
- Behaviour-preserving objective has been maintained (S78 flow)
|
||||
- Core journeys remained stable through regression checks
|
||||
- Structural clarity has improved in the highest-risk areas
|
||||
- The codebase is in a safer state for future controlled change
|
||||
|
||||
---
|
||||
|
||||
## Slice completion status (current)
|
||||
|
||||
All planned slices are complete:
|
||||
|
||||
1. Slice 1 — XML/Form derivation extraction ✅
|
||||
2. Slice 2a — Payload cleanup helpers ✅
|
||||
3. Slice 2b — File merge/dedupe helpers ✅
|
||||
4. Slice 3 — Side-effect facade wrappers ✅
|
||||
5. Slice 4 — BuildSection UI extraction ✅
|
||||
6. Slice 5 — BuildCheckSection UI extraction ✅
|
||||
7. Slice 6 — BuildCheckRow formatter-map cleanup ✅
|
||||
8. Slice 7 — Nested prop-boundary simplification ✅
|
||||
9. Slice 8 — Start flow readability cleanup (`createCase` / `aboutyou`) ✅
|
||||
|
||||
Tracker reference: `context/refactor-tracker.md`
|
||||
|
||||
---
|
||||
|
||||
## What changed overall
|
||||
|
||||
### Structural improvements delivered
|
||||
|
||||
- Repeated XML/form derivation logic moved into dedicated lib helpers
|
||||
- Payload shaping and file-list merge/dedupe logic normalized into focused helpers
|
||||
- Side effects wrapped behind clearer journey-effect boundaries
|
||||
- Large UI-heavy files split with clearer presentational structure (Slices 4–5)
|
||||
- Check-row formatting logic simplified into more maintainable mapping style
|
||||
- Deep nested prop access reduced with local boundary aliases
|
||||
- Start-flow components (`createCase`, `aboutyou`) made easier to read using local predicates/aliases
|
||||
|
||||
### What was intentionally preserved
|
||||
|
||||
- Business rules (including S78 behaviour)
|
||||
- HASCAS mapping behaviour
|
||||
- Appellant/agent branching behaviour
|
||||
- Field names/IDs and payload shape
|
||||
- Redirect/query construction (`lpa`, `apt`, `id`)
|
||||
- Validation messages/rules
|
||||
- Navigation and side-effect ordering/timing
|
||||
- EN/CY parity
|
||||
|
||||
---
|
||||
|
||||
## Current architecture quality (post-stream)
|
||||
|
||||
### Stronger than before
|
||||
|
||||
- Better separation between derivation/formatting/helpers and component rendering
|
||||
- Reduced duplication in payload/file processing paths
|
||||
- Safer change surfaces in `buildsection`, `buildchecksection`, and start flow
|
||||
- Improved readability of high-churn components
|
||||
|
||||
### Still sensitive / needs caution
|
||||
|
||||
- `buildsection.js` and `buildchecksection.js` remain central orchestration points
|
||||
- Save/resume and submit/finalise paths still require strict regression discipline
|
||||
- Some legacy patterns/comments remain and can obscure intent
|
||||
|
||||
---
|
||||
|
||||
## Risk position now
|
||||
|
||||
### Risks reduced
|
||||
|
||||
- Regression risk from duplicated transformation logic
|
||||
- Coupling risk from deep nested prop traversal
|
||||
- Reviewability risk in large mixed-concern components
|
||||
- Formatting-logic drift risk in check-answers rendering
|
||||
|
||||
### Risks still present (manageable)
|
||||
|
||||
- Live-flow sensitivity for save/resume/upload/submit remains inherently high
|
||||
- Manual regression burden is still significant for confidence
|
||||
- Further cleanup must continue to avoid behaviour changes
|
||||
|
||||
---
|
||||
|
||||
## Deferred follow-up items (post-stream)
|
||||
|
||||
These are **not part of Slice 1–8** and should be handled as separate scoped work:
|
||||
|
||||
1. Add/expand targeted automated regression coverage for protected journeys
|
||||
2. Remove stale/commented legacy code in touched components (safe cleanup slices)
|
||||
3. Document critical invariants for save/resume/submit paths in a concise maintainer note
|
||||
4. Continue incremental decomposition of remaining orchestration-heavy components only if behaviour can be proven unchanged
|
||||
|
||||
---
|
||||
|
||||
## Validation position
|
||||
|
||||
Regression evidence recorded across slices indicates preserved behaviour for:
|
||||
|
||||
- Start appeal and section navigation
|
||||
- Save and exit
|
||||
- Resume
|
||||
- Upload and file handling
|
||||
- Check answers rendering
|
||||
- Submit/finalisation
|
||||
- Confirmation/email paths
|
||||
- EN/CY parity
|
||||
- CRM insertion smoke
|
||||
- Lint baseline (warnings-only, no new blocking errors)
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
Compared to the earlier analysis, the new appeal flow is now in a materially safer and more maintainable state while preserving live behaviour.
|
||||
|
||||
The planned refactor stream (Slices 1–8) is complete. Any additional improvements should proceed as new, tightly scoped, behaviour-preserving slices.
|
||||
|
||||
---
|
||||
|
||||
## What should be done next
|
||||
|
||||
## 1) Stabilise and release safely (immediate)
|
||||
|
||||
Before any merge into `SIPS-Development`, run a formal readiness pass on `refactor`:
|
||||
|
||||
1. `npm run lint` (confirm no new blocking issues)
|
||||
2. Full protected-journey manual regression:
|
||||
- start appeal
|
||||
- save and exit
|
||||
- resume
|
||||
- upload documents
|
||||
- check answers
|
||||
- submit
|
||||
- confirmation
|
||||
3. EN/CY parity pass on touched paths
|
||||
4. Negative-path checks on validation and conditional branches
|
||||
5. CRM insertion smoke validation
|
||||
6. Rollback verification (revert path by merge commit)
|
||||
|
||||
If any check fails, fix in a separate, minimal safety slice.
|
||||
|
||||
## 2) Close out stream documentation (short, high-signal)
|
||||
|
||||
Add a concise closeout note (or PR summary block) that captures:
|
||||
|
||||
- stream scope and completed slices
|
||||
- behaviour-preservation statement
|
||||
- validation evidence references
|
||||
- known constraints that must not change in future work
|
||||
|
||||
Keep this short and avoid duplicating implementation detail.
|
||||
|
||||
## 3) Start a post-stream backlog (separate from Slice 1–8)
|
||||
|
||||
Create a small follow-up backlog of **new, independent slices** (do not reopen completed slices):
|
||||
|
||||
- automated regression coverage for protected journeys
|
||||
- safe removal of stale commented/dead code
|
||||
- targeted cleanup of remaining orchestration hotspots (`buildsection`, `buildchecksection`) only when backed by tests/regression checks
|
||||
- brief maintainer notes for critical invariants (payload shape, route/query contract, side-effect ordering)
|
||||
|
||||
## 4) Recommended execution order for follow-ups
|
||||
|
||||
Safest sequence:
|
||||
|
||||
1. Release-readiness and merge safety checks
|
||||
2. Regression automation improvements
|
||||
3. Documentation/invariant notes
|
||||
4. Small structural cleanups in high-risk files (one concern per slice)
|
||||
|
||||
## 5) Guardrails for all next work
|
||||
|
||||
Continue to enforce refactor-branch rules:
|
||||
|
||||
- no business-rule changes
|
||||
- no payload/field-name/query-shape drift
|
||||
- no side-effect timing/order changes unless explicitly scoped and proven safe
|
||||
- preserve EN/CY parity
|
||||
- keep slices small, reversible, and independently testable
|
||||
|
||||
---
|
||||
|
||||
## Potential improvements to logic and functionality (future work)
|
||||
|
||||
These items are intentionally listed as **future enhancements** (not part of completed refactor slices).
|
||||
|
||||
Any item that changes user-facing behaviour, business rules, or decision logic should be delivered as product-approved feature work with explicit regression protection.
|
||||
|
||||
## 1) Validation and data-quality improvements
|
||||
|
||||
- Strengthen cross-field validation where dependencies are currently implicit (for example, conditional requirements tied to prior answers).
|
||||
- Introduce clearer, field-level validation feedback for complex text/rich-text inputs.
|
||||
- Add stricter normalization for user-entered contact data (format quality, consistency) while preserving accepted legacy cases.
|
||||
|
||||
## 2) Save/resume robustness
|
||||
|
||||
- Add structured save/resume integrity checks to detect and recover from partial or stale payloads.
|
||||
- Improve diagnostics around resume hydration mismatches (field missing, incompatible shape, stale section state).
|
||||
- Add explicit handling for interrupted upload/save sequences to reduce user recovery friction.
|
||||
|
||||
## 3) Upload/document experience and safety
|
||||
|
||||
- Improve duplicate-document handling feedback (clear user messaging when dedupe occurs).
|
||||
- Provide more explicit upload status and retry outcomes for failed or partial uploads.
|
||||
- Add stronger document-type validation messaging and edge-case handling for borderline file metadata.
|
||||
|
||||
## 4) Submit/finalisation resilience
|
||||
|
||||
- Add clearer user-safe fallback states for finalisation failures (e.g., PDF generation or downstream completion messaging failures).
|
||||
- Improve idempotency safeguards in final submission steps to reduce accidental duplicate side effects.
|
||||
- Strengthen auditability of submit transitions for operational support/debugging.
|
||||
|
||||
## 5) Start-flow usability enhancements
|
||||
|
||||
- Improve guidance for appeal-type selection and conditional HASCAS confirmation to reduce user error.
|
||||
- Consider smarter defaults/prefill where policy allows, especially for known user profile/contact fields.
|
||||
- Improve branch-specific helper copy for appellant vs agent pathways.
|
||||
|
||||
## 6) Performance and maintainability improvements
|
||||
|
||||
- Reduce repeated in-component derivation cost by introducing safe memoization where values are stable.
|
||||
- Continue incremental decomposition of remaining orchestration-heavy components into testable units.
|
||||
- Expand targeted automated tests (journey-level + high-risk utility-level) to lower regression burden.
|
||||
|
||||
## 7) Recommended delivery model for these improvements
|
||||
|
||||
- Treat each enhancement area as a separate, tightly scoped slice.
|
||||
- Prioritise by user risk and operational impact:
|
||||
1. save/resume robustness
|
||||
2. submit/finalisation resilience
|
||||
3. validation/data-quality improvements
|
||||
4. upload/document UX resilience
|
||||
5. usability/performance refinements
|
||||
|
||||
- For each slice, require:
|
||||
- explicit statement of intended behaviour change (or confirmation of no change)
|
||||
- EN/CY parity checks
|
||||
- negative-path checks
|
||||
- rollback plan
|
||||
|
||||
---
|
||||
|
||||
## Evolving this into a multi-appeal-type platform (beyond S78)
|
||||
|
||||
To move from an S78-focused journey to a reusable **new appeal platform**, the safest path is to keep S78 as the baseline contract and introduce platform capabilities in additive slices.
|
||||
|
||||
## Platform principles
|
||||
|
||||
- Keep **S78 as reference behaviour** (do not regress)
|
||||
- Drive variation via **configuration and schema**, not hardcoded branching
|
||||
- Keep a stable core runtime for:
|
||||
- start/resume/save
|
||||
- section rendering
|
||||
- validation execution
|
||||
- upload/check/submit/finalise
|
||||
- Isolate appeal-type specifics behind explicit adapters
|
||||
|
||||
## Recommended platform workstreams
|
||||
|
||||
## 1) Appeal-type definition contract
|
||||
|
||||
Define a typed “appeal definition” model for each appeal type, covering:
|
||||
|
||||
- metadata (id, route key, labels, EN/CY text references)
|
||||
- section order and visibility rules
|
||||
- field/validation rules
|
||||
- required-doc rules
|
||||
- submission/finalisation behaviour flags
|
||||
|
||||
Start by expressing S78 in this contract first, then add new appeal types using the same contract.
|
||||
|
||||
## 2) Start-flow strategy model
|
||||
|
||||
Generalise `createCase`/`aboutyou` into a strategy-driven start flow:
|
||||
|
||||
- shared start-flow shell
|
||||
- per-appeal-type start-step configuration
|
||||
- clear branching policy hooks (e.g., HASCAS-style gates as config)
|
||||
|
||||
This avoids adding new hardcoded conditions per appeal type.
|
||||
|
||||
## 3) Validation engine boundary
|
||||
|
||||
Move towards a validation layer that consumes appeal-definition rules:
|
||||
|
||||
- common validator primitives
|
||||
- per-appeal rule sets loaded by appeal type
|
||||
- explicit cross-field dependency maps
|
||||
|
||||
Preserve existing messages/behaviour for S78 while introducing extension points.
|
||||
|
||||
## 4) Section and field rendering registry
|
||||
|
||||
Introduce a registry pattern for section/field rendering rules:
|
||||
|
||||
- field-type renderer map (already partially established)
|
||||
- appeal-type-specific section decorators where needed
|
||||
- strict fallback behaviour for unsupported fields
|
||||
|
||||
This enables incremental onboarding of new forms without rewriting core flow components.
|
||||
|
||||
## 5) Submission and integration adapters
|
||||
|
||||
Define adapter interfaces for variation points:
|
||||
|
||||
- payload mapping by appeal type
|
||||
- finalisation actions by appeal type
|
||||
- notification templates by appeal type/locale
|
||||
|
||||
Core workflow should call adapters, not branch inline by appeal type.
|
||||
|
||||
## 6) Platform test matrix and release gates
|
||||
|
||||
Adopt a matrix-driven regression model:
|
||||
|
||||
- rows: appeal types
|
||||
- columns: protected journeys + EN/CY parity + negative paths
|
||||
|
||||
Require S78 baseline pass + new-type pass before release.
|
||||
|
||||
## Suggested delivery sequence (safe)
|
||||
|
||||
1. Define appeal-type contract (S78 only)
|
||||
2. Map current S78 start flow to strategy model (no behaviour change)
|
||||
3. Introduce validation/section registries behind existing behaviour
|
||||
4. Add adapter boundaries for submit/finalise integrations
|
||||
5. Pilot one additional appeal type behind feature flag/config gating
|
||||
6. Expand matrix automation and observability
|
||||
|
||||
## Guardrails while platformising
|
||||
|
||||
- Do not change S78 business behaviour unintentionally
|
||||
- No payload/field/query contract drift for existing types
|
||||
- Preserve save/resume compatibility
|
||||
- Preserve EN/CY parity across all onboarded types
|
||||
- Keep rollout incremental with clear rollback per slice
|
||||
Reference in New Issue
Block a user