From d73903e82e48ec7ad96866a9c7af5e2238492b31 Mon Sep 17 00:00:00 2001
From: Will Miao <willmiao.dev@gmail.com>
Date: Mon, 3 Nov 2025 20:13:48 +0800
Subject: [PATCH] feat: prevent duplicate banner entries in recent history

Add duplicate detection to banner recording to prevent multiple entries
for the same banner ID in recent history. This prevents duplicate history
entries when pages refresh or banners are shown multiple times.

- Check if banner ID already exists in recentHistory before adding
- Return early if duplicate found to prevent adding same banner multiple times
- Add comprehensive tests for banner history functionality including:
  - Adding new banners to history
  - Preventing duplicate entries
  - Handling multiple different banners
- Clear history between tests to ensure test isolation
---
 static/js/managers/BannerService.js           |  7 ++
 tests/frontend/managers/BannerService.test.js | 78 +++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/static/js/managers/BannerService.js b/static/js/managers/BannerService.js
index ce8022b7..f4461fdc 100644
--- a/static/js/managers/BannerService.js
+++ b/static/js/managers/BannerService.js
@@ -348,6 +348,13 @@ class BannerService {
             return;
         }
 
+        // Check if banner with this ID already exists in recent history
+        // Only record if it's not already in history (prevents duplicates on page refresh)
+        const existingEntry = this.recentHistory.find(entry => entry.id === banner.id);
+        if (existingEntry) {
+            return; // Banner already exists in history, don't add again
+        }
+
         const sanitizedActions = Array.isArray(banner.actions)
             ? banner.actions.map(action => ({
                 text: action.text,
diff --git a/tests/frontend/managers/BannerService.test.js b/tests/frontend/managers/BannerService.test.js
index 94956dad..078f061e 100644
--- a/tests/frontend/managers/BannerService.test.js
+++ b/tests/frontend/managers/BannerService.test.js
@@ -35,6 +35,7 @@ describe('BannerService', () => {
         // Reset banner service state
         bannerService.banners.clear();
         bannerService.initialized = false;
+        bannerService.recentHistory = []; // Clear history for each test
         
         // Clear DOM
         document.body.innerHTML = '<div id="banner-container"></div>';
@@ -216,4 +217,81 @@ describe('BannerService', () => {
             expect(storageHelpers.setStorageItem).not.toHaveBeenCalled();
         });
     });
+
+    describe('Banner History', () => {
+        const testBanner = {
+            id: 'test-banner',
+            title: 'Test Banner',
+            content: 'This is a test banner',
+            actions: [
+                {
+                    text: 'Action 1',
+                    icon: 'fas fa-check',
+                    url: 'https://example.com',
+                    type: 'primary'
+                }
+            ]
+        };
+
+        it('should add banner to history when first recorded', () => {
+            // Mock storage to return empty array
+            storageHelpers.getStorageItem.mockImplementation((key, defaultValue) => {
+                if (key === 'banner_history') {
+                    return [];
+                }
+                return defaultValue;
+            });
+
+            // Record the banner appearance
+            bannerService.recordBannerAppearance(testBanner);
+
+            // Should have added the banner to history
+            expect(bannerService.recentHistory).toHaveLength(1);
+            expect(bannerService.recentHistory[0].id).toBe('test-banner');
+            expect(bannerService.recentHistory[0].title).toBe('Test Banner');
+        });
+
+        it('should not add duplicate banner to history when recorded multiple times', () => {
+            // Mock storage to return empty array
+            storageHelpers.getStorageItem.mockImplementation((key, defaultValue) => {
+                if (key === 'banner_history') {
+                    return [];
+                }
+                return defaultValue;
+            });
+
+            // Record the same banner twice
+            bannerService.recordBannerAppearance(testBanner);
+            bannerService.recordBannerAppearance(testBanner);
+
+            // Should only have one entry in history
+            expect(bannerService.recentHistory).toHaveLength(1);
+            expect(bannerService.recentHistory[0].id).toBe('test-banner');
+        });
+
+        it('should add different banners to history', () => {
+            // Mock storage to return empty array
+            storageHelpers.getStorageItem.mockImplementation((key, defaultValue) => {
+                if (key === 'banner_history') {
+                    return [];
+                }
+                return defaultValue;
+            });
+
+            const anotherBanner = {
+                id: 'another-banner',
+                title: 'Another Banner',
+                content: 'This is another test banner'
+            };
+
+            // Record two different banners
+            bannerService.recordBannerAppearance(testBanner);
+            bannerService.recordBannerAppearance(anotherBanner);
+
+            // Should have two entries in history
+            expect(bannerService.recentHistory).toHaveLength(2);
+            expect(bannerService.recentHistory[0].id).toBe('another-banner');
+            expect(bannerService.recentHistory[1].id).toBe('test-banner');
+        });
+    });
 });
\ No newline at end of file