[CORE-8140] 2.4.y.z/missing-pulsecheck-trunk

docs/fixes/dead-code-audit-notificationsservice.md

@@ -0,0 +1,145 @@
+# Dead Code Audit: NotificationsService and PopUpComponent
+
+## Date: February 4, 2026
+
+## Scope
+Audit of unused modal methods and modal types in:
+- `projects/v3/src/app/services/notifications.service.ts`
+- `projects/v3/src/app/components/pop-up/pop-up.component.html`
+
+## Findings
+
+### Removed in This Session
+
+#### 1. `trackInfo()` Method ✅ REMOVED
+- **Location**: `notifications.service.ts` (lines 1055-1064)
+- **Purpose**: Display pulse check status information modal
+- **Status**: **UNUSED** - superseded by home page local implementation
+- **Evidence**:
+  - Home page uses local `onTrackInfo()` method instead
+  - No references found in codebase (except deleted method)
+  - Not in documentation or API contracts
+- **Action**: Removed
+
+#### 2. `pulseCheckStatus` Modal Type ✅ REMOVED
+- **Location**: `pop-up.component.html` (lines 48-59)
+- **Purpose**: Template case for pulse check status info
+- **Status**: **UNUSED** - only referenced by deleted `trackInfo()` method
+- **Evidence**:
+  - Only caller was `trackInfo()` method
+  - Home page traffic light display uses different binding (`pulseCheckStatus` data property, not modal type)
+  - No other references in codebase
+- **Action**: Removed
+
+### Active Modal Methods (All Used)
+
+#### Public Modal Creation Methods
+1. **`popUp(type, data, redirect)`** ✅ USED
+   - Used by: auth-forgot-password, auth-registration, contact-number-form, home page
+   - Creates PopUpComponent modals with types: `forgotPasswordConfirmation`, `shortMessage`, `guidelines`
+
+2. **`achievementPopUp(type, achievement, options)`** ✅ USED
+   - Used by: home page, shared service, notifications service (for achievement notifications)
+   - Creates AchievementPopUpComponent modals
+
+3. **`lockTeamAssessmentPopUp(data, event)`** ✅ USED
+   - Used by: assessment components when user clicks locked team assessment
+   - Creates LockTeamAssessmentPopUpComponent modals
+
+4. **`activityCompletePopUp(activityId, activityCompleted)`** ✅ USED
+   - Used by: activity service when activity is completed
+   - Creates ActivityCompletePopUpComponent modals
+
+5. **`popUpReviewRating(reviewId, redirect, hasReviewRating)`** ✅ USED
+   - Used by: activity-desktop, assessment-mobile, devtool pages
+   - Creates ReviewRatingComponent modals
+
+6. **`fastFeedbackModal(props, options)`** ✅ USED
+   - Used by: fast-feedback service for pulse check
+   - Creates FastFeedbackComponent modals
+
+#### Supporting Methods
+7. **`modal(component, componentProps, options, event)`** ✅ USED
+   - Internal wrapper around modalOnly
+   - Used by all modal creation methods
+
+8. **`modalOnly(component, componentProps, options, event)`** ✅ USED
+   - Core modal creation with queue management
+   - Uses ModalService to handle modal queue
+
+#### Utility Methods
+9. **`alert(config)`** ✅ USED
+   - Used throughout app for alert dialogs
+   - AlertController wrapper
+
+10. **`presentToast(message, options)`** ✅ USED
+    - Used throughout app for toast notifications
+    - ToastController wrapper
+
+11. **`loading(opts)`** ✅ USED
+    - Used for loading spinners
+    - LoadingController wrapper
+
+12. **`dismiss()`** ✅ USED
+    - Used to dismiss modals
+    - ModalController wrapper
+
+### Active PopUpComponent Modal Types (All Used)
+
+1. **`forgotPasswordConfirmation`** ✅ USED
+   - Used by: auth-forgot-password component
+   - Shows email confirmation message
+
+2. **`shortMessage`** ✅ USED
+   - Used by: contact-number-form, auth-registration, home page
+   - Generic short message display
+
+3. **`guidelines`** ✅ USED
+   - Used by: home page
+   - Shows message with clickable route links
+
+## Audit Summary
+
+- **Total methods audited**: 12 public methods + 2 internal helpers
+- **Unused methods found**: 1 (`trackInfo()`)
+- **Unused modal types found**: 1 (`pulseCheckStatus`)
+- **All other methods**: Actively used and verified
+
+## Verification Method
+
+1. Grep search for method names across entire codebase
+2. Check documentation files for references
+3. Verify template usage in PopUpComponent
+4. Cross-reference with home page implementation
+
+## Recommendations
+
+### Immediate Actions ✅ COMPLETED
+- Remove `trackInfo()` method from notifications.service.ts
+- Remove `pulseCheckStatus` case from pop-up.component.html
+- Update notifications.service.spec.ts to remove any tests for `trackInfo()`
+
+### Future Considerations
+
+1. **No additional dead code found**: All other modal methods are actively used
+2. **Modal queue pattern is healthy**: ModalService properly manages modal display queue
+3. **PopUpComponent types are lean**: Only 3 modal types remain, all in active use
+
+## Testing Impact
+
+### Test Files Affected
+- `projects/v3/src/app/services/notifications.service.spec.ts`
+  - Remove any tests for `trackInfo()` method if present
+
+### No Tests Expected For
+- `pulseCheckStatus` modal type (template-only change)
+
+## Related Documentation
+
+See [review-rating-duplicate-api-fix.md](./review-rating-duplicate-api-fix.md) for context on why this audit was performed.
+
+## Notes
+
+- Home page's local `onTrackInfo()` implementation uses AlertController directly instead of NotificationsService
+- This is a better pattern as it keeps the UI logic local to the page component
+- No breaking changes expected from removals as code was genuinely unused

docs/fixes/review-rating-duplicate-api-fix.md

@@ -0,0 +1,167 @@
+# Review Rating Duplicate API Call Fix
+
+## Issue Summary
+
+When the review rating modal is displayed after feedback reading, the pulse check API (`pullFastFeedback()`) was being called twice:
+1. Once by the ReviewRatingComponent when the modal is dismissed
+2. Once by the page component (activity-desktop/assessment-mobile) after `reviewRatingPopUp()` returns
+
+This resulted in unnecessary duplicate GraphQL API calls, impacting performance and potentially causing race conditions.
+
+## Root Cause
+
+The original implementation added an explicit `pullFastFeedback()` call in the page components to fix the case where the review rating modal was skipped (when `assessment.hasReviewRating === false`). However, when the modal IS shown, the ReviewRatingComponent's `dismissModal()` method already calls `pullFastFeedback()` through `fastFeedbackOrRedirect()`, resulting in the duplicate call.
+
+## Solution
+
+### Approach
+
+**Conditional pulse check execution**: Only call `pullFastFeedback()` from page components when the review rating modal was actually skipped.
+
+- **When modal is skipped** (user/assessment has review rating disabled): Page component calls `pullFastFeedback()`
+- **When modal is shown**: ReviewRatingComponent handles `pullFastFeedback()` on dismiss (no longer - see architectural change below)
+
+### Architectural Change
+
+To follow separation of concerns and match the pattern used in assessment submissions:
+- **Removed** pulse check responsibility from ReviewRatingComponent
+- **Centralized** pulse check calls in page components (activity-desktop, assessment-mobile)
+- ReviewRatingComponent now only handles redirect navigation
+
+## Implementation Flow
+
+### Before Fix
+
+```mermaid
+sequenceDiagram
+    participant User
+    participant Page as Activity/Assessment Page
+    participant Modal as ReviewRatingComponent
+    participant API as FastFeedbackService
+
+    alt Review Rating Enabled
+        User->>Page: Read feedback
+        Page->>Modal: reviewRatingPopUp()
+        Modal->>User: Show modal
+        User->>Modal: Dismiss
+        Modal->>API: pullFastFeedback() ❌ Call #1
+        Modal->>Page: Return
+        Page->>API: pullFastFeedback() ❌ Call #2 (DUPLICATE)
+    else Review Rating Disabled
+        User->>Page: Read feedback
+        Page->>Page: reviewRatingPopUp() returns early
+        Page->>API: pullFastFeedback() ✅ Call #1
+    end
+```
+
+### After Fix
+
+```mermaid
+sequenceDiagram
+    participant User
+    participant Page as Activity/Assessment Page
+    participant Modal as ReviewRatingComponent
+    participant API as FastFeedbackService
+
+    alt Review Rating Enabled
+        User->>Page: Read feedback
+        Page->>Modal: reviewRatingPopUp()
+        Modal->>User: Show modal
+        User->>Modal: Dismiss (rate or skip)
+        Modal->>Page: Return (redirect only if needed)
+        Note over Page: Check: was modal skipped?
+        Page->>Page: hasReviewRating is TRUE
+        Page->>API: pullFastFeedback() ✅ Call #1 (no duplicate)
+    else Review Rating Disabled
+        User->>Page: Read feedback
+        Page->>Page: reviewRatingPopUp() returns early
+        Note over Page: Check: was modal skipped?
+        Page->>Page: hasReviewRating is FALSE
+        Page->>API: pullFastFeedback() ✅ Call #1
+    end
+```
+
+## Files Modified
+
+### Code Changes
+
+1. **projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts**
+   - Added conditional check in `readFeedback()` method
+   - Only calls `pullFastFeedback()` when `hasReviewRating === false`
+
+2. **projects/v3/src/app/pages/assessment-mobile/assessment-mobile.page.ts**
+   - Applied identical conditional logic in `readFeedback()` method
+   - Ensures mobile and desktop behavior is consistent
+
+3. **projects/v3/src/app/components/review-rating/review-rating.component.ts**
+   - Refactored `fastFeedbackOrRedirect()` method
+   - Removed pulse check call when `!redirect`
+   - Now only handles redirect navigation
+
+### Dead Code Removal
+
+4. **projects/v3/src/app/services/notifications.service.ts**
+   - Removed unused `trackInfo()` method (lines 1055-1064)
+   - Method was superseded by home page's local `onTrackInfo()` implementation
+
+5. **projects/v3/src/app/components/pop-up/pop-up.component.html**
+   - Removed unused `pulseCheckStatus` modal case (lines 48-59)
+   - Only referenced by deleted `trackInfo()` method
+
+## Testing Approach
+
+### Manual Testing Scenarios
+
+1. **Review rating enabled (user level + assessment level)**
+   - Read feedback → Review rating modal shows → Dismiss
+   - Verify pulse check modal appears once
+   - Check network tab for single `pullFastFeedback` GraphQL call
+
+2. **Review rating disabled (user level)**
+   - Read feedback → No review rating modal
+   - Verify pulse check modal appears immediately
+   - Check network tab for single `pullFastFeedback` GraphQL call
+
+3. **Review rating disabled (assessment level)**
+   - Read feedback → No review rating modal
+   - Verify pulse check modal appears immediately
+   - Check network tab for single `pullFastFeedback` GraphQL call
+
+4. **Review rating with redirect**
+   - Complete review rating → Submit with redirect
+   - Verify navigation to next task works correctly
+   - Verify pulse check appears before redirect
+
+### Test Files to Update
+
+- `projects/v3/src/app/pages/activity-desktop/activity-desktop.page.spec.ts`
+- `projects/v3/src/app/pages/assessment-mobile/assessment-mobile.page.spec.ts`
+- `projects/v3/src/app/components/review-rating/review-rating.component.spec.ts`
+- `projects/v3/src/app/services/notifications.service.spec.ts`
+
+## Performance Impact
+
+**Before**: 2 GraphQL API calls per review rating flow (when modal shown)
+**After**: 1 GraphQL API call per review rating flow
+
+**Estimated savings**: 50% reduction in pulse check API calls for review rating flows where modal is displayed
+
+## Edge Cases Handled
+
+1. **User-level review rating disabled**: `storageService.getUser().hasReviewRating === false`
+2. **Assessment-level review rating disabled**: `assessmentService.assessment?.hasReviewRating === false`
+3. **Redirect scenarios**: Review rating component still handles redirect navigation correctly
+4. **No redirect scenarios**: Pulse check now handled by page components
+
+## Related Changes
+
+- Initial implementation: Added `hasReviewRating` parameter to `popUpReviewRating()` method
+- Bug fix: Added explicit `pullFastFeedback()` calls when modal skipped
+- Optimization: This fix to prevent duplicate calls when modal shown
+- Cleanup: Removed unused `trackInfo()` method and `pulseCheckStatus` modal type
+
+## Future Considerations
+
+- Consider returning modal reference from `popUpReviewRating()` if more modal state detection is needed
+- Monitor for similar duplicate API call patterns in other modal workflows
+- Consider extracting modal lifecycle detection into a reusable pattern/service

projects/v3/src/app/components/pop-up/pop-up.component.html

@@ -45,19 +45,6 @@
         </div>
         </ng-container>
 
-      <!-- Pulsecheck Status Information -->
-      <ng-container *ngSwitchCase="'pulseCheckStatus'">
-        <div class="div-after-logo">
-          <p class="status-info">
-            This shows how you, your team, and reviewers of your team's work feel about whether your project is on track.
-          </p>
-          <p class="status-info">
-            <span class="green">Green</span> is on track, <span class="red">red</span> is potentially off track. <span
-              class="yellow">Yellow</span> means mixed views.
-          </p>
-        </div>
-        </ng-container>
-
     </ng-container>
 
     <div class="div-after-logo">

projects/v3/src/app/components/review-rating/review-rating.component.spec.ts

@@ -104,7 +104,7 @@ describe('ReviewRatingComponent', () => {
   });
 
   describe('submitReviewRating() - straightforward test', () => {
-    it('should trigger pulse check API when stay on same view', () => {
+    it('should NOT trigger pulse check API (handled by page components)', () => {
       component.redirect = null;
 
       component.ratingData = {
@@ -122,7 +122,8 @@ describe('ReviewRatingComponent', () => {
       expect(serviceSpy.submitRating.calls.first().args[0].rating).toEqual(0.12);
       expect(component.isSubmitting).toBe(false);
       expect(routerSpy.navigate.calls.count()).toBe(0);
-      expect(fastfeedbackSpy.pullFastFeedback).toHaveBeenCalledTimes(1);
+      // pulse check is now handled by page components, not ReviewRatingComponent
+      expect(fastfeedbackSpy.pullFastFeedback).not.toHaveBeenCalled();
     });
   });
 

projects/v3/src/app/components/review-rating/review-rating.component.ts

@@ -107,9 +107,10 @@ export class ReviewRatingComponent implements OnInit {
   }
 
   private async fastFeedbackOrRedirect(): Promise<any> {
-    // if this.redirect == false, don't redirect to another page
-    if (!this.redirect) {
-      return await this.fastFeedbackService.pullFastFeedback().toPromise();
+    // pulse check is now handled by page components (activity-desktop, assessment-mobile)
+    // this method only handles redirect navigation when redirect is provided
+    if (!this.redirect || typeof this.redirect === 'boolean') {
+      return;
     }
 
     if (!this.utils.isMobile()) {
@@ -153,6 +154,6 @@ export class ReviewRatingComponent implements OnInit {
 
   async dismissModal(): Promise<void> {
     await this.modalController.dismiss(null, 'cancel', `review-popup-${this.reviewId}`);
-    await this.fastFeedbackOrRedirect();
+    await this.fastFeedbackOrRedirect(); // check if fast feedback available after review rating modal is closed
   }
 }