fix(#64): Address PR comments

This commit is contained in:
Josh Creek
2026-01-10 18:45:57 +00:00
parent f17b350e4e
commit 5657c2b2ab
6 changed files with 164 additions and 192 deletions
+122
View File
@@ -0,0 +1,122 @@
import type { Pokedex } from '$lib/models/Pokedex';
import type { SupabaseClient } from '@supabase/supabase-js';
/**
* Calculate expected pokedex entries based on pokedex configuration
* Applies the same filtering logic as CombinedDataRepository
* @throws Error if the Supabase query fails
*/
export 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, error: regionError } = await supabase
.from('region_game_mappings')
.select('region')
.eq('game', pokedex.gameScope)
.maybeSingle();
if (regionError) {
console.error('Error looking up region for gameScope:', pokedex.gameScope, regionError);
throw new Error(
`Failed to lookup region for gameScope "${pokedex.gameScope}": ${regionError.message}`
);
}
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);
throw new Error(`Failed to calculate expected entries: ${error.message}`);
}
return data?.map((entry) => entry.id) || [];
}
/**
* Populate pokedex_entries_mapping table for a pokedex
* Uses chunked upsert to avoid UNIQUE constraint violations and request size limits
* @throws Error if the Supabase upsert fails
*/
export 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
}));
// Process in chunks to avoid request size limits
const CHUNK_SIZE = 500;
for (let i = 0; i < mappings.length; i += CHUNK_SIZE) {
const chunk = mappings.slice(i, i + CHUNK_SIZE);
const { error } = await supabase.from('pokedex_entries_mapping').upsert(chunk, {
onConflict: 'pokedexId,pokedexEntryId',
ignoreDuplicates: true
});
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)
* Uses a single atomic RPC call to prevent orphaned pokedexes
* @throws Error if the Supabase RPC call fails
*/
export async function recalculatePokedexMappings(
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;
}
// Call the atomic RPC function to delete old mappings and insert new ones
const { error } = await supabase.rpc('recalculate_pokedex_mappings', {
p_pokedex_id: pokedexId,
p_entry_ids: expectedEntryIds
});
if (error) {
console.error('Error recalculating pokedex mappings:', error);
throw new Error(`Failed to recalculate pokedex mappings: ${error.message}`);
}
}
+1 -85
View File
@@ -3,91 +3,7 @@ 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
* @throws Error if the Supabase query fails
*/
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)
.maybeSingle();
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);
throw new Error(`Failed to calculate expected entries: ${error.message}`);
}
return data?.map((entry) => entry.id) || [];
}
/**
* Populate pokedex_entries_mapping table for a pokedex
* Uses chunked upsert to avoid UNIQUE constraint violations and request size limits
* @throws Error if the Supabase upsert fails
*/
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
}));
// Process in chunks to avoid request size limits
const CHUNK_SIZE = 500;
for (let i = 0; i < mappings.length; i += CHUNK_SIZE) {
const chunk = mappings.slice(i, i + CHUNK_SIZE);
const { error } = await supabase.from('pokedex_entries_mapping').upsert(chunk, {
onConflict: 'pokedexId,pokedexEntryId',
ignoreDuplicates: true
});
if (error) {
console.error('Error populating pokedex mappings:', error);
throw new Error(`Failed to populate pokedex mappings: ${error.message}`);
}
}
}
import { populatePokedexMappings } from '$lib/services/PokedexMappingService';
// GET: List all pokedexes for user
export const GET = async (event: RequestEvent) => {
+3 -80
View File
@@ -3,81 +3,7 @@ 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
* @throws Error if the Supabase query fails
*/
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)
.maybeSingle();
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);
throw new Error(`Failed to calculate expected entries: ${error.message}`);
}
return data?.map((entry) => entry.id) || [];
}
/**
* Recalculate pokedex_entries_mapping table for a pokedex (used on update)
* Uses a single atomic RPC call to prevent orphaned pokedexes
*/
async function recalculatePokedexMappings(
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;
}
// Call the atomic RPC function to delete old mappings and insert new ones
const { error } = await supabase.rpc('recalculate_pokedex_mappings', {
p_pokedex_id: pokedexId,
p_entry_ids: expectedEntryIds
});
if (error) {
console.error('Error recalculating pokedex mappings:', error);
throw new Error(`Failed to recalculate pokedex mappings: ${error.message}`);
}
}
import { recalculatePokedexMappings } from '$lib/services/PokedexMappingService';
// GET: Get single pokedex by ID
export const GET = async (event: RequestEvent) => {
@@ -146,13 +72,10 @@ export const PUT = async (event: RequestEvent) => {
}
// Recalculate pokedex_entries_mapping if configuration changed
// Only recalculate if type flags or gameScope actually changed
// Only recalculate if isFormDex or gameScope actually changed (these are the only fields that affect mapping)
const configChanged =
(data.isFormDex !== undefined && existingPokedex.isFormDex !== data.isFormDex) ||
(data.gameScope !== undefined && existingPokedex.gameScope !== data.gameScope) ||
(data.isLivingDex !== undefined && existingPokedex.isLivingDex !== data.isLivingDex) ||
(data.isShinyDex !== undefined && existingPokedex.isShinyDex !== data.isShinyDex) ||
(data.isOriginDex !== undefined && existingPokedex.isOriginDex !== data.isOriginDex);
(data.gameScope !== undefined && existingPokedex.gameScope !== data.gameScope);
if (configChanged) {
await recalculatePokedexMappings(event.locals.supabase, id, pokedex);
+21
View File
@@ -0,0 +1,21 @@
import { redirect } from '@sveltejs/kit';
import PokedexRepository from '$lib/repositories/PokedexRepository';
import type { PageServerLoad } from './$types';
export const load: PageServerLoad = async ({ locals }) => {
const { safeGetSession, supabase } = locals;
const { session, user } = await safeGetSession();
// Require authentication
if (!session || !user) {
throw redirect(303, '/signin');
}
// Fetch pokedexes for the authenticated user
const repo = new PokedexRepository(supabase, user.id);
const pokedexes = await repo.findAll();
return {
pokedexes
};
};
+2 -27
View File
@@ -1,16 +1,11 @@
<script lang="ts">
import { onDestroy, onMount } from 'svelte';
import { goto } from '$app/navigation';
import { user } from '$lib/stores/user';
import type { Pokedex } from '$lib/models/Pokedex';
import PokedexCard from '$lib/components/pokedex/PokedexCard.svelte';
import PokedexForm from '$lib/components/pokedex/PokedexForm.svelte';
let pokedexes: Pokedex[] = [];
const unsubscribe = user.subscribe((value) => {
console.log('[MyPokedexes] User store updated:', value ? 'User found' : 'User null');
});
onDestroy(unsubscribe);
export let data;
let { pokedexes } = data;
let showModal = false;
let editingPokedex: Pokedex | null = null;
let formData: Partial<Pokedex> = {
@@ -25,26 +20,6 @@
$: mode = editingPokedex ? ('edit' as const) : ('create' as const);
onMount(async () => {
// Wait a short time for the user store to be populated (in case of race condition)
// This gives the layout's getUser() time to complete
if (!$user) {
console.log('[MyPokedexes] onMount() - No user yet, waiting 100ms...');
await new Promise((resolve) => setTimeout(resolve, 100));
console.log(
'[MyPokedexes] onMount() - After wait, user store value:',
$user ? 'User found' : 'User null'
);
}
if (!$user) {
goto('/signin');
return;
}
await loadPokedexes();
});
async function loadPokedexes() {
const response = await fetch('/api/pokedexes', { credentials: 'include' });
if (response.ok) {
@@ -11,12 +11,27 @@ SECURITY DEFINER
SET search_path = public, pg_temp
LANGUAGE plpgsql
AS $$
DECLARE
v_user_id UUID;
BEGIN
-- Validate that entry_ids is not empty to avoid accidental deletion
IF p_entry_ids IS NULL OR array_length(p_entry_ids, 1) IS NULL THEN
RAISE EXCEPTION 'entry_ids cannot be null or empty';
END IF;
-- Verify that the caller owns the target pokedex
SELECT "userId" INTO v_user_id
FROM pokedexes
WHERE id = p_pokedex_id;
IF v_user_id IS NULL THEN
RAISE EXCEPTION 'pokedex not found';
END IF;
IF v_user_id != auth.uid() THEN
RAISE EXCEPTION 'not authorized: you do not own this pokedex';
END IF;
-- Delete old mappings and insert new mappings in a single transaction
-- This ensures atomicity - either both operations succeed or both fail
DELETE FROM pokedex_entries_mapping