Merged PR 2242: Refactor: New Appeal flow structural cleanup (Slices 1–8, no behaviour change)
## Summary This PR merges the completed new-appeal refactor stream (Slices 1–8) into SIPS-Development. This refactor was executed in controlled slices with regression validation at each step. ## Scope Structural and readability improvements only: - XML/form derivation extraction - payload and file helper extraction - side-effect facade introduction - BuildSection and BuildCheckSection decomposition - BuildCheckRow formatter map refactor - nested prop boundary cleanup - start-flow cleanup (CreateCase / AboutYou) ## Behaviour No intended behavioural changes. The following were explicitly preserved: - S78 journey behaviour - payload shapes and field names/ids - HASCAS mapping logic - appellant/agent branching - validation rules and messages - redirect/query parameters (`lpa`, `apt`, `id`) - navigation and side-effect sequencing - EN/CY output parity ## Validation Full regression has been performed on: - start flow (CreateCase / AboutYou) - save and resume flows - file upload handling - check answers rendering - submit/finalisation sequence - confirmation flow - CRM insertion path - EN/CY parity Additional checks: - docsOffline branch behaviour - completion and partial-save email paths - negative-path validation scenarios - lint (warnings baseline unchanged) ## Risk Low: - changes are structural only - no business logic changes - no contract changes ## Rollback Safe rollback via reverting this merge commit. ## Notes This refactor reduces coupling and prepares the new-appeal flow for future appeal-type expansion. Related work items: #22570, #22576, #22577, #22583, #22586, #22587, #22588, #22590
This commit is contained in:
Robert Bond
@@ -0,0 +1,176 @@
|
||||
# Contributing with AI (Refactor Branch — PEDW FrontEnd)
|
||||
|
||||
## Purpose
|
||||
|
||||
This guide defines how engineers and AI agents should work safely and efficiently in this refactor branch.
|
||||
|
||||
This branch focuses on **safe, behaviour-preserving refactor of the new appeal flow**.
|
||||
|
||||
---
|
||||
|
||||
## Start Here (Required Order)
|
||||
|
||||
1. Read `.clinerules/refactor-branch-rules.md`
|
||||
2. Read `GUARDRAILS.md`
|
||||
3. Read core context:
|
||||
- `context/refactor-branch-charter.md`
|
||||
- `context/newappeal-refactor-guardrails.md`
|
||||
- `context/architecture.md`
|
||||
- `context/domain-flows.md`
|
||||
|
||||
Only read additional files when directly relevant.
|
||||
|
||||
Do **not** load all context, templates, or memory-bank files by default.
|
||||
|
||||
---
|
||||
|
||||
## Core Principle
|
||||
|
||||
This is a **refactor branch**, not a feature branch.
|
||||
|
||||
- Preserve current live S78 behaviour
|
||||
- Do not introduce business-rule changes unless explicitly requested
|
||||
- Prefer small, safe, reversible changes
|
||||
- Focus on structure, not behaviour
|
||||
|
||||
---
|
||||
|
||||
## Standard AI-Assisted Workflow
|
||||
|
||||
### 1. Scope
|
||||
|
||||
- Define requirement
|
||||
- Confirm non-goals
|
||||
- Identify impacted flow areas
|
||||
- Identify risk level
|
||||
|
||||
### 2. Plan
|
||||
|
||||
- Keep plan small and incremental
|
||||
- Identify safe boundaries
|
||||
- Avoid mixing refactor + feature work
|
||||
|
||||
### 3. Implement
|
||||
|
||||
- Make the smallest viable change
|
||||
- Prefer extraction over rewrite
|
||||
- Preserve interfaces and payload shapes
|
||||
|
||||
### 4. Validate
|
||||
|
||||
Minimum required:
|
||||
|
||||
- `npm run lint`
|
||||
- targeted manual checks
|
||||
- EN/CY parity checks (if user-facing)
|
||||
- negative-path checks for sensitive flows
|
||||
|
||||
### 5. Document (only when needed)
|
||||
|
||||
- update `memory-bank/change-log.md` for non-trivial work
|
||||
- record decisions or pitfalls if useful
|
||||
|
||||
---
|
||||
|
||||
## Templates (Use Only When Needed)
|
||||
|
||||
Templates are optional tools, not default context.
|
||||
|
||||
Use **only one** template if needed:
|
||||
|
||||
- feature implementation
|
||||
- bug fix
|
||||
- refactor
|
||||
- debugging
|
||||
- code review
|
||||
|
||||
Do not load all templates.
|
||||
Do not treat templates as baseline context.
|
||||
|
||||
---
|
||||
|
||||
## Context Loading Rule (Critical)
|
||||
|
||||
Always use the **minimum required context**.
|
||||
|
||||
### Default context for this branch:
|
||||
|
||||
- `context/refactor-branch-charter.md`
|
||||
- `context/newappeal-refactor-guardrails.md`
|
||||
- `context/architecture.md`
|
||||
- `context/domain-flows.md`
|
||||
|
||||
### Only load additional files when:
|
||||
|
||||
- working with integrations → `integration-map.md`
|
||||
- reviewing test gaps → `test-coverage-map.md`
|
||||
- performing release work → `runbook.md`
|
||||
- working on specific planned sequence → `context/plans/*`
|
||||
|
||||
Never load:
|
||||
|
||||
- all context files
|
||||
- all templates
|
||||
- onboarding or overview docs
|
||||
|
||||
---
|
||||
|
||||
## Branching Protocol
|
||||
|
||||
This repository currently uses different branch rules depending on the work type.
|
||||
|
||||
### For BAU / normal delivery work
|
||||
|
||||
- create work branches from `SIPS-Development`
|
||||
|
||||
### For new appeal refactor work
|
||||
|
||||
- use the `refactor` branch as the base branch
|
||||
- do refactor implementation either:
|
||||
- directly on `refactor`, if that is the agreed working model, or
|
||||
- on a short-lived feature branch created from `refactor`
|
||||
|
||||
Do not perform refactor implementation directly on `SIPS-Development`.
|
||||
|
||||
If unsure which branch model applies, stop and confirm before making changes.
|
||||
|
||||
---
|
||||
|
||||
## No-Break Rules (Always Enforce)
|
||||
|
||||
- Do not weaken auth/session behaviour
|
||||
- Do not weaken security headers/CSP
|
||||
- Keep EN/CY behaviour aligned
|
||||
- Do not expose secrets or personal data
|
||||
- Protect save/resume/upload/submit journeys
|
||||
|
||||
---
|
||||
|
||||
## PR Minimum (AI-Assisted)
|
||||
|
||||
Every change must include:
|
||||
|
||||
- Scope summary
|
||||
- Files changed
|
||||
- Risk notes (auth/data/i18n/a11y)
|
||||
- Validation evidence
|
||||
- Rollback plan
|
||||
|
||||
---
|
||||
|
||||
## Handling Unknowns
|
||||
|
||||
If unsure:
|
||||
|
||||
1. Choose the safer option
|
||||
2. Do not change behaviour
|
||||
3. call out assumptions clearly
|
||||
4. log open questions if needed
|
||||
|
||||
---
|
||||
|
||||
## Summary Rule
|
||||
|
||||
If in doubt:
|
||||
|
||||
> Preserve behaviour, reduce risk, keep changes small.
|
||||
@@ -0,0 +1,95 @@
|
||||
# Default Rules (Reference Only)
|
||||
|
||||
## Important
|
||||
|
||||
This file is **not the active rule set for this refactor branch**.
|
||||
|
||||
Active working rules are defined in:
|
||||
|
||||
- `.clinerules/refactor-branch-rules.md`
|
||||
- `context/refactor-branch-charter.md`
|
||||
- `context/newappeal-refactor-guardrails.md`
|
||||
|
||||
Do not load this file by default.
|
||||
|
||||
---
|
||||
|
||||
## Purpose
|
||||
|
||||
This file provides general repository-wide guidance and may be referenced when needed for broader context.
|
||||
|
||||
---
|
||||
|
||||
## Repository Mission
|
||||
|
||||
Deliver safe, accessible, bilingual (EN/CY), and reliable public-service functionality without regressing core journeys.
|
||||
|
||||
---
|
||||
|
||||
## General Priorities
|
||||
|
||||
1. Safety and security of user data
|
||||
2. Stability of public-facing journeys
|
||||
3. Accessibility and bilingual parity
|
||||
4. Maintainability of code
|
||||
5. Controlled, safe delivery
|
||||
|
||||
---
|
||||
|
||||
## General Coding Guidance
|
||||
|
||||
- Follow existing repo patterns
|
||||
- Keep page-level logic thin
|
||||
- Move reusable logic into `components/`, `actions/`, or `lib/`
|
||||
- Avoid broad refactors in feature or bugfix work
|
||||
- Prefer small, focused changes
|
||||
|
||||
---
|
||||
|
||||
## Architecture Guardrails (General)
|
||||
|
||||
- Do not bypass auth/session handling
|
||||
- Do not weaken security controls
|
||||
- Preserve relay request integrity
|
||||
- Maintain EN/CY route parity
|
||||
- Keep Redux hydration/persistence stable
|
||||
|
||||
---
|
||||
|
||||
## Validation Expectations (General)
|
||||
|
||||
- linting must pass
|
||||
- user-facing changes must be verified
|
||||
- accessibility and EN/CY checks required
|
||||
- sensitive flows require negative-path checks
|
||||
|
||||
---
|
||||
|
||||
## When To Use This File
|
||||
|
||||
Only reference this file when:
|
||||
|
||||
- broader repository rules are required
|
||||
- behaviour conflicts are unclear
|
||||
- no branch-specific guidance exists
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
This file is background guidance only.
|
||||
|
||||
For this refactor branch:
|
||||
|
||||
- follow branch-specific rules first
|
||||
- prioritise behaviour preservation
|
||||
- keep context minimal and focused
|
||||
|
||||
## Branching Guidance (Reference)
|
||||
|
||||
Branching depends on work type:
|
||||
|
||||
- BAU work typically branches from `SIPS-Development`
|
||||
- refactor-stream work for the new appeal flow uses `refactor` as the base branch
|
||||
|
||||
Follow branch-specific rules first.
|
||||
@@ -0,0 +1,228 @@
|
||||
# Refactor Branch Rules (Active)
|
||||
|
||||
## Purpose
|
||||
|
||||
This file defines the active working rules for this refactor branch.
|
||||
|
||||
This branch is focused on **safe, behaviour-preserving refactor of the new appeal flow**.
|
||||
|
||||
---
|
||||
|
||||
## Core Rule (Highest Priority)
|
||||
|
||||
Do not change live S78 behaviour unless explicitly instructed.
|
||||
|
||||
Refactor = improve structure, not behaviour.
|
||||
|
||||
---
|
||||
|
||||
## Primary Objectives
|
||||
|
||||
1. Make the new appeal flow easier to understand and maintain
|
||||
2. Reduce risk when making future changes
|
||||
3. Prepare the system for multiple appeal types
|
||||
4. Improve separation of concerns (UI, workflow, data, integrations)
|
||||
|
||||
---
|
||||
|
||||
## Non-Negotiable Rules
|
||||
|
||||
- Preserve all current user journeys:
|
||||
- start appeal
|
||||
- save and exit
|
||||
- resume appeal
|
||||
- upload documents
|
||||
- check answers
|
||||
- submit appeal
|
||||
- confirmation
|
||||
|
||||
- Do not:
|
||||
- change payload structures
|
||||
- change validation rules
|
||||
- change business logic
|
||||
- hardcode logic that is currently dynamic/config-driven
|
||||
- mix refactor with feature work
|
||||
|
||||
---
|
||||
|
||||
## Refactor Approach
|
||||
|
||||
Always:
|
||||
|
||||
1. Understand current behaviour first
|
||||
2. Identify smallest safe change
|
||||
3. Prefer extraction over rewrite
|
||||
4. Keep public interfaces stable
|
||||
5. Make changes easy to review and revert
|
||||
|
||||
---
|
||||
|
||||
## Change Size Guidance
|
||||
|
||||
- Prefer small PRs (<400 LOC where possible)
|
||||
- Avoid large multi-concern changes
|
||||
- Split work into safe slices
|
||||
|
||||
---
|
||||
|
||||
## Context Usage Rules (Critical for Efficiency)
|
||||
|
||||
### Default context (only load these):
|
||||
|
||||
- `context/refactor-branch-charter.md`
|
||||
- `context/newappeal-refactor-guardrails.md`
|
||||
- `context/architecture.md`
|
||||
- `context/domain-flows.md`
|
||||
|
||||
### Only load additional context when needed:
|
||||
|
||||
- integrations → `integration-map.md`
|
||||
- testing gaps → `test-coverage-map.md`
|
||||
- release concerns → `runbook.md`
|
||||
- specific work plan → `context/plans/*`
|
||||
|
||||
### Never load by default:
|
||||
|
||||
- all context files
|
||||
- onboarding or overview docs
|
||||
- all templates
|
||||
- memory-bank contents
|
||||
|
||||
---
|
||||
|
||||
## Templates Usage
|
||||
|
||||
Templates are optional helpers.
|
||||
|
||||
- Use only ONE template when needed
|
||||
- Do not load all templates
|
||||
- Do not treat templates as baseline context
|
||||
|
||||
---
|
||||
|
||||
## Safety Rules
|
||||
|
||||
- Do not weaken authentication or session logic
|
||||
- Do not weaken security headers or middleware
|
||||
- Do not expose secrets or personal data
|
||||
- Preserve EN/CY parity
|
||||
- Maintain accessibility standards
|
||||
|
||||
---
|
||||
|
||||
## Testing & Validation Expectations
|
||||
|
||||
Minimum:
|
||||
|
||||
- `npm run lint`
|
||||
- targeted manual verification
|
||||
- EN/CY checks (if user-facing)
|
||||
- negative-path checks (for sensitive flows)
|
||||
|
||||
Before changing critical logic:
|
||||
|
||||
- add or update regression protection where possible
|
||||
|
||||
---
|
||||
|
||||
## When Unsure
|
||||
|
||||
If any uncertainty exists:
|
||||
|
||||
1. Choose the safest option
|
||||
2. Do not change behaviour
|
||||
3. Call out assumptions
|
||||
4. Keep the change minimal
|
||||
|
||||
---
|
||||
|
||||
## Definition of Success
|
||||
|
||||
A successful change:
|
||||
|
||||
- preserves behaviour
|
||||
- reduces complexity
|
||||
- improves clarity or structure
|
||||
- is small and safe to merge
|
||||
- does not introduce regression risk
|
||||
|
||||
---
|
||||
|
||||
## One-Line Rule
|
||||
|
||||
If in doubt:
|
||||
|
||||
> Keep behaviour the same, reduce risk, and make the smallest safe change.
|
||||
|
||||
---
|
||||
|
||||
## Documentation Rule (Minimal and Targeted)
|
||||
|
||||
Only document information that is necessary to safely understand or change the system later.
|
||||
|
||||
Document:
|
||||
|
||||
- key decisions (why something was changed)
|
||||
- non-obvious behaviour
|
||||
- risks or constraints (what must not be changed)
|
||||
- important assumptions
|
||||
|
||||
Do not document:
|
||||
|
||||
- obvious code behaviour
|
||||
- step-by-step implementation details
|
||||
- temporary or experimental work
|
||||
- duplicated explanations across files
|
||||
|
||||
Prefer short, focused notes over long explanations.
|
||||
|
||||
If unsure:
|
||||
|
||||
> Will someone break the system in the future if this is not written down?
|
||||
|
||||
If yes → document it
|
||||
If no → do not document it
|
||||
|
||||
Documentation should be minimal, high-signal, and never outweigh the value of the code itself.
|
||||
|
||||
## Branch Safety Rule
|
||||
|
||||
This refactor stream uses the `refactor` branch as its working base branch.
|
||||
|
||||
For refactor work:
|
||||
|
||||
- do all implementation from the `refactor` branch or a short-lived feature branch created from `refactor`
|
||||
- do not implement refactor work directly on `SIPS-Development`
|
||||
|
||||
`SIPS-Development` remains the BAU integration branch.
|
||||
Refactor changes may be merged into `SIPS-Development` only when proven safe.
|
||||
|
||||
If branch context is unclear, stop and confirm before making changes.
|
||||
|
||||
## Branch Model
|
||||
|
||||
- `SIPS-Development` = BAU branch
|
||||
- `refactor` = refactor integration branch
|
||||
- optional short-lived working branches for individual slices should be created from `refactor`
|
||||
|
||||
## Regression Safety Rule (Critical)
|
||||
|
||||
This is a live system. All refactor work must prove behaviour is unchanged.
|
||||
|
||||
Before completing any slice:
|
||||
|
||||
- verify core journey behaviour has not changed
|
||||
- verify protected flows still work:
|
||||
- save and exit
|
||||
- resume appeal
|
||||
- upload documents
|
||||
- check answers
|
||||
- submit appeal
|
||||
- confirmation
|
||||
|
||||
- verify EN/CY parity for any affected areas
|
||||
|
||||
A slice is NOT complete until regression behaviour is confirmed.
|
||||
|
||||
If regression cannot be confidently ruled out:
|
||||
→ do not proceed to next slice
|
||||
@@ -0,0 +1,15 @@
|
||||
# Bugfix Workflow
|
||||
|
||||
1. Clarify the bug symptoms and expected behavior.
|
||||
2. Inspect the relevant files and execution path.
|
||||
3. Identify the most likely root cause.
|
||||
4. Implement the smallest safe fix.
|
||||
5. Check for:
|
||||
- side effects
|
||||
- regression risk
|
||||
- validation gaps
|
||||
- missing error handling
|
||||
6. Summarize:
|
||||
- root cause
|
||||
- fix
|
||||
- preventive improvements worth noting
|
||||
@@ -0,0 +1,17 @@
|
||||
# Code Review Workflow
|
||||
|
||||
Review the relevant code or changes for:
|
||||
- correctness
|
||||
- edge cases
|
||||
- data integrity
|
||||
- security concerns
|
||||
- performance concerns
|
||||
- maintainability
|
||||
- readability
|
||||
- consistency with repo conventions
|
||||
|
||||
Return:
|
||||
1. key findings
|
||||
2. impact / severity where useful
|
||||
3. suggested improvements
|
||||
4. strengths worth preserving
|
||||
@@ -0,0 +1,18 @@
|
||||
# Database Migration Workflow
|
||||
|
||||
1. Understand the desired schema or data change.
|
||||
2. Inspect current data model, queries, and dependent code.
|
||||
3. Identify:
|
||||
- forward migration steps
|
||||
- backward compatibility concerns
|
||||
- rollback considerations
|
||||
4. Implement or propose the minimal safe migration path.
|
||||
5. Review for:
|
||||
- data integrity
|
||||
- ordering issues
|
||||
- application compatibility
|
||||
- deployment timing risk
|
||||
6. Summarize:
|
||||
- schema/data changes
|
||||
- impacted code paths
|
||||
- rollout / rollback notes
|
||||
@@ -0,0 +1,17 @@
|
||||
# Debugging Workflow
|
||||
|
||||
1. Capture the symptom, error, or incorrect behavior.
|
||||
2. Inspect the relevant path and recent changes.
|
||||
3. Form the most likely hypotheses.
|
||||
4. Narrow the cause using the smallest practical evidence checks.
|
||||
5. Propose or implement the minimal safe fix.
|
||||
6. Review for:
|
||||
- hidden assumptions
|
||||
- unhandled states
|
||||
- logging or observability gaps
|
||||
- regression risk
|
||||
7. Summarize:
|
||||
- symptom
|
||||
- cause
|
||||
- fix
|
||||
- prevention ideas
|
||||
@@ -0,0 +1,10 @@
|
||||
# Documentation Update Workflow
|
||||
|
||||
1. Identify what changed in the code or workflow.
|
||||
2. Inspect the affected docs and support files.
|
||||
3. Update only the relevant sections.
|
||||
4. Keep the docs concise, factual, and current.
|
||||
5. Summarize:
|
||||
- what changed
|
||||
- which docs were updated
|
||||
- anything still uncertain
|
||||
@@ -0,0 +1,16 @@
|
||||
# Feature Implementation Workflow
|
||||
|
||||
1. Understand the requirement and desired outcome.
|
||||
2. Inspect the relevant files, patterns, and data flows.
|
||||
3. Propose a short implementation plan for non-trivial work.
|
||||
4. Implement the smallest maintainable change that satisfies the requirement.
|
||||
5. Review for:
|
||||
- correctness
|
||||
- edge cases
|
||||
- error handling
|
||||
- compatibility with existing behavior
|
||||
- maintainability
|
||||
6. Summarize:
|
||||
- what changed
|
||||
- why
|
||||
- any follow-up considerations
|
||||
@@ -0,0 +1,13 @@
|
||||
# Planning Workflow
|
||||
|
||||
1. Clarify the objective.
|
||||
2. Inspect the relevant code areas and constraints.
|
||||
3. Identify implementation options.
|
||||
4. Compare trade-offs.
|
||||
5. Recommend the most practical path.
|
||||
6. Return:
|
||||
- objective summary
|
||||
- proposed plan
|
||||
- risks
|
||||
- dependencies
|
||||
- suggested execution order
|
||||
@@ -0,0 +1,16 @@
|
||||
# PR Review Workflow
|
||||
|
||||
Review the pull request or change set for:
|
||||
- correctness
|
||||
- scope discipline
|
||||
- regression risk
|
||||
- compatibility impact
|
||||
- tests or validation gaps
|
||||
- security and performance concerns
|
||||
- maintainability
|
||||
|
||||
Return:
|
||||
1. summary of change intent
|
||||
2. key risks
|
||||
3. requested changes
|
||||
4. optional improvements
|
||||
@@ -0,0 +1,16 @@
|
||||
# Refactor Workflow
|
||||
|
||||
1. Understand the current behavior that must be preserved.
|
||||
2. Inspect the relevant modules and dependencies.
|
||||
3. Identify safe refactor boundaries.
|
||||
4. Propose a scoped plan.
|
||||
5. Refactor incrementally.
|
||||
6. Review for:
|
||||
- preserved behavior
|
||||
- reduced complexity
|
||||
- improved readability
|
||||
- regression risk
|
||||
7. Summarize:
|
||||
- what was simplified
|
||||
- what was preserved
|
||||
- remaining debt
|
||||
@@ -0,0 +1,15 @@
|
||||
# Release Readiness Workflow
|
||||
|
||||
1. Review the scope of changes.
|
||||
2. Check:
|
||||
- build readiness
|
||||
- environment assumptions
|
||||
- configuration changes
|
||||
- database implications
|
||||
- CI/CD impact
|
||||
- rollback expectations
|
||||
3. Identify release risks and missing checks.
|
||||
4. Summarize:
|
||||
- release blockers
|
||||
- risks
|
||||
- final recommended checks
|
||||
Reference in New Issue
Block a user