Skip to content

Commit 8a3b7fe

Browse files
trtshentrtshen
authored
Merge pull request #2637 from intersective/2.4.8/CORE-8166/review-missing-local-answer
[CORE-8166] local and remote answer display for review
2 parents 998823a + 59b5b85 commit 8a3b7fe

13 files changed

Lines changed: 463 additions & 64 deletions

‎docs/fixes/CORE-8166-8167-pagination-answer-persistence.md‎

Lines changed: 286 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,286 @@
1+
# CORE-8166 / CORE-8167 — Pagination Answer Persistence Fix
2+
3+
> **Branch:** `2.4.8/CORE-8166/review-missing-local-answer` and `2.4.8/CORE-8167/standalone-pagination-completed-review-trunk`
4+
> **PR:** [#2636](https://github.com/intersective/app/pull/2636)
5+
> **Date:** March 2026
6+
7+
---
8+
9+
## Problem Summary
10+
11+
When an assessment uses **pagination** (more than 8 questions, split across pages), navigating between pages causes previously entered answers to be **cleared from the UI**. The user's selections (checkboxes, radio buttons, text, etc.) disappear when they leave a page and return to it.
12+
13+
This affects both:
14+
- **Reviewer mode** (`doReview`) — reviewer's in-progress answers lost on page navigation
15+
- **Assessment mode** (`doAssessment`) — learner's in-progress answers lost on page navigation (specific to `multi-team-member-selector`)
16+
17+
---
18+
19+
## Root Cause Analysis
20+
21+
### How Pagination Works
22+
23+
Pagination splits assessment groups into pages via `pagesGroups` array. The template renders only the current page's questions using:
24+
25+
```html
26+
<ng-container *ngFor="let group of pagedGroups">
27+
```
28+
29+
Where `pagedGroups` returns `this.pagesGroups[this.pageIndex]`.
30+
31+
**When the user navigates to a different page, Angular destroys the question components on the current page and creates new ones for the target page.** When navigating back, fresh component instances are created — they run `ngOnInit()` and `_showSavedAnswers()` again.
32+
33+
The `FormGroup` (`questionsForm`) **persists** across page changes — only the visual components are destroyed/recreated. Form controls retain their values.
34+
35+
### The ControlValueAccessor Lifecycle
36+
37+
Each question component implements `ControlValueAccessor`. The lifecycle on component creation is:
38+
39+
1. Component `ngOnInit()` runs → calls `_showSavedAnswers()`
40+
2. `FormControlName` directive's `ngOnInit()` runs → calls `writeValue()` with the current form control value
41+
3. `registerOnChange()` is called — `propagateChange()` becomes functional
42+
43+
This means `propagateChange()` is a **no-op** during step 1, and `writeValue()` in step 2 is the authoritative source of the form control's current value.
44+
45+
### Three Distinct Bugs
46+
47+
#### Bug 1: `_showSavedAnswers()` Overwrites Dirty Form Controls
48+
49+
**Affected modes:** `doReview`, `doAssessment`
50+
**Affected types:** all question types
51+
52+
When a component is recreated on pagination return, `_showSavedAnswers()` in `ngOnInit()` unconditionally read from `@Input` data (e.g., `this.review.answer`, `this.submission.answer`) — which is the **original API data**, not the user's edits. This overwrites `innerValue` with stale data.
53+
54+
**Fix:** check `control.pristine` before deciding the data source:
55+
- If `control.pristine` → use API data (no local edits exist)
56+
- If `!control.pristine` (dirty) → use `control.value` (preserves local edits)
57+
58+
```typescript
59+
// example from oneof.component.ts
60+
if (this.control && !this.control.pristine) {
61+
this.innerValue = this.control.value;
62+
this.comment = this.control.value?.comment ?? this.review.comment;
63+
} else {
64+
this.innerValue = {
65+
answer: this.review.answer,
66+
comment: this.review.comment,
67+
};
68+
this.comment = this.review.comment;
69+
}
70+
```
71+
72+
#### Bug 2: Template Bindings Read From Stale `@Input` Data
73+
74+
**Affected modes:** `doReview`
75+
**Affected types:** `multiple`, `oneof`, `team-member-selector`, `multi-team-member-selector`
76+
77+
Even after fixing `_showSavedAnswers()`, templates in review mode were binding directly to `@Input` properties (e.g., `review.answer`) instead of the local `innerValue`. So checkbox `[checked]` and radio `[value]` bindings showed the original API answers, not the user's edits.
78+
79+
**Fix:** change all review-mode template bindings to use `innerValue`:
80+
81+
| Component | Before | After |
82+
|---|---|---|
83+
| `multiple` | `review.answer.includes(choice.id)` | `innerValue?.answer?.includes(choice.id)` |
84+
| `oneof` | `review.answer` | `innerValue?.answer` |
85+
| `team-member-selector` | `review?.answer` | `innerValue?.answer` |
86+
| `multi-team-member-selector` | `isSelectedInReview(teamMember)` | `isSelected(teamMember)` |
87+
88+
#### Bug 3: Array Type Initialization Mismatch
89+
90+
**Affected modes:** `doReview` (multiple, multi-team-member-selector), `doAssessment` (multi-team-member-selector)
91+
**Affected types:** `multiple`, `multi-team-member-selector`
92+
93+
For checkbox-based question types, the form control was initialized with `answer: ''` (empty string) instead of `answer: []` (empty array). When `writeValue()` populated `innerValue` with this string value, subsequent calls to `addOrRemove()` crashed with `TypeError: arrayInput.push is not a function` because an empty string is not an array. This silent error prevented `propagateChange()` from executing, so the form control stayed pristine with no user edits actually saved.
94+
95+
**Fix (assessment.component.ts `_populateQuestionsForm()`):**
96+
```typescript
97+
if (this.action === 'review') {
98+
const arrayTypes = ['multiple', 'multi team member selector'];
99+
quesCtrl = {
100+
comment: '',
101+
answer: arrayTypes.includes(question.type) ? [] : '',
102+
file: null
103+
};
104+
} else {
105+
// assessment mode: multi-team-member-selector uses a plain array
106+
if (question.type === 'multi team member selector') {
107+
quesCtrl = [];
108+
}
109+
}
110+
```
111+
112+
**Fix (component-level guards):** added array coercion in `writeValue()`, `onChange()`, and `_showSavedAnswers()` for both `multiple` and `multi-team-member-selector`:
113+
```typescript
114+
// writeValue guard
115+
if (this.doReview && this.innerValue && !Array.isArray(this.innerValue.answer)) {
116+
this.innerValue = { ...this.innerValue, answer: [] };
117+
}
118+
119+
// onChange guard
120+
if (!Array.isArray(this.innerValue.answer)) {
121+
this.innerValue.answer = [];
122+
}
123+
```
124+
125+
---
126+
127+
## Why `doAssessment` Mode Was Also Affected
128+
129+
The `multi-team-member-selector` component was specifically affected in assessment mode because of a **data shape mismatch**:
130+
131+
- In **assessment mode**, the component treats `innerValue` as a **plain array** (e.g., `['key1', 'key2']`). Methods like `onChange()`, `isSelected()`, and `triggerSave()` all operate on `innerValue` directly as an array.
132+
- However, `_populateQuestionsForm()` initialized the form control with `null` (the default for all assessment-mode controls).
133+
- When `writeValue(null)` was called, the null check `if (value)` skipped setting `innerValue`, leaving it undefined.
134+
- On first checkbox click, `onChange()` called `this.utils.addOrRemove(this.innerValue, value)` — which crashed because `this.innerValue` was not an array.
135+
136+
**Fix:** initialize `multi team member selector` with `[]` in assessment mode:
137+
```typescript
138+
if (question.type === 'multi team member selector') {
139+
quesCtrl = [];
140+
}
141+
```
142+
143+
Plus a defensive guard in `writeValue()`:
144+
```typescript
145+
if (this.doAssessment && !Array.isArray(this.innerValue)) {
146+
this.innerValue = Array.isArray(this.innerValue?.answer) ? this.innerValue.answer : [];
147+
}
148+
```
149+
150+
Other question types in assessment mode were **not affected** because:
151+
- `multiple` already had a null guard: `if (!this.innerValue) { this.innerValue = []; }`
152+
- Scalar types (`oneof`, `text`, `slider`, `team-member-selector`) use direct assignment, not array operations
153+
- `file-upload` uses `fileRequestFormat()` which safely returns `{}` for null
154+
155+
---
156+
157+
## Question Types Affected
158+
159+
| Question Type | Component | Review Mode Fix | Assessment Mode Fix |
160+
|---|---|---|---|
161+
| Radio (single choice) | `app-oneof` | pristine check + template binding | — |
162+
| Checkbox (multiple choice) | `app-multiple` | pristine check + template binding + array init | — |
163+
| Text / Textarea | `app-text` | pristine check | — |
164+
| Slider | `app-slider` | pristine check | — |
165+
| File Upload | `app-file-upload` | pristine check | — |
166+
| Team Member (single) | `app-team-member-selector` | pristine check + template binding | — |
167+
| Team Member (multi) | `app-multi-team-member-selector` | pristine check + template binding + array init | array init + writeValue guard |
168+
169+
---
170+
171+
## Files Changed
172+
173+
### Parent Component
174+
- **assessment.component.ts** — `_populateQuestionsForm()`: proper initial values for array-type controls in both review and assessment modes; consolidated `_prefillForm()` method
175+
176+
### Question Components (TypeScript)
177+
- **multiple.component.ts** — `_showSavedAnswers()`, `writeValue()`, `onChange()`: pristine check, array coercion
178+
- **oneof.component.ts** — `_showSavedAnswers()`, `writeValue()`: pristine check, comment restoration
179+
- **text.component.ts** — `_showSavedAnswers()`, `writeValue()`: pristine check, object-vs-string handling
180+
- **slider.component.ts** — `_showSavedAnswers()`, `writeValue()`: pristine check, comment restoration
181+
- **team-member-selector.component.ts** — `_showSavedAnswers()`, `writeValue()`: pristine check, comment restoration
182+
- **multi-team-member-selector.component.ts** — `_showSavedAnswers()`, `writeValue()`, `onChange()`: pristine check, array coercion, assessment mode plain-array guard
183+
- **file-upload.component.ts** — `_showSavedAnswers()`: pristine check
184+
185+
### Question Components (Templates)
186+
- **multiple.component.html** — `[checked]` binding: `review.answer.includes()` → `innerValue?.answer?.includes()`; `onLabelToggle` passes `'answer'` type in review mode
187+
- **oneof.component.html** — `[value]` binding: `review.answer` → `innerValue?.answer`
188+
- **team-member-selector.component.html** — `[value]` binding: `review?.answer` → `innerValue?.answer`
189+
- **multi-team-member-selector.component.html** — `[checked]` binding: `isSelectedInReview()` → `isSelected()` in doReview section
190+
191+
---
192+
193+
## The `control.pristine` Pattern
194+
195+
All question components now follow a consistent pattern in `_showSavedAnswers()`:
196+
197+
```
198+
┌─────────────────────────────────────────────────┐
199+
│ Component recreated on pagination return │
200+
│ │
201+
│ ngOnInit() → _showSavedAnswers() │
202+
│ │
203+
│ Is control.pristine? │
204+
│ YES → Use @Input data (API/original) │
205+
│ NO → Use control.value (user's local edits) │
206+
│ │
207+
│ writeValue() called by FormControlName │
208+
│ → Sets innerValue from form control value │
209+
│ → Template binds to innerValue (not @Input) │
210+
└─────────────────────────────────────────────────┘
211+
```
212+
213+
**Why `pristine` works as the discriminator:**
214+
- `_prefillForm()` calls `control.setValue(value, { emitEvent: false })` — this does NOT mark the control as dirty (it stays pristine)
215+
- User interactions call `propagateChange()` → which DOES mark the control as dirty
216+
- So `pristine = true` means "only API data, no user edits" and `pristine = false` means "user has made changes"
217+
218+
**Wait — `setValue()` does mark the control as dirty in some Angular versions.** Actually, `setValue()` with `{ emitEvent: false }` still changes the pristine state to false. The key insight is:
219+
- On first load, `_prefillForm()` sets the value → `pristine = false`
220+
- `_showSavedAnswers()` reads `control.value` which already has the prefilled value
221+
- So either path (pristine or not) produces the correct result on first load
222+
- On pagination return (component recreated), the form control still has the user's edits from `propagateChange()`, and `control.pristine = false`, so `_showSavedAnswers()` correctly reads from `control.value`
223+
224+
---
225+
226+
## Data Shape Reference
227+
228+
### Review Mode
229+
Form control value is always an **object**:
230+
```typescript
231+
{ answer: any, comment: string, file?: any }
232+
```
233+
- `answer` is `[]` for checkbox types, `''` or scalar for others
234+
- Components access `innerValue.answer` and `innerValue.comment` separately
235+
236+
### Assessment Mode
237+
Form control value varies by type:
238+
```typescript
239+
// oneof, team-member-selector: scalar (string/number)
240+
'choice-id' or 5
241+
242+
// multiple: array
243+
[1, 3, 5]
244+
245+
// multi-team-member-selector: array of JSON strings
246+
['{"userId":1,"name":"..."}', '{"userId":2,"name":"..."}']
247+
248+
// text: string
249+
'answer text'
250+
251+
// slider: number
252+
75
253+
254+
// file-upload: FileInput object
255+
{ url: '...', name: 'file.pdf', ... }
256+
```
257+
258+
---
259+
260+
## Three Selection Check Functions in `multi-team-member-selector`
261+
262+
| Function | Data Source | Purpose | Used In Template Sections |
263+
|---|---|---|---|
264+
| `isSelected()` | `this.innerValue` (local state) | current working state including unsaved edits | `doAssessment`, `doReview` — checkbox `[checked]` binding |
265+
| `isSelectedInSubmission()` | `this.submission.answer` (@Input, API data) | learner's original submission | `doReview`, `isDisplayOnly` — "Learner's answer" badge |
266+
| `isSelectedInReview()` | `this.review.answer` (@Input, API data) | reviewer's original review | `isDisplayOnly` — "Expert's answer" badge |
267+
268+
`isSelected()` is used for checkbox bindings in **both** `doAssessment` and `doReview` because it reads from `innerValue` which preserves user edits across pagination. The other two only display static badges from API data.
269+
270+
---
271+
272+
## Testing Verification
273+
274+
Verified via browser screenshots on `localhost:4200`:
275+
276+
### Review Mode — "150 Questions" assessment
277+
- Text field: "Persist test!!!" persisted across page 1 → page 4 → page 1
278+
- Radio (oneof): 2nd choice selection persisted
279+
- Checkbox (multiple): unchecked 1st checkbox stayed unchecked after pagination
280+
- No console errors (previous `TypeError: arrayInput.push` resolved)
281+
282+
### Assessment Mode — "1 group of 9 questions" assessment
283+
- Multi-team-member-selector: selected `learner_reg_091` and `learner 004`, navigated page 1 → page 2 → page 1, both selections persisted
284+
285+
### Review Mode — "1 group of 10 questions" assessment
286+
- Radio selections persisted across pagination

‎projects/v3/src/app/components/assessment/assessment.component.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -530,15 +530,18 @@ Best regards`;
530530
let quesCtrl: { answer: any; comment?: string; file?: any } | any = null;
531531

532532
if (this.action === 'review') {
533+
// use array initial value for checkbox-based question types
534+
const arrayTypes = ['multiple', 'multi team member selector'];
533535
quesCtrl = {
534536
comment: '',
535-
answer: question.type === 'multi team member selector' ? [] : '',
537+
answer: arrayTypes.includes(question.type) ? [] : '',
536538
file: null
537539
};
538540
} else {
539-
// For assessment mode, initialize multi team member selector with proper structure
541+
// for assessment mode, multi-team-member-selector uses a plain array
542+
// (not an object) because onChange/isSelected/triggerSave treat innerValue as an array
540543
if (question.type === 'multi team member selector') {
541-
quesCtrl = { answer: [] };
544+
quesCtrl = [];
542545
}
543546
}
544547

‎projects/v3/src/app/components/file-upload/file-upload.component.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -274,17 +274,26 @@ export class FileUploadComponent implements OnInit, OnDestroy {
274274
// adding save values to from control
275275
private _showSavedAnswers() {
276276
if ((['in progress', 'not start'].includes(this.reviewStatus)) && this.doReview && this.review) {
277-
this.innerValue = {
278-
answer: {},
279-
comment: ''
280-
};
281-
this.innerValue.comment = this.review.comment;
282-
this.comment = this.review.comment;
283-
this.innerValue.answer = this.review.answer;
284-
this.innerValue.file = this.review.file;
277+
// when the control has been modified (e.g. user edited during pagination),
278+
// preserve their edits; otherwise use the saved review data
279+
if (this.control && !this.control.pristine) {
280+
this.innerValue = this.control.value;
281+
this.comment = this.control.value?.comment ?? this.review.comment;
282+
} else {
283+
this.innerValue = {
284+
answer: this.review.answer,
285+
comment: this.review.comment,
286+
file: this.review.file,
287+
};
288+
this.comment = this.review.comment;
289+
}
285290
}
286291
if ((this.submissionStatus === 'in progress') && (this.doAssessment)) {
287-
this.innerValue = this.submission?.answer;
292+
if (this.control && !this.control.pristine) {
293+
this.innerValue = this.control.value;
294+
} else {
295+
this.innerValue = this.submission?.answer;
296+
}
288297
}
289298
if (this.control) {
290299
this.control.setValue(this.innerValue);

‎projects/v3/src/app/components/multi-team-member-selector/multi-team-member-selector.component.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ <h3 class="for-accessibility" [id]="'multi-team-member-selector-question-' + que
7979

8080
<ion-checkbox color="success"
8181
[attr.aria-label]="teamMember.userName"
82-
[checked]="isSelectedInReview(teamMember)"
82+
[checked]="isSelected(teamMember)"
8383
[value]="teamMember.key"
8484
justify="start"
8585
slot="start"

0 commit comments

Comments
 (0)