From 68cc2a0712ceccf096a309d639e1dc171339cf23 Mon Sep 17 00:00:00 2001
From: Josh Creek <8179928+jcreek@users.noreply.github.com>
Date: Sat, 10 Jan 2026 17:06:52 +0000
Subject: [PATCH] fix(#64): Address PR comments

---
 src/lib/components/SignUp.svelte              |  18 ++-
 src/routes/+page.svelte                       |  32 ++++-
 src/routes/api/pokedexes/+server.ts           |  78 +++++++++++++
 src/routes/api/pokedexes/[id]/+server.ts      | 110 ++++++++++++++++++
 ...10144000_fix_stats_functions_ambiguity.sql |  15 ++-
 ...0110150000_add_pokedex_entries_mapping.sql |  46 ++++++++
 ...001_populate_existing_pokedex_mappings.sql |  25 ++++
 7 files changed, 308 insertions(+), 16 deletions(-)
 create mode 100644 supabase/migrations/20260110150000_add_pokedex_entries_mapping.sql
 create mode 100644 supabase/migrations/20260110150001_populate_existing_pokedex_mappings.sql

diff --git a/src/lib/components/SignUp.svelte b/src/lib/components/SignUp.svelte
index 09951f4..df40179 100644
--- a/src/lib/components/SignUp.svelte
+++ b/src/lib/components/SignUp.svelte
@@ -1,4 +1,7 @@
 <script lang="ts">
+	import { createEventDispatcher } from 'svelte';
+	const dispatch = createEventDispatcher();
+
 	let email = '';
 	let password = '';
 
@@ -17,13 +20,18 @@
 			});
 
 			if (error) {
-				throw error;
+				console.error('Sign up error:', error);
+				alert(`Sign up failed: ${error.message}`);
+				return;
 			}
 
-			// Handle success (optional)
-		} catch (error) {
-			console.error('Sign up error:', error.message);
-			// Handle error
+			if (data) {
+				// Emit signedUp event to notify parent component
+				dispatch('signedUp', {});
+			}
+		} catch (err) {
+			console.error('Sign up error:', err);
+			alert('Sign up failed');
 		}
 	}
 </script>
diff --git a/src/routes/+page.svelte b/src/routes/+page.svelte
index 575dd80..f5f1d0b 100644
--- a/src/routes/+page.svelte
+++ b/src/routes/+page.svelte
@@ -1,8 +1,9 @@
 <script lang="ts">
-	import { onDestroy } from 'svelte';
+	import { onDestroy, onMount } from 'svelte';
 	import { user } from '$lib/stores/user.js';
 	import { type User } from '@supabase/auth-js';
 	import SignUp from '$lib/components/SignUp.svelte';
+	import { goto } from '$app/navigation';
 
 	export let data;
 	let { supabase, stats } = data;
@@ -14,6 +15,28 @@
 	});
 	onDestroy(unsubscribe);
 
+	onMount(() => {
+		checkSessionAndRedirect();
+	});
+
+	async function checkSessionAndRedirect() {
+		try {
+			const {
+				data: { session }
+			} = await supabase.auth.getSession();
+
+			if (session) {
+				await goto('/my-pokedexes');
+			}
+		} catch (error) {
+			console.error('Error checking session:', error);
+		}
+	}
+
+	async function handleSignedUp() {
+		await goto('/welcome');
+	}
+
 	// Format large numbers (e.g., 1000 -> 1K, 1000000 -> 1M)
 	function formatNumber(num: number): string {
 		if (num >= 1000000) {
@@ -140,7 +163,7 @@
 		<div class="card shrink-0 w-full max-w-sm shadow-2xl bg-neutral">
 			<div class="card-body">
 				<h2 class="card-title text-2xl mb-4">Get Started Free</h2>
-				<SignUp {supabase} />
+				<SignUp {supabase} on:signedUp={handleSignedUp} />
 			</div>
 		</div>
 	</div>
@@ -153,17 +176,14 @@
 			<div class="stat">
 				<div class="stat-title">Pokémon caught</div>
 				<div class="stat-value text-primary">{pokemonCaught}</div>
-				<div class="stat-desc">Since January 2026</div>
 			</div>
 			<div class="stat">
 				<div class="stat-title">Users</div>
 				<div class="stat-value text-primary">{users}</div>
-				<div class="stat-desc">↗︎ 400 (22%)</div>
 			</div>
 			<div class="stat">
 				<div class="stat-title">Living Dexes Completed</div>
 				<div class="stat-value text-primary">{livingDexesCompleted}</div>
-				<div class="stat-desc">Trainers worldwide</div>
 			</div>
 		</div>
 	</div>
@@ -325,7 +345,7 @@
 			<div class="card shrink-0 w-full max-w-sm shadow-2xl bg-neutral">
 				<div class="card-body">
 					<h3 class="card-title text-xl mb-4">Sign Up Now</h3>
-					<SignUp {supabase} />
+					<SignUp {supabase} on:signedUp={handleSignedUp} />
 				</div>
 			</div>
 		</div>
diff --git a/src/routes/api/pokedexes/+server.ts b/src/routes/api/pokedexes/+server.ts
index 9365ab8..cf3758b 100644
--- a/src/routes/api/pokedexes/+server.ts
+++ b/src/routes/api/pokedexes/+server.ts
@@ -3,6 +3,80 @@ import type { Pokedex } from '$lib/models/Pokedex';
 import PokedexRepository from '$lib/repositories/PokedexRepository';
 import { requireAuth } from '$lib/utils/auth';
 import type { RequestEvent } from '@sveltejs/kit';
+import type { SupabaseClient } from '@supabase/supabase-js';
+
+/**
+ * Calculate expected pokedex entries based on pokedex configuration
+ * Applies the same filtering logic as CombinedDataRepository
+ */
+async function calculateExpectedEntries(
+	supabase: SupabaseClient,
+	pokedex: Pokedex
+): Promise<number[]> {
+	let query = supabase.from('pokedex_entries').select('id');
+
+	// Apply form filter: if isFormDex is false, only include base forms
+	// Base forms have form IS NULL, except Unown which has no base form (use 'A')
+	if (!pokedex.isFormDex) {
+		query = query.or('form.is.null,and(pokemon.eq.Unown,form.eq.A)');
+	}
+
+	// Apply region filter: if gameScope is specified, filter by region
+	if (pokedex.gameScope) {
+		// Determine region from gameScope using region_game_mappings
+		const { data: regionData } = await supabase
+			.from('region_game_mappings')
+			.select('region')
+			.eq('game', pokedex.gameScope)
+			.single();
+
+		if (regionData?.region) {
+			query = query.eq('regionToCatchIn', regionData.region);
+		}
+	}
+
+	// Apply game filter: if gameScope is specified, filter by gamesToCatchIn array
+	if (pokedex.gameScope) {
+		query = query.contains('gamesToCatchIn', [pokedex.gameScope]);
+	}
+
+	const { data, error } = await query;
+
+	if (error) {
+		console.error('Error calculating expected entries:', error);
+		return [];
+	}
+
+	return data?.map((entry) => entry.id) || [];
+}
+
+/**
+ * Populate pokedex_entries_mapping table for a pokedex
+ */
+async function populatePokedexMappings(
+	supabase: SupabaseClient,
+	pokedexId: string,
+	pokedex: Pokedex
+): Promise<void> {
+	const expectedEntryIds = await calculateExpectedEntries(supabase, pokedex);
+
+	if (expectedEntryIds.length === 0) {
+		console.warn(`No expected entries calculated for pokedex ${pokedexId}`);
+		return;
+	}
+
+	const mappings = expectedEntryIds.map((pokedexEntryId) => ({
+		pokedexId,
+		pokedexEntryId
+	}));
+
+	const { error } = await supabase.from('pokedex_entries_mapping').insert(mappings);
+
+	if (error) {
+		console.error('Error populating pokedex mappings:', error);
+		throw new Error(`Failed to populate pokedex mappings: ${error.message}`);
+	}
+}
 
 // GET: List all pokedexes for user
 export const GET = async (event: RequestEvent) => {
@@ -31,6 +105,10 @@ export const POST = async (event: RequestEvent) => {
 
 		const repo = new PokedexRepository(event.locals.supabase, userId);
 		const pokedex = await repo.create(data);
+
+		// Populate pokedex_entries_mapping table with expected entries
+		await populatePokedexMappings(event.locals.supabase, pokedex._id, pokedex);
+
 		return json(pokedex);
 	} catch (err) {
 		console.error('Error in POST /api/pokedexes:', err);
diff --git a/src/routes/api/pokedexes/[id]/+server.ts b/src/routes/api/pokedexes/[id]/+server.ts
index c4b71e9..41b80bf 100644
--- a/src/routes/api/pokedexes/[id]/+server.ts
+++ b/src/routes/api/pokedexes/[id]/+server.ts
@@ -3,6 +3,103 @@ import type { Pokedex } from '$lib/models/Pokedex';
 import PokedexRepository from '$lib/repositories/PokedexRepository';
 import { requireAuth } from '$lib/utils/auth';
 import type { RequestEvent } from '@sveltejs/kit';
+import type { SupabaseClient } from '@supabase/supabase-js';
+
+/**
+ * Calculate expected pokedex entries based on pokedex configuration
+ * Applies the same filtering logic as CombinedDataRepository
+ */
+async function calculateExpectedEntries(
+	supabase: SupabaseClient,
+	pokedex: Pokedex
+): Promise<number[]> {
+	let query = supabase.from('pokedex_entries').select('id');
+
+	// Apply form filter: if isFormDex is false, only include base forms
+	// Base forms have form IS NULL, except Unown which has no base form (use 'A')
+	if (!pokedex.isFormDex) {
+		query = query.or('form.is.null,and(pokemon.eq.Unown,form.eq.A)');
+	}
+
+	// Apply region filter: if gameScope is specified, filter by region
+	if (pokedex.gameScope) {
+		// Determine region from gameScope using region_game_mappings
+		const { data: regionData } = await supabase
+			.from('region_game_mappings')
+			.select('region')
+			.eq('game', pokedex.gameScope)
+			.single();
+
+		if (regionData?.region) {
+			query = query.eq('regionToCatchIn', regionData.region);
+		}
+	}
+
+	// Apply game filter: if gameScope is specified, filter by gamesToCatchIn array
+	if (pokedex.gameScope) {
+		query = query.contains('gamesToCatchIn', [pokedex.gameScope]);
+	}
+
+	const { data, error } = await query;
+
+	if (error) {
+		console.error('Error calculating expected entries:', error);
+		return [];
+	}
+
+	return data?.map((entry) => entry.id) || [];
+}
+
+/**
+ * Populate pokedex_entries_mapping table for a pokedex
+ */
+async function populatePokedexMappings(
+	supabase: SupabaseClient,
+	pokedexId: string,
+	pokedex: Pokedex
+): Promise<void> {
+	const expectedEntryIds = await calculateExpectedEntries(supabase, pokedex);
+
+	if (expectedEntryIds.length === 0) {
+		console.warn(`No expected entries calculated for pokedex ${pokedexId}`);
+		return;
+	}
+
+	const mappings = expectedEntryIds.map((pokedexEntryId) => ({
+		pokedexId,
+		pokedexEntryId
+	}));
+
+	const { error } = await supabase.from('pokedex_entries_mapping').insert(mappings);
+
+	if (error) {
+		console.error('Error populating pokedex mappings:', error);
+		throw new Error(`Failed to populate pokedex mappings: ${error.message}`);
+	}
+}
+
+/**
+ * Recalculate pokedex_entries_mapping table for a pokedex (used on update)
+ */
+async function recalculatePokedexMappings(
+	supabase: SupabaseClient,
+	pokedexId: string,
+	pokedex: Pokedex
+): Promise<void> {
+	// Delete old mappings
+	const { error: deleteError } = await supabase
+		.from('pokedex_entries_mapping')
+		.delete()
+		.eq('pokedexId', pokedexId);
+
+	if (deleteError) {
+		console.error('Error deleting old pokedex mappings:', deleteError);
+		throw new Error(`Failed to delete old pokedex mappings: ${deleteError.message}`);
+	}
+
+	// Populate new mappings
+	await populatePokedexMappings(supabase, pokedexId, pokedex);
+}
 
 // GET: Get single pokedex by ID
 export const GET = async (event: RequestEvent) => {
@@ -63,6 +160,19 @@ export const PUT = async (event: RequestEvent) => {
 			return json({ error: 'Pokedex not found' }, { status: 404 });
 		}
 
+		// Recalculate pokedex_entries_mapping if configuration changed
+		// Only recalculate if type flags or gameScope changed
+		const configChanged =
+			data.isFormDex !== undefined ||
+			data.gameScope !== undefined ||
+			(data.isLivingDex !== undefined && pokedex.isLivingDex !== data.isLivingDex) ||
+			(data.isShinyDex !== undefined && pokedex.isShinyDex !== data.isShinyDex) ||
+			(data.isOriginDex !== undefined && pokedex.isOriginDex !== data.isOriginDex);
+
+		if (configChanged) {
+			await recalculatePokedexMappings(event.locals.supabase, id, { ...pokedex, ...data });
+		}
+
 		return json(pokedex);
 	} catch (err) {
 		console.error(err);
diff --git a/supabase/migrations/20260110144000_fix_stats_functions_ambiguity.sql b/supabase/migrations/20260110144000_fix_stats_functions_ambiguity.sql
index 106d302..a998986 100644
--- a/supabase/migrations/20260110144000_fix_stats_functions_ambiguity.sql
+++ b/supabase/migrations/20260110144000_fix_stats_functions_ambiguity.sql
@@ -30,13 +30,18 @@ BEGIN
   FROM auth.users;
 
   -- Calculate current stats: count of completed pokedexes
-  -- A pokedex is completed when all its entries have caught = true
+  -- A pokedex is completed when all its expected entries have caught = true
+  -- Join catch_records to pokedex_entries_mapping to validate against explicit expected entries
   SELECT COUNT(*) INTO v_completed_pokedexes
   FROM (
-    SELECT "pokedexId"
-    FROM catch_records
-    GROUP BY "pokedexId"
-    HAVING COUNT(*) = COUNT(*) FILTER (WHERE caught = true)
+    SELECT cr."pokedexId"
+    FROM catch_records cr
+    INNER JOIN pokedex_entries_mapping pem
+      ON cr."pokedexId" = pem."pokedexId"
+      AND cr."pokedexEntryId" = pem."pokedexEntryId"
+    GROUP BY cr."pokedexId"
+    HAVING COUNT(*) = COUNT(*) FILTER (WHERE cr.caught = true)
+      AND COUNT(*) = (SELECT COUNT(*) FROM pokedex_entries_mapping WHERE "pokedexId" = cr."pokedexId")
   ) completed;
 
   -- Update cache using UPSERT pattern
diff --git a/supabase/migrations/20260110150000_add_pokedex_entries_mapping.sql b/supabase/migrations/20260110150000_add_pokedex_entries_mapping.sql
new file mode 100644
index 0000000..dc8ca88
--- /dev/null
+++ b/supabase/migrations/20260110150000_add_pokedex_entries_mapping.sql
@@ -0,0 +1,46 @@
+-- Add pokedex_entries_mapping table to explicitly define which entries belong to each pokedex
+-- This fixes the stats functions ambiguity issue by providing an explicit definition of expected entries
+
+-- Create pokedex_entries_mapping table
+CREATE TABLE pokedex_entries_mapping (
+  id BIGSERIAL PRIMARY KEY,
+  "pokedexId" UUID NOT NULL REFERENCES pokedexes(id) ON DELETE CASCADE,
+  "pokedexEntryId" BIGINT NOT NULL REFERENCES pokedex_entries(id) ON DELETE CASCADE,
+  UNIQUE("pokedexId", "pokedexEntryId")
+);
+
+-- Create indexes for performance
+CREATE INDEX idx_pokedex_entries_mapping_pokedex_id ON pokedex_entries_mapping("pokedexId");
+CREATE INDEX idx_pokedex_entries_mapping_entry_id ON pokedex_entries_mapping("pokedexEntryId");
+
+-- Enable RLS
+ALTER TABLE pokedex_entries_mapping ENABLE ROW LEVEL SECURITY;
+
+-- RLS Policies
+
+-- Public read access (mapping is reference data)
+CREATE POLICY "Anyone can view pokedex entries mapping"
+  ON pokedex_entries_mapping
+  FOR SELECT USING (true);
+
+-- Users can only insert mappings for their own pokedexes
+CREATE POLICY "Users can insert own pokedex mappings"
+  ON pokedex_entries_mapping
+  FOR INSERT WITH CHECK (
+    EXISTS (
+      SELECT 1 FROM pokedexes
+      WHERE pokedexes.id = pokedex_entries_mapping."pokedexId"
+      AND pokedexes."userId" = auth.uid()
+    )
+  );
+
+-- Users can only delete mappings for their own pokedexes
+CREATE POLICY "Users can delete own pokedex mappings"
+  ON pokedex_entries_mapping
+  FOR DELETE USING (
+    EXISTS (
+      SELECT 1 FROM pokedexes
+      WHERE pokedexes.id = pokedex_entries_mapping."pokedexId"
+      AND pokedexes."userId" = auth.uid()
+    )
+  );
diff --git a/supabase/migrations/20260110150001_populate_existing_pokedex_mappings.sql b/supabase/migrations/20260110150001_populate_existing_pokedex_mappings.sql
new file mode 100644
index 0000000..d1b69bb
--- /dev/null
+++ b/supabase/migrations/20260110150001_populate_existing_pokedex_mappings.sql
@@ -0,0 +1,25 @@
+-- Populate pokedex_entries_mapping for existing pokedexes
+-- This is a one-time migration to backfill data for pokedexes created before the mapping table
+
+-- Insert mappings for all existing pokedexes
+-- Apply the same filtering logic that the application uses to determine expected entries
+INSERT INTO pokedex_entries_mapping ("pokedexId", "pokedexEntryId")
+SELECT
+  p.id AS "pokedexId",
+  pe.id AS "pokedexEntryId"
+FROM pokedexes p
+CROSS JOIN pokedex_entries pe
+WHERE
+  -- Form filter: if isFormDex is false, only include base forms
+  -- Base forms have form IS NULL, except Unown which has no base form (use 'A')
+  (p."isFormDex" = true OR (pe.form IS NULL OR (pe.pokemon = 'Unown' AND pe.form = 'A')))
+  -- Game scope filter: if gameScope is specified, filter by game
+  AND (p."gameScope" IS NULL OR pe."gamesToCatchIn" @> ARRAY[p."gameScope"]::TEXT[])
+  -- Region filter: if gameScope is specified, filter by region
+  AND (p."gameScope" IS NULL OR pe."regionToCatchIn" = (
+    SELECT region FROM region_game_mappings WHERE game = p."gameScope" LIMIT 1
+  ))
+ON CONFLICT ("pokedexId", "pokedexEntryId") DO NOTHING;
+
+-- Add comment to document this migration
+COMMENT ON TABLE pokedex_entries_mapping IS 'Junction table defining which pokedex_entries belong to each pokedex. Used for accurate stats calculation.';