fix(#64): Address PR comments

This commit is contained in:
Josh Creek
2026-01-10 18:27:01 +00:00
parent 68cc2a0712
commit f17b350e4e
9 changed files with 434 additions and 340 deletions
+17
View File
@@ -24,6 +24,19 @@
onMount(async () => { onMount(async () => {
await getUser(); await getUser();
// Listen for auth state changes to keep the user store in sync
const {
data: { subscription }
} = supabase.auth.onAuthStateChange((event, session) => {
console.log('[Layout] Auth state changed:', event, session ? 'Session found' : 'No session');
if (session) {
localUser = session.user;
} else {
localUser = null;
}
user.set(localUser);
});
if (pwaInfo) { if (pwaInfo) {
const { registerSW } = await import('virtual:pwa-register'); const { registerSW } = await import('virtual:pwa-register');
registerSW({ registerSW({
@@ -41,6 +54,10 @@
} }
}); });
} }
return () => {
subscription.unsubscribe();
};
}); });
async function getUser() { async function getUser() {
+26 -4
View File
@@ -15,6 +15,8 @@
}); });
onDestroy(unsubscribe); onDestroy(unsubscribe);
let isCheckingSession = true;
onMount(() => { onMount(() => {
checkSessionAndRedirect(); checkSessionAndRedirect();
}); });
@@ -30,6 +32,8 @@
} }
} catch (error) { } catch (error) {
console.error('Error checking session:', error); console.error('Error checking session:', error);
} finally {
isCheckingSession = false;
} }
} }
@@ -67,6 +71,14 @@
/> />
</svelte:head> </svelte:head>
{#if isCheckingSession}
<!-- Loading placeholder while checking session -->
<div class="hero bg-base-100 my-36">
<div class="hero-content flex-col">
<span class="loading loading-spinner loading-lg"></span>
</div>
</div>
{:else}
<!-- Hero Section --> <!-- Hero Section -->
<div class="hero bg-base-100 my-36"> <div class="hero bg-base-100 my-36">
<div class="hero-content flex-col lg:flex-row-reverse"> <div class="hero-content flex-col lg:flex-row-reverse">
@@ -78,6 +90,7 @@
class="h-4 w-4" class="h-4 w-4"
viewBox="0 0 20 20" viewBox="0 0 20 20"
fill="currentColor" fill="currentColor"
aria-hidden="true"
> >
<path <path
fill-rule="evenodd" fill-rule="evenodd"
@@ -93,6 +106,7 @@
class="h-4 w-4" class="h-4 w-4"
viewBox="0 0 20 20" viewBox="0 0 20 20"
fill="currentColor" fill="currentColor"
aria-hidden="true"
> >
<path <path
d="M9.049 2.927c.3-.921 1.603-.921 1.902 0l1.07 3.292a1 1 0 00.95.69h3.462c.969 0 1.371 1.24.588 1.81l-2.8 2.034a1 1 0 00-.364 1.118l1.07 3.292c.3.921-.755 1.688-1.54 1.118l-2.8-2.034a1 1 0 00-1.175 0l-2.8 2.034c-.784.57-1.838-.197-1.539-1.118l1.07-3.292a1 1 0 00-.364-1.118L2.98 8.72c-.783-.57-.38-1.81.588-1.81h3.461a1 1 0 00.951-.69l1.07-3.292z" d="M9.049 2.927c.3-.921 1.603-.921 1.902 0l1.07 3.292a1 1 0 00.95.69h3.462c.969 0 1.371 1.24.588 1.81l-2.8 2.034a1 1 0 00-.364 1.118l1.07 3.292c.3.921-.755 1.688-1.54 1.118l-2.8-2.034a1 1 0 00-1.175 0l-2.8 2.034c-.784.57-1.838-.197-1.539-1.118l1.07-3.292a1 1 0 00-.364-1.118L2.98 8.72c-.783-.57-.38-1.81.588-1.81h3.461a1 1 0 00.951-.69l1.07-3.292z"
@@ -106,6 +120,7 @@
class="h-4 w-4" class="h-4 w-4"
viewBox="0 0 20 20" viewBox="0 0 20 20"
fill="currentColor" fill="currentColor"
aria-hidden="true"
> >
<path <path
fill-rule="evenodd" fill-rule="evenodd"
@@ -127,6 +142,7 @@
target="_blank" target="_blank"
rel="noopener noreferrer" rel="noopener noreferrer"
class="btn btn-primary btn-lg" class="btn btn-primary btn-lg"
aria-label="Join the Living Dex Tracker Discord community"
> >
<svg <svg
xmlns="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg"
@@ -145,6 +161,7 @@
target="_blank" target="_blank"
rel="noopener noreferrer" rel="noopener noreferrer"
class="btn btn-outline btn-lg" class="btn btn-outline btn-lg"
aria-label="View the Living Dex Tracker project on GitHub"
> >
<svg <svg
xmlns="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg"
@@ -221,6 +238,7 @@
stroke="currentColor" stroke="currentColor"
viewBox="0 0 24 24" viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
> >
<path <path
stroke-linecap="round" stroke-linecap="round"
@@ -251,6 +269,7 @@
stroke="currentColor" stroke="currentColor"
viewBox="0 0 24 24" viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
> >
<path <path
stroke-linecap="round" stroke-linecap="round"
@@ -263,8 +282,8 @@
<div> <div>
<h2 class="card-title text-xl mb-2">Free & Open Source</h2> <h2 class="card-title text-xl mb-2">Free & Open Source</h2>
<p class="text-base-content/80"> <p class="text-base-content/80">
Completely open source and free to use, enabling the community to contribute updates Completely open source and free to use, enabling the community to contribute
as soon as new Pokémon are released. updates as soon as new Pokémon are released.
</p> </p>
</div> </div>
</div> </div>
@@ -281,6 +300,7 @@
stroke="currentColor" stroke="currentColor"
viewBox="0 0 24 24" viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
> >
<path <path
stroke-linecap="round" stroke-linecap="round"
@@ -311,6 +331,7 @@
stroke="currentColor" stroke="currentColor"
viewBox="0 0 24 24" viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
> >
<path <path
stroke-linecap="round" stroke-linecap="round"
@@ -361,10 +382,11 @@
<p class="text-sm text-base-content/70"> <p class="text-sm text-base-content/70">
Living Dex Tracker is a fan-made project. We do not claim ownership of any Pokémon Living Dex Tracker is a fan-made project. We do not claim ownership of any Pokémon
characters, images, or other content featured on this website. This project is not characters, images, or other content featured on this website. This project is not
affiliated with, endorsed, sponsored, or specifically approved by Nintendo, Game Freak, or affiliated with, endorsed, sponsored, or specifically approved by Nintendo, Game Freak,
The Pokémon Company. or The Pokémon Company.
</p> </p>
</div> </div>
</div> </div>
</div> </div>
</div> </div>
{/if}
+14 -3
View File
@@ -8,6 +8,7 @@ import type { SupabaseClient } from '@supabase/supabase-js';
/** /**
* Calculate expected pokedex entries based on pokedex configuration * Calculate expected pokedex entries based on pokedex configuration
* Applies the same filtering logic as CombinedDataRepository * Applies the same filtering logic as CombinedDataRepository
* @throws Error if the Supabase query fails
*/ */
async function calculateExpectedEntries( async function calculateExpectedEntries(
supabase: SupabaseClient, supabase: SupabaseClient,
@@ -28,7 +29,7 @@ async function calculateExpectedEntries(
.from('region_game_mappings') .from('region_game_mappings')
.select('region') .select('region')
.eq('game', pokedex.gameScope) .eq('game', pokedex.gameScope)
.single(); .maybeSingle();
if (regionData?.region) { if (regionData?.region) {
query = query.eq('regionToCatchIn', regionData.region); query = query.eq('regionToCatchIn', regionData.region);
@@ -44,7 +45,7 @@ async function calculateExpectedEntries(
if (error) { if (error) {
console.error('Error calculating expected entries:', error); console.error('Error calculating expected entries:', error);
return []; throw new Error(`Failed to calculate expected entries: ${error.message}`);
} }
return data?.map((entry) => entry.id) || []; return data?.map((entry) => entry.id) || [];
@@ -52,6 +53,8 @@ async function calculateExpectedEntries(
/** /**
* Populate pokedex_entries_mapping table for a pokedex * 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( async function populatePokedexMappings(
supabase: SupabaseClient, supabase: SupabaseClient,
@@ -70,13 +73,21 @@ async function populatePokedexMappings(
pokedexEntryId pokedexEntryId
})); }));
const { error } = await supabase.from('pokedex_entries_mapping').insert(mappings); // 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) { if (error) {
console.error('Error populating pokedex mappings:', error); console.error('Error populating pokedex mappings:', error);
throw new Error(`Failed to populate pokedex mappings: ${error.message}`); throw new Error(`Failed to populate pokedex mappings: ${error.message}`);
} }
} }
}
// GET: List all pokedexes for user // GET: List all pokedexes for user
export const GET = async (event: RequestEvent) => { export const GET = async (event: RequestEvent) => {
+27 -42
View File
@@ -8,6 +8,7 @@ import type { SupabaseClient } from '@supabase/supabase-js';
/** /**
* Calculate expected pokedex entries based on pokedex configuration * Calculate expected pokedex entries based on pokedex configuration
* Applies the same filtering logic as CombinedDataRepository * Applies the same filtering logic as CombinedDataRepository
* @throws Error if the Supabase query fails
*/ */
async function calculateExpectedEntries( async function calculateExpectedEntries(
supabase: SupabaseClient, supabase: SupabaseClient,
@@ -28,7 +29,7 @@ async function calculateExpectedEntries(
.from('region_game_mappings') .from('region_game_mappings')
.select('region') .select('region')
.eq('game', pokedex.gameScope) .eq('game', pokedex.gameScope)
.single(); .maybeSingle();
if (regionData?.region) { if (regionData?.region) {
query = query.eq('regionToCatchIn', regionData.region); query = query.eq('regionToCatchIn', regionData.region);
@@ -44,16 +45,17 @@ async function calculateExpectedEntries(
if (error) { if (error) {
console.error('Error calculating expected entries:', error); console.error('Error calculating expected entries:', error);
return []; throw new Error(`Failed to calculate expected entries: ${error.message}`);
} }
return data?.map((entry) => entry.id) || []; return data?.map((entry) => entry.id) || [];
} }
/** /**
* Populate pokedex_entries_mapping table for a pokedex * Recalculate pokedex_entries_mapping table for a pokedex (used on update)
* Uses a single atomic RPC call to prevent orphaned pokedexes
*/ */
async function populatePokedexMappings( async function recalculatePokedexMappings(
supabase: SupabaseClient, supabase: SupabaseClient,
pokedexId: string, pokedexId: string,
pokedex: Pokedex pokedex: Pokedex
@@ -65,42 +67,18 @@ async function populatePokedexMappings(
return; return;
} }
const mappings = expectedEntryIds.map((pokedexEntryId) => ({ // Call the atomic RPC function to delete old mappings and insert new ones
pokedexId, const { error } = await supabase.rpc('recalculate_pokedex_mappings', {
pokedexEntryId p_pokedex_id: pokedexId,
})); p_entry_ids: expectedEntryIds
});
const { error } = await supabase.from('pokedex_entries_mapping').insert(mappings);
if (error) { if (error) {
console.error('Error populating pokedex mappings:', error); console.error('Error recalculating pokedex mappings:', error);
throw new Error(`Failed to populate pokedex mappings: ${error.message}`); throw new Error(`Failed to recalculate 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 // GET: Get single pokedex by ID
export const GET = async (event: RequestEvent) => { export const GET = async (event: RequestEvent) => {
try { try {
@@ -154,6 +132,13 @@ export const PUT = async (event: RequestEvent) => {
} }
const repo = new PokedexRepository(event.locals.supabase, userId); const repo = new PokedexRepository(event.locals.supabase, userId);
// Fetch the existing pokedex before update to compare values
const existingPokedex = await repo.findById(id);
if (!existingPokedex) {
return json({ error: 'Pokedex not found' }, { status: 404 });
}
const pokedex = await repo.update(id, data); const pokedex = await repo.update(id, data);
if (!pokedex) { if (!pokedex) {
@@ -161,16 +146,16 @@ export const PUT = async (event: RequestEvent) => {
} }
// Recalculate pokedex_entries_mapping if configuration changed // Recalculate pokedex_entries_mapping if configuration changed
// Only recalculate if type flags or gameScope changed // Only recalculate if type flags or gameScope actually changed
const configChanged = const configChanged =
data.isFormDex !== undefined || (data.isFormDex !== undefined && existingPokedex.isFormDex !== data.isFormDex) ||
data.gameScope !== undefined || (data.gameScope !== undefined && existingPokedex.gameScope !== data.gameScope) ||
(data.isLivingDex !== undefined && pokedex.isLivingDex !== data.isLivingDex) || (data.isLivingDex !== undefined && existingPokedex.isLivingDex !== data.isLivingDex) ||
(data.isShinyDex !== undefined && pokedex.isShinyDex !== data.isShinyDex) || (data.isShinyDex !== undefined && existingPokedex.isShinyDex !== data.isShinyDex) ||
(data.isOriginDex !== undefined && pokedex.isOriginDex !== data.isOriginDex); (data.isOriginDex !== undefined && existingPokedex.isOriginDex !== data.isOriginDex);
if (configChanged) { if (configChanged) {
await recalculatePokedexMappings(event.locals.supabase, id, { ...pokedex, ...data }); await recalculatePokedexMappings(event.locals.supabase, id, pokedex);
} }
return json(pokedex); return json(pokedex);
+17 -1
View File
@@ -1,5 +1,5 @@
<script lang="ts"> <script lang="ts">
import { onMount } from 'svelte'; import { onDestroy, onMount } from 'svelte';
import { goto } from '$app/navigation'; import { goto } from '$app/navigation';
import { user } from '$lib/stores/user'; import { user } from '$lib/stores/user';
import type { Pokedex } from '$lib/models/Pokedex'; import type { Pokedex } from '$lib/models/Pokedex';
@@ -7,6 +7,10 @@
import PokedexForm from '$lib/components/pokedex/PokedexForm.svelte'; import PokedexForm from '$lib/components/pokedex/PokedexForm.svelte';
let pokedexes: Pokedex[] = []; let pokedexes: Pokedex[] = [];
const unsubscribe = user.subscribe((value) => {
console.log('[MyPokedexes] User store updated:', value ? 'User found' : 'User null');
});
onDestroy(unsubscribe);
let showModal = false; let showModal = false;
let editingPokedex: Pokedex | null = null; let editingPokedex: Pokedex | null = null;
let formData: Partial<Pokedex> = { let formData: Partial<Pokedex> = {
@@ -22,10 +26,22 @@
$: mode = editingPokedex ? ('edit' as const) : ('create' as const); $: mode = editingPokedex ? ('edit' as const) : ('create' as const);
onMount(async () => { 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) { if (!$user) {
goto('/signin'); goto('/signin');
return; return;
} }
await loadPokedexes(); await loadPokedexes();
}); });
+2 -2
View File
@@ -97,10 +97,10 @@
<p class="text-sm opacity-70 mt-2"> <p class="text-sm opacity-70 mt-2">
Still having trouble? Still having trouble?
<a <a
href="mailto:support@example.com" href="https://github.com/jcreek/LivingDexTracker/issues"
class="link link-primary font-semibold hover:underline" class="link link-primary font-semibold hover:underline"
> >
Contact support Raise an issue on our GitHub
</a> for assistance. </a> for assistance.
</p> </p>
</div> </div>
@@ -8,6 +8,7 @@ SELECT
p.id AS "pokedexId", p.id AS "pokedexId",
pe.id AS "pokedexEntryId" pe.id AS "pokedexEntryId"
FROM pokedexes p FROM pokedexes p
LEFT JOIN region_game_mappings rg ON rg.game = p."gameScope"
CROSS JOIN pokedex_entries pe CROSS JOIN pokedex_entries pe
WHERE WHERE
-- Form filter: if isFormDex is false, only include base forms -- Form filter: if isFormDex is false, only include base forms
@@ -15,10 +16,8 @@ WHERE
(p."isFormDex" = true OR (pe.form IS NULL OR (pe.pokemon = 'Unown' AND pe.form = '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 -- Game scope filter: if gameScope is specified, filter by game
AND (p."gameScope" IS NULL OR pe."gamesToCatchIn" @> ARRAY[p."gameScope"]::TEXT[]) AND (p."gameScope" IS NULL OR pe."gamesToCatchIn" @> ARRAY[p."gameScope"]::TEXT[])
-- Region filter: if gameScope is specified, filter by region -- Region filter: only enforce when a mapping exists
AND (p."gameScope" IS NULL OR pe."regionToCatchIn" = ( AND (p."gameScope" IS NULL OR rg.region IS NULL OR pe."regionToCatchIn" = rg.region)
SELECT region FROM region_game_mappings WHERE game = p."gameScope" LIMIT 1
))
ON CONFLICT ("pokedexId", "pokedexEntryId") DO NOTHING; ON CONFLICT ("pokedexId", "pokedexEntryId") DO NOTHING;
-- Add comment to document this migration -- Add comment to document this migration
@@ -0,0 +1,28 @@
-- Create function to atomically recalculate pokedex entries mapping
-- This function replaces the two-step delete+insert process with a single atomic operation
-- to prevent orphaned pokedexes if repopulation fails or yields an empty list
CREATE OR REPLACE FUNCTION recalculate_pokedex_mappings(
p_pokedex_id UUID,
p_entry_ids BIGINT[]
)
RETURNS VOID
SECURITY DEFINER
SET search_path = public, pg_temp
LANGUAGE plpgsql
AS $$
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;
-- 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
WHERE "pokedexId" = p_pokedex_id;
INSERT INTO pokedex_entries_mapping ("pokedexId", "pokedexEntryId")
SELECT p_pokedex_id, unnest(p_entry_ids);
END;
$$;
@@ -0,0 +1,16 @@
-- Fix RLS SELECT policy on pokedex_entries_mapping to restrict reads to owning user
-- This prevents exposing all user mappings and ensures consistency with INSERT/DELETE policies
-- Drop the overly permissive public SELECT policy
DROP POLICY IF EXISTS "Anyone can view pokedex entries mapping" ON pokedex_entries_mapping;
-- Create a new SELECT policy that restricts reads to the owning user
CREATE POLICY "Users can view own pokedex entries mapping"
ON pokedex_entries_mapping
FOR SELECT USING (
EXISTS (
SELECT 1 FROM pokedexes
WHERE pokedexes.id = pokedex_entries_mapping."pokedexId"
AND pokedexes."userId" = auth.uid()
)
);