Refactor Notification

MARK_ALL_READ_CACHE_ISSUE.md

@@ -0,0 +1,140 @@
+# Mark All As Read - Cache Issue Analysis
+
+**Date**: 2026-01-01  
+**Issue**: After marking all as read, list is empty but count still shows 66
+
+---
+
+## 🔍 Problem Analysis
+
+### Current Flow
+
+1. **User clicks "Mark all as read"**
+2. **`markAllAsRead()` is called**
+3. **Fetches notifications**: `this.getNotifications(userId, 1, 1000)`
+   - ⚠️ **PROBLEM**: This goes through `NotificationService.getNotifications()`
+   - ⚠️ **PROBLEM**: Which uses **CACHED** data if available
+   - ⚠️ **PROBLEM**: Cached notifications still have `isRead: false`
+4. **Filters unread**: Gets 66 unread from cached data
+5. **Marks each as read**: Calls Leantime API for each
+6. **Invalidates cache**: After marking completes
+7. **Count is fetched**: But might use stale cache or be fetched before invalidation
+
+### The Issue
+
+**Cache Race Condition**:
+- `markAllAsRead` uses cached notifications (which are stale)
+- Marks them as read in Leantime
+- Invalidates cache
+- But count might be fetched from cache **before** invalidation completes
+- Or count cache might not be properly invalidated
+
+**Why List is Empty**:
+- After marking, all notifications are read
+- List might filter to show only unread
+- So list is empty (correct behavior)
+- But count still shows 66 (stale cache)
+
+---
+
+## 🔧 Root Causes
+
+### 1. Using Cached Data in `markAllAsRead`
+
+**Current Code**:
+```typescript
+// In markAllAsRead
+const allNotifications = await this.getNotifications(userId, 1, 1000);
+```
+
+**Problem**: `getNotifications()` uses cache, so we're working with stale data.
+
+**Solution**: Fetch directly from Leantime API, bypassing cache.
+
+---
+
+### 2. Cache Invalidation Timing
+
+**Current Flow**:
+1. Mark all as read (uses cached data)
+2. Invalidate cache
+3. Count is fetched (might use stale cache if fetched too soon)
+
+**Problem**: Race condition between invalidation and count fetch.
+
+**Solution**: 
+- Invalidate cache **before** marking (or fetch fresh data)
+- Force immediate count refresh after marking
+- Add delay before count fetch to ensure cache is cleared
+
+---
+
+### 3. Count Cache Not Properly Invalidated
+
+**Current Code**:
+```typescript
+if (success) {
+  await this.invalidateCache(userId);
+}
+```
+
+**Problem**: If `markAllAsRead` fails partially, cache might not be invalidated.
+
+**Solution**: Always invalidate cache, even on partial success.
+
+---
+
+## ✅ Recommended Fixes
+
+### Fix 1: Bypass Cache in `markAllAsRead`
+
+**Change**: Fetch notifications directly from Leantime API, not through cached service.
+
+**Implementation**:
+- Add a method to fetch notifications directly from adapter (bypassing cache)
+- Or add a `forceRefresh` parameter to `getNotifications`
+- Or fetch directly in `markAllAsRead` using Leantime API
+
+### Fix 2: Always Invalidate Cache
+
+**Change**: Invalidate cache even if some notifications fail to mark.
+
+**Implementation**:
+- Invalidate cache if **any** notifications were successfully marked
+- Not just if **all** succeeded
+
+### Fix 3: Force Fresh Count After Marking
+
+**Change**: After marking, force an immediate fresh count fetch.
+
+**Implementation**:
+- After `markAllAsRead` completes, immediately call `getNotificationCount()` with cache bypass
+- Or add a delay before count fetch to ensure cache is cleared
+
+---
+
+## 📊 Expected Behavior After Fixes
+
+### After Mark All As Read
+
+**Before**:
+- List: Empty (all read) ✅
+- Count: 66 (stale cache) ❌
+
+**After**:
+- List: Empty (all read) ✅
+- Count: 0 (fresh data) ✅
+
+---
+
+## 🎯 Next Steps
+
+1. **Fix cache usage in `markAllAsRead`**: Fetch fresh data, not cached
+2. **Improve cache invalidation**: Always invalidate, even on partial success
+3. **Force count refresh**: Immediately fetch fresh count after marking
+4. **Test**: Verify count updates correctly after marking
+
+---
+
+**Status**: Analysis complete. Ready to implement fixes.
+

lib/services/notifications/leantime-adapter.ts

@@ -266,14 +266,53 @@ export class LeantimeAdapter implements NotificationAdapter {
       console.log(`[LEANTIME_ADAPTER] markAllAsRead - Leantime user ID: ${leantimeUserId}`);
       
       // Leantime doesn't have a "mark all as read" method, so we need to:
-      // 1. Fetch all unread notifications
+      // 1. Fetch all unread notifications directly from API (bypassing any cache)
       // 2. Mark each one individually using markNotificationRead
       
-      console.log(`[LEANTIME_ADAPTER] markAllAsRead - Fetching all unread notifications`);
-      const allNotifications = await this.getNotifications(userId, 1, 1000); // Get up to 1000 notifications
-      const unreadNotifications = allNotifications.filter(n => !n.isRead);
+      console.log(`[LEANTIME_ADAPTER] markAllAsRead - Fetching all unread notifications directly from API`);
       
-      console.log(`[LEANTIME_ADAPTER] markAllAsRead - Found ${unreadNotifications.length} unread notifications to mark`);
+      // Fetch all notifications directly from API (up to 1000) to get fresh data (not cached)
+      const jsonRpcBody = {
+        jsonrpc: '2.0',
+        method: 'leantime.rpc.Notifications.Notifications.getAllNotifications',
+        params: {
+          userId: leantimeUserId,
+          showNewOnly: 0, // Get all, not just unread
+          limitStart: 0,
+          limitEnd: 1000,
+          filterOptions: []
+        },
+        id: 1
+      };
+      
+      console.log(`[LEANTIME_ADAPTER] markAllAsRead - Fetching notifications from API`);
+      const fetchResponse = await fetch(`${this.apiUrl}/api/jsonrpc`, {
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/json',
+          'X-API-Key': this.apiToken
+        },
+        body: JSON.stringify(jsonRpcBody)
+      });
+      
+      if (!fetchResponse.ok) {
+        console.error(`[LEANTIME_ADAPTER] markAllAsRead - Failed to fetch notifications: HTTP ${fetchResponse.status}`);
+        return false;
+      }
+      
+      const fetchData = await fetchResponse.json();
+      if (fetchData.error) {
+        console.error(`[LEANTIME_ADAPTER] markAllAsRead - Error fetching notifications:`, fetchData.error);
+        return false;
+      }
+      
+      // Transform the raw Leantime notifications to our format
+      const rawNotifications = Array.isArray(fetchData.result) ? fetchData.result : [];
+      const unreadNotifications = rawNotifications
+        .filter((n: any) => n.read === 0 || n.read === false || n.read === '0')
+        .map((n: any) => ({ id: n.id, sourceId: String(n.id) }));
+      
+      console.log(`[LEANTIME_ADAPTER] markAllAsRead - Found ${unreadNotifications.length} unread notifications to mark (from ${rawNotifications.length} total)`);
       
       if (unreadNotifications.length === 0) {
         console.log(`[LEANTIME_ADAPTER] markAllAsRead - No unread notifications, returning success`);
@@ -282,9 +321,8 @@ export class LeantimeAdapter implements NotificationAdapter {
       
       // Mark each notification as read
       const markPromises = unreadNotifications.map(async (notification) => {
-        // Extract the numeric ID from our compound ID (format: "leantime-123")
-        const sourceId = notification.sourceId;
-        const notificationId = parseInt(sourceId);
+        // Use the ID directly (we already have it from the API response)
+        const notificationId = typeof notification.id === 'number' ? notification.id : parseInt(String(notification.id || notification.sourceId));
         
         if (isNaN(notificationId)) {
           console.error(`[LEANTIME_ADAPTER] markAllAsRead - Invalid notification ID: ${sourceId}`);

lib/services/notifications/notification-service.ts

@@ -287,14 +287,17 @@ export class NotificationService {
     console.log(`[NOTIFICATION_SERVICE] markAllAsRead results:`, results);
     
     const success = results.every(result => result);
-    console.log(`[NOTIFICATION_SERVICE] markAllAsRead overall success: ${success}`);
+    const anySuccess = results.some(result => result);
+    console.log(`[NOTIFICATION_SERVICE] markAllAsRead overall success: ${success}, any success: ${anySuccess}`);
     
-    if (success) {
-      console.log(`[NOTIFICATION_SERVICE] Invalidating caches for user ${userId}`);
-      // Invalidate caches
+    // Always invalidate cache if any notifications were marked (even partial success)
+    // This ensures count and list are refreshed after marking
+    if (anySuccess) {
+      console.log(`[NOTIFICATION_SERVICE] Invalidating caches for user ${userId} (some notifications were marked)`);
+      // Invalidate caches to force fresh data on next fetch
       await this.invalidateCache(userId);
     } else {
-      console.log(`[NOTIFICATION_SERVICE] Not invalidating caches - operation failed`);
+      console.log(`[NOTIFICATION_SERVICE] Not invalidating caches - no notifications were marked`);
     }
     
     return success;