Browse Source

fix(news): delete cached items absent from latest feed

Stale-item deletion was computing `idsToDelete` as feed items that were
filtered out by version match, so DB items removed from the feed itself
were never cleaned up (Requirement 1.3 violation).

Introduce `NewsService.deleteItemsNotInFeed(feedExternalIds)` which uses
a `$nin` filter against the full feed externalId list, and switch the
cron service to pass the full feed externalIds directly. The previous
`deleteNewsItemsByExternalIds` is left in place for the follow-up
cleanup commit.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Ryotaro Nagahara 3 weeks ago
parent
commit
df5196c8b1

+ 8 - 5
apps/app/src/features/news/server/services/news-cron-service.spec.ts

@@ -1,17 +1,17 @@
 // Hoisted mocks
 const mocks = vi.hoisted(() => {
   const upsertNewsItems = vi.fn();
-  const deleteNewsItemsByExternalIds = vi.fn();
+  const deleteItemsNotInFeed = vi.fn();
   const mockFetch = vi.fn();
   const getGrowiVersion = vi.fn(() => '7.5.0');
 
   return {
     NewsService: vi.fn(() => ({
       upsertNewsItems,
-      deleteNewsItemsByExternalIds,
+      deleteItemsNotInFeed,
     })),
     upsertNewsItems,
-    deleteNewsItemsByExternalIds,
+    deleteItemsNotInFeed,
     mockFetch,
     getGrowiVersion,
   };
@@ -146,7 +146,10 @@ describe('NewsCronService', () => {
       await service.executeJob();
 
       expect(mocks.upsertNewsItems).toHaveBeenCalledWith(VALID_FEED.items);
-      expect(mocks.deleteNewsItemsByExternalIds).toHaveBeenCalledWith([]);
+      expect(mocks.deleteItemsNotInFeed).toHaveBeenCalledWith([
+        'item-001',
+        'item-002',
+      ]);
     });
 
     test('should NOT update DB when fetch fails', async () => {
@@ -156,7 +159,7 @@ describe('NewsCronService', () => {
       await service.executeJob();
 
       expect(mocks.upsertNewsItems).not.toHaveBeenCalled();
-      expect(mocks.deleteNewsItemsByExternalIds).not.toHaveBeenCalled();
+      expect(mocks.deleteItemsNotInFeed).not.toHaveBeenCalled();
     });
 
     test('should NOT update DB when fetch throws', async () => {

+ 6 - 7
apps/app/src/features/news/server/services/news-cron-service.ts

@@ -132,15 +132,14 @@ export class NewsCronService extends CronService {
         : undefined,
     }));
 
-    const feedIds = new Set(filteredItems.map((item) => item.id));
-
-    // Get all existing external IDs to find which ones are no longer in the feed
-    // We pass all filtered items' IDs — items not in the feed are determined by exclusion
-    const allFeedIds = feedJson.items.map((item) => item.id);
-    const idsToDelete = allFeedIds.filter((id) => !feedIds.has(id));
+    // Pass the full set of feed externalIds so the service can delete any DB
+    // item that is no longer present in the feed (Requirement 1.3). Includes
+    // items filtered out by version match — those remain "in the feed" and
+    // are allowed to age out via the NewsItem TTL.
+    const feedExternalIds = feedJson.items.map((item) => item.id);
 
     const service = new NewsService();
     await service.upsertNewsItems(newsItemInputs);
-    await service.deleteNewsItemsByExternalIds(idsToDelete);
+    await service.deleteItemsNotInFeed(feedExternalIds);
   }
 }

+ 22 - 0
apps/app/src/features/news/server/services/news-service.spec.ts

@@ -437,4 +437,26 @@ describe('NewsService', () => {
       expect(mocks.newsItemDeleteMany).not.toHaveBeenCalled();
     });
   });
+
+  describe('deleteItemsNotInFeed', () => {
+    test('should call deleteMany with $nin filter for items not in feed', async () => {
+      mocks.newsItemDeleteMany.mockResolvedValue({ deletedCount: 1 });
+
+      await service.deleteItemsNotInFeed(['ext-001', 'ext-002']);
+
+      expect(mocks.newsItemDeleteMany).toHaveBeenCalledWith({
+        externalId: { $nin: ['ext-001', 'ext-002'] },
+      });
+    });
+
+    test('should call deleteMany with $nin: [] when feed is empty (deletes all cached items)', async () => {
+      mocks.newsItemDeleteMany.mockResolvedValue({ deletedCount: 5 });
+
+      await service.deleteItemsNotInFeed([]);
+
+      expect(mocks.newsItemDeleteMany).toHaveBeenCalledWith({
+        externalId: { $nin: [] },
+      });
+    });
+  });
 });

+ 12 - 0
apps/app/src/features/news/server/services/news-service.ts

@@ -178,4 +178,16 @@ export class NewsService {
 
     await NewsItem.deleteMany({ externalId: { $in: externalIds } });
   }
+
+  /**
+   * Delete every cached news item whose externalId is NOT in the supplied set.
+   * Caller passes the full list of externalIds present in the latest feed; any DB
+   * item missing from that list is considered stale and removed (Requirement 1.3).
+   *
+   * Note: passing an empty array means "feed has no items" and will delete every
+   * cached news item. Callers must only invoke this after a successful feed fetch.
+   */
+  async deleteItemsNotInFeed(feedExternalIds: string[]): Promise<void> {
+    await NewsItem.deleteMany({ externalId: { $nin: feedExternalIds } });
+  }
 }