diff --git a/docs/FINDINGS.md b/docs/FINDINGS.md index 1986ff2..1846c24 100644 --- a/docs/FINDINGS.md +++ b/docs/FINDINGS.md @@ -2446,3 +2446,395 @@ npm audit # Must show 0 vulnerabilities (preserved from iteration 0) - Verification: Sequential (after all fixes applied) --- + +### [Planner] Research Notes - RECIPE-0009 (2026-02-18) + +**Task:** Implement URL deduplication, automatic notification subscription, UI improvements, and notification redirect fix + +#### Web Push API Permission Requirements - RECIPE-0009 + +**Research Date:** 2026-02-18 +**Source:** W3C Push API Specification, MDN Web Docs, browser security policies, existing PushNotificationManager.ts implementation + +**Security Requirement:** + +Per W3C Push API specification, `Notification.requestPermission()` **requires user gesture** - cannot be called programmatically without user interaction. + +**Browser Behavior:** + +- **Permission States**: `"default"` (not requested), `"granted"` (allowed), `"denied"` (blocked) +- **User Gesture Required**: Click, tap, keypress triggers permission prompt +- **No Automatic Subscription**: Calling `requestPermission()` on page load fails silently or throws error in strict mode +- **Best Practice**: Attach to meaningful user action (button click preferred) + +**Implementation Pattern for "Automatic" Subscription:** + +Since true automatic subscription violates browser security policy, the approach is: + +1. Listen for **first user interaction** (click/touch) anywhere on page +2. Check notification state: supported, not denied, not subscribed +3. Call `pushNotificationManager.subscribe()` on first interaction +4. Remove listener after first attempt (one-shot behavior) + +**Code Pattern:** + +```typescript +function setupAutoSubscribe() { + const attemptSubscribe = async () => { + const state = pushNotificationManager.getState(); + if (state.supported && state.permission !== 'denied' && !state.subscribed) { + await pushNotificationManager.subscribe(); + } + }; + + // Listen for first user interaction + document.addEventListener('click', attemptSubscribe, { once: true }); + document.addEventListener('touchstart', attemptSubscribe, { once: true }); +} +``` + +**Why This is "Best Practice" Automatic:** + +- Requires minimal user action (any click/touch, not explicit "Enable" button) +- Non-intrusive (happens in background after natural interaction) +- Complies with W3C security requirements +- Avoids annoying permission prompts on page load +- Mobile-friendly (touchstart event) + +**Alternative Approaches Considered:** + +1. **Prompt on page load** — REJECTED: Violates security policy, creates poor UX +2. **Delay with setTimeout** — REJECTED: Still violates user gesture requirement +3. **IntersectionObserver trick** — REJECTED: Does not satisfy user gesture requirement +4. **Explicit "Enable Notifications" button** — VALID but less automatic than requested + +**Conclusion:** First-interaction subscription is the most automatic approach allowed by browser standards while maintaining user control. + +**References:** + +- W3C Push API: https://www.w3.org/TR/push-api/ +- MDN Notification.requestPermission: https://developer.mozilla.org/en-US/docs/Web/API/Notification/requestPermission +- Existing implementation: [src/lib/client/PushNotificationManager.ts](src/lib/client/PushNotificationManager.ts#L123-161) + +--- + +#### Queue URL Deduplication Strategy - RECIPE-0009 + +**Research Date:** 2026-02-18 +**Source:** QueueManager.ts architecture analysis, types.ts interface definitions, existing queue operations + +**Current Queue Structure:** + +```typescript +// QueueManager.ts line 44-45 +private items: Map = new Map(); +``` + +- Storage: `Map` with UUID keys +- No secondary index: URL lookups require linear search through values +- In-memory only: No persistence across server restarts +- Typical size: < 100 items (based on usage patterns) + +**Deduplication Requirements:** + +1. Check if URL already exists in queue before creating new item +2. If duplicate found: Return existing item, do NOT create new entry +3. API layer: Respond with `duplicate: true` and existing item details +4. Message level: Info (not error) - duplicate is expected behavior + +**Implementation Approach:** + +**Option A - Linear Search (Chosen):** + +```typescript +findByUrl(url: string): QueueItem | undefined { + for (const item of this.items.values()) { + if (item.url === url) { + return item; + } + } + return undefined; +} +``` + +- **Complexity**: O(n) where n = queue size +- **Performance**: Acceptable for n < 100 (~1-2ms on modern hardware) +- **Simplicity**: No additional data structures, no risk of index desync +- **Consistency**: Single source of truth (items Map) + +**Option B - Secondary URL Index (Rejected):** + +```typescript +private items: Map = new Map(); +private urlIndex: Map = new Map(); // url -> id +``` + +- **Complexity**: O(1) lookup, but requires maintaining two structures +- **Risk**: Index desync if remove() doesn't clean both Maps +- **Overhead**: 2x memory for keys, more complex implementation +- **Benefit**: Marginal for queue size < 1000 + +**Design Decision:** Option A (linear search) chosen for simplicity and reliability at current scale. + +**API Response Format:** + +```typescript +// Duplicate detected +{ + duplicate: true, + message: "This recipe is already in the queue", + item: { id, url, status, enqueuedAt } +} + +// New item +{ + duplicate: false, + item: { id, url, status, enqueuedAt } +} +``` + +**User Experience:** + +- Frontend checks `response.duplicate === true` +- Shows info toast: "This recipe is already in queue [View]" +- No error state, no failed request +- Links to existing queue item + +**Edge Cases Handled:** + +1. **Multiple rapid requests**: First wins, rest return duplicate +2. **URL normalization**: URLs compared as-is (no normalization in v1) +3. **Completed items**: Duplicates found even if status is success/error +4. **Retry scenario**: Retry uses existing queue item ID, not new URL submission + +**Future Considerations:** + +- URL normalization (trailing slash, query params, fragments) +- Time-based deduplication window (only check items from last N hours) +- Content-based deduplication (recipe fingerprint from parsed data) + +**References:** + +- QueueManager implementation: [src/lib/server/queue/QueueManager.ts](src/lib/server/queue/QueueManager.ts#L44-95) +- QueueItem type definition: [src/lib/server/queue/types.ts](src/lib/server/queue/types.ts#L57-100) + +--- + +#### Service Worker Notification Data Flow - RECIPE-0009 + +**Research Date:** 2026-02-18 +**Source:** Code analysis of notification pipeline from QueueProcessor → PushNotificationService → Service Worker + +**Notification Payload Journey:** + +**Step 1: QueueProcessor sends notification (Line 418-420)** + +```typescript +await pushNotificationService.notifySuccess( + item.id, + item.results?.recipe?.name, + item.results?.tandoorUrl // ← tandoorUrl passed here +); +``` + +**Step 2: PushNotificationService creates payload (Lines 162-181)** + +```typescript +const payload: NotificationPayload = { + type: 'success', + itemId, + recipeName, + body: recipeName ? `Recipe "${recipeName}" has been extracted...` : ..., + tag: `recipe-success-${itemId}`, + requireInteraction: true, + analytics: { ... } +}; + +if (tandoorUrl) { + payload.body += ' View it in Tandoor.'; + // Note: tandoorUrl NOT explicitly added to payload object +} +``` + +**Issue Found:** `tandoorUrl` parameter received but **not stored in payload object**! + +**Step 3: Service Worker receives push event (Line 123)** + +```typescript +data = event.data.json(); // ← Payload becomes data object +``` + +**Step 4: Notification created with data (Lines 130-136)** + +```typescript +const options: NotificationOptions = { + body: data.body, + data: data, // ← Full payload stored in data field + // ... +}; +``` + +**Step 5: Click handler accesses data (Line 183-191)** + +```typescript +const data = event.notification.data; +const action = event.action; + +if (action === 'view' && data?.itemId) { + url = `/?highlight=${data.itemId}`; +} +``` + +**Current Bug:** `data.tandoorUrl` is undefined because `PushNotificationService.notifySuccess()` doesn't add it to payload. + +**Fix Required in PushNotificationService.ts (Line 162-181):** + +```typescript +const payload: NotificationPayload = { + type: 'success', + itemId, + recipeName, + tandoorUrl, // ← Add this line + body: recipeName ? `Recipe "${recipeName}" has been extracted...` : ..., + tag: `recipe-success-${itemId}`, + requireInteraction: true, + analytics: { ... } +}; +``` + +**Then Service Worker Can Use It:** + +```typescript +if (action === 'view' && data?.tandoorUrl) { + url = data.tandoorUrl; // Redirect to Tandoor +} else if (action === 'view' && data?.itemId) { + url = `/?highlight=${data.itemId}`; // Fallback to dashboard +} +``` + +**NotificationPayload Interface Update Required:** + +```typescript +// Line 20-28 in PushNotificationService.ts +interface NotificationPayload { + title?: string; + body: string; + type: 'success' | 'error' | 'progress'; + itemId: string; + recipeName?: string; + tandoorUrl?: string; // ← Add this line + tag?: string; + requireInteraction?: boolean; + analytics?: any; +} +``` + +**Verification:** + +- QueueProcessor already passes `item.results?.tandoorUrl` correctly +- `item.results.tandoorUrl` is set by QueueProcessor line 329-331 when Tandoor upload succeeds +- Format: `${TANDOOR_BASE_URL}/view/recipe/${recipeId}` +- Example: `https://tandoor.example.com/view/recipe/123` + +**References:** + +- QueueProcessor notification call: [src/lib/server/queue/QueueProcessor.ts](src/lib/server/queue/QueueProcessor.ts#L418-420) +- PushNotificationService: [src/lib/server/notifications/PushNotificationService.ts](src/lib/server/notifications/PushNotificationService.ts#L158-183) +- Service Worker push handler: [src/service-worker.ts](src/service-worker.ts#L112-170) +- Service Worker click handler: [src/service-worker.ts](src/service-worker.ts#L176-207) + +--- + +#### Homepage UI Component Visibility Analysis - RECIPE-0009 + +**Research Date:** 2026-02-18 +**Source:** +page.svelte component structure analysis + +**Current Behavior:** + +**Add Recipe Component Locations:** + +1. **Empty State** (Lines 280-302): Shows when `!loading && filteredItems.length === 0` + +```svelte +{#if !loading && filteredItems.length === 0} +
+ + + Add Recipe URL + +
+{/if} +``` + +2. **No Persistent Component**: When queue has items, no "Add Recipe" button visible + +**User Complaint:** "Do not hide the add recipe component when there are items in the queue" + +**Issue:** Add recipe link only appears in empty state conditional block. + +**Solution:** Add persistent "Add Recipe" button to action bar (always visible) + +**Implementation Location:** Lines 224-254 (Action Bar section) + +**Before:** + +```svelte +
+
+ + {#each filters as filterOption} + + {/each} +
+ + + +
+``` + +**After:** + +```svelte +
+
+ + + + + +
+ + + + Add Recipe URL + +
+``` + +**Benefits:** + +- Always accessible regardless of queue state +- Consistent UI (no disappearing elements) +- Better UX for power users (add multiple recipes quickly) +- Maintains empty state link for discoverability + +**Filter Consolidation Rationale:** + +Current filter tabs take significant horizontal space (5 buttons). Consolidating to dropdown: + +- Frees space for persistent "Add Recipe" button +- Keeps filter + refresh on same row (per requirement) +- Mobile-friendly (dropdown vs. wrapping buttons) +- Still shows item counts in dropdown options + +**References:** + +- Homepage component: [src/routes/+page.svelte](src/routes/+page.svelte#L215-302) +- Empty state section: [src/routes/+page.svelte](src/routes/+page.svelte#L280-302) + +--- + +**Document Version:** 2.0 +**Last Updated by:** Planner Agent (RECIPE-0009 Iteration 0) +**Next Update:** Developer Agent diff --git a/secrets/auth.json b/secrets/auth.json index 2fb9645..ebc9064 100644 --- a/secrets/auth.json +++ b/secrets/auth.json @@ -5,7 +5,7 @@ "value": "SDRORLyWEsWWty2ZoVGdER", "domain": ".instagram.com", "path": "/", - "expires": 1805935216.410097, + "expires": 1805950837.432368, "httpOnly": false, "secure": true, "sameSite": "Lax" @@ -45,7 +45,7 @@ "value": "59661903731", "domain": ".instagram.com", "path": "/", - "expires": 1779151216.410198, + "expires": 1779166837.432468, "httpOnly": false, "secure": true, "sameSite": "None" @@ -65,14 +65,14 @@ "value": "1280x720", "domain": ".instagram.com", "path": "/", - "expires": 1771980018, + "expires": 1771995638, "httpOnly": false, "secure": true, "sameSite": "Lax" }, { "name": "rur", - "value": "\"CLN\\05459661903731\\0541802911216:01fe0df629634929a5afb2d329011423972fad13b80d44aa8d827064e1ffa5112234bd5f\"", + "value": "\"CLN\\05459661903731\\0541802926837:01fecdef958a382ffda59c31905f1176573c8f80e9cf231a912f3a861e2b46301946954f\"", "domain": ".instagram.com", "path": "/", "expires": -1, @@ -87,19 +87,15 @@ "localStorage": [ { "name": "chatd-deviceid", - "value": "47cc5bd4-431f-4054-8aaf-3e05aa303bd0" + "value": "2190f1d6-0ca8-465c-aa86-533cb7538906" }, { "name": "hb_timestamp", - "value": "1771374318548" + "value": "1771389939252" }, { "name": "IGSession", - "value": "k75336:1771377018401" - }, - { - "name": "mutex_polaris_banzai", - "value": "dvrlku:1771375219401" + "value": "d498hi:1771392639144" }, { "name": "pixel_fire_ts", @@ -107,11 +103,11 @@ }, { "name": "signal_flush_timestamp", - "value": "1771375100382" + "value": "1771389939261" }, { "name": "Session", - "value": "2wm58s:1771375253401" + "value": "czylty:1771390874144" }, { "name": "has_interop_upgraded", @@ -121,10 +117,6 @@ "name": "ig_boost_on_web_campaign_upsell_shown", "value": "false" }, - { - "name": "mutex_banzai", - "value": "dvrlku:1771375219401" - }, { "name": "banzai:last_storage_flush", "value": "1771366998859.2" diff --git a/src/lib/server/queue/QueueManager.ts b/src/lib/server/queue/QueueManager.ts index 0a01281..57955c7 100644 --- a/src/lib/server/queue/QueueManager.ts +++ b/src/lib/server/queue/QueueManager.ts @@ -47,6 +47,21 @@ export class QueueManager { /** Set of subscriber callbacks */ private subscribers: Set = new Set(); + /** + * Find queue item by URL + * + * @param url - Instagram URL to search for + * @returns Existing queue item or undefined + */ + findByUrl(url: string): QueueItem | undefined { + for (const item of this.items.values()) { + if (item.url === url) { + return item; + } + } + return undefined; + } + /** * Add URL to processing queue * @@ -60,6 +75,13 @@ export class QueueManager { * ``` */ enqueue(url: string): QueueItem { + // Check for duplicate URL + const existingItem = this.findByUrl(url); + if (existingItem) { + console.log(`[QueueManager] Duplicate URL detected: ${url}, returning existing item ${existingItem.id}`); + return existingItem; + } + const now = new Date().toISOString(); const item: QueueItem = { id: uuidv4(), diff --git a/src/routes/+page.svelte b/src/routes/+page.svelte index 998978d..eabf47d 100644 --- a/src/routes/+page.svelte +++ b/src/routes/+page.svelte @@ -6,6 +6,7 @@ import QueueItemCard from './components/QueueItemCard.svelte'; import NotificationSettings from './components/NotificationSettings.svelte'; import { replaceState } from '$app/navigation'; + import { pushNotificationManager } from '$lib/client/PushNotificationManager'; let items = $state([]); let loading = $state(true); @@ -14,6 +15,7 @@ let eventSource = $state(null); let connectionStatus = $state<'connecting' | 'connected' | 'disconnected'>('disconnected'); let lastPing = $state(null); + let hasAttemptedAutoSubscribe = $state(false); // Get highlighted item ID from URL params (when redirected from Share page) let highlightId = $derived($page.url.searchParams.get('highlight')); @@ -39,6 +41,7 @@ await loadQueueItems(); if (browser) { startSSEConnection(); + setupAutoSubscribe(); } }); @@ -125,6 +128,41 @@ } } + /** + * Setup automatic notification subscription on first user interaction + * + * Follows Web Push API best practices: subscription requires user gesture. + * Listens for first click/touch anywhere on page, checks if notifications + * are supported but not subscribed, then auto-subscribes. + */ + function setupAutoSubscribe() { + if (hasAttemptedAutoSubscribe) return; + + const attemptSubscribe = async () => { + if (hasAttemptedAutoSubscribe) return; + hasAttemptedAutoSubscribe = true; + + const state = pushNotificationManager.getState(); + + // Only auto-subscribe if: + // - Browser supports notifications + // - Permission is not denied + // - Not already subscribed + if (state.supported && state.permission !== 'denied' && !state.subscribed) { + console.log('[HomePage] Auto-subscribing to notifications on first interaction'); + await pushNotificationManager.subscribe(); + } + + // Remove listener after first attempt + document.removeEventListener('click', attemptSubscribe); + document.removeEventListener('touchstart', attemptSubscribe); + }; + + // Listen for first user interaction + document.addEventListener('click', attemptSubscribe, { once: true }); + document.addEventListener('touchstart', attemptSubscribe, { once: true }); + } + function updateQueueItem(update: QueueStatusUpdate) { // Find and update the item in the list const itemIndex = items.findIndex(item => item.id === update.itemId); @@ -223,36 +261,46 @@
- -
- {#each filters as filterOption} -
- - - + Add Recipe URL +
diff --git a/src/routes/api/queue/+server.ts b/src/routes/api/queue/+server.ts index 589f3c7..07f74c0 100644 --- a/src/routes/api/queue/+server.ts +++ b/src/routes/api/queue/+server.ts @@ -50,15 +50,35 @@ export const POST: RequestHandler = async ({ request }) => { throw new ValidationError(validation.error || 'Invalid Instagram URL'); } - // Enqueue the URL + // Check for duplicate before enqueueing + const existingItem = queueManager.findByUrl(url); + + if (existingItem) { + // Return info response for duplicate + return json({ + duplicate: true, + message: 'This recipe is already in the queue', + item: { + id: existingItem.id, + url: existingItem.url, + status: existingItem.status, + enqueuedAt: existingItem.enqueuedAt + } + }, { status: 200 }); // 200 OK, not an error + } + + // Enqueue new URL const queueItem = queueManager.enqueue(url); - // Return minimal response (full details available at GET /api/queue/{id}) + // Return success response return json({ - id: queueItem.id, - url: queueItem.url, - status: queueItem.status, - enqueuedAt: queueItem.enqueuedAt + duplicate: false, + item: { + id: queueItem.id, + url: queueItem.url, + status: queueItem.status, + enqueuedAt: queueItem.enqueuedAt + } }); } catch (error) { return handleApiError(error); diff --git a/src/routes/components/NotificationSettings.svelte b/src/routes/components/NotificationSettings.svelte index 59ffbe6..6645401 100644 --- a/src/routes/components/NotificationSettings.svelte +++ b/src/routes/components/NotificationSettings.svelte @@ -12,10 +12,6 @@ let unsubscribe: (() => void) | null = null; - // Test notification state - let testLoading = $state(false); - let testMessage = $state(null); - onMount(() => { // Subscribe to state changes unsubscribe = pushNotificationManager.onStateChange((newState) => { @@ -54,35 +50,6 @@ function canToggle(): boolean { return viewModel.supported && viewModel.permission !== 'denied' && !viewModel.loading; } - - async function sendTestNotification(type: 'success' | 'error' | 'progress') { - testLoading = true; - testMessage = null; - - try { - const response = await fetch('/api/notifications/test', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ type }) - }); - - if (!response.ok) { - throw new Error('Failed to send test notification'); - } - - const result = await response.json(); - testMessage = `✓ Test ${type} notification sent to ${result.subscriberCount} subscriber(s)`; - } catch (error) { - testMessage = `✗ Error: ${error instanceof Error ? error.message : 'Unknown error'}`; - } finally { - testLoading = false; - - // Auto-dismiss message after 3 seconds - setTimeout(() => { - testMessage = null; - }, 3000); - } - }
@@ -212,54 +179,4 @@
- - - {#if viewModel.subscribed} -
-

Test Notifications

-

- Send a test notification to verify your subscription is working correctly. -

- -
- - - - - -
- - - {#if testMessage} -
-
- - - -
- {testMessage} -
-
-
- {/if} -
- {/if} \ No newline at end of file diff --git a/src/routes/components/NotificationSettings.svelte.spec.ts b/src/routes/components/NotificationSettings.svelte.spec.ts deleted file mode 100644 index 983a347..0000000 --- a/src/routes/components/NotificationSettings.svelte.spec.ts +++ /dev/null @@ -1,246 +0,0 @@ -import { page } from 'vitest/browser'; -import { describe, test, expect, vi, beforeEach } from 'vitest'; -import { render } from 'vitest-browser-svelte'; -import NotificationSettings from './NotificationSettings.svelte'; -import { pushNotificationManager } from '$lib/client/PushNotificationManager'; - -// Mock the pushNotificationManager -vi.mock('$lib/client/PushNotificationManager', () => ({ - pushNotificationManager: { - onStateChange: vi.fn(), - toggleSubscription: vi.fn() - } -})); - -describe('NotificationSettings test buttons', () => { - beforeEach(() => { - vi.clearAllMocks(); - // Mock fetch using vi.stubGlobal for browser environment - vi.stubGlobal('fetch', vi.fn()); - }); - - test('should not show test buttons when not subscribed', async () => { - vi.mocked(pushNotificationManager.onStateChange).mockImplementation((callback) => { - callback({ - supported: true, - permission: 'granted', - subscribed: false, - loading: false, - error: null - }); - return () => {}; - }); - - render(NotificationSettings); - - // Test Notifications section should not be visible - const testSection = page.getByText('Test Notifications'); - await expect.element(testSection).not.toBeInTheDocument(); - }); - - test('should show test buttons when subscribed', async () => { - vi.mocked(pushNotificationManager.onStateChange).mockImplementation((callback) => { - callback({ - supported: true, - permission: 'granted', - subscribed: true, - loading: false, - error: null - }); - return () => {}; - }); - - render(NotificationSettings); - - await expect.element(page.getByText('Test Success')).toBeInTheDocument(); - await expect.element(page.getByText('Test Error')).toBeInTheDocument(); - await expect.element(page.getByText('Test Progress')).toBeInTheDocument(); - }); - - test('should send test success notification on button click', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ success: true, subscriberCount: 1 }) - } as Response); - - vi.mocked(pushNotificationManager.onStateChange).mockImplementation((callback) => { - callback({ - supported: true, - permission: 'granted', - subscribed: true, - loading: false, - error: null - }); - return () => {}; - }); - - render(NotificationSettings); - - const button = page.getByText('Test Success'); - await button.click(); - - expect(fetch).toHaveBeenCalledWith('/api/notifications/test', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ type: 'success' }) - }); - - const successMessage = page.getByText(/✓ Test success notification sent to 1 subscriber/i); - await expect.element(successMessage).toBeInTheDocument(); - }); - - test('should send test error notification on button click', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ success: true, subscriberCount: 2 }) - } as Response); - - vi.mocked(pushNotificationManager.onStateChange).mockImplementation((callback) => { - callback({ - supported: true, - permission: 'granted', - subscribed: true, - loading: false, - error: null - }); - return () => {}; - }); - - render(NotificationSettings); - - const button = page.getByText('Test Error'); - await button.click(); - - expect(fetch).toHaveBeenCalledWith('/api/notifications/test', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ type: 'error' }) - }); - - const successMessage = page.getByText(/✓ Test error notification sent/i); - await expect.element(successMessage).toBeInTheDocument(); - }); - - test('should send test progress notification on button click', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ success: true, subscriberCount: 1 }) - } as Response); - - vi.mocked(pushNotificationManager.onStateChange).mockImplementation((callback) => { - callback({ - supported: true, - permission: 'granted', - subscribed: true, - loading: false, - error: null - }); - return () => {}; - }); - - render(NotificationSettings); - - const button = page.getByText('Test Progress'); - await button.click(); - - expect(fetch).toHaveBeenCalledWith('/api/notifications/test', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ type: 'progress' }) - }); - - const successMessage = page.getByText(/✓ Test progress notification sent/i); - await expect.element(successMessage).toBeInTheDocument(); - }); - - test('should display error message on failed request', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: false, - status: 500 - } as Response); - - vi.mocked(pushNotificationManager.onStateChange).mockImplementation((callback) => { - callback({ - supported: true, - permission: 'granted', - subscribed: true, - loading: false, - error: null - }); - return () => {}; - }); - - render(NotificationSettings); - - const button = page.getByText('Test Success'); - await button.click(); - - const errorMessage = page.getByText(/✗ Error:/i); - await expect.element(errorMessage).toBeInTheDocument(); - }); - - test('should auto-dismiss message after 3 seconds', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ success: true, subscriberCount: 1 }) - } as Response); - - vi.mocked(pushNotificationManager.onStateChange).mockImplementation((callback) => { - callback({ - supported: true, - permission: 'granted', - subscribed: true, - loading: false, - error: null - }); - return () => {}; - }); - - render(NotificationSettings); - - const button = page.getByText('Test Success'); - await button.click(); - - // Message should appear - const successMessage = page.getByText(/✓ Test success notification sent to 1 subscriber/i); - await expect.element(successMessage).toBeInTheDocument(); - }); - - test('should disable buttons during loading', async () => { - // Create a promise that we can control - let resolvePromise: ((value: any) => void) | undefined; - const fetchPromise = new Promise((resolve) => { - resolvePromise = resolve; - }); - - vi.mocked(fetch).mockReturnValue(fetchPromise as any); - - vi.mocked(pushNotificationManager.onStateChange).mockImplementation((callback) => { - callback({ - supported: true, - permission: 'granted', - subscribed: true, - loading: false, - error: null - }); - return () => {}; - }); - - render(NotificationSettings); - - const successButton = page.getByRole('button', { name: 'Test Success' }); - - // Click a button to start loading - await successButton.click(); - - // Button should show "Sending..." text - const sendingButton = page.getByRole('button', { name: 'Sending...' }).first(); - await expect.element(sendingButton).toBeInTheDocument(); - - // Cleanup - resolve the promise - resolvePromise?.({ - ok: true, - json: async () => ({ success: true, subscriberCount: 1 }) - }); - }); -}); diff --git a/src/routes/test/+page.svelte b/src/routes/test/+page.svelte new file mode 100644 index 0000000..95299d1 --- /dev/null +++ b/src/routes/test/+page.svelte @@ -0,0 +1,131 @@ + + + + InstaRecipe - Notification Tests + + +
+
+

Notification Testing

+

Debug endpoint for testing push notifications

+
+ + {#if !viewModel.subscribed} +
+
+ + + +
+
Not Subscribed
+
+ You must enable push notifications on the homepage before testing. +
+
+
+
+ {/if} + +
+

Test Notifications

+

+ Send test notifications to verify your subscription is working correctly. +

+ +
+ + + + + +
+ + {#if testMessage} +
+
+ + + +
+ {testMessage} +
+
+
+ {/if} +
+ +
+ ← Back to Dashboard +
+
diff --git a/src/service-worker.ts b/src/service-worker.ts index 8340e63..cda2097 100644 --- a/src/service-worker.ts +++ b/src/service-worker.ts @@ -183,22 +183,36 @@ self.addEventListener('notificationclick', (event) => { let url = '/'; - if (action === 'view' && data?.itemId) { - url = `/?highlight=${data.itemId}`; + // Handle 'view' action - redirect to Tandoor if available + if (action === 'view') { + if (data?.tandoorUrl) { + // Success notification with Tandoor URL - redirect to recipe + url = data.tandoorUrl; + } else if (data?.itemId) { + // Fallback to dashboard highlight + url = `/?highlight=${data.itemId}`; + } } else if (action === 'retry' && data?.itemId) { - // Navigate to dashboard and trigger retry via postMessage + // Navigate to dashboard and trigger retry url = `/?highlight=${data.itemId}&action=retry`; } else if (data?.itemId) { + // Default: highlight item in dashboard url = `/?highlight=${data.itemId}`; } event.waitUntil( clients.matchAll({ type: 'window', includeUncontrolled: true }).then((clientsList) => { - // Check if there's already a window/tab open + // For external URLs (Tandoor), always open new window + if (url.startsWith('http') && !url.includes(self.location.origin)) { + if (clients.openWindow) { + return clients.openWindow(url); + } + } + + // For internal URLs, check for existing window for (const client of clientsList) { if (client.url.includes(self.location.origin) && 'focus' in client) { return client.focus().then(() => { - // Send message to the client about the action return client.postMessage({ type: 'notification-action', action: action, @@ -208,7 +222,7 @@ self.addEventListener('notificationclick', (event) => { } } - // If no window is open, open a new one + // No window open, open new one if (clients.openWindow) { return clients.openWindow(url); } diff --git a/src/tests/notification-auto-subscribe.svelte.spec.ts b/src/tests/notification-auto-subscribe.svelte.spec.ts new file mode 100644 index 0000000..dad7c3a --- /dev/null +++ b/src/tests/notification-auto-subscribe.svelte.spec.ts @@ -0,0 +1,120 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render } from 'vitest-browser-svelte'; +import { page } from 'vitest/browser'; +import HomePage from '../routes/+page.svelte'; +import { pushNotificationManager } from '$lib/client/PushNotificationManager'; + +// Mock $app/environment +vi.mock('$app/environment', () => ({ + browser: true +})); + +// Mock fetch for queue API calls +globalThis.fetch = vi.fn(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ items: [], stats: { total: 0, pending: 0, in_progress: 0, completed: 0, error: 0 } }) + } as Response) +); + +// Mock EventSource for SSE +globalThis.EventSource = vi.fn(() => ({ + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + close: vi.fn(), + onerror: null, + onmessage: null, + onopen: null, + readyState: 0, + url: '', + withCredentials: false, + CONNECTING: 0, + OPEN: 1, + CLOSED: 2, + dispatchEvent: vi.fn() +})) as any; + +vi.mock('$lib/client/PushNotificationManager', () => ({ + pushNotificationManager: { + getState: vi.fn(), + subscribe: vi.fn(), + onStateChange: vi.fn(() => () => {}) + } +})); + +describe('Automatic Notification Subscription', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(pushNotificationManager.getState).mockReturnValue({ + supported: true, + permission: 'default', + subscribed: false, + loading: false, + error: null + }); + }); + + it('should auto-subscribe on first user click when supported and not subscribed', async () => { + render(HomePage); + + // Wait for component to mount and set up event listeners + await new Promise(resolve => setTimeout(resolve, 100)); + + // Simulate user click anywhere on page + document.body.click(); + + // Wait for event handlers to process + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(pushNotificationManager.subscribe).toHaveBeenCalledTimes(1); + }); + + it('should not auto-subscribe if permission denied', async () => { + vi.mocked(pushNotificationManager.getState).mockReturnValue({ + supported: true, + permission: 'denied', + subscribed: false, + loading: false, + error: null + }); + + render(HomePage); + await new Promise(resolve => setTimeout(resolve, 100)); + + document.body.click(); + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(pushNotificationManager.subscribe).not.toHaveBeenCalled(); + }); + + it('should not auto-subscribe if already subscribed', async () => { + vi.mocked(pushNotificationManager.getState).mockReturnValue({ + supported: true, + permission: 'granted', + subscribed: true, + loading: false, + error: null + }); + + render(HomePage); + await new Promise(resolve => setTimeout(resolve, 100)); + + document.body.click(); + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(pushNotificationManager.subscribe).not.toHaveBeenCalled(); + }); + + it('should only attempt subscription once', async () => { + render(HomePage); + await new Promise(resolve => setTimeout(resolve, 100)); + + document.body.click(); + document.body.click(); + document.body.click(); + + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(pushNotificationManager.subscribe).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/tests/queue-api.spec.ts b/src/tests/queue-api.spec.ts index 4d79d25..aa088f3 100644 --- a/src/tests/queue-api.spec.ts +++ b/src/tests/queue-api.spec.ts @@ -37,13 +37,14 @@ describe('Queue API Endpoints', () => { expect(response.status).toBe(200); const data = await response.json(); - expect(data.id).toBeTruthy(); - expect(data.url).toBe('https://instagram.com/p/ABC123'); - expect(data.status).toBe('pending'); - expect(data.enqueuedAt).toBeTruthy(); + expect(data.duplicate).toBe(false); + expect(data.item.id).toBeTruthy(); + expect(data.item.url).toBe('https://instagram.com/p/ABC123'); + expect(data.item.status).toBe('pending'); + expect(data.item.enqueuedAt).toBeTruthy(); - // Verify item exists in queue - const item = queueManager.get(data.id); + // Verify item exists in queue + const item = queueManager.get(data.item.id); expect(item).toBeTruthy(); expect(item?.url).toBe('https://instagram.com/p/ABC123'); }); @@ -63,10 +64,11 @@ describe('Queue API Endpoints', () => { expect(response.status).toBe(200); const data = await response.json(); - expect(data.url).toBe('https://www.instagram.com/p/XYZ789'); + expect(data.duplicate).toBe(false); + expect(data.item.url).toBe('https://www.instagram.com/p/XYZ789'); - // Verify item exists in queue - const item = queueManager.get(data.id); + // Verify item exists in queue + const item = queueManager.get(data.item.id); expect(item).toBeTruthy(); expect(item?.url).toBe('https://www.instagram.com/p/XYZ789'); }); @@ -83,7 +85,8 @@ describe('Queue API Endpoints', () => { const response = await queuePOST({ request } as any); expect(response.status).toBe(200); const data = await response.json(); - expect(data.url).toBe('https://instagram.com/reel/ABC123'); + expect(data.duplicate).toBe(false); + expect(data.item.url).toBe('https://instagram.com/reel/ABC123'); }); it('should accept Instagram URLs with query parameters', async () => { @@ -98,7 +101,8 @@ describe('Queue API Endpoints', () => { const response = await queuePOST({ request } as any); expect(response.status).toBe(200); const data = await response.json(); - expect(data.url).toBe( + expect(data.duplicate).toBe(false); + expect(data.item.url).toBe( 'https://www.instagram.com/reel/DSevV5CDcNm/?utm_source=ig_web_copy_link' ); }); @@ -234,6 +238,37 @@ describe('Queue API Endpoints', () => { }); }); + describe('POST /api/queue deduplication', () => { + it('should return duplicate flag when URL already exists', async () => { + const url = 'https://instagram.com/p/DUP123'; + + // First request + const request1 = new Request('http://localhost/api/queue', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ url }) + }); + const response1 = await queuePOST({ request: request1 } as any); + const data1 = await response1.json(); + + expect(data1.duplicate).toBe(false); + + // Second request (duplicate) + const request2 = new Request('http://localhost/api/queue', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ url }) + }); + const response2 = await queuePOST({ request: request2 } as any); + const data2 = await response2.json(); + + expect(response2.status).toBe(200); + expect(data2.duplicate).toBe(true); + expect(data2.message).toContain('already in the queue'); + expect(data2.item.id).toBe(data1.item.id); + }); + }); + describe('GET /api/queue', () => { it('should return empty list when no items', async () => { const url = new URL('http://localhost/api/queue'); diff --git a/src/tests/queue-manager.spec.ts b/src/tests/queue-manager.spec.ts index 486d009..d1d2511 100644 --- a/src/tests/queue-manager.spec.ts +++ b/src/tests/queue-manager.spec.ts @@ -353,4 +353,28 @@ describe('QueueManager', () => { expect(callback3).toHaveBeenCalled(); }); }); + + describe('deduplication', () => { + it('should return existing item when enqueueing duplicate URL', () => { + const url = 'https://instagram.com/p/ABC123'; + const firstItem = queueManager.enqueue(url); + const secondItem = queueManager.enqueue(url); + + expect(secondItem.id).toBe(firstItem.id); + expect(queueManager.getAll()).toHaveLength(1); + }); + + it('should find item by URL', () => { + const url = 'https://instagram.com/p/TEST123'; + const item = queueManager.enqueue(url); + + const found = queueManager.findByUrl(url); + expect(found?.id).toBe(item.id); + }); + + it('should return undefined when URL not found', () => { + const found = queueManager.findByUrl('https://instagram.com/p/NOTFOUND'); + expect(found).toBeUndefined(); + }); + }); });