Merged PR 2258: refactor(summary-types): standardise case summary markup with SummaryCard/SummaryRow wrappers
Related work items: #22441
This commit is contained in:
Robert Bond
@@ -6,8 +6,8 @@ Base branch: `refactor`
|
||||
|
||||
## Status
|
||||
|
||||
Current slice: Slice R7 — Summary / Case-Type Rendering Simplification
|
||||
Status: IMPLEMENTED (manual validation in progress)
|
||||
Current slice: Slice R9 — Summary Rollout (Batch 1)
|
||||
Status: COMPLETE
|
||||
|
||||
---
|
||||
|
||||
@@ -17,202 +17,41 @@ Status: IMPLEMENTED (manual validation in progress)
|
||||
|
||||
### Slice R1 — Representation Entry Logic Extraction
|
||||
|
||||
**Goal:**
|
||||
Extract representation eligibility and CTA logic from `CaseSummary`.
|
||||
|
||||
**Scope:**
|
||||
|
||||
- showRepButton logic
|
||||
- date window checks (showReps / showRepsEnded)
|
||||
- appeal type branching
|
||||
- LPA-specific behaviour
|
||||
- CTA route construction
|
||||
|
||||
**Rules:**
|
||||
|
||||
- No behaviour change
|
||||
- No UI change
|
||||
- Same inputs/outputs
|
||||
|
||||
**Completion notes (this slice):**
|
||||
|
||||
- Extracted representation-entry decision logic from `components/case/summary.js` into `components/case/summary/utils/representationEntry.js`.
|
||||
- Preserved existing branch structure/order and duplicated consultation/general render branches.
|
||||
- Preserved existing data-source usage (`detailsObj` vs `searchDetailsObj`) and existing constants/field names.
|
||||
- Preserved local date-check behaviour via extracted local-equivalent helpers (`showRepsLocal`, `showRepsEndedLocal`).
|
||||
- Validation evidence:
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- Manual regression checks reported as passed (APP/IP/Agent/LPA), including EN/CY parity and route/query behaviour.
|
||||
[UNCHANGED]
|
||||
|
||||
---
|
||||
|
||||
### Slice R2 — Representation Page Loader Separation
|
||||
|
||||
**Goal:**
|
||||
Separate SSR/data-loading paths.
|
||||
|
||||
**Scope:**
|
||||
|
||||
- existing representation (state param)
|
||||
- new representation (from search/case)
|
||||
|
||||
**Target:**
|
||||
|
||||
- `loadExistingRepresentation()`
|
||||
- `loadNewRepresentation()`
|
||||
|
||||
**Completion notes (this slice):**
|
||||
|
||||
- Extracted representation page SSR/data-loading orchestration into `lib/representation/pageLoaders.js` with:
|
||||
- `loadRepresentationBootstrap({ ctx })`
|
||||
- `loadExistingRepresentation({ store, ctx, bootstrap })`
|
||||
- `loadNewRepresentation({ store, ctx, bootstrap })`
|
||||
- `loadRepresentationPage({ store, ctx })`
|
||||
- Kept `pages/myportal/representation.js` render/UI unchanged and converted `getServerSideProps` to a thin wrapper calling `loadRepresentationPage(...)`.
|
||||
- Preserved behaviour-critical details:
|
||||
- shared pre-branch bootstrap fetch timing
|
||||
- exact branch condition `query.hasOwnProperty("state")`
|
||||
- existing dispatch ordering and payload shapes
|
||||
- existing fallback/data-shape logic and typo-field usage
|
||||
- unchanged SSR returned props (`containerID`, `docsOffline`)
|
||||
- Validation evidence:
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` re-run completed: **6 passed, 1 failed**.
|
||||
- Failing test: `[chromium] tests/loggedin/raiserep.spec.js:203:5` (`Raise Statement representation as an Interested Party`) with error `no statement option` / missing `Statement` option in combobox.
|
||||
- Manual regression checks confirmed passed (APP/IP/Agent/LPA), including EN/CY parity.
|
||||
[UNCHANGED]
|
||||
|
||||
---
|
||||
|
||||
### Slice R3 — Journey Step Resolution Extraction
|
||||
|
||||
**Goal:**
|
||||
Isolate step navigation logic.
|
||||
|
||||
**Scope:**
|
||||
|
||||
- current step determination
|
||||
- next/previous step logic
|
||||
- validation gating between steps
|
||||
|
||||
**Completion notes (this slice):**
|
||||
|
||||
- Added pure step-resolution helpers in `components/case/representation/utils/stepResolution.js`:
|
||||
- `resolveJourneyStageFlags({ representationSubmit, representationSubmitConfirmation })`
|
||||
- `resolveRepresentationControlKey({ isLPA, representationCapacity, repDetailsCapacity, appealType, normalizeCapacity })`
|
||||
- `resolveSubmitTransition({ representationType, showQuestionnaireSection, questionnaireCount })`
|
||||
- `getQuestionnaireNextSection(currentSection, questionnaireCount)`
|
||||
- `getQuestionnairePreviousSection(currentSection)`
|
||||
- Updated `components/case/representation/index.js` to call helpers for:
|
||||
- check-stage flag resolution (`showCheckStage`)
|
||||
- control key resolution in `whichControl()`
|
||||
- questionnaire/non-questionnaire submit transition decision
|
||||
- questionnaire next-section increment calculation
|
||||
- Preserved behaviour-critical constraints:
|
||||
- no JSX structure redesign
|
||||
- no route/query changes
|
||||
- no payload/Redux shape changes
|
||||
- no dispatch/API side-effect sequencing changes
|
||||
- no representation-type availability logic changes (`buildRepsArr` unchanged)
|
||||
- Validation evidence:
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` re-run completed: **6 passed**.
|
||||
- Manual APP/IP/Agent/LPA and EN/CY checks: **pending for this slice**.
|
||||
[UNCHANGED]
|
||||
|
||||
---
|
||||
|
||||
### Slice R4 — Flow Shell Decomposition
|
||||
|
||||
**Goal:**
|
||||
Break large flow component into smaller parts.
|
||||
|
||||
**Target components:**
|
||||
|
||||
- Capacity selection
|
||||
- Representation type selection
|
||||
- Content entry
|
||||
- Check answers
|
||||
- Completion
|
||||
[UNCHANGED]
|
||||
|
||||
---
|
||||
|
||||
### Slice R5 — Representation Elements Normalisation
|
||||
|
||||
**Goal:**
|
||||
Clean up shared UI helpers and duplicated patterns.
|
||||
|
||||
**Scope:**
|
||||
|
||||
- input rendering
|
||||
- file upload handling (structure only, not behaviour)
|
||||
- shared UI logic extraction
|
||||
|
||||
**Completion notes (this slice):**
|
||||
|
||||
- Implemented on feature branch created from `refactor`: `rep-slice-r5-elements-normalisation`.
|
||||
- Added render-only shared fragments under `components/case/representation/elements/`:
|
||||
- `RepresentationTypeSelectorBlock.js`
|
||||
- `RepresentationActionButtons.js`
|
||||
- Integrated these fragments into main representation variants only:
|
||||
- `representationAppellant.js`
|
||||
- `representationAgent.js`
|
||||
- `representationInterestedPartyPerson.js`
|
||||
- Preserved scope constraints for Slice R5:
|
||||
- kept parent components as logic owners
|
||||
- no submission/update side-effect refactor
|
||||
- no upload service behaviour changes
|
||||
- no payload/route/query/Redux/validation business-rule changes
|
||||
- consultation variants intentionally not normalised in this slice
|
||||
- Validation evidence:
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` completed: **7 passed**.
|
||||
- Manual APP/IP/Agent/LPA checks: **passed**.
|
||||
- Manual EN/CY parity checks: **passed**.
|
||||
[UNCHANGED]
|
||||
|
||||
---
|
||||
|
||||
### Slice R6 — Async Data / Service Layer Cleanup
|
||||
|
||||
**Goal:**
|
||||
Reduce duplication and coupling in data fetching.
|
||||
|
||||
**Scope:**
|
||||
|
||||
- getCase → getPortalModuleDetails chains
|
||||
- blob/file retrieval
|
||||
- representation detail aggregation
|
||||
|
||||
**Completion notes (this slice):**
|
||||
|
||||
- Implemented on feature branch created from `refactor`: `rep-slice-r6-data-service-cleanup`.
|
||||
- Added deterministic representation-specific data-resolution helpers in:
|
||||
- `components/case/representation/utils/dataResolution.js`
|
||||
- Updated `components/case/representation/index.js` to use extracted helper functions for:
|
||||
- initial case details/results source resolution
|
||||
- details/results lookup by current context (state/edit/ticket/created)
|
||||
- submit-path details source resolution and details lookup
|
||||
- Preserved slice boundary constraints:
|
||||
- no service/API call execution movement
|
||||
- no dispatch sequencing changes
|
||||
- no loader branch redesign
|
||||
- no blob retrieval orchestration changes
|
||||
- no payload contract/Redux/route/query/EN-CY changes
|
||||
- Validation evidence:
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` completed: **7 passed**.
|
||||
- Manual APP/IP/Agent/LPA checks: **passed**.
|
||||
- Manual EN/CY parity checks: **passed**.
|
||||
[UNCHANGED]
|
||||
|
||||
---
|
||||
|
||||
### Slice R7 — Summary / Case-Type Rendering Simplification (Optional)
|
||||
|
||||
**Goal:**
|
||||
Reduce duplication in summary-type components.
|
||||
|
||||
**Scope:**
|
||||
|
||||
- unify repeated rendering patterns
|
||||
- introduce shared summary renderer where safe
|
||||
### Slice R7 — Summary / Case-Type Rendering Simplification (Proof Slice)
|
||||
|
||||
**Completion notes (this slice):**
|
||||
|
||||
@@ -220,129 +59,90 @@ Reduce duplication in summary-type components.
|
||||
- Added render-only summary primitives:
|
||||
- `components/case/summary/components/SummaryCard.js`
|
||||
- `components/case/summary/components/SummaryRow.js`
|
||||
- Applied these primitives to a small proof subset of summary-type files only:
|
||||
- `components/case/summaryTypes/pinswg_planningappeals78id.js`
|
||||
- `components/case/summaryTypes/pinswg_planningconditionss73s79id.js`
|
||||
- Preserved Slice R7 constraints:
|
||||
- no CaseSummary orchestration changes
|
||||
- no translation/date/bilingual logic moved into shared primitives
|
||||
- no linked-cases extraction (kept inline)
|
||||
- no row/card order changes intended
|
||||
- no route/query/payload/Redux/business-rule changes
|
||||
- Applied to proof subset:
|
||||
- `pinswg_planningappeals78id.js`
|
||||
- `pinswg_planningconditionss73s79id.js`
|
||||
- Preserved constraints:
|
||||
- no translation/date/bilingual logic moved
|
||||
- no linked-cases extraction
|
||||
- no ordering/visibility changes
|
||||
- Validation evidence:
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` completed: **7 passed**.
|
||||
- Manual APP/IP/Agent/LPA and EN/CY checks: **in progress** (Welsh IP statement option-label parity issue detected and patched in representation flow check display).
|
||||
|
||||
---
|
||||
|
||||
### Post-R7 follow-up patch — Welsh IP statement label parity
|
||||
|
||||
**Issue observed during manual check:**
|
||||
|
||||
- In Welsh journey (Interested Party → Statement), option-label parity regressed:
|
||||
- selection step showed duplicated Welsh “No” labels
|
||||
- check/submit summary displayed raw English `No`
|
||||
|
||||
**Fix applied (minimal, behaviour-preserving):**
|
||||
|
||||
- `components/case/representation/representationInterestedPartyPerson.js`
|
||||
- restored canonical option values for conditional radio to `options={["Yes", "No"]}` so `RenderCondtionalRadioList` translation mapping behaves correctly.
|
||||
- `components/case/representation/representationCompleteSubmit.js`
|
||||
- added display mapping for `representationOnBehalfOf` summary value:
|
||||
- `Yes` → `t("myrepresentations:questionnaire-yes")`
|
||||
- `No` → `t("myrepresentations:questionnaire-no")`
|
||||
- fallback unchanged for non-boolean/unexpected values
|
||||
|
||||
**Validation evidence (follow-up patch):**
|
||||
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` completed: **7 passed (4.6m)**.
|
||||
- Manual Welsh IP statement check: input labels and check-summary label now aligned via translation mapping.
|
||||
|
||||
---
|
||||
|
||||
### Post-R7 follow-up patch 2 — Welsh IP conditional details submit validation
|
||||
|
||||
**Issue observed during manual check:**
|
||||
|
||||
- In Welsh journey (Interested Party → Statement), when selecting “Yes” and entering conditional details, submit could still block with required validation on the conditional text field.
|
||||
|
||||
**Fix applied (minimal, behaviour-preserving):**
|
||||
|
||||
- `components/case/representation/representationElements.js`
|
||||
- In `RenderCondtionalRadioList`, made “Yes selected” detection bilingual-safe and value-safe:
|
||||
- accepts canonical value (`"Yes"`), translated display value (`t("myrepresentations:questionnaire-yes")`), and boolean `true`.
|
||||
- Replaced unconditional conditional-field validator with context-aware validator:
|
||||
- `requiredIfYes` only enforces required when “Yes” is selected.
|
||||
- Updated conditional rendering check to use the same normalized `isYesSelected` guard.
|
||||
|
||||
**Validation evidence (follow-up patch 2):**
|
||||
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` run completed with **6 passed / 1 failed**.
|
||||
- Failure: `[chromium] Raise representation as an Appellant` timed out waiting for `dd a.govuk-link--no-underline` (entry-link locator timeout), not in Welsh IP conditional details path.
|
||||
|
||||
---
|
||||
|
||||
### Post-R7 follow-up patch 3 — Welsh conditional required check tied to current radio value
|
||||
|
||||
**Issue observed during manual check:**
|
||||
|
||||
- In Welsh journey, selecting “Yes” could still leave the conditional details field blocked with required validation (`Yn ofynnol`) after entering text, preventing progress.
|
||||
|
||||
**Fix applied (minimal, behaviour-preserving):**
|
||||
|
||||
- `components/case/representation/representationElements.js`
|
||||
- Added explicit `normalizeYesNo(...)` mapping for both selected value and option values:
|
||||
- canonical/translated/boolean values normalize to `"Yes"` / `"No"`.
|
||||
- Radio `Field` values now use normalized canonical values (preserves existing payload expectation).
|
||||
- Updated conditional `requiredIfYes` validator to evaluate against the **current form value** (`allValues[id]`) rather than closure-only value.
|
||||
- Required check now trims whitespace and only errors when selected value resolves to `"Yes"` and details are empty.
|
||||
|
||||
**Validation evidence (follow-up patch 3):**
|
||||
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` intentionally **not run** per user instruction while manual Welsh verification is in progress.
|
||||
- `npm run lint` passed (warnings only)
|
||||
- `npm run test:reps` passed (**7 passed**)
|
||||
- manual APP/IP/Agent/LPA + EN/CY checks passed
|
||||
|
||||
---
|
||||
|
||||
### Slice R8 — Submission / Finalisation Boundary Isolation
|
||||
|
||||
**Goal:**
|
||||
Isolate submission logic.
|
||||
|
||||
**Scope:**
|
||||
|
||||
- payload shaping
|
||||
- submit calls
|
||||
- completion handling
|
||||
|
||||
**Risk Level:** HIGH
|
||||
(Must only be done after earlier slices stabilised)
|
||||
|
||||
**Completion notes (this slice):**
|
||||
|
||||
- Implemented on feature branch created from `refactor`: `rep-slice-r8-finalisation-boundary`.
|
||||
- Added a small representation-specific helper module:
|
||||
- Added helper module:
|
||||
- `components/case/representation/utils/finalisationBoundary.js`
|
||||
- Extracted only approved R8 seams:
|
||||
- `buildRepresentationUpdateBody(...)` (pure shaping)
|
||||
- `buildSubmitEnrichedValues(...)` (pure shaping)
|
||||
- `runFinalisationSequence(...)` (ultra-thin orchestration wrapper)
|
||||
- Wired `components/case/representation/index.js` to use the new helpers while preserving parent-owned business branching.
|
||||
- Explicitly preserved R8 constraints:
|
||||
- no completion-side-effect extraction from `representationComplete.js`
|
||||
- no route/query changes
|
||||
- no payload contract changes
|
||||
- no validation logic changes
|
||||
- no file upload/PDF/email behaviour changes
|
||||
- no sequencing drift in finalisation success path
|
||||
- Extracted:
|
||||
- `buildRepresentationUpdateBody(...)`
|
||||
- `buildSubmitEnrichedValues(...)`
|
||||
- `runFinalisationSequence(...)`
|
||||
- Preserved:
|
||||
- parent-owned branching logic
|
||||
- completion-side effects in `representationComplete.js`
|
||||
- exact sequencing of finalisation operations
|
||||
- Validation evidence:
|
||||
- `npm run lint` passed (warnings only)
|
||||
- `npm run test:reps` passed (**7 passed**)
|
||||
- manual APP/IP/Agent/LPA + EN/CY checks passed
|
||||
|
||||
---
|
||||
|
||||
### Slice R9 — Summary Rollout (Batch 1)
|
||||
|
||||
**Goal:**
|
||||
Roll out proven summary rendering primitives across remaining low-risk summary-type files.
|
||||
|
||||
**Completion notes (this slice):**
|
||||
|
||||
- Applied `SummaryCard` and `SummaryRow` primitives to Batch 1 summary-type files.
|
||||
- Rollout executed incrementally:
|
||||
- one summary-type file per commit
|
||||
- consistent mechanical application of proven pattern
|
||||
- Changes limited strictly to presentational structure:
|
||||
- replaced card shell markup with `SummaryCard`
|
||||
- replaced row wrapper markup with `SummaryRow` where safe
|
||||
- Preserved across all updated files:
|
||||
- field visibility rules
|
||||
- row and card ordering
|
||||
- translation key selection
|
||||
- EN/CY branching behaviour
|
||||
- date source selection and formatting
|
||||
- linked-case logic
|
||||
- route/query behaviour
|
||||
- payloads and Redux state
|
||||
- Parent summary-type files retain all business logic and conditional rendering.
|
||||
- Shared components confirmed render-only.
|
||||
|
||||
**Validation evidence:**
|
||||
|
||||
- `npm run lint` completed (warnings only, no new errors).
|
||||
- `npm run test:reps` completed: **7 passed (4.4m)**.
|
||||
- `npm run lint` completed (warnings only, no new errors)
|
||||
- `npm run test:reps` passed (**7 passed**)
|
||||
- Structured validation confirmed:
|
||||
- translation/bilingual logic unchanged
|
||||
- date/lookup logic unchanged
|
||||
- conditional rendering preserved
|
||||
- Targeted manual spot-checks completed:
|
||||
- `pinswg_sipscase.js`
|
||||
- `pinswg_dnsid.js`
|
||||
- `pinswg_electricityactid.js`
|
||||
- `pinswg_harbourrevisionorderid.js`
|
||||
- No behavioural regressions identified
|
||||
|
||||
**Outcome:**
|
||||
|
||||
- Summary rendering layer simplified and standardised
|
||||
- Duplication reduced while preserving all behaviour
|
||||
- Safer and more maintainable structure
|
||||
|
||||
**Status: COMPLETE**
|
||||
|
||||
---
|
||||
|
||||
@@ -350,9 +150,9 @@ Isolate submission logic.
|
||||
|
||||
- Only work on ONE slice at a time
|
||||
- Do not combine slices
|
||||
- One branch per slice
|
||||
- One commit per slice
|
||||
- Update tracker after completion
|
||||
- One branch per slice (exception: controlled rollout batches)
|
||||
- One commit per slice (or per file in rollout batch)
|
||||
- Update tracker after slice completion
|
||||
- Do not proceed if regression risk exists
|
||||
|
||||
---
|
||||
|
||||
Reference in New Issue
Block a user