diff --git a/MARK_ALL_READ_CACHE_ISSUE.md b/MARK_ALL_READ_CACHE_ISSUE.md new file mode 100644 index 00000000..499d9061 --- /dev/null +++ b/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. + diff --git a/lib/services/notifications/leantime-adapter.ts b/lib/services/notifications/leantime-adapter.ts index e9b3833d..fa680c26 100644 --- a/lib/services/notifications/leantime-adapter.ts +++ b/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}`); diff --git a/lib/services/notifications/notification-service.ts b/lib/services/notifications/notification-service.ts index 92f76ee7..683a011a 100644 --- a/lib/services/notifications/notification-service.ts +++ b/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;