Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions docs/fixes/CORE-8002-pulsecheck-workflow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# CORE-8002: Pulse check popup displayed twice

## issue summary
In a moderated assessment flow (`feedback available` -> mark review as read -> continue), learners could receive the same pulse check popup twice:
1. first popup after feedback/read + review rating flow
2. second popup after navigating to Home

## root cause
`fastFeedbackOpening` was cleared too early in `FastFeedbackService.pullFastFeedback()`.

- the lock was set before opening the modal
- it was reset in `finalize()` right after modal creation/enqueue (not after modal dismissal)
- during this gap, another pulse check check (for example Home `updateDashboard()`) could run and queue/open the same pulse check again

## fix applied
Updated `projects/v3/src/app/services/fast-feedback.service.ts`:
- moved `retry` before `switchMap` so retries only re-run the API fetch, never the modal open
- replaced `catchError` with `finalize` on the modal observable so the lock is released on success, error, and unsubscribe

### operator ordering change

```
before (retry after switchMap — retried modal opens):
fetch() → switchMap(modal()) → retry(3)

after (retry before switchMap — only retries fetch):
fetch() → retry(3) → switchMap(modal())
```

`retry` re-subscribes to everything **above** it in the pipe. placing it before `switchMap` means a fetch error retries the fetch, but a modal error passes through without re-triggering the entire pipeline.

## lock state diagram

the `fastFeedbackOpening` storage flag acts as a mutex to prevent concurrent modal opens:

```
pullFastFeedback() called
┌───────────────┐
│ lock = true? │──── YES ──→ skip modal, return of(res)
└───────┬───────┘
│ NO
set lock = TRUE ← acquire
open modal popup
┌───────┴────────┐
│ │
success error/dismiss
│ │
▼ ▼
finalize() finalize() ← both paths release
│ │
▼ ▼
set lock = FALSE ← release
```

`finalize` runs when the observable completes, errors, or is unsubscribed — covering all exit paths:
- user submits pulse check → observable completes → lock released
- modal throws error → observable errors → lock released
- user navigates away (component destroyed) → subscription unsubscribed → lock released

## why this is safe
- preserves existing behavior for successful pulse check display
- prevents duplicate modal opens from concurrent trigger paths
- `finalize` guarantees lock release on every exit path — no deadlock possible

## reference trigger paths observed
- review rating close path may request pulse check (`ReviewRatingComponent.fastFeedbackOrRedirect`)
- Home dashboard load also requests pulse check (`HomePage.updateDashboard`)
- assessment submission path (`AssessmentService._afterSubmit`)
- review submission path (`ReviewDesktopPage`)
- manual user click on traffic light (`TrafficLightGroupComponent.navigateToPulseCheck`)

the first two can fire back-to-back in the reported reproduce steps.

## verification checklist
- reproduce original scenario with moderated assessment feedback-read flow
- confirm only one pulse check popup is shown
- confirm pulse check can still be shown later when a genuinely new pulse check is available
8 changes: 4 additions & 4 deletions projects/v3/src/app/services/fast-feedback.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ export class FastFeedbackService {
closable: false,
}): Observable<any> {
return this._getFastFeedback(options.skipChecking, options.type).pipe(
retry({
count: 3,
delay: 1000
}),
switchMap((res) => {
try {
// don't open it again if there's one opening
Expand Down Expand Up @@ -168,10 +172,6 @@ export class FastFeedbackService {
});
}
}),
retry({
count: 3,
delay: 1000
}),
);
}

Expand Down