diff --git a/src/lib/notifications/email.ts b/src/lib/notifications/email.ts index 1a24b5b01..2d6e4e404 100644 --- a/src/lib/notifications/email.ts +++ b/src/lib/notifications/email.ts @@ -5,7 +5,9 @@ * Falls back gracefully if Resend is not configured. */ +import type { SupabaseClient } from '@supabase/supabase-js'; import { createClient } from '@/lib/supabase/server'; +import type { Database } from '@/lib/types/database'; import type { NotificationPreferences, NotificationType, EmailTemplate } from './types'; import { DEFAULT_NOTIFICATION_PREFERENCES } from './types'; @@ -37,10 +39,15 @@ async function getResendClient() { * Falls back to defaults if preferences not set or column missing. */ export async function getUserNotificationPreferences( - userId: string + userId: string, + client?: SupabaseClient ): Promise { try { - const supabase = await createClient(); + // Accept a caller-supplied client so server-side dispatchers (e.g. the + // background insight-notifier / cron push paths) can hand in a service-role + // admin client. The cookie-backed request client throws "cookies was called + // outside a request scope" when invoked with no active request. + const supabase = client ?? (await createClient()); // .maybeSingle() returns null (no error) when the user row does not exist. // The cron roster sweep can hand us stale or detached player_ids whose // backing `users` row was deleted; .single() would throw PGRST116 diff --git a/src/lib/notifications/push.ts b/src/lib/notifications/push.ts index 6f239cdab..c1e2f4ff0 100644 --- a/src/lib/notifications/push.ts +++ b/src/lib/notifications/push.ts @@ -1,6 +1,6 @@ 'use server'; -import { createClient } from '@/lib/supabase/server'; +import { createAdminClient } from '@/lib/supabase/admin'; import type { NotificationType, NotificationPreferences } from './types'; import { getUserNotificationPreferences } from './email'; @@ -125,14 +125,22 @@ export async function sendPushNotification( data: Record ): Promise<{ success: boolean; error?: string }> { try { - // Check user preferences - const prefs = await getUserNotificationPreferences(userId); + // Server-side dispatcher: use the service-role admin client for ALL DB + // access here. This works in BOTH request and background contexts (cron, + // roster-sweep, the post-write insight-notifier) — the cookie-backed + // request client throws "cookies was called outside a request scope" when + // invoked outside a request, and RLS would also hide the *recipient's* + // device tokens whenever the sender differs from the recipient. The + // send-apns-push Edge Function below already authenticates with this same + // service-role key. + const supabase = createAdminClient(); + + // Check user preferences (recipient row, read via the admin client above). + const prefs = await getUserNotificationPreferences(userId, supabase); if (!shouldSendPush(type, prefs)) { return { success: true }; // User opted out } - const supabase = await createClient(); - // Get user's active device tokens // eslint-disable-next-line @typescript-eslint/no-explicit-any const { data: tokens, error: tokenError } = await (supabase as any) diff --git a/src/test/lib/notifications/push.test.ts b/src/test/lib/notifications/push.test.ts new file mode 100644 index 000000000..828d73375 --- /dev/null +++ b/src/test/lib/notifications/push.test.ts @@ -0,0 +1,118 @@ +/** + * Regression test for src/lib/notifications/push.ts — `sendPushNotification`. + * + * Guards the production fix for: + * "insight-notifier: push send reported failure: `cookies` was called + * outside a request scope" + * + * sendPushNotification is invoked from BACKGROUND contexts (the post-write + * insight-notifier, the event-reminders cron) where there is no active request, + * so it must NOT use the cookie-backed request client. It now reads prefs + + * device tokens through the service-role admin client (the same key the + * send-apns-push Edge Function already uses), which also lets it read a + * *recipient's* device tokens when the sender differs from the recipient. + * + * The cookie client (`@/lib/supabase/server`) is mocked to THROW so the test + * fails loudly if any code path regresses back to it. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { DEFAULT_NOTIFICATION_PREFERENCES } from '@/lib/notifications/types'; + +// --- Controllable mock state ---------------------------------------------- + +let prefs: Record = { ...DEFAULT_NOTIFICATION_PREFERENCES, push_events: true }; +let deviceTokens: Array<{ token: string; platform: string }> = []; + +// The cookie-backed request client MUST NOT be touched in a background context. +const serverCreateClient = vi.fn(async () => { + throw new Error('`cookies` was called outside a request scope'); +}); +vi.mock('@/lib/supabase/server', () => ({ + createClient: serverCreateClient, +})); + +// Service-role admin client — chainable fake for `users` + `device_tokens`. +const adminFrom = vi.fn((table: string) => { + if (table === 'users') { + return { + select: () => ({ + eq: () => ({ + maybeSingle: async () => ({ + data: { notification_preferences: prefs }, + error: null, + }), + }), + }), + }; + } + // device_tokens + return { + select: () => ({ + eq: () => ({ + // .from().select().eq('user_id').eq('active') is awaited directly. + eq: async () => ({ data: deviceTokens, error: null }), + }), + }), + update: () => ({ + eq: async () => ({ data: null, error: null }), + }), + }; +}); +const createAdminClient = vi.fn(() => ({ from: adminFrom })); +vi.mock('@/lib/supabase/admin', () => ({ createAdminClient })); + +// --- fetch mock ----------------------------------------------------------- + +const fetchSpy = vi.fn(async () => ({ ok: true, text: async () => '' })); + +let consoleErrorSpy: ReturnType; + +beforeEach(() => { + prefs = { ...DEFAULT_NOTIFICATION_PREFERENCES, push_events: true }; + deviceTokens = [{ token: 'devicetoken123', platform: 'ios' }]; + serverCreateClient.mockClear(); + adminFrom.mockClear(); + createAdminClient.mockClear(); + fetchSpy.mockClear(); + vi.stubGlobal('fetch', fetchSpy); + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + consoleErrorSpy.mockRestore(); +}); + +describe('sendPushNotification (background-safe)', () => { + it('delivers via the service-role admin client without touching the cookie request client', async () => { + const { sendPushNotification } = await import('@/lib/notifications/push'); + const result = await sendPushNotification('coachhelm_insight', 'user-1', { insightTitle: 'x' }); + + expect(result.success).toBe(true); + expect(createAdminClient).toHaveBeenCalled(); + // The regression guard: the no-request-scope cookie client is never used. + expect(serverCreateClient).not.toHaveBeenCalled(); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it('honors the recipient opt-out (push_events=false) and sends nothing', async () => { + prefs = { ...DEFAULT_NOTIFICATION_PREFERENCES, push_events: false }; + const { sendPushNotification } = await import('@/lib/notifications/push'); + const result = await sendPushNotification('coachhelm_insight', 'user-1', { insightTitle: 'x' }); + + expect(result.success).toBe(true); + expect(serverCreateClient).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('returns success when the recipient has no active device tokens', async () => { + deviceTokens = []; + const { sendPushNotification } = await import('@/lib/notifications/push'); + const result = await sendPushNotification('coachhelm_insight', 'user-1', { insightTitle: 'x' }); + + expect(result.success).toBe(true); + expect(serverCreateClient).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/supabase/tests/rls/golf_coach_insights_cross_tenant_select.sql b/supabase/tests/rls/golf_coach_insights_cross_tenant_select.sql index bf2f24223..08a63caf0 100644 --- a/supabase/tests/rls/golf_coach_insights_cross_tenant_select.sql +++ b/supabase/tests/rls/golf_coach_insights_cross_tenant_select.sql @@ -83,9 +83,9 @@ BEGIN (id, coach_id, player_id, team_id, insight_type, title, content, priority, category) VALUES ('00000000-0000-0000-0000-0000000000af', v_coach_a, v_player_a, v_team_a, - 'value_derived', 'A insight', 'tenant A only', 'high', 'putting'), + 'putting', 'A insight', 'tenant A only', 'high', 'putting'), ('00000000-0000-0000-0000-0000000000bf', v_coach_b, v_player_b, v_team_b, - 'value_derived', 'B insight', 'tenant B only', 'high', 'putting') + 'putting', 'B insight', 'tenant B only', 'high', 'putting') ON CONFLICT DO NOTHING; END $$;