feat(favorites): migrate favorites to use resource URLs instead of IDs - #13
Conversation
- Update database schema to store resource URLs in user_favorites table - Add helper function to get resource URL from resource object - Modify favorites logic to work with both authenticated users and localStorage fallback - Update UI components to use new favorites system - Fix AdBlockDetector to use environment variable for API key - Improve Showcase page layout consistency with aspect ratio
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR refactors the favorites system from ID-based to URL-based storage, including database migrations to update the schema, enhanced hooks with localStorage and event-driven sync support, and component updates to leverage URLs for identifying resources. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Action
participant Component as FavoritesTab/ResourceCard
participant HeartedHook as useHeartedResources
participant LocalStorage as Browser localStorage
participant Events as Event Emitter
participant AuthHook as useUserFavorites
participant SupabaseAPI as Supabase API
User->>Component: Click Heart Icon
Component->>HeartedHook: toggleHeart(resourceUrl)
alt Authenticated User
HeartedHook->>AuthHook: Check if user exists
AuthHook->>SupabaseAPI: Mutate favorite (insert/delete with resourceUrl)
SupabaseAPI-->>AuthHook: Success response
AuthHook->>HeartedHook: Return userFavorites (isLoading, data)
HeartedHook->>LocalStorage: Update localStorage with resourceUrl
HeartedHook->>Events: Emit localFavoritesChanged
else Unauthenticated User
HeartedHook->>LocalStorage: Toggle resourceUrl in localStorage
HeartedHook->>Events: Emit localFavoritesChanged
Events-->>Component: Listeners notified
end
HeartedHook-->>Component: Return updated heartedResources
Component->>Component: Update UI (heart filled/empty)
Component-->>User: Visual feedback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/oc review the code Review this pull request: |
|
ProviderModelNotFoundError |
|
/oc review the code |
|
ProviderModelNotFoundError |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
src/components/AdBlockDetector.tsx (1)
17-17:isBlockedstate is set but never read.
isBlockedis updated on line 57 but isn't referenced anywhere in the component's render output or logic — onlyisOpendrives the dialog. Consider removing it to avoid dead state.♻️ Proposed cleanup
export function AdBlockDetector() { - const [isBlocked, setIsBlocked] = useState(false); const [isOpen, setIsOpen] = useState(false);} catch (error) { console.warn("PostHog request failed, likely blocked:", error); - setIsBlocked(true); setIsOpen(true); }Also applies to: 57-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/AdBlockDetector.tsx` at line 17, The isBlocked state and its setter (const [isBlocked, setIsBlocked] = useState(false)) in the AdBlockDetector component are never read; remove the dead state and any calls to setIsBlocked (e.g., the update on line 57) to simplify the component, leaving only the isOpen state and its logic (dialog rendering and handlers such as any functions referencing isBlocked) intact; if the boolean was intended to control UI, instead wire that logic to isOpen or the appropriate existing state before deleting.src/pages/Showcase.tsx (1)
81-81: Doubleaspect-videoon non-image/video previews is redundant but harmless.For fonts (line 81), JSON (line 102), and document (line 121) previews,
aspect-videois set on the inner element while the outer wrapper (line 155) also appliesaspect-video. The outer constraint already sizes the container, making the inner one a no-op. Consider removingaspect-videofrom the inner elements for clarity, or from the outer wrapper if you want each preview type to control its own aspect ratio.Also applies to: 102-102, 121-121, 155-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Showcase.tsx` at line 81, The inner preview containers (the divs using className values like "w-full h-full flex flex-col items-center justify-center bg-muted/20 p-6 aspect-video ..." for font previews and similar inner elements for JSON and document previews) redundantly include "aspect-video" while the outer wrapper (the parent container that also applies "aspect-video") already constrains sizing; remove "aspect-video" from those inner className strings ("w-full h-full ... aspect-video") to avoid the no-op duplication, or alternatively remove it from the outer wrapper if you want each preview to manage its own aspect ratio—update the className on the inner preview divs referenced above (font/JSON/document preview elements) accordingly.src/pages/ResourcesHub.tsx (1)
80-86: Tab state and URL are not kept in sync.The
tabquery parameter is read once on mount, but clicking the Resources/Favorites toggle buttons (lines 142-159) doesn't update the URL. This means:
- The URL still shows
?tab=favoritesafter switching back to Resources.- Browser back/forward won't toggle the tab.
- Sharing/bookmarking the URL after toggling won't reflect the current view.
Consider using
useSearchParamsfrom react-router to keep the URL and component state synchronized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/ResourcesHub.tsx` around lines 80 - 86, The component currently reads the tab query param once in the useEffect and updates local state via setShowFavorites, but doesn't update the URL when the user toggles tabs; replace that pattern with react-router's useSearchParams: initialize show state from const [searchParams, setSearchParams] = useSearchParams() (instead of new URLSearchParams), derive showFavorites from searchParams.get('tab') === 'favorites', and update setSearchParams({ tab: 'favorites' }) or setSearchParams({}) inside the Resources/Favorites toggle handlers (the same functions that currently call setShowFavorites) so the URL and component state stay synchronized and browser navigation/sharing works correctly.src/hooks/useUserFavorites.ts (1)
10-16:isSchemaReadyhas no recovery path — once false, favorites are permanently disabled until remount.When
setIsSchemaReady(false)is called (line 25), the query perpetually returns[](line 16) and mutations are blocked (line 43). SinceisSchemaReadylives in component state, it cannot recover without unmounting and remounting. Consider either addingisSchemaReadyto the query'senabledcondition to stop pointless refetches, or providing a retry/recovery mechanism.Minimal fix: disable refetches when schema is not ready
enabled: !!user?.id, + // Stop refetching when the schema is known to be outdated + enabled: !!user?.id && isSchemaReady,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useUserFavorites.ts` around lines 10 - 16, The query currently returns [] whenever isSchemaReady is false and never recovers because isSchemaReady is local state; fix by preventing the query from running while the schema is not ready: in the useQuery call add an enabled condition that includes isSchemaReady (e.g., enabled: Boolean(user?.id) && isSchemaReady) so queryFn isn’t executed when the schema is unavailable, and likewise guard any mutation calls (the code path that checks isSchemaReady before mutating) to skip or queue mutations until setIsSchemaReady(true) is called to allow recovery.src/hooks/useHeartedResources.ts (1)
60-64:isHeartedreads from localStorage on every call instead of using component state.For unauthenticated users,
isHeartedcallsgetLocalHeartedResources()(alocalStorage.getItem+JSON.parse) on every invocation, while the returnedheartedResourcesuses state. In a list rendering N resource cards, this causes N synchronous localStorage reads per render. Using theheartedResourcesstate would be more efficient and consistent.Use state instead of localStorage read
const isHearted = (resource: Resource | string) => { const resourceUrl = typeof resource === 'string' ? resource : getResourceUrl(resource); if (!resourceUrl) return false; - return getLocalHeartedResources().includes(resourceUrl); + return heartedResources.includes(resourceUrl); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useHeartedResources.ts` around lines 60 - 64, The isHearted function currently calls getLocalHeartedResources() (reading localStorage) on every invocation; change it to read from the hook's heartedResources state instead so repeated renders/cards don't synchronously hit localStorage. Locate isHearted in useHeartedResources and replace getLocalHeartedResources().includes(...) with heartedResources.includes(resourceUrl) (after computing resourceUrl the same way), and ensure heartedResources is defined in the closure or passed into isHearted; keep the early return (if !resourceUrl) false and preserve behavior for authenticated users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/resources/FavoritesTab.tsx`:
- Around line 46-58: The current key for ResourceCard uses
getResourceUrl(resource) which can collide for different favorites; update the
key in the favorites rendering to use a composite key combining a unique
resource identifier with the URL (for example use resource.id plus the
getResourceUrl(resource) value) so change the key on the ResourceCard rendered
inside favoriteResources.map to a composite like `${resource.id}-${resourceUrl}`
to guarantee uniqueness while keeping the rest of the render (ResourceCard,
resource prop, onClick={setSelectedResource}) unchanged.
In `@src/hooks/useHeartedResources.ts`:
- Around line 66-73: Authenticated branch returns userFavorites.toggleFavorite
and userFavorites.isFavorited which only accept string, causing an inconsistent
API compared to the unauthenticated branch that accepts Resource | string and
resolves via getResourceUrl; fix by wrapping the authenticated handlers so
toggleHeart(resource: Resource | string) first resolves to a URL (use
getResourceUrl or the same resolution helper used in the unauthenticated branch)
and then calls userFavorites.toggleFavorite(resolvedUrl), and similarly wrap
isHearted to resolve the resource before calling
userFavorites.isFavorited(resolvedUrl); keep heartedResources and isLoading
unchanged and preserve original function names userFavorites.toggleFavorite and
userFavorites.isFavorited in the implementation.
In `@src/hooks/useUserFavorites.ts`:
- Line 34: In useUserFavorites (the data mapping that currently does return
data?.map(fav => fav.resource_url.toString()) || []), guard against
fav.resource_url being null before calling .toString(): filter out or handle
null resource_url values (e.g., skip nulls or coerce safely) so you never call
.toString() on null and the function always returns an array of strings; update
the mapping to check fav.resource_url (or use a safe coercion) and ensure the
function still returns [] when data is absent.
In `@supabase/migrations/20260217130000_add_resource_url_to_user_favorites.sql`:
- Around line 21-23: The partial unique index
user_favorites_user_id_resource_url_key allows multiple (user_id, NULL)
duplicates; to enforce uniqueness reliably, first deduplicate any existing rows
in public.user_favorites where resource_url IS NULL (collapse duplicates per
user_id or delete extras), then either add a NOT NULL constraint on resource_url
or set a canonical non-NULL value, and finally recreate the unique index without
the WHERE clause (i.e., unique on (user_id, resource_url) using the same index
name) so NULLs cannot produce duplicate entries during the transition.
- Around line 13-17: After the COALESCE backfill (the UPDATE using
public.user_favorites uf and public.resources r), add a cleanup DELETE to remove
any remaining user_favorites whose resource_url is still NULL (these represent
favorites that either reference missing resources or resources with all NULL URL
columns) so the subsequent ALTER TABLE ... ALTER COLUMN resource_url SET NOT
NULL will not fail; specifically, add a statement like DELETE FROM
public.user_favorites uf WHERE uf.resource_url IS NULL (optionally join or check
existence in public.resources if you want to distinguish missing resources vs
unfillable ones), immediately after the backfill UPDATE and before the ALTER
COLUMN change.
In `@supabase/migrations/20260217140000_fix_user_favorites_constraints.sql`:
- Around line 8-25: Backfill may create duplicate (user_id, resource_url) and
leave NULL resource_url values, so after running the update (the UPDATE on
public.user_favorites using public.resources) deduplicate and purge NULLs before
adding the constraint: remove duplicate rows in public.user_favorites so only
one row per (user_id, resource_url) remains (use a grouped selection retaining
the desired row id/ctid) and delete or otherwise resolve any rows where
resource_url IS NULL, then add the constraint
user_favorites_user_id_resource_url_key (unique (user_id, resource_url)); also
keep the existing drop of user_favorites_user_id_resource_id_key and drop of the
partial index as-is.
- Around line 2-6: The migration references user_favorites.resource_id
unconditionally which will fail on fresh DBs because the resource_id column may
not exist; modify the migration to first check for the column before running the
DELETE/UPDATE by wrapping those statements in a DO $$ BEGIN IF EXISTS (SELECT 1
FROM information_schema.columns WHERE table_name='user_favorites' AND
column_name='resource_id') THEN ... END IF; END $$; or alternatively use
conditional SQL (IF EXISTS) for each operation so both the DELETE and any UPDATE
that reference resource_id are skipped when the column is absent.
---
Nitpick comments:
In `@src/components/AdBlockDetector.tsx`:
- Line 17: The isBlocked state and its setter (const [isBlocked, setIsBlocked] =
useState(false)) in the AdBlockDetector component are never read; remove the
dead state and any calls to setIsBlocked (e.g., the update on line 57) to
simplify the component, leaving only the isOpen state and its logic (dialog
rendering and handlers such as any functions referencing isBlocked) intact; if
the boolean was intended to control UI, instead wire that logic to isOpen or the
appropriate existing state before deleting.
In `@src/hooks/useHeartedResources.ts`:
- Around line 60-64: The isHearted function currently calls
getLocalHeartedResources() (reading localStorage) on every invocation; change it
to read from the hook's heartedResources state instead so repeated renders/cards
don't synchronously hit localStorage. Locate isHearted in useHeartedResources
and replace getLocalHeartedResources().includes(...) with
heartedResources.includes(resourceUrl) (after computing resourceUrl the same
way), and ensure heartedResources is defined in the closure or passed into
isHearted; keep the early return (if !resourceUrl) false and preserve behavior
for authenticated users.
In `@src/hooks/useUserFavorites.ts`:
- Around line 10-16: The query currently returns [] whenever isSchemaReady is
false and never recovers because isSchemaReady is local state; fix by preventing
the query from running while the schema is not ready: in the useQuery call add
an enabled condition that includes isSchemaReady (e.g., enabled:
Boolean(user?.id) && isSchemaReady) so queryFn isn’t executed when the schema is
unavailable, and likewise guard any mutation calls (the code path that checks
isSchemaReady before mutating) to skip or queue mutations until
setIsSchemaReady(true) is called to allow recovery.
In `@src/pages/ResourcesHub.tsx`:
- Around line 80-86: The component currently reads the tab query param once in
the useEffect and updates local state via setShowFavorites, but doesn't update
the URL when the user toggles tabs; replace that pattern with react-router's
useSearchParams: initialize show state from const [searchParams,
setSearchParams] = useSearchParams() (instead of new URLSearchParams), derive
showFavorites from searchParams.get('tab') === 'favorites', and update
setSearchParams({ tab: 'favorites' }) or setSearchParams({}) inside the
Resources/Favorites toggle handlers (the same functions that currently call
setShowFavorites) so the URL and component state stay synchronized and browser
navigation/sharing works correctly.
In `@src/pages/Showcase.tsx`:
- Line 81: The inner preview containers (the divs using className values like
"w-full h-full flex flex-col items-center justify-center bg-muted/20 p-6
aspect-video ..." for font previews and similar inner elements for JSON and
document previews) redundantly include "aspect-video" while the outer wrapper
(the parent container that also applies "aspect-video") already constrains
sizing; remove "aspect-video" from those inner className strings ("w-full h-full
... aspect-video") to avoid the no-op duplication, or alternatively remove it
from the outer wrapper if you want each preview to manage its own aspect
ratio—update the className on the inner preview divs referenced above
(font/JSON/document preview elements) accordingly.
| return ( | ||
| <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-4"> | ||
| {favoriteResources.map(resource => ( | ||
| <div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4"> | ||
| {favoriteResources.map(resource => { | ||
| const resourceUrl = getResourceUrl(resource); | ||
| return ( | ||
| <ResourceCard | ||
| key={resource.id} | ||
| key={resourceUrl} | ||
| resource={resource} | ||
| onClick={setSelectedResource} | ||
| /> | ||
| ))} | ||
| ); | ||
| })} | ||
| </div> |
There was a problem hiding this comment.
Duplicate React keys possible if two favorite resources share the same URL.
getResourceUrl(resource) is used as the key prop (line 52). While uncommon, if two distinct resources resolve to the same URL (e.g., same download_url), React would see duplicate keys and log warnings / mis-reconcile. Using a composite key like `${resource.id}-${resourceUrl}` is safer.
Suggested fix
- key={resourceUrl}
+ key={`${resource.id}-${resourceUrl}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-4"> | |
| {favoriteResources.map(resource => ( | |
| <div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4"> | |
| {favoriteResources.map(resource => { | |
| const resourceUrl = getResourceUrl(resource); | |
| return ( | |
| <ResourceCard | |
| key={resource.id} | |
| key={resourceUrl} | |
| resource={resource} | |
| onClick={setSelectedResource} | |
| /> | |
| ))} | |
| ); | |
| })} | |
| </div> | |
| return ( | |
| <div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4"> | |
| {favoriteResources.map(resource => { | |
| const resourceUrl = getResourceUrl(resource); | |
| return ( | |
| <ResourceCard | |
| key={`${resource.id}-${resourceUrl}`} | |
| resource={resource} | |
| onClick={setSelectedResource} | |
| /> | |
| ); | |
| })} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/resources/FavoritesTab.tsx` around lines 46 - 58, The current
key for ResourceCard uses getResourceUrl(resource) which can collide for
different favorites; update the key in the favorites rendering to use a
composite key combining a unique resource identifier with the URL (for example
use resource.id plus the getResourceUrl(resource) value) so change the key on
the ResourceCard rendered inside favoriteResources.map to a composite like
`${resource.id}-${resourceUrl}` to guarantee uniqueness while keeping the rest
of the render (ResourceCard, resource prop, onClick={setSelectedResource})
unchanged.
| if (user) { | ||
| return { | ||
| heartedResources: userFavorites.favorites, | ||
| toggleHeart: userFavorites.toggleFavorite, | ||
| isHearted: userFavorites.isFavorited, | ||
| isLoading: userFavorites.isLoading, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's inspect the useHeartedResources.ts file to see the actual code
cat -n src/hooks/useHeartedResources.tsRepository: Coder-soft/renderdragon.org
Length of output: 3198
🏁 Script executed:
# Check how toggleHeart and isHearted are used in the codebase
rg -n --type=ts --type=tsx 'toggleHeart\|isHearted' -g '!**/useHeartedResources*' --max-count=100Repository: Coder-soft/renderdragon.org
Length of output: 98
🏁 Script executed:
# Let's also check the useUserFavorites hook to understand the parameter types
fd -e ts -e tsx 'useUserFavorites' --exec cat -n {} +Repository: Coder-soft/renderdragon.org
Length of output: 4206
🏁 Script executed:
# Check the types of Resource and getResourceUrl
rg -n 'type Resource|interface Resource|export.*getResourceUrl' --type=ts --type=tsxRepository: Coder-soft/renderdragon.org
Length of output: 98
🏁 Script executed:
# Search for toggleHeart usage with proper syntax
rg -n 'toggleHeart' --type ts -g '!useHeartedResources' -A 3 -B 1Repository: Coder-soft/renderdragon.org
Length of output: 1984
🏁 Script executed:
# Search for isHearted usage
rg -n 'isHearted' --type ts -g '!useHeartedResources' -A 3 -B 1Repository: Coder-soft/renderdragon.org
Length of output: 1653
🏁 Script executed:
# Check getResourceUrl implementation
rg -n 'getResourceUrl|export.*Resource' --type ts | head -40Repository: Coder-soft/renderdragon.org
Length of output: 2997
Critical: Authenticated path doesn't resolve Resource objects—type signature inconsistency.
The unauthenticated toggleHeart and isHearted accept Resource | string and resolve to a URL via getResourceUrl. However, the authenticated path directly returns userFavorites.toggleFavorite and userFavorites.isFavorited, which only accept string. This creates an inconsistent API contract:
- Unauthenticated:
toggleHeart(resource: Resource | string)→ resolves Resource to URL - Authenticated:
toggleHeart(resourceUrl: string)→ expects string only
While current codebase usage (e.g., ResourceCard.tsx line 37) pre-resolves Resource objects before calling these functions, the type mismatch violates the API contract. A developer passing a Resource object when authenticated would pass [object Object] to the database.
Proposed fix: Wrap authenticated methods with URL resolution
if (user) {
+ const resolveUrl = (resource: Resource | string): string =>
+ typeof resource === 'string' ? resource : getResourceUrl(resource);
return {
heartedResources: userFavorites.favorites,
- toggleHeart: userFavorites.toggleFavorite,
- isHearted: userFavorites.isFavorited,
+ toggleHeart: (resource: Resource | string) => {
+ const url = resolveUrl(resource);
+ if (!url) return;
+ userFavorites.toggleFavorite(url);
+ },
+ isHearted: (resource: Resource | string) => {
+ const url = resolveUrl(resource);
+ if (!url) return false;
+ return userFavorites.isFavorited(url);
+ },
isLoading: userFavorites.isLoading,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (user) { | |
| return { | |
| heartedResources: userFavorites.favorites, | |
| toggleHeart: userFavorites.toggleFavorite, | |
| isHearted: userFavorites.isFavorited, | |
| isLoading: userFavorites.isLoading, | |
| }; | |
| } | |
| if (user) { | |
| const resolveUrl = (resource: Resource | string): string => | |
| typeof resource === 'string' ? resource : getResourceUrl(resource); | |
| return { | |
| heartedResources: userFavorites.favorites, | |
| toggleHeart: (resource: Resource | string) => { | |
| const url = resolveUrl(resource); | |
| if (!url) return; | |
| userFavorites.toggleFavorite(url); | |
| }, | |
| isHearted: (resource: Resource | string) => { | |
| const url = resolveUrl(resource); | |
| if (!url) return false; | |
| return userFavorites.isFavorited(url); | |
| }, | |
| isLoading: userFavorites.isLoading, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useHeartedResources.ts` around lines 66 - 73, Authenticated branch
returns userFavorites.toggleFavorite and userFavorites.isFavorited which only
accept string, causing an inconsistent API compared to the unauthenticated
branch that accepts Resource | string and resolves via getResourceUrl; fix by
wrapping the authenticated handlers so toggleHeart(resource: Resource | string)
first resolves to a URL (use getResourceUrl or the same resolution helper used
in the unauthenticated branch) and then calls
userFavorites.toggleFavorite(resolvedUrl), and similarly wrap isHearted to
resolve the resource before calling userFavorites.isFavorited(resolvedUrl); keep
heartedResources and isLoading unchanged and preserve original function names
userFavorites.toggleFavorite and userFavorites.isFavorited in the
implementation.
| } | ||
|
|
||
| return data?.map(fav => fav.resource_id.toString()) || []; | ||
| return data?.map(fav => fav.resource_url.toString()) || []; |
There was a problem hiding this comment.
fav.resource_url could be null at runtime if migrations haven't fully run.
The schema enforces NOT NULL after the final migration, but if the migration sequence is partially applied, resource_url may still be nullable. .toString() on null throws a TypeError.
Defensive fix
- return data?.map(fav => fav.resource_url.toString()) || [];
+ return data?.map(fav => String(fav.resource_url ?? '')) .filter(Boolean) || [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return data?.map(fav => fav.resource_url.toString()) || []; | |
| return data?.map(fav => String(fav.resource_url ?? '')) .filter(Boolean) || []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useUserFavorites.ts` at line 34, In useUserFavorites (the data
mapping that currently does return data?.map(fav => fav.resource_url.toString())
|| []), guard against fav.resource_url being null before calling .toString():
filter out or handle null resource_url values (e.g., skip nulls or coerce
safely) so you never call .toString() on null and the function always returns an
array of strings; update the mapping to check fav.resource_url (or use a safe
coercion) and ensure the function still returns [] when data is absent.
| update public.user_favorites as uf | ||
| set resource_url = coalesce(r.download_url, r.preview_url, r.image_url) | ||
| from public.resources as r | ||
| where uf.resource_url is null | ||
| and uf.resource_id::text = r.id::text; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine the follow-up migration mentioned
fd "20260217140000" --type f -e sqlRepository: Coder-soft/renderdragon.org
Length of output: 140
🏁 Script executed:
#!/bin/bash
# Also examine the full current migration file for complete context
find . -name "*20260217130000*" -type f -exec cat -n {} \;Repository: Coder-soft/renderdragon.org
Length of output: 915
🏁 Script executed:
#!/bin/bash
# Check the resources table schema to understand the URL fields
git ls-files | grep -E "(migration|schema)" | head -20Repository: Coder-soft/renderdragon.org
Length of output: 572
🏁 Script executed:
#!/bin/bash
# Examine the follow-up migration that fixes constraints
cat -n supabase/migrations/20260217140000_fix_user_favorites_constraints.sqlRepository: Coder-soft/renderdragon.org
Length of output: 1512
🏁 Script executed:
#!/bin/bash
# Check the resources table schema to see if URL fields can be NULL
fd "create_resources" -o "create.*resources" --type f -e sql | xargs cat -nRepository: Coder-soft/renderdragon.org
Length of output: 224
🏁 Script executed:
#!/bin/bash
# Search for resources table creation or schema definition
rg "create table.*resources" -A 20Repository: Coder-soft/renderdragon.org
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Search for resources table creation
rg "create table" . --type sql -A 30 | grep -A 30 "public.resources"Repository: Coder-soft/renderdragon.org
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Also check for any constraints on resources URL fields
rg "download_url|preview_url|image_url" . --type sql -B 3 -A 1 | head -100Repository: Coder-soft/renderdragon.org
Length of output: 2020
🏁 Script executed:
#!/bin/bash
# Check the full resources table schema to see if URL fields have NOT NULL constraints
cat -n supabase/migrations/20250702100614-84c46566-7fc5-4a4f-86ad-c3e3603c3acc.sql | head -100Repository: Coder-soft/renderdragon.org
Length of output: 2977
🏁 Script executed:
#!/bin/bash
# Also search for any constraints added to resources URL fields in later migrations
rg "alter table.*resources" . --type sql -A 5 | grep -E "download_url|preview_url|image_url" -B 3 -A 1Repository: Coder-soft/renderdragon.org
Length of output: 53
Backfill can leave resource_url NULL, causing the follow-up migration to fail.
The resources table allows download_url, preview_url, and image_url to all be NULL. When all three are NULL, COALESCE returns NULL, leaving resource_url NULL. The follow-up migration (20260217140000) attempts to fix this by deleting only non-existent resources (lines 2-6) and backfilling again (lines 9-13), but it has the same issue. When it then executes ALTER COLUMN resource_url SET NOT NULL (lines 28-29), the migration will fail on any rows where the resource exists but all URL fields are NULL.
Add a cleanup step to delete or handle these unfillable favorites:
Proposed addition after the backfill block
end $$;
+-- Remove favorites that couldn't be backfilled (resource has no URLs)
+delete from public.user_favorites
+where resource_url is null;
+
create unique index if not exists user_favorites_user_id_resource_url_key🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260217130000_add_resource_url_to_user_favorites.sql`
around lines 13 - 17, After the COALESCE backfill (the UPDATE using
public.user_favorites uf and public.resources r), add a cleanup DELETE to remove
any remaining user_favorites whose resource_url is still NULL (these represent
favorites that either reference missing resources or resources with all NULL URL
columns) so the subsequent ALTER TABLE ... ALTER COLUMN resource_url SET NOT
NULL will not fail; specifically, add a statement like DELETE FROM
public.user_favorites uf WHERE uf.resource_url IS NULL (optionally join or check
existence in public.resources if you want to distinguish missing resources vs
unfillable ones), immediately after the backfill UPDATE and before the ALTER
COLUMN change.
| create unique index if not exists user_favorites_user_id_resource_url_key | ||
| on public.user_favorites (user_id, resource_url) | ||
| where resource_url is not null; |
There was a problem hiding this comment.
Partial unique index permits duplicate (user_id, NULL) pairs.
The WHERE resource_url IS NOT NULL clause means multiple rows for the same user with resource_url = NULL won't violate uniqueness. During the transition window (before the follow-up migration), a user could accumulate duplicate NULL-url favorites. This is acceptable only if the follow-up migration reliably cleans them up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260217130000_add_resource_url_to_user_favorites.sql`
around lines 21 - 23, The partial unique index
user_favorites_user_id_resource_url_key allows multiple (user_id, NULL)
duplicates; to enforce uniqueness reliably, first deduplicate any existing rows
in public.user_favorites where resource_url IS NULL (collapse duplicates per
user_id or delete extras), then either add a NOT NULL constraint on resource_url
or set a canonical non-NULL value, and finally recreate the unique index without
the WHERE clause (i.e., unique on (user_id, resource_url) using the same index
name) so NULLs cannot produce duplicate entries during the transition.
| delete from public.user_favorites | ||
| where resource_url is null | ||
| and not exists ( | ||
| select 1 from public.resources r where r.id::text = user_favorites.resource_id::text | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find migration files related to user_favorites
fd "202602171[234]0000" --type fRepository: Coder-soft/renderdragon.org
Length of output: 275
🏁 Script executed:
# Also search more broadly for these migration files
find . -name "*20260217120000*" -o -name "*20260217130000*" -o -name "*20260217140000*" 2>/dev/nullRepository: Coder-soft/renderdragon.org
Length of output: 281
🏁 Script executed:
# Search in migrations directory
git ls-files | grep -i migration | grep -i user_favoritesRepository: Coder-soft/renderdragon.org
Length of output: 275
🏁 Script executed:
cat -n supabase/migrations/20260217120000_create_user_favorites.sqlRepository: Coder-soft/renderdragon.org
Length of output: 1229
🏁 Script executed:
cat -n supabase/migrations/20260217130000_add_resource_url_to_user_favorites.sqlRepository: Coder-soft/renderdragon.org
Length of output: 915
🏁 Script executed:
cat -n supabase/migrations/20260217140000_fix_user_favorites_constraints.sqlRepository: Coder-soft/renderdragon.org
Length of output: 1512
Migration will fail on a fresh database—resource_id column is never created before this migration runs.
Migration 120000 creates the table without a resource_id column. Migration 130000 does not create it either; it only conditionally references it if it already exists (see the DO $$ BEGIN IF EXISTS pattern at lines 4–19). When migration 140000 executes on a fresh database, the unconditional references to user_favorites.resource_id on lines 5 and 13 will cause a column-not-found error.
Wrap the DELETE and UPDATE statements in a DO block that checks for column existence first (similar to the pattern in migration 130000), or use if exists clauses for the operations themselves.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260217140000_fix_user_favorites_constraints.sql` around
lines 2 - 6, The migration references user_favorites.resource_id unconditionally
which will fail on fresh DBs because the resource_id column may not exist;
modify the migration to first check for the column before running the
DELETE/UPDATE by wrapping those statements in a DO $$ BEGIN IF EXISTS (SELECT 1
FROM information_schema.columns WHERE table_name='user_favorites' AND
column_name='resource_id') THEN ... END IF; END $$; or alternatively use
conditional SQL (IF EXISTS) for each operation so both the DELETE and any UPDATE
that reference resource_id are skipped when the column is absent.
| -- Backfill remaining missing resource_url values from resources table | ||
| update public.user_favorites as uf | ||
| set resource_url = coalesce(r.download_url, r.preview_url, r.image_url) | ||
| from public.resources as r | ||
| where uf.resource_url is null | ||
| and uf.resource_id::text = r.id::text; | ||
|
|
||
| -- Drop the old unique constraint on (user_id, resource_id) | ||
| alter table public.user_favorites | ||
| drop constraint if exists user_favorites_user_id_resource_id_key; | ||
|
|
||
| -- Drop the partial unique index on (user_id, resource_url) | ||
| drop index if exists public.user_favorites_user_id_resource_url_key; | ||
|
|
||
| -- Create a proper unique constraint on (user_id, resource_url) | ||
| alter table public.user_favorites | ||
| add constraint user_favorites_user_id_resource_url_key | ||
| unique (user_id, resource_url); |
There was a problem hiding this comment.
Backfill may produce duplicate (user_id, resource_url) pairs, breaking the constraint.
If a user had multiple favorites pointing to different resource_id values that resolve to the same URL via COALESCE(download_url, preview_url, image_url), the backfill will create duplicate (user_id, resource_url) rows. The subsequent ADD CONSTRAINT … UNIQUE on line 24 will then fail.
Additionally, if a resource has all three URL columns NULL, COALESCE returns NULL, so resource_url stays NULL after the backfill. The SET NOT NULL on line 29 will fail for those rows.
Consider deduplicating after the backfill and cleaning up remaining NULLs before applying the constraint:
Proposed fix: deduplicate and clean NULLs before constraint
-- Backfill remaining missing resource_url values from resources table
update public.user_favorites as uf
set resource_url = coalesce(r.download_url, r.preview_url, r.image_url)
from public.resources as r
where uf.resource_url is null
and uf.resource_id::text = r.id::text;
+-- Remove rows that still have no resource_url after backfill
+delete from public.user_favorites where resource_url is null;
+
+-- Deduplicate (user_id, resource_url) keeping the earliest favorite
+delete from public.user_favorites
+where id not in (
+ select min(id) from public.user_favorites
+ where resource_url is not null
+ group by user_id, resource_url
+);
+
-- Drop the old unique constraint on (user_id, resource_id)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- Backfill remaining missing resource_url values from resources table | |
| update public.user_favorites as uf | |
| set resource_url = coalesce(r.download_url, r.preview_url, r.image_url) | |
| from public.resources as r | |
| where uf.resource_url is null | |
| and uf.resource_id::text = r.id::text; | |
| -- Drop the old unique constraint on (user_id, resource_id) | |
| alter table public.user_favorites | |
| drop constraint if exists user_favorites_user_id_resource_id_key; | |
| -- Drop the partial unique index on (user_id, resource_url) | |
| drop index if exists public.user_favorites_user_id_resource_url_key; | |
| -- Create a proper unique constraint on (user_id, resource_url) | |
| alter table public.user_favorites | |
| add constraint user_favorites_user_id_resource_url_key | |
| unique (user_id, resource_url); | |
| -- Backfill remaining missing resource_url values from resources table | |
| update public.user_favorites as uf | |
| set resource_url = coalesce(r.download_url, r.preview_url, r.image_url) | |
| from public.resources as r | |
| where uf.resource_url is null | |
| and uf.resource_id::text = r.id::text; | |
| -- Remove rows that still have no resource_url after backfill | |
| delete from public.user_favorites where resource_url is null; | |
| -- Deduplicate (user_id, resource_url) keeping the earliest favorite | |
| delete from public.user_favorites | |
| where id not in ( | |
| select min(id) from public.user_favorites | |
| where resource_url is not null | |
| group by user_id, resource_url | |
| ); | |
| -- Drop the old unique constraint on (user_id, resource_id) | |
| alter table public.user_favorites | |
| drop constraint if exists user_favorites_user_id_resource_id_key; | |
| -- Drop the partial unique index on (user_id, resource_url) | |
| drop index if exists public.user_favorites_user_id_resource_url_key; | |
| -- Create a proper unique constraint on (user_id, resource_url) | |
| alter table public.user_favorites | |
| add constraint user_favorites_user_id_resource_url_key | |
| unique (user_id, resource_url); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260217140000_fix_user_favorites_constraints.sql` around
lines 8 - 25, Backfill may create duplicate (user_id, resource_url) and leave
NULL resource_url values, so after running the update (the UPDATE on
public.user_favorites using public.resources) deduplicate and purge NULLs before
adding the constraint: remove duplicate rows in public.user_favorites so only
one row per (user_id, resource_url) remains (use a grouped selection retaining
the desired row id/ctid) and delete or otherwise resolve any rows where
resource_url IS NULL, then add the constraint
user_favorites_user_id_resource_url_key (unique (user_id, resource_url)); also
keep the existing drop of user_favorites_user_id_resource_id_key and drop of the
partial index as-is.
|
/oc review code |
|
ProviderModelNotFoundError |
- AdBlockDetector: remove dead isBlocked state (set but never read) - Showcase: remove redundant aspect-video from inner font/json/document previews - ResourcesHub: replace useState/useEffect tab with useSearchParams for URL sync - useUserFavorites: add isSchemaReady to query enabled condition to prevent pointless refetches - useHeartedResources: use heartedResources state instead of localStorage read in isHearted/toggleHeart
This reverts commit b30732d.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements