Skip to content

Commit 924945d

Browse files
authored
refactor: eliminate workarounds, replace with idiomatic patterns (#70)
## hooks.server.ts - Remove redundant getSession() call (getUser() is sufficient for JWT validation) - Remove manual expired-session check (supabase handles this internally) - Replace 2 separate DB queries (memberships + subscriptions) with get_user_context RPC — consistent with layout.server.ts, saves 2 round-trips per protected request ## routes/+page.server.ts - Replace raw memberships query with get_user_context RPC (consistent with hooks) ## admin/settings/+page.server.ts action - Replace get_user_context RPC call (fetches too much) with targeted memberships query for just tenant_id — right tool for the job ## format.ts - Remove locale hack in formatCurrency (was manually mapping EUR→de-DE, USD→en-US, GBP→en-GB) - Use undefined locale so Intl.NumberFormat picks the correct locale for the currency automatically ## supabase.ts - Replace typeof window !== 'undefined' with browser from $app/environment (consistent with rest of codebase) ## (app)/+layout.svelte - Change $effect → $effect.pre for session/settings init — ensures stores are populated BEFORE first render, not after ## bookings/[type]/+page.svelte - Remove dead client-side isValidType guard (server already throws 404 for invalid types) - Use $derived for type/icon to properly track reactive data prop changes - Use satisfies for icons record type safety ## SQL: search_products - Change 'english' dictionary → 'simple' for language-agnostic full-text search - Adds ILIKE fallback for partial/typo matching - Fixes Greek product name search that was broken with English stemming ## SQL: mv_best_sellers - Remove synchronous refresh trigger that was blocking every order INSERT transaction - Convert to scheduled refresh via keep-alive cron (daily) using service_role RPC - Add refresh_mv_best_sellers() call to keep-alive endpoint ## mocks.ts - Add rpc() mock to createSupabaseMock (now required since hooks uses get_user_context) - Build get_user_context response from existing config shape - Add rpc to return type signature ## tests - Remove session-expiry test (the feature was removed from hooks — getSession no longer called)
1 parent a57af76 commit 924945d

File tree

12 files changed

+75
-71
lines changed

12 files changed

+75
-71
lines changed

src/hooks.server.test.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -54,23 +54,6 @@ describe("hooks.server", () => {
5454
});
5555
});
5656

57-
describe("session expiry", () => {
58-
it("signs out and redirects if session is expired", async () => {
59-
const config = scenarios.activeSubscription("admin");
60-
const expiredAt = Math.floor(Date.now() / 1000) - 3600;
61-
const mock = createSupabaseMock(config);
62-
// Override getSession to return expired session, add signOut stub
63-
mock.auth.getSession = vi.fn().mockResolvedValue({
64-
data: { session: { expires_at: expiredAt, access_token: "x", refresh_token: "y", user: {} } },
65-
error: null,
66-
});
67-
(mock.auth as Record<string, unknown>).signOut = vi.fn().mockResolvedValue({});
68-
mockSupabaseClient.mockReturnValue(mock);
69-
const event = { url: new URL("http://localhost/admin"), cookies: { get: vi.fn(), set: vi.fn(), delete: vi.fn() }, locals: {}, request: new Request("http://localhost/admin") };
70-
await expect(runHandle(event)).rejects.toMatchObject({ location: "/" });
71-
});
72-
});
73-
7457
describe("subscription validation", () => {
7558
it("redirects to /billing if expired", async () => {
7659
await expect(runHandle(createEvent("/admin", scenarios.expiredTrial()))).rejects.toMatchObject({ location: "/billing" });

src/hooks.server.ts

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,34 +21,27 @@ export const handle: Handle = async ({ event, resolve }) => {
2121

2222
event.locals.supabase = supabase;
2323
const { data: { user } } = await supabase.auth.getUser();
24-
const { data: { session } } = await supabase.auth.getSession();
2524
event.locals.user = user;
2625

27-
// Reject missing or expired sessions
28-
if (session && (!session.expires_at || new Date(session.expires_at * 1000) < new Date())) {
29-
await supabase.auth.signOut();
30-
throw redirect(307, "/");
31-
}
32-
3326
const path = event.url.pathname;
3427
const isPublic = publicRoutes.includes(path) || path.startsWith("/api/");
3528
const isAuthOnly = authOnlyRoutes.includes(path);
3629

37-
// Fetch membership + subscription once, lazily
38-
let _membership: { role: MemberRole | null; tenantId: string | null; facilityId: string | null; active: boolean } | null = null;
30+
// Fetch membership + subscription once via RPC, lazily (same as layout.server.ts)
31+
let _ctx: { role: MemberRole | null; tenantId: string | null; facilityId: string | null; active: boolean } | null = null;
3932
const getMembership = async (): Promise<{ role: MemberRole | null; tenantId: string | null; facilityId: string | null; active: boolean }> => {
40-
if (_membership) return _membership;
41-
if (!user) return (_membership = { role: null, tenantId: null, facilityId: null, active: false });
33+
if (_ctx) return _ctx;
34+
if (!user) return (_ctx = { role: null, tenantId: null, facilityId: null, active: false });
4235

43-
const { data: m } = await supabase.from("memberships").select("role, tenant_id, facility_id").eq("user_id", user.id).order("is_primary", { ascending: false }).limit(1).single();
44-
if (!m) return (_membership = { role: null, tenantId: null, facilityId: null, active: false });
36+
const { data: ctx } = await supabase.rpc("get_user_context", { p_user_id: user.id });
37+
if (!ctx?.membership) return (_ctx = { role: null, tenantId: null, facilityId: null, active: false });
4538

46-
const { data: s } = await supabase.from("subscriptions").select("status, current_period_end, trial_end").eq("tenant_id", m.tenant_id).single();
47-
const now = Date.now();
48-
const active = s && ["trialing", "active"].includes(s.status as SubscriptionStatus) &&
49-
((s.current_period_end && new Date(s.current_period_end).getTime() > now) || (s.trial_end && new Date(s.trial_end).getTime() > now));
39+
const sub = ctx.subscription;
40+
const now = new Date();
41+
const active = sub && ["trialing", "active"].includes(sub.status as SubscriptionStatus) &&
42+
((sub.periodEnd && new Date(sub.periodEnd) > now) || (sub.trialEnd && new Date(sub.trialEnd) > now));
5043

51-
return (_membership = { role: m.role as MemberRole, tenantId: m.tenant_id, facilityId: m.facility_id, active: !!active });
44+
return (_ctx = { role: ctx.membership.role as MemberRole, tenantId: ctx.membership.tenantId, facilityId: ctx.membership.facilityId, active: !!active });
5245
};
5346

5447
if (path === "/" && user) {

src/lib/testing/mocks.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export interface SupabaseMockConfig { user?: MockUser | null; memberships?: Mock
2020
export const createSupabaseMock = (config: SupabaseMockConfig = {}): {
2121
auth: { getUser: ReturnType<typeof vi.fn>; getSession: ReturnType<typeof vi.fn> };
2222
from: ReturnType<typeof vi.fn>;
23+
rpc: ReturnType<typeof vi.fn>;
2324
} => {
2425
const { user, memberships = [], subscriptions = [], tenants = [] } = config;
2526
const authUser: User | null = user ? { id: user.id, email: user.email, aud: "authenticated", role: "authenticated", email_confirmed_at: new Date().toISOString(), phone: "", confirmed_at: new Date().toISOString(), last_sign_in_at: new Date().toISOString(), app_metadata: {}, user_metadata: { full_name: "Test" }, identities: [], created_at: new Date().toISOString(), updated_at: new Date().toISOString() } : null;
@@ -52,9 +53,26 @@ export const createSupabaseMock = (config: SupabaseMockConfig = {}): {
5253
return b;
5354
};
5455

56+
// Build get_user_context RPC response from config
57+
const primaryMembership = memberships[0] ?? null;
58+
const subscription = primaryMembership ? subscriptions.find(s => s.tenantId === primaryMembership.tenantId) ?? null : null;
59+
const tenant = primaryMembership ? tenants.find(t => t.id === primaryMembership.tenantId) ?? null : null;
60+
61+
const userContextData = primaryMembership ? {
62+
membership: { role: primaryMembership.role, tenantId: primaryMembership.tenantId, facilityId: primaryMembership.facilityId },
63+
subscription: subscription ? { status: subscription.status, trialEnd: subscription.trialEnd?.toISOString() ?? null, periodEnd: subscription.currentPeriodEnd?.toISOString() ?? null } : null,
64+
tenant: tenant ? { id: tenant.id, name: tenant.name, settings: {} } : null,
65+
profile: { fullName: authUser?.user_metadata?.full_name ?? "Test" },
66+
activeSession: null,
67+
} : null;
68+
5569
return {
5670
auth: { getUser: vi.fn().mockResolvedValue({ data: { user: authUser }, error: null }), getSession: vi.fn().mockResolvedValue({ data: { session }, error: null }) },
5771
from: vi.fn().mockImplementation((t: string) => createBuilder(t)),
72+
rpc: vi.fn().mockImplementation((fn: string) => {
73+
if (fn === "get_user_context") return Promise.resolve({ data: userContextData, error: null });
74+
return Promise.resolve({ data: null, error: null });
75+
}),
5876
};
5977
};
6078

src/lib/utils/format.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ export function formatDate(date: string | Date, s?: FormatSettings | null, inclu
2222

2323
export function formatCurrency(value: number, s?: FormatSettings | null): string {
2424
const currency = s?.currency_code ?? "EUR";
25-
return new Intl.NumberFormat(currency === "USD" ? "en-US" : currency === "GBP" ? "en-GB" : "de-DE", { style: "currency", currency }).format(value);
25+
// Use undefined locale so the runtime picks the best locale for the currency
26+
return new Intl.NumberFormat(undefined, { style: "currency", currency }).format(value);
2627
}
2728

2829
const CURRENCY_SYMBOLS: Record<string, string> = {

src/lib/utils/supabase.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createClient } from "@supabase/supabase-js";
22
import { env as publicEnv } from "$env/dynamic/public";
3+
import { browser } from "$app/environment";
34

45
const { PUBLIC_SUPABASE_URL: url, PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY: anonKey } =
56
publicEnv as {
@@ -15,7 +16,7 @@ export const supabase = createClient(url, anonKey);
1516

1617
// Keep server and client in sync: when auth state changes on the client,
1718
// update the HTTP-only cookies on the server so SSR sees the session on reload.
18-
if (typeof window !== "undefined") {
19+
if (browser) {
1920
const postAuthCallback = async (
2021
event: string,
2122
session: { access_token: string; refresh_token: string } | null,

src/routes/(app)/+layout.svelte

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
77
const { data, children } = $props();
88
9-
// Sync server data into client stores on every navigation
10-
$effect(() => {
9+
// Sync server data into client stores before render on every navigation
10+
$effect.pre(() => {
1111
session.setUser(data.user);
1212
if (data.settings) settings.setSettings(data.settings);
1313
});

src/routes/(app)/admin/settings/+page.server.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,14 @@ export const actions: Actions = {
2828
const { supabase, user } = locals;
2929
if (!user) return fail(401, { error: "Unauthorized" });
3030

31-
const { data: ctx } = await supabase.rpc("get_user_context", { p_user_id: user.id });
32-
const tenantId = ctx?.membership?.tenantId;
31+
const { data: m } = await supabase
32+
.from("memberships")
33+
.select("tenant_id")
34+
.eq("user_id", user.id)
35+
.order("is_primary", { ascending: false })
36+
.limit(1)
37+
.single();
38+
const tenantId = m?.tenant_id;
3339
if (!tenantId) return fail(400, { error: "No tenant" });
3440

3541
const formData = await request.formData();
Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
<script lang="ts">
22
import BookingPage from "$lib/components/features/booking-page.svelte";
33
import { Cake, Dribbble, Calendar, MoreHorizontal } from "@lucide/svelte";
4-
import { BOOKING_TYPE, type BookingTypeValue } from "$lib/constants";
4+
import { BOOKING_TYPE } from "$lib/constants";
5+
import type { BookingTypeValue } from "$lib/constants";
56
67
const { data } = $props();
78
@@ -10,13 +11,11 @@
1011
[BOOKING_TYPE.FOOTBALL]: Dribbble,
1112
event: Calendar,
1213
other: MoreHorizontal,
13-
} as const;
14+
} satisfies Record<string, typeof Cake>;
1415
15-
const validTypes = Object.values(BOOKING_TYPE);
16-
type ValidType = BookingTypeValue;
17-
const isValidType = (t: string): t is ValidType => validTypes.includes(t as ValidType);
16+
// data.type is BookingType (includes event/other), cast to BookingTypeValue for BookingPage
17+
const type = $derived(data.type as BookingTypeValue);
18+
const icon = $derived(icons[data.type] ?? Calendar);
1819
</script>
1920

20-
{#if isValidType(data.type)}
21-
<BookingPage type={data.type} bookings={data.bookings} user={data.user} icon={icons[data.type]} />
22-
{/if}
21+
<BookingPage {type} bookings={data.bookings} user={data.user} {icon} />

src/routes/+page.server.ts

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,10 @@ import { getHomeForRole } from "$lib/config/auth";
44

55
export const load: PageServerLoad = async ({ locals }) => {
66
const { user, supabase } = locals;
7-
87
if (!user) return {};
98

10-
const { data: membership } = await supabase
11-
.from("memberships")
12-
.select("role")
13-
.eq("user_id", user.id)
14-
.order("is_primary", { ascending: false })
15-
.limit(1)
16-
.single();
17-
18-
if (!membership) throw redirect(307, "/onboarding");
19-
throw redirect(307, getHomeForRole(membership.role));
9+
// Hooks already redirects authenticated users away from / — this is a safety net
10+
const { data: ctx } = await supabase.rpc("get_user_context", { p_user_id: user.id });
11+
if (!ctx?.membership) throw redirect(307, "/onboarding");
12+
throw redirect(307, getHomeForRole(ctx.membership.role as Parameters<typeof getHomeForRole>[0]));
2013
};

src/routes/api/keep-alive/+server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ export const GET: RequestHandler = async ({ request }) => {
2929
throw error(500, `Keep-alive upsert failed: ${upsertError.message}`);
3030
}
3131

32+
// Also refresh the best-sellers materialized view daily
33+
await admin.rpc("refresh_mv_best_sellers");
34+
3235
return json(
3336
{
3437
success: true,

0 commit comments

Comments
 (0)