Fixes to responsive styling #1325
Conversation
|
@NiallJoeMaher is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughLayout and UI spacing adjustments across article/feed/home headers, mobile header/logo and content card visuals; filter prop renames and padding tweaks; global CSS adds Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
NiallJoeMaher
had a problem deploying
to
production
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/editor/editor/index.tsx (1)
34-41: Accessibility issue now exposed - div needs keyboard support.The ESLint disable was removed, but the underlying accessibility issue remains: the div has an
onClickhandler but lacks keyboard interaction support. Screen reader and keyboard users cannot focus the editor via this click target.♿ Suggested fix for keyboard accessibility
<div className="relative" + role="button" + tabIndex={0} onClick={() => { editor?.chain().focus().run(); }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + editor?.chain().focus().run(); + } + }} >Alternatively, if this wrapper is just for click-to-focus convenience and not meant to be a focusable element itself, you could suppress the lint rule with a comment explaining the rationale.
🤖 Fix all issues with AI agents
In @app/(editor)/create/[[...paramsArr]]/_client.tsx:
- Around line 307-313: The link-post branch currently only checks presence of
linkUrl and linkTitle but not URL format; update the publish/submit handler
handling the "link" tab (the branch using linkUrl and linkTitle) to validate
linkUrl is a well-formed URL (e.g., attempt new URL(linkUrl) in a try/catch or
use a robust URL regex), and if validation fails call toast.error with a clear
message and return to prevent publishing; ensure the check happens before any
network/save calls so invalid URLs are rejected early.
- Around line 142-143: The debouncedValue currently only combines title + body
so edits to linkUrl/linkTitle won't trigger auto-save for link posts; change the
useDebounce input to include linkUrl and linkTitle (e.g., combine title, body,
linkUrl, linkTitle or construct a single string/tuple that includes those four
values) so link-field edits are debounced too, and update the later auto-save
comparison that uses debouncedValue (the check that decides whether to save) to
reflect the expanded debounced shape.
- Around line 584-590: The publish confirmation always uses the title variable
so link posts show "Untitled" even when a linkTitle exists; update the render
logic used in the <p> (around isPostScheduled, activeTab, publishedTime) to
derive a displayTitle variable (e.g., use linkTitle when activeTab === "link",
falling back to title, then "Untitled") and use that displayTitle in the
scheduled and non-scheduled messages so link posts show their linkTitle in the
confirmation dialog.
In @app/(editor)/create/[[...paramsArr]]/navigation.tsx:
- Around line 141-158: Menu.Item's render prop previously provided an active
boolean for keyboard/focus highlighting but the current map ignores it; update
the Menu.Item usage for both branches so you accept the render prop (e.g.,
Menu.Item(({ active }) => ...)) and apply conditional classes based on active to
the button and Link elements (the block that renders item.onClick and the Link
branch), ensuring the active class names (for example hover/active background
and text) are appended when active is true so keyboard navigation shows visual
feedback.
In @app/global-error.tsx:
- Around line 21-22: Remove the empty JSX expression `{}` and stop bypassing TS
by passing `undefined as any` to the NextError component; locate the JSX that
renders NextError (component name: NextError) and replace `statusCode={undefined
as any}` with a valid status code (e.g., `statusCode={500}`) or, if you truly
must keep an unknown value, restore a targeted ESLint/TS suppression with a
clear comment explaining why and where, but prefer passing an explicit numeric
status code for type safety.
In @components/Header/MinimalHeader.tsx:
- Around line 165-183: The MenuItem render prop’s active state isn’t being used
for keyboard/focus styling in MinimalHeader.tsx; update the MenuItem invocation
inside the userNavigation.map so it uses the render-prop form (e.g.,
MenuItem={({ active }) => (...)}) and apply conditional classes based on active
for both the button and the Link elements (add the same active class used
elsewhere, e.g., hover/bg or text changes) so keyboard navigation highlights
match hover styles; ensure you preserve existing props like onClick and to when
wrapping the item name.
In @components/markdocNodes/FallbackMedia/FallbackMedia.tsx:
- Around line 8-10: The iframe in the FallbackMedia component is missing a
required title attribute; update the FallbackMedia.tsx component to accept a
title prop (e.g., title?: string in the component props/interface), provide a
sensible default (like "Embedded content" or a more specific default constant)
when no title is passed, and ensure the iframe element includes title={title}
alongside the existing style and {...props} so screen readers receive the
description; also update any prop types/TS types and call sites if necessary so
callers can pass a descriptive title.
- Line 9: Remove the useless empty JSX expression from the FallbackMedia
component: locate the empty curly braces "{}" inside the FallbackMedia JSX
render (in FallbackMedia or its return/JSX block) and delete them so there is no
dead JSX expression left; ensure the component still returns the intended markup
and run linting to confirm no leftover ESLint disable/whitespace artifacts
remain.
In @components/PostEditor/components/UrlMetadataPreview.tsx:
- Line 84: In the UrlMetadataPreview component remove the stray empty JSX
expression `{}` that renders nothing; locate the JSX return in
UrlMetadataPreview (where an isolated `{}` appears) and delete that token so the
rendered output is clean and no-op fragments are removed.
In @components/PostEditor/extensions/s3-image-upload.ts:
- Around line 169-185: The FileReader onload uses a stale transaction `tr`
created before the async callback; recreate the transaction inside
`reader.onload` to avoid invalid positions: inside `reader.onload` call `const
tr = view.state.tr` (not the outer `tr`), then `tr.setMeta(uploadKey, { add: {
id, pos, src: reader.result } })` and `view.dispatch(tr)` so the metadata uses a
fresh transaction based on the current document/selection when dispatched.
In @components/PostEditor/hooks/useArticleEditor.ts:
- Around line 102-107: The effect in useArticleEditor that sets initialContent
can run multiple times because relying on !editor.getText().trim() is brittle;
add a persistent flag (e.g., hasLoadedInitialContentRef) and update the
useEffect to: only run if editor, initialContent present and
hasLoadedInitialContentRef.current is false, then call
editor.commands.setContent(initialContent), set lastContentRef.current =
initialContent and set hasLoadedInitialContentRef.current = true; this
guarantees editor.commands.setContent (in the useEffect that currently
references editor and initialContent) runs exactly once even if initialContent
is whitespace/formatting.
In @components/PostEditor/PostEditor.tsx:
- Around line 118-140: The handlers handleTitleChange, handleBodyChange, and
handleTagsChange currently call onContentChange(getCurrentData()) immediately
after setTitle/setBody/setTags causing stale data due to async state updates;
remove the onContentChange calls from those handlers and instead add a useEffect
that watches [title, body, tags] (or their state setters) and calls
onContentChange(getCurrentData()) whenever the derived content changes so the
callback receives up-to-date data; keep getCurrentData and onContentChange names
so the effect can call getCurrentData() and guard with onContentChange?.
In @components/SideBar/AppSidebar.tsx:
- Line 180: The template currently lowercases item.customStyle in the
AppSidebar.tsx className, which can alter Tailwind hex color token casing (e.g.,
bg-[#0A66C2]) and break JIT matching; remove the toLowerCase() call and instead
interpolate the original value (safely coalesced to an empty string or trimmed)
so hex classes keep their original casing and null/undefined values are handled
(update the className string interpolation around item.customStyle in
AppSidebar.tsx to use the original item.customStyle with a fallback).
In @components/ui-components/sidebar.tsx:
- Around line 143-144: The Tailwind variant `[[data-collapsed=true]_&]:max-w-9`
is non-standard; update the collapsed-state selector in the class string that
starts with "max-w-full transition-[max-width] duration-300 ease-in-out" to use
a valid parent/data-attribute or group variant: either use Tailwind's parent
selector form with the parent selector token (e.g.,
`[.parent[data-collapsed=true]_&]:max-w-9`) or switch to the group pattern
`group-data-[collapsed=true]:max-w-9` and ensure the parent element has the
`group` class; replace the incorrect `[[data-collapsed=true]_&]:max-w-9` token
accordingly.
In @drizzle/seed-sources.ts:
- Line 15: The PostgresJsDatabase type needs its schema generic filled for full
typing: change the declaration of db from "const db: PostgresJsDatabase =
drizzle(client);" to include your schema generic, e.g. "const db:
PostgresJsDatabase<YourSchemaType> = drizzle(client);" and make sure
YourSchemaType (the generated or defined schema interface) is imported or
declared in this file so the drizzle(client) result is fully typed.
- Around line 433-435: The current loop over feedSourcesRaw uses
generateSlug(source.name) and then truncates to 40 chars for username, which can
cause collisions; change the logic that assigns username so it ensures
uniqueness: after computing baseSlug = generateSlug(source.name), derive a
baseUsername (e.g., baseSlug or baseSlug plus a short unique suffix) and then,
before finalizing username, check against already-used usernames (or a users
table lookup) and append/increment a numeric suffix until an unused 40-character
username is found; update the variable currently named username to this
collision-resistant value so subsequent user reuse is correct.
- Around line 451-460: The default avatar path "/images/person.png" used when
inserting the bot user (see newUserId and the db.insert(user).values block)
points to a missing asset; either add the image file at public/images/person.png
or update the inserted image value to an existing asset path (or a
config/constant) to prevent broken avatars—ensure the chosen image is committed
to the repo and referenced exactly as used in the image field.
🧹 Nitpick comments (36)
drizzle/seed-sources.ts (3)
18-25: Consider trimming leading/trailing hyphens.The slug generation doesn't remove leading or trailing hyphens that may result from the character replacements.
♻️ Suggested enhancement
const generateSlug = (name: string): string => { return name .toLowerCase() .replace(/[^a-z0-9\s-]/g, "") .replace(/\s+/g, "-") .replace(/-+/g, "-") + .replace(/^-+|-+$/g, "") .trim(); };
28-422: Consider externalizing feed sources data.The large hardcoded array makes maintenance more difficult. Moving this data to a separate JSON or configuration file would improve maintainability and allow non-developers to update feed sources.
433-500: Consider wrapping operations in a transaction.The current implementation could leave orphaned bot users if feed source creation fails after user creation. While seed scripts often accept this tradeoff, wrapping each source's user + feed_source operations in a transaction would ensure data consistency.
♻️ Transaction pattern example
for (const source of feedSourcesRaw) { await db.transaction(async (tx) => { const slug = generateSlug(source.name); const username = slug.substring(0, 40); // ... user creation/lookup logic using tx instead of db // ... feed source creation/update logic using tx instead of db }); }components/editor/editor/plugins/upload-images.tsx (1)
7-7: Address outstanding TODO comment and type safety warnings.The file contains unresolved technical debt:
- Line 7: TODO comment about "red squigly bois" is still present
- Line 13 & 60: Type assertions with
as unknownand acknowledged type uncertainty ("Type might be wrong here")These suggest lingering type safety issues that should be resolved before merge. Are these intentional deferred items or blockers?
Also applies to: 18-43, 60-60
server/api/router/comment.ts (2)
693-721: LGTM - ESLint suppression removed.The removal of ESLint suppression comments is acceptable. The code remains functional.
For future improvement, consider adding proper TypeScript types to the
commentparameter instead of usinganyto improve type safety throughout the codebase.
724-758: LGTM - ESLint suppression removed.The removal of ESLint suppression comments is acceptable. The function logic remains correct.
For future improvement, consider typing the
rootCommentanddescendantsparameters properly instead of usinganyto enhance type safety.EDITOR_SHORTCUTS.MD (1)
7-50: LGTM - Improved table formatting.The conversion to pipe-delimited Markdown tables improves readability and follows standard Markdown conventions.
Minor style improvements (entirely optional):
- Line 24: Capitalize "Windows" (proper noun for Microsoft's OS)
- Line 25: Consider "double-click" with a hyphen for compound verb usage
- Lines 40: Capitalize "YouTube" (proper brand name)
These are pedantic style points and don't affect functionality.
components/ui-components/link.tsx (1)
9-10: Consider handling additional URL schemes.The current external link detection covers HTTP(S) and protocol-relative URLs, but other schemes like
mailto:,tel:,ftp:, etc., would be treated as internal links. While Next.js Link may handle these gracefully, explicit handling would be more robust.♻️ Optional: More comprehensive external link detection
const isExternal = - props.href.startsWith("http") || props.href.startsWith("//"); + props.href.startsWith("http") || + props.href.startsWith("//") || + props.href.startsWith("mailto:") || + props.href.startsWith("tel:");Note:
mailto:andtel:links typically shouldn't open in a new tab, so you may want separate handling:const isExternal = props.href.startsWith("http") || props.href.startsWith("//"); const isSpecialScheme = props.href.startsWith("mailto:") || props.href.startsWith("tel:"); if (isExternal) { return ( <Headless.DataInteractive> <a {...props} ref={ref} target="_blank" rel="noopener noreferrer" /> </Headless.DataInteractive> ); } if (isSpecialScheme) { return ( <Headless.DataInteractive> <a {...props} ref={ref} /> </Headless.DataInteractive> ); }app/(app)/draft/[id]/page.tsx (1)
67-77: Consider using Next.jsImagecomponent for optimized loading.The native
<img>element works but misses out on Next.js image optimization (lazy loading, responsive sizing, format optimization). Since this is a user avatar that could vary in size/quality, usingnext/imagewould provide better performance.♻️ Suggested refactor
+import Image from "next/image"; ... {post.user.image ? ( - <img + <Image src={post.user.image} alt="" className="h-5 w-5 rounded-full object-cover" + width={20} + height={20} /> ) : (components/PostEditor/extensions/s3-image-upload.ts (1)
25-27: Type assertion workaround noted.The
as unknown as PluginKeycast on line 27 suggests a type mismatch between the genericPluginKey<DecorationSet>and what the Plugin constructor expects. This works but is fragile.Consider defining the plugin key without the generic to avoid the double cast:
const uploadKey = new PluginKey("s3-upload-image");e2e/my-posts.spec.ts (1)
14-19: Loading indicator wait is good; hardcoded timeout may cause flakiness.Waiting for the loading message to disappear (lines 15-17) is the correct approach. However, the additional
waitForTimeout(500)on line 19 is a hardcoded delay that can cause flaky tests - it may be too short on slower CI runners or unnecessarily slow on fast machines.Consider waiting for a specific element that indicates content has rendered instead.
♻️ Suggested improvement
// Wait for content to load - wait for loading message to disappear await expect(page.getByText("Fetching your posts...")).toBeHidden({ timeout: 15000, }); - // Additional wait for content to render - await page.waitForTimeout(500); + // Wait for tab content container to be visible + await expect(page.locator('article').first()).toBeVisible({ timeout: 5000 }).catch(() => { + // Tab might be empty, which is fine + });e2e/home.spec.ts (1)
11-22: Consider adding more assertions for authenticated homepage.The test correctly verifies the "Trending" heading and "Your Posts" link for authenticated users. However, the test coverage appears reduced compared to the previous implementation.
Consider adding a few more assertions to verify key elements of the authenticated homepage are present, such as:
- Verify article/content feed is visible
- Check for expected sidebar sections
- Confirm user-specific elements are displayed
This would help catch regressions in the authenticated user experience more reliably.
components/Layout/SidebarAppLayout.tsx (1)
1-20: Well-structured component with clear types.The "use client" directive is correctly placed, and the type definitions are clear and explicit. The separation of
AlgoliaConfigandSidebarAppLayoutPropsprovides good type safety.Minor suggestion: Consider extracting the
AlgoliaConfigtype to a shared types file if it's used in multiple places across the codebase. This would improve maintainability and ensure consistency.context/SidebarContext.tsx (2)
26-28: Use functional update to avoid stale closure and unnecessary re-renders.The
toggleSidebarcallback capturesisCollapsedin its closure, causing a new function reference on every state change. Using a functional update pattern would make the callback stable and avoid potential stale closure issues.♻️ Suggested improvement
const toggleSidebar = useCallback(() => { - setIsCollapsed(!isCollapsed); -}, [isCollapsed, setIsCollapsed]); + setIsCollapsed((prev) => !prev); +}, [setIsCollapsed]);
37-43: Consider memoizing the context value to prevent unnecessary re-renders.The context value object is recreated on every render, which could cause all consumers to re-render even when the actual values haven't changed. Wrapping it in
useMemowould optimize this.♻️ Suggested improvement
+import { createContext, useContext, useCallback, useMemo, type ReactNode } from "react"; ... +const contextValue = useMemo( + () => ({ isCollapsed, toggleSidebar, setCollapsed }), + [isCollapsed, toggleSidebar, setCollapsed] +); return ( <SidebarContext.Provider - value={{ isCollapsed, toggleSidebar, setCollapsed }} + value={contextValue} > {children} </SidebarContext.Provider> );app/(app)/articles/_client.tsx (1)
44-45: Minor edge case in relative time calculation.When
diffMinsis exactly 0, the conditiondiffMins < 1is true, so "just now" is correctly returned. However, ifdiffMins < 60were checked first, you'd get "0m ago". The current order is correct, but consider adding a comment for clarity or usingdiffMins <= 0to make the intent explicit.components/UnifiedContentCard/UnifiedContentCard.tsx (1)
68-81: InconsistentgetRelativeTimeimplementation across files.This function differs from the one in
app/(app)/articles/_client.tsx:
- Here: returns
"0m ago"whendiffMins < 60anddiffMinsis 0- Articles: returns
"just now"fordiffMins < 1Consider extracting this to a shared utility in
@/utils/utilsto ensure consistent behavior across the app.components/PostEditor/components/UrlMetadataPreview.tsx (1)
89-92: Consider using state for image error handling.The current approach directly manipulates the DOM via
e.currentTarget.style.display. A more React-idiomatic approach would use state to conditionally render the image container.♻️ Alternative approach using state
+import { useState } from "react"; ... export function UrlMetadataPreview({ metadata, isLoading, error, url, onRetry, }: UrlMetadataPreviewProps) { + const [imageError, setImageError] = useState(false); + // Loading state ... - {metadata.image && ( + {metadata.image && !imageError && ( <div className="relative aspect-video w-full overflow-hidden bg-neutral-100 dark:bg-neutral-900"> <img src={metadata.image} alt={metadata.title || "Link preview"} className="h-full w-full object-cover" - onError={(e) => { - // Hide image on error - e.currentTarget.style.display = "none"; - }} + onError={() => setImageError(true)} /> </div> )}components/SideBar/AppSidebar.tsx (1)
94-114: Consider handling missing icons gracefully.The iconMap lookup at line 95 could return
undefinedifitem.icondoesn't match any key. While theIcon &&check on line 102 prevents a crash, it might be helpful to log a warning during development for missing icons.♻️ Optional: Add development warning
{sidebarNavigation.map((item) => { const Icon = iconMap[item.icon as keyof typeof iconMap]; + if (!Icon && process.env.NODE_ENV === "development") { + console.warn(`Missing icon mapping for: ${item.icon}`); + } return (components/PostEditor/tabs/WriteTab.tsx (1)
9-9: Unused import.
copyfromcopy-to-clipboardis imported but never used in this component.Proposed fix
- import copy from "copy-to-clipboard";app/api/fetch-metadata/route.ts (1)
150-157: Consider consolidating URL validation with the existing utility.There's an
isValidUrlfunction inutils/utils.tsthat performs similar validation. However, your version adds a protocol check which is better for security. Consider either:
- Updating the shared utility to include the protocol check
- Documenting why this local version is needed
Based on relevant code snippets, the existing utility only checks URL parsing without protocol validation.
components/Header/MinimalHeader.tsx (1)
39-41: Simplify boolean expression.The ternary can be simplified.
Proposed fix
const { data: count } = api.notification.getCount.useQuery(undefined, { - enabled: session ? true : false, + enabled: !!session, });app/(app)/[username]/[slug]/_userLinkDetail.tsx (2)
79-99: State initialization and sync pattern has a subtle issue.The initial state values from
useMemo(lines 70-77) are used to initializeuseState(lines 79-85), butuseStateonly uses the initial value on first render. WhenlinkContentloads asynchronously, the initialuseMemovalues will benull/0, and the subsequent sync logic (lines 95-99) handles updates.However, setting state directly during render (lines 95-99) is an anti-pattern that can cause extra re-renders. Consider using
useEffectfor synchronization:Suggested refactor using useEffect
- const serverVoteKey = `${currentUserVote}-${currentUpvotes}-${currentDownvotes}`; - const [lastSyncedKey, setLastSyncedKey] = useState(serverVoteKey); - - if (serverVoteKey !== lastSyncedKey && linkContent) { - setUserVote(currentUserVote); - setVotes({ upvotes: currentUpvotes, downvotes: currentDownvotes }); - setLastSyncedKey(serverVoteKey); - } + useEffect(() => { + if (linkContent) { + setUserVote(linkContent.userVote ?? null); + setVotes({ + upvotes: linkContent.upvotes ?? 0, + downvotes: linkContent.downvotes ?? 0, + }); + } + }, [linkContent?.userVote, linkContent?.upvotes, linkContent?.downvotes]);
24-35: External service dependency for favicons.The favicon URL relies on Google's S2 favicon service. While this is commonly used, consider adding a fallback for cases where Google's service is unavailable or blocked, or document this external dependency.
server/api/router/content.ts (1)
1177-1242: Consider combining user lookup and post query into a single join.The current implementation makes two sequential database calls: one to find the user by username, then another to find the post. These could be combined into a single query using a join on the username condition, reducing latency.
Combined query suggestion
- // Find the user - const userResult = await ctx.db - .select({ id: user.id }) - .from(user) - .where(eq(user.username, input.username)) - .limit(1); - - if (userResult.length === 0) { - throw new TRPCError({ - code: "NOT_FOUND", - message: "User not found", - }); - } - - const authorId = userResult[0].id; - - // Find the link post - const linkPostResults = await ctx.db + // Find the link post with author in a single query + const linkPostResults = await ctx.db .select({ // ... existing fields }) .from(posts) .leftJoin(user, eq(posts.authorId, user.id)) .where( and( eq(posts.slug, input.slug), - eq(posts.authorId, authorId), + eq(user.username, input.username), eq(posts.type, "link"), eq(posts.status, "published"), lte(posts.publishedAt, new Date().toISOString()), ), ) .limit(1); + + if (linkPostResults.length === 0) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "Link post not found", + }); + }app/(app)/[username]/[slug]/page.tsx (1)
105-179: Consider extracting shared logic between getUserPost and getUserLinkPost.The
getUserLinkPostfunction shares significant code withgetUserPost(lines 31-103). The main differences are:
type: "link"vstype: "article"- Additional fields:
externalUrl,coverImageA shared helper with a type parameter could reduce duplication.
Shared helper suggestion
async function getUserPostByType( username: string, postSlug: string, postType: "article" | "link" ) { // Shared user lookup and query logic // Return with type-specific fields }e2e/editor.spec.ts (2)
149-191: Keyboard shortcuts use Mac-specific Meta key.The tests use
Meta+a,Meta+b, andMeta+iwhich only work on macOS. For cross-platform compatibility in CI environments (which often run Linux), consider usingControlor detecting the platform:Cross-platform keyboard shortcut suggestion
- // Select all text using keyboard - await page.keyboard.press("Meta+a"); - - // Apply bold via keyboard shortcut - await page.keyboard.press("Meta+b"); + // Select all text using keyboard (use Control for cross-platform) + await page.keyboard.press("Control+a"); + + // Apply bold via keyboard shortcut + await page.keyboard.press("Control+b");Alternatively, use Playwright's platform detection:
const modifier = process.platform === 'darwin' ? 'Meta' : 'Control'; await page.keyboard.press(`${modifier}+a`);
556-598: Multiple uses of waitForTimeout may cause flaky tests.The tests use
waitForTimeout(500)in several places to wait for state updates. This can cause flaky tests if the application is slower than expected. Consider using more deterministic waits:Replace fixed timeouts with condition-based waits
- // Wait a moment for state to update - await page.waitForTimeout(500); - - // Both nav and action bar Publish buttons should be disabled - const navPublishButton = page.locator('nav button:has-text("Publish")'); - await expect(navPublishButton).toBeDisabled(); + // Wait for Publish button state to update + const navPublishButton = page.locator('nav button:has-text("Publish")'); + await expect(navPublishButton).toBeDisabled({ timeout: 5000 });components/PostEditor/tabs/LinkTab.tsx (3)
37-52: Duplicate title auto-population logic may cause unnecessary renders or race conditions.The
onSuccesscallback (lines 39-43) already sets the title when metadata is fetched. TheuseEffect(lines 48-52) duplicates this logic, which is redundant and could cause double state updates.Additionally, including
onTitleChangein the dependency array without memoization in the parent could cause the effect to re-run unnecessarily.Suggested fix: Remove the redundant useEffect
const { metadata, isLoading, error, refetch } = useLinkMetadata(url, { onSuccess: (data) => { // Auto-populate title if empty if (!title && data.title) { onTitleChange(data.title); } onMetadataFetched?.(data); }, }); - // Update title when metadata changes (if title is still empty) - useEffect(() => { - if (metadata?.title && !title) { - onTitleChange(metadata.title); - } - }, [metadata?.title, title, onTitleChange]);
84-97: Magic number inconsistency with URL validation logic.The condition
url.length > 10doesn't align withisValidUrlinuseLinkMetadata.tswhich usesurl.length < 10. This creates a gap where a 10-character URL would be considered valid but not show the preview section.Consider extracting this threshold to a shared constant or using
>= 10for consistency.Suggested fix
- {(url.length > 10 || isLoading || error || metadata) && ( + {(url.length >= 10 || isLoading || error || metadata) && (
123-133: Consider using lucide-react's Info icon for consistency.The component already imports from
lucide-react(line 6). Using an inline SVG here is inconsistent with the rest of the codebase.Suggested fix
+import { Link as LinkIcon, Info } from "lucide-react"; // Then in the JSX: - <svg - className="mt-0.5 h-5 w-5 flex-shrink-0 text-blue-600 dark:text-blue-400" - viewBox="0 0 24 24" - fill="none" - stroke="currentColor" - strokeWidth="2" - > - <circle cx="12" cy="12" r="10" /> - <path d="M12 16v-4" /> - <path d="M12 8h.01" /> - </svg> + <Info className="mt-0.5 h-5 w-5 flex-shrink-0 text-blue-600 dark:text-blue-400" />components/PostEditor/hooks/useLinkMetadata.ts (2)
66-71: Potential stale closure issue withmetadatain callback dependency.The
fetchMetadatacallback depends onmetadata(line 127), which causes it to be recreated after every successful fetch. The early return check on line 69 compares againstmetadatafrom the closure, but sincefetchMetadatais recreated when metadata changes, this check may not behave as expected.Consider using a ref for the metadata check or removing
metadatafrom dependencies:Suggested fix using ref
const debounceTimerRef = useRef<NodeJS.Timeout | null>(null); const abortControllerRef = useRef<AbortController | null>(null); const lastFetchedUrlRef = useRef<string>(""); + const metadataRef = useRef<LinkMetadata | null>(null); + // Keep ref in sync + useEffect(() => { + metadataRef.current = metadata; + }, [metadata]); const fetchMetadata = useCallback( async (targetUrl: string) => { // Skip if already fetched this URL - if (targetUrl === lastFetchedUrlRef.current && metadata) { + if (targetUrl === lastFetchedUrlRef.current && metadataRef.current) { return; } // ... rest of function }, - [metadata, onSuccess, onError], + [onSuccess, onError], );
102-105: Error response parsing could fail silently.If the server returns a non-JSON error response,
response.json()will throw, masking the actual HTTP error. Consider adding a fallback:Suggested fix
if (!response.ok) { - const errorData = await response.json(); - throw new Error(errorData.error || "Failed to fetch metadata"); + let errorMessage = "Failed to fetch metadata"; + try { + const errorData = await response.json(); + errorMessage = errorData.error || errorMessage; + } catch { + // Response wasn't JSON, use default message + } + throw new Error(errorMessage); }components/PostEditor/toolbar/ArticleToolbar.tsx (2)
55-60: URL input lacks validation before submission.The
UrlInputcomponent accepts any non-empty string. For link and YouTube inputs, consider validating the URL format before callingonSubmitto provide immediate user feedback.Suggested fix
const handleSubmit = (e: React.FormEvent) => { e.preventDefault(); - if (url.trim()) { - onSubmit(url.trim()); + const trimmedUrl = url.trim(); + if (trimmedUrl) { + try { + new URL(trimmedUrl); + onSubmit(trimmedUrl); + } catch { + // Optionally show validation error to user + } } };
327-333: Hidden file input lacks accessible labeling.The hidden file input should have an
aria-labelor be associated with a label for screen reader users.Suggested fix
<input ref={fileInputRef} type="file" accept="image/jpeg,image/png,image/gif,image/webp" onChange={handleFileChange} className="hidden" + aria-label="Upload image" />app/(editor)/create/[[...paramsArr]]/_client.tsx (1)
638-638: Non-standard Tailwind classz-60.
z-60is not a default Tailwind z-index utility. Either usez-[60]for arbitrary values or ensure this is defined in your Tailwind config.Suggested fix
- <div className="z-60 absolute bottom-0 left-0 right-0 top-0 bg-black opacity-25" /> + <div className="absolute inset-0 z-[60] bg-black opacity-25" />
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
.github/workflows/e2e-tests.ymlis excluded by!**/*.yml.github/workflows/greetings.ymlis excluded by!**/*.yml.github/workflows/pull-request.ymlis excluded by!**/*.ymlicons/linkedin.svgis excluded by!**/*.svg,!**/*.svgpackage.jsonis excluded by!**/*.jsonpublic/images/codu-logo.pngis excluded by!**/*.png,!**/*.pngseed.ymlis excluded by!**/*.yml
📒 Files selected for processing (80)
CONTRIBUTING.mdEDITOR_SHORTCUTS.MDLICENSE.mdapp/(app)/[username]/[slug]/_userLinkDetail.tsxapp/(app)/[username]/[slug]/page.tsxapp/(app)/[username]/_sourceProfileClient.tsxapp/(app)/[username]/_usernameClient.tsxapp/(app)/advertise/_client.tsxapp/(app)/articles/_client.tsxapp/(app)/auth/page.tsxapp/(app)/code-of-conduct/page.tsxapp/(app)/draft/[id]/page.tsxapp/(app)/error.tsxapp/(app)/feed/[sourceSlug]/_client.tsxapp/(app)/feed/_client.tsxapp/(app)/forbidden/page.tsxapp/(app)/jobs/create/_client.tsxapp/(app)/layout.tsxapp/(app)/page.tsxapp/(app)/settings/_client.tsxapp/(editor)/create/[[...paramsArr]]/_client.tsxapp/(editor)/create/[[...paramsArr]]/navigation.tsxapp/api/fetch-metadata/route.tsapp/global-error.tsxauth.tscdk/README.mdcomponents/CoduChallenge/CoduChallenge.tsxcomponents/Comments/CommentsArea.tsxcomponents/ContentDetail/Layout.tsxcomponents/Feed/Filters.tsxcomponents/Header/MinimalHeader.tsxcomponents/Layout/SidebarAppLayout.tsxcomponents/Nav/Nav.tsxcomponents/NewsletterCTA/NewsletterCTA.tsxcomponents/PostEditor/PostEditor.tsxcomponents/PostEditor/components/TagInput.tsxcomponents/PostEditor/components/UrlMetadataPreview.tsxcomponents/PostEditor/extensions/index.tscomponents/PostEditor/extensions/s3-image-upload.tscomponents/PostEditor/hooks/useArticleEditor.tscomponents/PostEditor/hooks/useLinkMetadata.tscomponents/PostEditor/index.tscomponents/PostEditor/tabs/LinkTab.tsxcomponents/PostEditor/tabs/WriteTab.tsxcomponents/PostEditor/toolbar/ArticleToolbar.tsxcomponents/SideBar/AppSidebar.tsxcomponents/Theme/ThemeProvider.tsxcomponents/TrendingPosts/TrendingPostsLoading.tsxcomponents/UnifiedContentCard/UnifiedContentCard.tsxcomponents/editor/editor/index.tsxcomponents/editor/editor/plugins/upload-images.tsxcomponents/markdocNodes/FallbackMedia/FallbackMedia.tsxcomponents/ui-components/link.tsxcomponents/ui-components/listbox.tsxcomponents/ui-components/navbar.tsxcomponents/ui-components/sidebar-layout.tsxcomponents/ui-components/sidebar.tsxcomponents/ui-components/stacked-layout.tsxcomponents/ui/Search.tsxconfig/site_settings.tscontext/SidebarContext.tsxcurriculum/intro-to-react/introduction.mddrizzle/0016_migrate_comments.sqldrizzle/seed-sources.tse2e/articles.spec.tse2e/constants/constants.tse2e/editor.spec.tse2e/home.spec.tse2e/login.spec.tse2e/my-posts.spec.tse2e/saved.spec.tse2e/setup.tse2e/utils/utils.tsmarkdoc/editor/shortcuts/shortcuts.markdoc.tsmarkdoc/tags/markdoc-example.markdoc.tsserver/api/router/comment.tsserver/api/router/content.tsserver/lib/posts.tsstyles/globals.csstypes/next-auth.d.ts
💤 Files with no reviewable changes (3)
- components/ui-components/listbox.tsx
- components/Nav/Nav.tsx
- components/CoduChallenge/CoduChallenge.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-10-22T08:43:13.236Z
Learnt from: dineshsutihar
Repo: codu-code/codu PR: 1153
File: app/(app)/articles/[slug]/page.tsx:91-104
Timestamp: 2024-10-22T08:43:13.236Z
Learning: In `app/(app)/articles/[slug]/page.tsx`, when rendering content, the `renderedContent` needs to be treated as a string using a type assertion because it's used with `dangerouslySetInnerHTML`, and removing the type assertion could lead to runtime issues.
Applied to files:
app/(app)/feed/[sourceSlug]/_client.tsxapp/(app)/auth/page.tsxapp/(app)/error.tsxapp/(app)/[username]/[slug]/page.tsxapp/(app)/articles/_client.tsxapp/(app)/forbidden/page.tsxcomponents/Comments/CommentsArea.tsxapp/(app)/page.tsxapp/(app)/[username]/[slug]/_userLinkDetail.tsxapp/(editor)/create/[[...paramsArr]]/_client.tsxapp/(app)/draft/[id]/page.tsx
📚 Learning: 2024-10-17T01:03:59.199Z
Learnt from: JohnAllenTech
Repo: codu-code/codu PR: 1139
File: e2e/articles.spec.ts:133-147
Timestamp: 2024-10-17T01:03:59.199Z
Learning: In `e2e/articles.spec.ts` (TypeScript Playwright tests), avoid refactoring duplicated code into helper functions unless the code is reused elsewhere.
Applied to files:
e2e/saved.spec.tse2e/my-posts.spec.tse2e/home.spec.tse2e/articles.spec.tse2e/editor.spec.tse2e/login.spec.ts
📚 Learning: 2024-10-20T01:10:48.663Z
Learnt from: JohnAllenTech
Repo: codu-code/codu PR: 1158
File: e2e/articles.spec.ts:208-224
Timestamp: 2024-10-20T01:10:48.663Z
Learning: In `e2e/articles.spec.ts`, tests within the `Authenticated Articles Page` describe block assume the user is already authenticated, so explicit authentication steps are not necessary within individual tests.
Applied to files:
e2e/saved.spec.tse2e/my-posts.spec.tse2e/home.spec.tse2e/articles.spec.tse2e/editor.spec.tse2e/login.spec.ts
📚 Learning: 2024-10-17T01:07:41.406Z
Learnt from: JohnAllenTech
Repo: codu-code/codu PR: 1139
File: e2e/articles.spec.ts:0-0
Timestamp: 2024-10-17T01:07:41.406Z
Learning: In our end-to-end tests, using `waitUntil: "networkidle"` in `page.waitForNavigation` is unreliable because of ongoing background requests, so it should be avoided.
Applied to files:
e2e/articles.spec.ts
📚 Learning: 2024-10-18T04:18:28.906Z
Learnt from: RangerCreaky
Repo: codu-code/codu PR: 1146
File: server/api/router/series.ts:109-135
Timestamp: 2024-10-18T04:18:28.906Z
Learning: In the TypeScript file `server/api/router/series.ts`, when comments are included to aid reviewers, avoid suggesting their removal.
Applied to files:
components/Comments/CommentsArea.tsxserver/api/router/comment.ts
📚 Learning: 2024-10-28T23:51:53.631Z
Learnt from: JohnAllenTech
Repo: codu-code/codu PR: 1187
File: e2e/setup.ts:23-25
Timestamp: 2024-10-28T23:51:53.631Z
Learning: In the e2e tests setup scripts, it's acceptable to hardcode IDs for test data.
Applied to files:
e2e/constants/constants.ts
🧬 Code graph analysis (22)
components/Layout/SidebarAppLayout.tsx (4)
context/SidebarContext.tsx (2)
useSidebar(46-52)SidebarProvider(20-44)components/ui-components/sidebar-layout.tsx (1)
SidebarLayout(67-129)components/SideBar/AppSidebar.tsx (1)
AppSidebar(74-190)components/Header/MinimalHeader.tsx (1)
MinimalHeader(34-207)
app/api/fetch-metadata/route.ts (2)
utils/utils.ts (1)
isValidUrl(8-15)server/auth.ts (1)
auth(3-3)
drizzle/seed-sources.ts (1)
server/db/index.ts (1)
db(30-33)
components/Header/MinimalHeader.tsx (4)
context/SidebarContext.tsx (1)
useSidebar(46-52)auth.ts (1)
signIn(73-85)components/ui-components/link.tsx (1)
Link(5-25)components/ui/Search.tsx (2)
Search(517-552)MobileSearch(554-580)
app/(app)/[username]/[slug]/page.tsx (3)
server/db/index.ts (1)
db(30-33)server/db/schema.ts (3)
user(114-170)posts(273-356)tag(663-681)server/trpc/server.ts (1)
headers(21-26)
auth.ts (1)
server/db/schema.ts (1)
user(114-170)
app/(app)/articles/_client.tsx (1)
auth.ts (1)
session(65-72)
components/PostEditor/components/TagInput.tsx (1)
components/PostEditor/index.ts (1)
TagInput(16-16)
e2e/articles.spec.ts (1)
e2e/constants/constants.ts (1)
articleContent(1-2)
components/PostEditor/tabs/LinkTab.tsx (2)
components/PostEditor/hooks/useLinkMetadata.ts (2)
LinkMetadata(5-11)useLinkMetadata(52-195)components/PostEditor/components/UrlMetadataPreview.tsx (1)
UrlMetadataPreview(19-130)
app/(app)/layout.tsx (1)
components/Layout/SidebarAppLayout.tsx (1)
SidebarAppLayout(47-53)
app/(app)/page.tsx (2)
components/Hero/Hero.tsx (1)
Hero(215-245)components/PopularTags/PopularTags.tsx (1)
PopularTags(7-31)
e2e/editor.spec.ts (3)
e2e/utils/utils.ts (1)
loggedInAsUserOne(13-33)server/db/schema.ts (1)
tag(663-681)e2e/constants/constants.ts (1)
articleContent(1-2)
app/(app)/settings/_client.tsx (5)
components/ui-components/heading.tsx (1)
Subheading(21-33)components/ui-components/text.tsx (1)
Text(4-18)components/ui-components/fieldset.tsx (2)
Field(49-67)Label(69-83)components/Switch/Switch.tsx (1)
Switch(20-20)components/ui-components/divider.tsx (1)
Divider(3-20)
app/(editor)/create/[[...paramsArr]]/_client.tsx (9)
components/PostEditor/PostEditor.tsx (1)
PostType(10-10)components/PostEditor/index.ts (5)
PostType(3-3)LinkMetadata(13-13)WriteTab(6-6)LinkTab(7-7)TagInput(16-16)components/PostEditor/hooks/useLinkMetadata.ts (1)
LinkMetadata(5-11)utils/post.ts (2)
PostStatus(5-5)getPostStatus(13-21)components/PromptService/PromptContext.tsx (1)
usePrompt(16-16)hooks/useDebounce.tsx (1)
useDebounce(3-15)server/trpc/react.tsx (1)
api(11-11)schema/content.ts (1)
ConfirmContentSchema(159-172)markdoc/components.ts (1)
markdocComponents(7-13)
components/PostEditor/toolbar/ArticleToolbar.tsx (2)
components/editor/editor/index.tsx (1)
Editor(17-49)components/PostEditor/index.ts (1)
ArticleToolbar(18-18)
app/(app)/draft/[id]/page.tsx (3)
next.config.js (1)
config(22-67)components/ui-components/link.tsx (1)
Link(5-25)markdoc/components.ts (1)
markdocComponents(7-13)
components/PostEditor/extensions/index.ts (3)
components/PostEditor/index.ts (2)
getArticleExtensions(21-21)ArticleEditorExtensions(22-22)components/ui-components/heading.tsx (1)
Heading(7-19)components/ui-components/link.tsx (1)
Link(5-25)
components/ui-components/sidebar-layout.tsx (1)
components/ui-components/navbar.tsx (1)
NavbarItem(62-122)
components/PostEditor/extensions/s3-image-upload.ts (3)
app/actions/getUploadUrl.ts (1)
getUploadUrl(15-44)utils/s3helpers.ts (1)
uploadFile(17-28)components/editor/editor/plugins/upload-images.tsx (1)
startImageUpload(64-117)
e2e/utils/utils.ts (3)
e2e/constants/constants.ts (1)
E2E_USER_ONE_ID(7-7)server/db/index.ts (1)
db(30-33)server/db/schema.ts (1)
posts(273-356)
server/api/router/content.ts (2)
schema/content.ts (1)
GetContentBySlugSchema(44-46)server/db/schema.ts (3)
user(114-170)posts(273-356)bookmarks(535-561)
🪛 ast-grep (0.40.4)
components/PostEditor/extensions/s3-image-upload.ts
[warning] 63-71: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: loadingOverlay.innerHTML = <div class="flex items-center gap-2 bg-white dark:bg-neutral-800 px-3 py-2 rounded-full shadow-lg"> <svg class="animate-spin h-4 w-4 text-pink-600" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"> <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4"></circle> <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path> </svg> <span class="text-sm font-medium text-neutral-700 dark:text-neutral-300">Uploading...</span> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 63-71: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: loadingOverlay.innerHTML = <div class="flex items-center gap-2 bg-white dark:bg-neutral-800 px-3 py-2 rounded-full shadow-lg"> <svg class="animate-spin h-4 w-4 text-pink-600" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"> <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4"></circle> <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path> </svg> <span class="text-sm font-medium text-neutral-700 dark:text-neutral-300">Uploading...</span> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🪛 LanguageTool
EDITOR_SHORTCUTS.MD
[uncategorized] ~24-~24: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ... to Use Hotkeys 1. Press the meta key (windows key or mac cmd key) with the desired ho...
(A_WINDOWS)
[grammar] ~25-~25: Use a hyphen to join words.
Context: ... combination and for this you can double click the word or phrase or press meta+b...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~40-~40: The official name of this popular video platform is spelled with a capital “T”.
Context: ... | {% media src="url" /%} | | /youtube | Embed a YouTube video with src ...
(YOUTUBE)
[uncategorized] ~40-~40: The official name of this popular video platform is spelled with a capital “T”.
Context: ... Embed a YouTube video with src | {% youtube src="url" /%} | | /codepen |...
(YOUTUBE)
LICENSE.md
[style] ~161-~161: ‘any and all’ might be wordy. Consider a shorter alternative.
Context: ...ge, computer failure or malfunction, or any and all other commercial damages or losses)...
(EN_WORDINESS_PREMIUM_ANY_AND_ALL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
| }, | ||
| }); | ||
|
|
||
| const { setTheme, resolvedTheme } = useTheme(); |
There was a problem hiding this comment.
Handle undefined resolvedTheme during initial render.
The resolvedTheme from useTheme() can be undefined during initial hydration, which means the expression resolvedTheme === "dark" will evaluate to false by default. This could cause a flash of incorrect theme state or hydration mismatch.
💡 Suggested fix to handle the undefined case
- <Switch
- color="pink"
- checked={resolvedTheme === "dark"}
- onChange={(checked) => setTheme(checked ? "dark" : "light")}
- />
+ <Switch
+ color="pink"
+ checked={resolvedTheme === "dark"}
+ onChange={(checked) => setTheme(checked ? "dark" : "light")}
+ disabled={!resolvedTheme}
+ />Alternatively, you could add a loading state indicator or render the toggle only when resolvedTheme is defined.
Also applies to: 455-455
| // Debounce for auto-save | ||
| const debouncedValue = useDebounce(title + body, 1500); |
There was a problem hiding this comment.
Debounce value doesn't include link tab state, potentially breaking auto-save for link posts.
The debouncedValue only combines title + body, but for link posts, changes to linkUrl and linkTitle should also trigger auto-save. This may cause link posts to not auto-save when the user types in the link fields.
Suggested fix
- const debouncedValue = useDebounce(title + body, 1500);
+ const debouncedValue = useDebounce(
+ activeTab === "write" ? title + body : linkUrl + linkTitle,
+ 1500
+ );Note: You'll also need to update the comparison on line 443 accordingly.
🤖 Prompt for AI Agents
In @app/(editor)/create/[[...paramsArr]]/_client.tsx around lines 142 - 143, The
debouncedValue currently only combines title + body so edits to
linkUrl/linkTitle won't trigger auto-save for link posts; change the useDebounce
input to include linkUrl and linkTitle (e.g., combine title, body, linkUrl,
linkTitle or construct a single string/tuple that includes those four values) so
link-field edits are debounced too, and update the later auto-save comparison
that uses debouncedValue (the check that decides whether to save) to reflect the
expanded debounced shape.
| } else { | ||
| // For link tab, validate URL and title | ||
| if (!linkUrl || !linkTitle) { | ||
| toast.error("URL and title are required for link posts"); | ||
| return; | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Link posts lack URL format validation before publishing.
The link tab validation only checks for presence of linkUrl and linkTitle, but doesn't validate that linkUrl is a properly formatted URL. A user could publish a link post with an invalid URL.
Suggested fix
} else {
// For link tab, validate URL and title
- if (!linkUrl || !linkTitle) {
+ if (!linkTitle) {
+ toast.error("Title is required for link posts");
+ return;
+ }
+ try {
+ const url = new URL(linkUrl);
+ if (!["http:", "https:"].includes(url.protocol)) {
+ throw new Error("Invalid protocol");
+ }
+ } catch {
- toast.error("URL and title are required for link posts");
+ toast.error("Please enter a valid URL (must start with http:// or https://)");
return;
}
}📝 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.
| } else { | |
| // For link tab, validate URL and title | |
| if (!linkUrl || !linkTitle) { | |
| toast.error("URL and title are required for link posts"); | |
| return; | |
| } | |
| return; | |
| } | |
| } else { | |
| // For link tab, validate URL and title | |
| if (!linkTitle) { | |
| toast.error("Title is required for link posts"); | |
| return; | |
| } | |
| try { | |
| const url = new URL(linkUrl); | |
| if (!["http:", "https:"].includes(url.protocol)) { | |
| throw new Error("Invalid protocol"); | |
| } | |
| } catch { | |
| toast.error("Please enter a valid URL (must start with http:// or https://)"); | |
| return; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @app/(editor)/create/[[...paramsArr]]/_client.tsx around lines 307 - 313, The
link-post branch currently only checks presence of linkUrl and linkTitle but not
URL format; update the publish/submit handler handling the "link" tab (the
branch using linkUrl and linkTitle) to validate linkUrl is a well-formed URL
(e.g., attempt new URL(linkUrl) in a try/catch or use a robust URL regex), and
if validation fails call toast.error with a clear message and return to prevent
publishing; ensure the check happens before any network/save calls so invalid
URLs are rejected early.
| <p className="mt-3 text-sm text-neutral-600 dark:text-neutral-400"> | ||
| {isPostScheduled | ||
| ? `"${title || "Untitled"}" will appear on ${new Date(publishedTime).toLocaleDateString(undefined, { weekday: "long", month: "long", day: "numeric" })} at ${new Date(publishedTime).toLocaleTimeString(undefined, { hour: "numeric", minute: "2-digit" })}` | ||
| : activeTab === "link" | ||
| ? "Share this gem with the community?" | ||
| : `Your masterpiece "${title || "Untitled"}" is about to go live!`} | ||
| </p> |
There was a problem hiding this comment.
Publish confirmation dialog shows wrong title for link posts.
The confirmation message uses title variable, but for link posts this would be empty (link posts use linkTitle). Users will see "Untitled" in the dialog even when they've entered a link title.
Suggested fix
+ const displayTitle = activeTab === "link" ? linkTitle : title;
<p className="mt-3 text-sm text-neutral-600 dark:text-neutral-400">
{isPostScheduled
- ? `"${title || "Untitled"}" will appear on ${new Date(publishedTime).toLocaleDateString(...)}...`
+ ? `"${displayTitle || "Untitled"}" will appear on ${new Date(publishedTime).toLocaleDateString(...)}...`
: activeTab === "link"
? "Share this gem with the community?"
- : `Your masterpiece "${title || "Untitled"}" is about to go live!`}
+ : `Your masterpiece "${displayTitle || "Untitled"}" is about to go live!`}
</p>📝 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.
| <p className="mt-3 text-sm text-neutral-600 dark:text-neutral-400"> | |
| {isPostScheduled | |
| ? `"${title || "Untitled"}" will appear on ${new Date(publishedTime).toLocaleDateString(undefined, { weekday: "long", month: "long", day: "numeric" })} at ${new Date(publishedTime).toLocaleTimeString(undefined, { hour: "numeric", minute: "2-digit" })}` | |
| : activeTab === "link" | |
| ? "Share this gem with the community?" | |
| : `Your masterpiece "${title || "Untitled"}" is about to go live!`} | |
| </p> | |
| const displayTitle = activeTab === "link" ? linkTitle : title; | |
| <p className="mt-3 text-sm text-neutral-600 dark:text-neutral-400"> | |
| {isPostScheduled | |
| ? `"${displayTitle || "Untitled"}" will appear on ${new Date(publishedTime).toLocaleDateString(undefined, { weekday: "long", month: "long", day: "numeric" })} at ${new Date(publishedTime).toLocaleTimeString(undefined, { hour: "numeric", minute: "2-digit" })}` | |
| : activeTab === "link" | |
| ? "Share this gem with the community?" | |
| : `Your masterpiece "${displayTitle || "Untitled"}" is about to go live!`} | |
| </p> |
🤖 Prompt for AI Agents
In @app/(editor)/create/[[...paramsArr]]/_client.tsx around lines 584 - 590, The
publish confirmation always uses the title variable so link posts show
"Untitled" even when a linkTitle exists; update the render logic used in the <p>
(around isPostScheduled, activeTab, publishedTime) to derive a displayTitle
variable (e.g., use linkTitle when activeTab === "link", falling back to title,
then "Untitled") and use that displayTitle in the scheduled and non-scheduled
messages so link posts show their linkTitle in the confirmation dialog.
| {userNavigation.map((item) => ( | ||
| <Menu.Item key={item.name}> | ||
| {({ active }) => | ||
| item.onClick ? ( | ||
| <button | ||
| onClick={item.onClick} | ||
| className={`${ | ||
| active ? "bg-neutral-100" : "" | ||
| } block w-full px-4 py-2 text-left text-sm text-neutral-700`} | ||
| > | ||
| {item.name} | ||
| </button> | ||
| ) : ( | ||
| <Link | ||
| href={item.href} | ||
| className={`${ | ||
| active ? "bg-neutral-100" : "" | ||
| } block px-4 py-2 text-sm text-neutral-700`} | ||
| > | ||
| {item.name} | ||
| </Link> | ||
| ) | ||
| } | ||
| {item.onClick ? ( | ||
| <button | ||
| onClick={item.onClick} | ||
| className="flex w-full rounded px-4 py-2 text-left text-sm text-neutral-700 hover:bg-neutral-200 dark:text-neutral-200 dark:hover:bg-zinc-700" | ||
| > | ||
| {item.name} | ||
| </button> | ||
| ) : ( | ||
| <Link | ||
| href={item.href} | ||
| className="block rounded px-4 py-2 text-sm text-neutral-700 hover:bg-neutral-200 dark:text-neutral-200 dark:hover:bg-zinc-700" | ||
| > | ||
| {item.name} | ||
| </Link> | ||
| )} | ||
| </Menu.Item> |
There was a problem hiding this comment.
Active state styling removed from menu items.
The Menu.Item render prop provides an active parameter for keyboard navigation highlighting, but it's no longer used. Users navigating the menu with keyboard won't see visual feedback for the currently focused item.
Proposed fix to restore active state
<Menu.Item key={item.name}>
- {item.onClick ? (
+ {({ active }) => item.onClick ? (
<button
onClick={item.onClick}
- className="flex w-full rounded px-4 py-2 text-left text-sm text-neutral-700 hover:bg-neutral-200 dark:text-neutral-200 dark:hover:bg-zinc-700"
+ className={`flex w-full rounded px-4 py-2 text-left text-sm text-neutral-700 dark:text-neutral-200 ${
+ active ? "bg-neutral-200 dark:bg-zinc-700" : ""
+ }`}
>
{item.name}
</button>
) : (
<Link
href={item.href}
- className="block rounded px-4 py-2 text-sm text-neutral-700 hover:bg-neutral-200 dark:text-neutral-200 dark:hover:bg-zinc-700"
+ className={`block rounded px-4 py-2 text-sm text-neutral-700 dark:text-neutral-200 ${
+ active ? "bg-neutral-200 dark:bg-zinc-700" : ""
+ }`}
>
{item.name}
</Link>
)}
</Menu.Item>🤖 Prompt for AI Agents
In @app/(editor)/create/[[...paramsArr]]/navigation.tsx around lines 141 - 158,
Menu.Item's render prop previously provided an active boolean for keyboard/focus
highlighting but the current map ignores it; update the Menu.Item usage for both
branches so you accept the render prop (e.g., Menu.Item(({ active }) => ...))
and apply conditional classes based on active to the button and Link elements
(the block that renders item.onClick and the Link branch), ensuring the active
class names (for example hover/active background and text) are appended when
active is true so keyboard navigation shows visual feedback.
| href={item.href} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className={`focus-style rounded-md p-1 text-neutral-400 transition-all duration-300 hover:scale-105 hover:text-white hover:brightness-110 focus:scale-105 focus:text-white focus:brightness-110 ${item.customStyle.toLowerCase()}`} |
There was a problem hiding this comment.
toLowerCase() on customStyle may break hex color classes.
The customStyle.toLowerCase() call could break Tailwind classes with hex colors like bg-[#0A66C2] (LinkedIn blue), converting it to bg-[#0a66c2]. While CSS hex colors are case-insensitive, Tailwind's JIT compiler may not recognize the lowercased version if the original is defined differently.
🔧 Suggested fix
-className={`focus-style rounded-md p-1 text-neutral-400 transition-all duration-300 hover:scale-105 hover:text-white hover:brightness-110 focus:scale-105 focus:text-white focus:brightness-110 ${item.customStyle.toLowerCase()}`}
+className={`focus-style rounded-md p-1 text-neutral-400 transition-all duration-300 hover:scale-105 hover:text-white hover:brightness-110 focus:scale-105 focus:text-white focus:brightness-110 ${item.customStyle}`}📝 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.
| className={`focus-style rounded-md p-1 text-neutral-400 transition-all duration-300 hover:scale-105 hover:text-white hover:brightness-110 focus:scale-105 focus:text-white focus:brightness-110 ${item.customStyle.toLowerCase()}`} | |
| className={`focus-style rounded-md p-1 text-neutral-400 transition-all duration-300 hover:scale-105 hover:text-white hover:brightness-110 focus:scale-105 focus:text-white focus:brightness-110 ${item.customStyle}`} |
🤖 Prompt for AI Agents
In @components/SideBar/AppSidebar.tsx at line 180, The template currently
lowercases item.customStyle in the AppSidebar.tsx className, which can alter
Tailwind hex color token casing (e.g., bg-[#0A66C2]) and break JIT matching;
remove the toLowerCase() call and instead interpolate the original value (safely
coalesced to an empty string or trimmed) so hex classes keep their original
casing and null/undefined values are handled (update the className string
interpolation around item.customStyle in AppSidebar.tsx to use the original
item.customStyle with a fallback).
| // Collapsed state - animate max-width to contain hover area (matches container 300ms animation) | ||
| "max-w-full transition-[max-width] duration-300 ease-in-out [[data-collapsed=true]_&]:max-w-9", |
There was a problem hiding this comment.
Verify the collapsed state selector syntax.
The selector [[data-collapsed=true]_&]:max-w-9 uses non-standard Tailwind arbitrary variant syntax. The double brackets with underscore may not work as intended.
If you're targeting a parent with data-collapsed="true", the correct Tailwind syntax should be:
- For parent selector:
[.parent[data-collapsed=true]_&]:max-w-9 - Or using Tailwind's group pattern:
group-data-[collapsed=true]:max-w-9(withgroupclass on parent)
Please verify this works correctly and consider using the standard group/data attribute pattern:
💡 Suggested approach using group pattern
- "max-w-full transition-[max-width] duration-300 ease-in-out [[data-collapsed=true]_&]:max-w-9",
+ "max-w-full transition-[max-width] duration-300 ease-in-out group-data-[collapsed=true]:max-w-9",Then ensure the parent container has the group class applied.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @components/ui-components/sidebar.tsx around lines 143 - 144, The Tailwind
variant `[[data-collapsed=true]_&]:max-w-9` is non-standard; update the
collapsed-state selector in the class string that starts with "max-w-full
transition-[max-width] duration-300 ease-in-out" to use a valid
parent/data-attribute or group variant: either use Tailwind's parent selector
form with the parent selector token (e.g.,
`[.parent[data-collapsed=true]_&]:max-w-9`) or switch to the group pattern
`group-data-[collapsed=true]:max-w-9` and ensure the parent element has the
`group` class; replace the incorrect `[[data-collapsed=true]_&]:max-w-9` token
accordingly.
| } | ||
|
|
||
| const client = postgres(DATABASE_URL, { max: 1 }); | ||
| const db: PostgresJsDatabase = drizzle(client); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Complete the TypeScript type annotation.
The PostgresJsDatabase type requires a schema generic parameter for full type safety.
♻️ Suggested fix
+import type { schema } from "../server/db/schema";
+
const client = postgres(DATABASE_URL, { max: 1 });
-const db: PostgresJsDatabase = drizzle(client);
+const db: PostgresJsDatabase<typeof schema> = drizzle(client, { schema });📝 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.
| const db: PostgresJsDatabase = drizzle(client); | |
| import type { schema } from "../server/db/schema"; | |
| import { drizzle } from "drizzle-orm/postgres-js"; | |
| import postgres from "postgres"; | |
| import { nanoid } from "nanoid"; | |
| const DATABASE_URL = process.env.DATABASE_URL; | |
| if (!DATABASE_URL) { | |
| throw new Error("DATABASE_URL is not defined"); | |
| } | |
| const client = postgres(DATABASE_URL, { max: 1 }); | |
| const db: PostgresJsDatabase<typeof schema> = drizzle(client, { schema }); |
🤖 Prompt for AI Agents
In @drizzle/seed-sources.ts at line 15, The PostgresJsDatabase type needs its
schema generic filled for full typing: change the declaration of db from "const
db: PostgresJsDatabase = drizzle(client);" to include your schema generic, e.g.
"const db: PostgresJsDatabase<YourSchemaType> = drizzle(client);" and make sure
YourSchemaType (the generated or defined schema interface) is imported or
declared in this file so the drizzle(client) result is fully typed.
| for (const source of feedSourcesRaw) { | ||
| const slug = generateSlug(source.name); | ||
| const username = slug.substring(0, 40); // Max 40 chars for username |
There was a problem hiding this comment.
Risk of username collisions from truncation.
Truncating the slug to 40 characters on line 435 could cause two different feed sources with similar names to share the same username, leading to incorrect user reuse. For example, sources named "Josh W Comeau Blog Updates Newsletter" and "Josh W Comeau Blog Updates Archives" would both truncate to the same 40-character username.
🔧 Suggested fix
Consider appending a unique suffix before truncation, or check for existing usernames and add a numeric suffix if collision detected:
for (const source of feedSourcesRaw) {
const slug = generateSlug(source.name);
- const username = slug.substring(0, 40); // Max 40 chars for username
+ let username = slug.substring(0, 40);
// Check if user already exists
- const existingUser = await db
+ let existingUser = await db
.select({ id: user.id })
.from(user)
.where(eq(user.username, username))
.limit(1);
+
+ // Handle collision by appending counter
+ let counter = 1;
+ while (existingUser.length > 0 && counter < 100) {
+ const suffix = `-${counter}`;
+ username = slug.substring(0, 40 - suffix.length) + suffix;
+ existingUser = await db
+ .select({ id: user.id })
+ .from(user)
+ .where(eq(user.username, username))
+ .limit(1);
+ counter++;
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @drizzle/seed-sources.ts around lines 433 - 435, The current loop over
feedSourcesRaw uses generateSlug(source.name) and then truncates to 40 chars for
username, which can cause collisions; change the logic that assigns username so
it ensures uniqueness: after computing baseSlug = generateSlug(source.name),
derive a baseUsername (e.g., baseSlug or baseSlug plus a short unique suffix)
and then, before finalizing username, check against already-used usernames (or a
users table lookup) and append/increment a numeric suffix until an unused
40-character username is found; update the variable currently named username to
this collision-resistant value so subsequent user reuse is correct.
| const newUserId = `feed_${slug}_${nanoid(8)}`; | ||
| const [newUser] = await db | ||
| .insert(user) | ||
| .values({ | ||
| id: newUserId, | ||
| username: username, | ||
| name: source.name, | ||
| websiteUrl: source.websiteUrl || "", | ||
| bio: source.description || "", | ||
| image: "/images/person.png", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for person.png in the repository
fd -t f "person.png" .Repository: codu-code/codu
Length of output: 40
🏁 Script executed:
# Check for public directory structure
ls -la public/ 2>/dev/null || echo "public/ directory not found"Repository: codu-code/codu
Length of output: 237
🏁 Script executed:
# Search for any images directory
fd -t d "images" . -type dRepository: codu-code/codu
Length of output: 228
🏁 Script executed:
# Search for other usages of /images/ paths in the codebase
rg "/images/" --type ts --type tsx --type jsRepository: codu-code/codu
Length of output: 85
🏁 Script executed:
# Check if there are any other default avatar/image assignments
rg "person\.png|default.*image|avatar" --type ts --type tsx --type js -iRepository: codu-code/codu
Length of output: 85
🏁 Script executed:
# Look at the database schema to understand image field requirements
fd -t f "schema" . | grep -E "\.(ts|prisma|sql)$" | head -5Repository: codu-code/codu
Length of output: 113
🏁 Script executed:
# Search for /images/ path usage without specifying type
rg "/images/"Repository: codu-code/codu
Length of output: 3753
🏁 Script executed:
# Search for person.png, default image, or avatar references
rg "person\.png|avatar|default.*image" -iRepository: codu-code/codu
Length of output: 5563
🏁 Script executed:
# Check the database schema for user.image field
cat server/db/schema.tsRepository: codu-code/codu
Length of output: 50370
Add missing default avatar image file to avoid broken images.
Line 460 uses a hardcoded path /images/person.png as the default bot user avatar. This file does not exist in the repository. Create the image file at public/images/person.png or update the default to point to an existing image asset.
🤖 Prompt for AI Agents
In @drizzle/seed-sources.ts around lines 451 - 460, The default avatar path
"/images/person.png" used when inserting the bot user (see newUserId and the
db.insert(user).values block) points to a missing asset; either add the image
file at public/images/person.png or update the inserted image value to an
existing asset path (or a config/constant) to prevent broken avatars—ensure the
chosen image is committed to the repo and referenced exactly as used in the
image field.
- Add PostEditor with WriteTab and LinkTab for article/link creation - Add URL metadata fetching for link posts - Add article toolbar with formatting options - Add E2E editor tests and test utilities - Update E2E setup with link post test data - Update slash command and image upload extensions
…ling and state management
…lly and prevent layout shift with scrollbar-gutter in globals.css
… Filters components
NiallJoeMaher
force-pushed
the
feat/layout-updates
branch
from
f18a3f2 to
3a48106
Compare
NiallJoeMaher
had a problem deploying
to
production
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/Feed/Filters.tsx (1)
1-1: Fix Prettier formatting issues before merging.The pipeline indicates formatting issues that need to be resolved. Please run the following command to fix:
prettier --write components/Feed/Filters.tsx
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/(app)/articles/_client.tsxapp/(app)/feed/_client.tsxapp/(app)/page.tsxcomponents/Feed/Filters.tsxcomponents/Header/MinimalHeader.tsxcomponents/UnifiedContentCard/UnifiedContentCard.tsxstyles/globals.css
🚧 Files skipped from review as they are similar to previous changes (5)
- components/Header/MinimalHeader.tsx
- app/(app)/feed/_client.tsx
- app/(app)/page.tsx
- components/UnifiedContentCard/UnifiedContentCard.tsx
- app/(app)/articles/_client.tsx
🧰 Additional context used
🪛 GitHub Actions: Code Quality Checks
components/Feed/Filters.tsx
[warning] 1-1: Code style issues found in the file. Run 'prettier --write' to fix.
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix format issues in this file.
[error] 1-1: Process completed with exit code 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (2)
styles/globals.css (1)
3-6: LGTM! Good use of scrollbar-gutter for layout stability.This CSS property effectively prevents layout shift when the scrollbar appears or disappears, improving the user experience. The implementation is correct and the comment clearly explains the intent.
components/Feed/Filters.tsx (1)
83-83: LGTM! Responsive spacing improvements are well-implemented.The changes make the filter controls more compact on mobile while maintaining comfortable spacing on larger screens. The responsive pattern is consistent across all three dropdown buttons.
Also applies to: 87-87, 132-132, 177-177
NiallJoeMaher
had a problem deploying
to
production
GitHub Actions
Failure
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
NiallJoeMaher
had a problem deploying
to
production
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @e2e/saved.spec.ts:
- Line 55: Replace the unreliable page.waitForLoadState("networkidle") call in
saved.spec.ts with an explicit wait for the saved-items UI or network response:
remove the page.waitForLoadState("networkidle") and instead wait for the saved
list to render via an explicit wait (e.g., wait for the saved items container
selector or the first saved article element) or wait for the bookmarking network
response before asserting; update the relevant test in saved.spec.ts that calls
page.waitForLoadState to use these explicit waits (e.g., wait for the saved
items selector or the bookmark API response) so the test no longer relies on
"networkidle".
🧹 Nitpick comments (1)
e2e/saved.spec.ts (1)
50-51: Consider replacing fixed wait with explicit state check.While the 1000ms wait on line 51 is better than the previous 500ms, and the improvements on lines 60-62 using
filterwithhasTextare excellent, the fixed timeout could still be replaced with an explicit state verification for better reliability.♻️ Optional refactor
- // Wait for bookmark mutation to complete - await page.waitForTimeout(1000); + // Wait for bookmark mutation to complete - button should show bookmarked state + await expect(bookmarkButton).toHaveAttribute("aria-pressed", "true", { timeout: 2000 }).catch(() => {});Note: This assumes the bookmark button updates its
aria-pressedattribute. Adjust the selector/attribute based on the actual implementation. If no reliable state indicator exists, the current timeout is acceptable.Also applies to: 60-62
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
e2e/articles.spec.tse2e/my-posts.spec.tse2e/saved.spec.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-17T01:03:59.199Z
Learnt from: JohnAllenTech
Repo: codu-code/codu PR: 1139
File: e2e/articles.spec.ts:133-147
Timestamp: 2024-10-17T01:03:59.199Z
Learning: In `e2e/articles.spec.ts` (TypeScript Playwright tests), avoid refactoring duplicated code into helper functions unless the code is reused elsewhere.
Applied to files:
e2e/saved.spec.tse2e/my-posts.spec.tse2e/articles.spec.ts
📚 Learning: 2024-10-20T01:10:48.663Z
Learnt from: JohnAllenTech
Repo: codu-code/codu PR: 1158
File: e2e/articles.spec.ts:208-224
Timestamp: 2024-10-20T01:10:48.663Z
Learning: In `e2e/articles.spec.ts`, tests within the `Authenticated Articles Page` describe block assume the user is already authenticated, so explicit authentication steps are not necessary within individual tests.
Applied to files:
e2e/saved.spec.tse2e/my-posts.spec.tse2e/articles.spec.ts
📚 Learning: 2024-10-17T01:07:41.406Z
Learnt from: JohnAllenTech
Repo: codu-code/codu PR: 1139
File: e2e/articles.spec.ts:0-0
Timestamp: 2024-10-17T01:07:41.406Z
Learning: In our end-to-end tests, using `waitUntil: "networkidle"` in `page.waitForNavigation` is unreliable because of ongoing background requests, so it should be avoided.
Applied to files:
e2e/my-posts.spec.tse2e/articles.spec.ts
🧬 Code graph analysis (1)
e2e/my-posts.spec.ts (2)
e2e/constants/constants.ts (1)
articleExcerpt(4-4)e2e/utils/constants.ts (1)
articleExcerpt(4-4)
🪛 GitHub Actions: Code Quality Checks
e2e/saved.spec.ts
[error] 55-55: Playwright: Unexpected use of networkidle (Playwright rule violation).
🪛 GitHub Check: Run ESLint and Prettier
e2e/saved.spec.ts
[failure] 55-55:
Unexpected use of networkidle
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (5)
e2e/my-posts.spec.ts (2)
14-23: LGTM! Excellent improvement to test reliability.Replacing the fixed 500ms wait with explicit state-based waits is a significant improvement. The approach correctly waits for the loading indicator to be hidden, then verifies content is visible. The timeouts (20s and 15s) are appropriate for potentially slow CI environments.
73-73: Good addition of explicit timeouts for slower environments.Adding 15-second timeouts to these visibility assertions improves test stability in slower CI/mobile environments while maintaining reasonable wait times.
Also applies to: 79-79, 87-87
e2e/articles.spec.ts (2)
111-139: LGTM! Robust handling of hydration and missing data.The enhanced sorting verification correctly:
- Waits for articles to render and hydrate
- Uses a try-catch pattern to handle cases where time elements may not be present
- Defaults to
nullfor missing dates- Only verifies sort order when sufficient data exists
- Ensures articles loaded even if dates aren't available
This makes the test significantly more reliable across different rendering conditions.
346-349: Good addition of explicit timeout.The 10-second timeout accommodates slower mobile browsers and network conditions when verifying the bookmark state change.
e2e/saved.spec.ts (1)
36-48: LGTM! Improved reliability with explicit visibility checks.These changes correctly:
- Wait for the first article to be visible with appropriate timeout
- Extract the article heading with explicit visibility verification
- Check bookmark button visibility before interaction
This makes the test more resilient to timing variations.
e2e/saved.spec.ts
Outdated
|
|
||
| // Navigate to saved page | ||
| await page.goto("http://localhost:3000/saved"); | ||
| await page.waitForLoadState("networkidle"); |
There was a problem hiding this comment.
Replace networkidle with explicit waits.
The use of waitForLoadState("networkidle") violates project conventions and is flagged by the linter. Based on learnings, networkidle is unreliable in this codebase due to ongoing background requests.
🔧 Proposed fix
Replace the networkidle wait with an explicit wait for the saved items to be present:
- await page.waitForLoadState("networkidle");
+ // Wait for saved items section to be loaded
+ await page.waitForSelector("article", { timeout: 10000 }).catch(() => {});Alternatively, if you expect the saved list to always contain items after bookmarking, you could wait for the first article directly:
- await page.waitForLoadState("networkidle");
+ await page.waitForSelector("article", { timeout: 10000 });Based on learnings, networkidle is unreliable in end-to-end tests.
📝 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.
| await page.waitForLoadState("networkidle"); | |
| await page.waitForSelector("article", { timeout: 10000 }); |
🧰 Tools
🪛 GitHub Actions: Code Quality Checks
[error] 55-55: Playwright: Unexpected use of networkidle (Playwright rule violation).
🪛 GitHub Check: Run ESLint and Prettier
[failure] 55-55:
Unexpected use of networkidle
🤖 Prompt for AI Agents
In @e2e/saved.spec.ts at line 55, Replace the unreliable
page.waitForLoadState("networkidle") call in saved.spec.ts with an explicit wait
for the saved-items UI or network response: remove the
page.waitForLoadState("networkidle") and instead wait for the saved list to
render via an explicit wait (e.g., wait for the saved items container selector
or the first saved article element) or wait for the bookmarking network response
before asserting; update the relevant test in saved.spec.ts that calls
page.waitForLoadState to use these explicit waits (e.g., wait for the saved
items selector or the bookmark API response) so the test no longer relies on
"networkidle".
issues - Remove unused imports from layout.tsx (headers, ThemeProvider, etc.) - Remove unused sortAPIToUI mapping from articles/_client.tsx - Fix type parameter naming in Filters.tsx (prefix with underscore) - Fix networkidle usage in saved.spec.ts (use domcontentloaded) - Use platform-aware keyboard modifier in editor.spec.ts (Meta on Mac, Control on Linux)
NiallJoeMaher
had a problem deploying
to
production
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/(app)/articles/_client.tsx (1)
428-437: Fix invalid heading markup (h1contains adiv)
<h1>should not contain a block<div>(invalid content model + can confuse semantics/AT). Use an inline element styled asinline-flexinstead.Proposed fix
- <h1 className="text-2xl font-bold tracking-tight text-neutral-800 dark:text-neutral-50"> + <h1 className="text-2xl font-bold tracking-tight text-neutral-800 dark:text-neutral-50"> {typeof tag === "string" ? ( - <div className="flex items-center justify-center"> + <span className="inline-flex items-center justify-center"> <TagIcon className="mr-3 h-6 w-6 text-neutral-800 dark:text-neutral-200" /> {getCamelCaseFromLower(tag)} - </div> + </span> ) : ( "Articles" )} </h1>
🧹 Nitpick comments (1)
app/(app)/articles/_client.tsx (1)
352-381: The proposed refactor improves code clarity, but the concern about API values in URLs is not substantiatedEvidence shows the app has always used only UI sort values (
"recent" | "trending" | "popular") in URLs. The API sort schema ("newest" | "oldest" | "top" | "trending") is never exposed to URL parameters. The current validation already handles unknown sort values safely by defaulting to"recent", so there is no silent behavior change for invalid values.The proposed
parseUISorthelper is a nice enhancement for documenting possible legacy values explicitly, but is not required for backward compatibility since the existing code already degrades gracefully.Also applies to: 404-419
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/(app)/articles/_client.tsxapp/(app)/layout.tsxcomponents/Feed/Filters.tsxe2e/editor.spec.tse2e/saved.spec.ts
💤 Files with no reviewable changes (1)
- app/(app)/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/saved.spec.ts
- components/Feed/Filters.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (2)
e2e/editor.spec.ts (2)
8-10: Excellent cross-platform solution!The MOD_KEY constant correctly handles platform-specific keyboard modifiers. This makes the tests portable across macOS (Meta/Command), Windows, and Linux (Control), addressing a common source of E2E test failures in cross-platform environments.
162-162: LGTM! Platform-aware keyboard shortcuts implemented correctly.All instances of hardcoded Meta key shortcuts have been properly replaced with the dynamic
MOD_KEYconstant. The template literal syntax integrates correctly with Playwright'skeyboard.press()method, ensuring these tests now work reliably on all platforms.Also applies to: 165-165, 185-185, 188-188, 692-692
NiallJoeMaher
had a problem deploying
to
production
GitHub Actions
Error
NiallJoeMaher
had a problem deploying
to
production
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @drizzle/0017_lowly_gunslinger.sql:
- Around line 174-210: The migration creates two duplicate tables—feed_sources
and FeedSource—with the same columns and different naming and enum types
(feed_source_status vs FeedSourceStatus); remove the duplication by
consolidating into a single canonical table (choose either snake_case
feed_sources or camelCase FeedSource), migrate any data if needed, standardize
the status enum to a single enum type and unify its values (e.g., 'active' vs
'ACTIVE'), keep necessary unique constraints (e.g., FeedSource_id_unique) and
timestamps, and drop the redundant table and enum declarations so only one
consistent table/schema remains.
- Around line 331-332: The ALTER TABLE for "PostVote" creates a foreign key
"PostVote_postId_Post_id_fk" that incorrectly references the legacy PascalCase
table "public"."Post"("id"); update this FK to reference the new snake_case
posts table ("public"."posts"("id")) and rename the constraint to a consistent
name (e.g., reflect post_id/posts_id) so it matches the migration's naming
convention and other migrations (tables like post_tags, post_votes, reports)
that reference "posts". Ensure the constraint definition on "PostVote" (and any
related index/constraint names) is changed accordingly so all references point
to "public"."posts"("id").
🧹 Nitpick comments (2)
drizzle/0017_lowly_gunslinger.sql (2)
162-163: Redundant UNIQUE constraint on PRIMARY KEY columns.
Discussion.idandFeedSource.idare alreadyPRIMARY KEY, which implicitly guarantees uniqueness. The additionalUNIQUEconstraints are unnecessary:-- Line 162: Discussion CONSTRAINT "Discussion_id_unique" UNIQUE("id") -- Line 209: FeedSource CONSTRAINT "FeedSource_id_unique" UNIQUE("id")♻️ Suggested fix
CREATE TABLE "Discussion" ( "id" serial PRIMARY KEY NOT NULL, ... - CONSTRAINT "Discussion_id_unique" UNIQUE("id") ); CREATE TABLE "FeedSource" ( "id" serial PRIMARY KEY NOT NULL, ... - CONSTRAINT "FeedSource_id_unique" UNIQUE("id") );Also applies to: 209-210
339-395: Good indexing strategy overall.The migration includes comprehensive indexes for:
- Foreign key columns (enabling efficient JOINs and cascading deletes)
- Frequently filtered columns (status, type, published, publishedAt)
- Unique constraints on slugs and URLs
One consideration:
ContentBookmarkandContentVotehave unique constraints on composite keys but lack a standalone index onuserIdfor queries like "fetch all bookmarks for user X". If this query pattern is common, consider adding:CREATE INDEX "ContentBookmark_userId_index" ON "ContentBookmark" USING btree ("userId"); CREATE INDEX "ContentVote_userId_index" ON "ContentVote" USING btree ("userId");
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
drizzle/meta/0017_snapshot.jsonis excluded by!**/*.jsondrizzle/meta/_journal.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
.claude/commands/do-test.mddrizzle/0017_lowly_gunslinger.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (6)
.claude/commands/do-test.md (1)
1-51: Well-structured documentation for development verification workflow.The file clearly defines a sequential testing pipeline with appropriate command examples, conditional E2E execution, and expected output format. The YAML frontmatter and markdown structure are appropriate for Claude IDE integration.
drizzle/0017_lowly_gunslinger.sql (5)
1-395: PR objective mismatch: Migration file doesn't align with "Fixes to responsive styling".This migration file introduces a comprehensive database schema (enums, tables, FKs, indexes) for content, voting, bookmarking, feeds, and reporting. The PR title and description mention only responsive styling fixes—there's no mention of schema changes.
Either this file was included unintentionally, or the PR description needs updating to reflect the scope of database changes.
295-295: VerifyFeedSourcetable exists before FK reference.Line 295 creates a foreign key from
AggregatedArticle.sourceIdtoFeedSource.id, butFeedSourceis created later in this same migration (lines 193-210). Drizzle typically handles ordering, but verify the generated SQL execution order is correct.
1-13: The duplicate enum types are intentional and represent different schema versions for backward compatibility. The codebase explicitly useslegacyprefixes inserver/db/schema.ts(e.g.,legacyFeedSourceStatus,legacyVoteType,legacyReportReason,legacyReportStatus), with both versions actively used: lowercase conventions for new tables and PascalCase for legacy tables. No consolidation is needed.Likely an incorrect or invalid review comment.
219-235: These are not duplicate tables but intentional legacy support tables.The schema contains two voting tables by design:
post_votes(lines 219–226): New system for the refactoredpoststable (uuid IDs)PostVote(lines 228–235): Legacy system for the originalPosttable (text IDs)Both tables coexist to support a phased data migration. Migration 0015 explicitly converts data from the legacy
PostVotesystem to the newpost_votessystem. The application code uses onlypost_votes; thePostVotetable is marked as legacy in the schema (legacyPostVotesrelation) and no longer actively referenced in business logic.
292-294: BothsessionandPosttables exist in earlier migrations; migration will succeed, but schema design warrants attention.The
sessiontable (renamed fromSessionin migration 0004) and thePosttable (created in 0000) both exist before this migration, so the ALTER TABLE statements will execute successfully. However, note that this migration introduces a parallel table structure:
- Modifies the legacy capitalized
Posttable (addingupvotesanddownvotescolumns)- Simultaneously creates a new lowercase
poststableThis pattern may indicate incomplete schema consolidation; consider clarifying the intended coexistence of
Postandpoststables.
drizzle/0017_lowly_gunslinger.sql
Outdated
| CREATE TABLE "feed_sources" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "name" text NOT NULL, | ||
| "url" text NOT NULL, | ||
| "website_url" text, | ||
| "logo_url" text, | ||
| "slug" varchar(100), | ||
| "category" varchar(50), | ||
| "description" text, | ||
| "status" "feed_source_status" DEFAULT 'active' NOT NULL, | ||
| "user_id" text, | ||
| "last_fetched_at" timestamp(3) with time zone, | ||
| "last_success_at" timestamp(3) with time zone, | ||
| "error_count" integer DEFAULT 0 NOT NULL, | ||
| "last_error" text, | ||
| "created_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "updated_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE "FeedSource" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "name" text NOT NULL, | ||
| "url" text NOT NULL, | ||
| "websiteUrl" text, | ||
| "logoUrl" text, | ||
| "category" varchar(50), | ||
| "slug" varchar(100), | ||
| "description" text, | ||
| "status" "FeedSourceStatus" DEFAULT 'ACTIVE' NOT NULL, | ||
| "lastFetchedAt" timestamp(3) with time zone, | ||
| "lastSuccessAt" timestamp(3) with time zone, | ||
| "errorCount" integer DEFAULT 0 NOT NULL, | ||
| "lastError" text, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "updatedAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "FeedSource_id_unique" UNIQUE("id") | ||
| ); |
There was a problem hiding this comment.
Duplicate tables: feed_sources vs FeedSource.
Both tables serve the same purpose (RSS/feed source management) with nearly identical columns but different naming conventions:
feed_sources(lines 174-191): snake_case columns, usesfeed_source_statusenumFeedSource(lines 193-210): camelCase columns, usesFeedSourceStatusenum
This duplication increases maintenance burden and risks data inconsistency. Clarify the intended use case for each, or consolidate.
🤖 Prompt for AI Agents
In @drizzle/0017_lowly_gunslinger.sql around lines 174 - 210, The migration
creates two duplicate tables—feed_sources and FeedSource—with the same columns
and different naming and enum types (feed_source_status vs FeedSourceStatus);
remove the duplication by consolidating into a single canonical table (choose
either snake_case feed_sources or camelCase FeedSource), migrate any data if
needed, standardize the status enum to a single enum type and unify its values
(e.g., 'active' vs 'ACTIVE'), keep necessary unique constraints (e.g.,
FeedSource_id_unique) and timestamps, and drop the redundant table and enum
declarations so only one consistent table/schema remains.
drizzle/0017_lowly_gunslinger.sql
Outdated
| ALTER TABLE "PostVote" ADD CONSTRAINT "PostVote_postId_Post_id_fk" FOREIGN KEY ("postId") REFERENCES "public"."Post"("id") ON DELETE cascade ON UPDATE cascade;--> statement-breakpoint | ||
| ALTER TABLE "PostVote" ADD CONSTRAINT "PostVote_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE cascade;--> statement-breakpoint |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for Post table creation in migration files
rg -n 'CREATE TABLE.*"Post"' drizzle/
# Also search for posts table (snake_case)
rg -n 'CREATE TABLE.*"posts"' drizzle/
# Get context around line 237 to see what table is being created
sed -n '230,240p' drizzle/0017_lowly_gunslinger.sql
# Get full context of lines 331-332
sed -n '325,335p' drizzle/0017_lowly_gunslinger.sqlRepository: codu-code/codu
Length of output: 2686
🏁 Script executed:
# Check for both PostVote and post_votes table creation
rg -n 'CREATE TABLE.*(PostVote|post_votes)' drizzle/
# Get more context around the PostVote creation to understand the migration pattern
sed -n '200,250p' drizzle/0017_lowly_gunslinger.sqlRepository: codu-code/codu
Length of output: 1999
PostVote FK references legacy Post table while migration establishes snake_case naming convention.
The FK constraint at line 331-332 references "public"."Post"("id") (PascalCase), but this migration introduces a new posts table (snake_case) and correctly updates other tables like post_tags, post_votes, and reports to reference "public"."posts"("id"). While the legacy Post table exists, this inconsistency suggests PostVote should reference posts instead to align with the migration's naming convention.
🤖 Prompt for AI Agents
In @drizzle/0017_lowly_gunslinger.sql around lines 331 - 332, The ALTER TABLE
for "PostVote" creates a foreign key "PostVote_postId_Post_id_fk" that
incorrectly references the legacy PascalCase table "public"."Post"("id"); update
this FK to reference the new snake_case posts table ("public"."posts"("id")) and
rename the constraint to a consistent name (e.g., reflect post_id/posts_id) so
it matches the migration's naming convention and other migrations (tables like
post_tags, post_votes, reports) that reference "posts". Ensure the constraint
definition on "PostVote" (and any related index/constraint names) is changed
accordingly so all references point to "public"."posts"("id").
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @drizzle/0017_lowly_gunslinger.sql:
- Around line 15-174: The ContentReport table currently allows both contentId
and discussionId to be null or both set; update the CREATE TABLE for
"ContentReport" to add a CHECK constraint (e.g., named
ContentReport_content_discussion_xor_check) that enforces exactly one of
contentId or discussionId is non-NULL (XOR semantics), so inserts/updates must
supply one and only one of contentId or discussionId; add the constraint to the
table definition (and migration) alongside the existing columns and FKs.
- Around line 293-339: The migration has idempotency gaps: direct ALTERs (e.g.,
adding "upvotes"/"downvotes" to "Post") and DO blocks adding constraints (e.g.,
"AggregatedArticle_sourceId_FeedSource_id_fk", "bookmarks_post_id_posts_id_fk",
"PostVote_postId_Post_id_fk", and the "session" primary key) can fail if
referenced tables don't exist; change direct ALTER TABLE statements to use ALTER
TABLE IF EXISTS for tables like "Post" and wrap constraint/PK additions' DO
blocks to catch WHEN undefined_object THEN null (instead of/in addition to
duplicate_object) so missing-table (42P01) errors are ignored and migrations
become safe in CI/reordered runs.
🧹 Nitpick comments (2)
drizzle/0017_lowly_gunslinger.sql (2)
340-396: Indexes look broadly sensible; consider partial indexes for low-cardinality booleans if this becomes large.E.g.
Content_published_indexon a boolean can be less useful than a partial index (onlypublished = true) depending on distribution. Not a blocker—just something to revisit with real query plans.
153-174: Redundant UNIQUE constraints on serial primary keys ("Discussion_id_unique","FeedSource_id_unique").They don’t hurt correctness, but they’re noise and add extra catalog objects. Worth dropping if you regenerate this migration.
Also applies to: 194-211
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
drizzle/meta/0017_snapshot.jsonis excluded by!**/*.jsondrizzle/meta/_journal.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
drizzle/0017_lowly_gunslinger.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (2)
drizzle/0017_lowly_gunslinger.sql (2)
266-339: No type/target mismatch exists—both voting systems have correctly aligned types.The
post_votes.post_id→posts.idforeign key usesuuidon both sides, andPostVote.postId→Post.idusestexton both sides. The schema shows two intentional voting systems coexisting after migration 0015: the legacyPostVotetable (for the originalPosttable, typetext) and the newpost_votestable (for the newpoststable, typeuuid). Each system is internally consistent. While having both voting systems active after migration may be confusing architecturally, the FK constraints are correctly typed and will not cause broken joins.
1-14: No action needed—enum duplication is intentional and properly separated.The duplicated enum sets follow a clear parallel migration architecture:
VoteType/ReportReason/etc. (PascalCase) are used exclusively by the newContent-based system, whilevote_type/report_reason/etc. (snake_case) are used exclusively by the legacypostssystem. No cross-contamination exists, and the naming convention makes accidental mixing unlikely. This is a safe, well-designed transitional pattern as acknowledged in the migration header comment.
| CREATE TABLE IF NOT EXISTS "AggregatedArticle" ( | ||
| "id" text PRIMARY KEY NOT NULL, | ||
| "sourceId" integer NOT NULL, | ||
| "shortId" varchar(20), | ||
| "title" text NOT NULL, | ||
| "slug" varchar(350) NOT NULL, | ||
| "excerpt" text, | ||
| "externalUrl" varchar(2000) NOT NULL, | ||
| "imageUrl" text, | ||
| "ogImageUrl" text, | ||
| "sourceAuthor" varchar(200), | ||
| "publishedAt" timestamp with time zone, | ||
| "fetchedAt" timestamp with time zone, | ||
| "upvotes" integer DEFAULT 0 NOT NULL, | ||
| "downvotes" integer DEFAULT 0 NOT NULL, | ||
| "clickCount" integer DEFAULT 0 NOT NULL, | ||
| "createdAt" timestamp with time zone DEFAULT now() NOT NULL, | ||
| "updatedAt" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "AggregatedArticleBookmark" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "articleId" text NOT NULL, | ||
| "userId" text NOT NULL, | ||
| "createdAt" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "AggregatedArticleTag" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "articleId" text NOT NULL, | ||
| "tagId" integer NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "AggregatedArticleVote" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "articleId" text NOT NULL, | ||
| "userId" text NOT NULL, | ||
| "voteType" "VoteType" NOT NULL, | ||
| "createdAt" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "bookmarks" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "post_id" uuid NOT NULL, | ||
| "user_id" text NOT NULL, | ||
| "created_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "bookmarks_post_id_user_id_key" UNIQUE("post_id","user_id") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "comment_votes" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "comment_id" uuid NOT NULL, | ||
| "user_id" text NOT NULL, | ||
| "vote_type" "vote_type" NOT NULL, | ||
| "created_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "comment_votes_comment_id_user_id_key" UNIQUE("comment_id","user_id") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "comments" ( | ||
| "id" uuid PRIMARY KEY NOT NULL, | ||
| "post_id" uuid NOT NULL, | ||
| "author_id" text NOT NULL, | ||
| "parent_id" uuid, | ||
| "path" text NOT NULL, | ||
| "depth" integer DEFAULT 0 NOT NULL, | ||
| "body" text NOT NULL, | ||
| "upvotes_count" integer DEFAULT 0 NOT NULL, | ||
| "downvotes_count" integer DEFAULT 0 NOT NULL, | ||
| "created_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "updated_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "deleted_at" timestamp(3) with time zone, | ||
| "legacy_comment_id" integer | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "Content" ( | ||
| "id" text PRIMARY KEY NOT NULL, | ||
| "type" "ContentType" NOT NULL, | ||
| "title" varchar(500) NOT NULL, | ||
| "body" text, | ||
| "excerpt" text, | ||
| "userId" text, | ||
| "externalUrl" varchar(2000), | ||
| "imageUrl" text, | ||
| "ogImageUrl" text, | ||
| "sourceId" integer, | ||
| "sourceAuthor" varchar(200), | ||
| "published" boolean DEFAULT false NOT NULL, | ||
| "publishedAt" timestamp(3) with time zone, | ||
| "upvotes" integer DEFAULT 0 NOT NULL, | ||
| "downvotes" integer DEFAULT 0 NOT NULL, | ||
| "readTimeMins" integer, | ||
| "clickCount" integer DEFAULT 0 NOT NULL, | ||
| "slug" varchar(300), | ||
| "canonicalUrl" text, | ||
| "coverImage" text, | ||
| "showComments" boolean DEFAULT true NOT NULL, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "updatedAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "ContentBookmark" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "contentId" text NOT NULL, | ||
| "userId" text NOT NULL, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "ContentBookmark_contentId_userId_key" UNIQUE("contentId","userId") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "ContentReport" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "contentId" text, | ||
| "discussionId" integer, | ||
| "reporterId" text NOT NULL, | ||
| "reason" "ReportReason" NOT NULL, | ||
| "details" text, | ||
| "status" "ReportStatus" DEFAULT 'PENDING' NOT NULL, | ||
| "reviewedById" text, | ||
| "reviewedAt" timestamp(3) with time zone, | ||
| "actionTaken" text, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "ContentTag" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "contentId" text NOT NULL, | ||
| "tagId" integer NOT NULL, | ||
| CONSTRAINT "ContentTag_contentId_tagId_key" UNIQUE("contentId","tagId") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "ContentVote" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "contentId" text NOT NULL, | ||
| "userId" text NOT NULL, | ||
| "voteType" "VoteType" NOT NULL, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "ContentVote_contentId_userId_key" UNIQUE("contentId","userId") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "Discussion" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "body" text NOT NULL, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "updatedAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "contentId" text NOT NULL, | ||
| "userId" text NOT NULL, | ||
| "parentId" integer, | ||
| "upvotes" integer DEFAULT 0 NOT NULL, | ||
| "downvotes" integer DEFAULT 0 NOT NULL, | ||
| CONSTRAINT "Discussion_id_unique" UNIQUE("id") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "DiscussionVote" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "discussionId" integer NOT NULL, | ||
| "userId" text NOT NULL, | ||
| "voteType" "VoteType" NOT NULL, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "DiscussionVote_discussionId_userId_key" UNIQUE("discussionId","userId") | ||
| ); | ||
| --> statement-breakpoint |
There was a problem hiding this comment.
ContentReport should enforce “exactly one of contentId / discussionId” (or whatever rule you intend).
Right now it’s possible to insert invalid rows (both NULL, or both set) and still satisfy FKs. Add a CHECK constraint if the business rule is one-or-the-other.
Proposed fix (CHECK constraint)
CREATE TABLE IF NOT EXISTS "ContentReport" (
"id" serial PRIMARY KEY NOT NULL,
"contentId" text,
"discussionId" integer,
"reporterId" text NOT NULL,
"reason" "ReportReason" NOT NULL,
"details" text,
"status" "ReportStatus" DEFAULT 'PENDING' NOT NULL,
"reviewedById" text,
"reviewedAt" timestamp(3) with time zone,
"actionTaken" text,
"createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL
+ ,
+ CONSTRAINT "ContentReport_exactly_one_target_chk"
+ CHECK ( (("contentId" IS NOT NULL)::int + ("discussionId" IS NOT NULL)::int) = 1 )
);🤖 Prompt for AI Agents
In @drizzle/0017_lowly_gunslinger.sql around lines 15 - 174, The ContentReport
table currently allows both contentId and discussionId to be null or both set;
update the CREATE TABLE for "ContentReport" to add a CHECK constraint (e.g.,
named ContentReport_content_discussion_xor_check) that enforces exactly one of
contentId or discussionId is non-NULL (XOR semantics), so inserts/updates must
supply one and only one of contentId or discussionId; add the constraint to the
table definition (and migration) alongside the existing columns and FKs.
| CREATE TABLE IF NOT EXISTS "feed_sources" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "name" text NOT NULL, | ||
| "url" text NOT NULL, | ||
| "website_url" text, | ||
| "logo_url" text, | ||
| "slug" varchar(100), | ||
| "category" varchar(50), | ||
| "description" text, | ||
| "status" "feed_source_status" DEFAULT 'active' NOT NULL, | ||
| "user_id" text, | ||
| "last_fetched_at" timestamp(3) with time zone, | ||
| "last_success_at" timestamp(3) with time zone, | ||
| "error_count" integer DEFAULT 0 NOT NULL, | ||
| "last_error" text, | ||
| "created_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "updated_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "FeedSource" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "name" text NOT NULL, | ||
| "url" text NOT NULL, | ||
| "websiteUrl" text, | ||
| "logoUrl" text, | ||
| "category" varchar(50), | ||
| "slug" varchar(100), | ||
| "description" text, | ||
| "status" "FeedSourceStatus" DEFAULT 'ACTIVE' NOT NULL, | ||
| "lastFetchedAt" timestamp(3) with time zone, | ||
| "lastSuccessAt" timestamp(3) with time zone, | ||
| "errorCount" integer DEFAULT 0 NOT NULL, | ||
| "lastError" text, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "updatedAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "FeedSource_id_unique" UNIQUE("id") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "post_tags" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "post_id" uuid NOT NULL, | ||
| "tag_id" integer NOT NULL, | ||
| CONSTRAINT "post_tags_post_id_tag_id_key" UNIQUE("post_id","tag_id") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "post_votes" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "post_id" uuid NOT NULL, | ||
| "user_id" text NOT NULL, | ||
| "vote_type" "vote_type" NOT NULL, | ||
| "created_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "post_votes_post_id_user_id_key" UNIQUE("post_id","user_id") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "PostVote" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "postId" text NOT NULL, | ||
| "userId" text NOT NULL, | ||
| "voteType" "VoteType" NOT NULL, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| CONSTRAINT "PostVote_postId_userId_key" UNIQUE("postId","userId") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "posts" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "type" "post_type" NOT NULL, | ||
| "author_id" text NOT NULL, | ||
| "title" varchar(500) NOT NULL, | ||
| "slug" varchar(300) NOT NULL, | ||
| "excerpt" text, | ||
| "body" text, | ||
| "canonical_url" text, | ||
| "cover_image" text, | ||
| "external_url" varchar(2000), | ||
| "source_id" integer, | ||
| "source_author" varchar(200), | ||
| "reading_time" integer, | ||
| "upvotes_count" integer DEFAULT 0 NOT NULL, | ||
| "downvotes_count" integer DEFAULT 0 NOT NULL, | ||
| "comments_count" integer DEFAULT 0 NOT NULL, | ||
| "views_count" integer DEFAULT 0 NOT NULL, | ||
| "status" "post_status" DEFAULT 'draft' NOT NULL, | ||
| "published_at" timestamp(3) with time zone, | ||
| "featured" boolean DEFAULT false NOT NULL, | ||
| "pinned_until" timestamp(3) with time zone, | ||
| "show_comments" boolean DEFAULT true NOT NULL, | ||
| "created_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "updated_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
| "legacy_post_id" text | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "reports" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "post_id" uuid, | ||
| "comment_id" uuid, | ||
| "reporter_id" text NOT NULL, | ||
| "reason" "report_reason" NOT NULL, | ||
| "details" text, | ||
| "status" "report_status" DEFAULT 'pending' NOT NULL, | ||
| "reviewed_by_id" text, | ||
| "reviewed_at" timestamp(3) with time zone, | ||
| "action_taken" text, | ||
| "created_at" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "SponsorInquiry" ( | ||
| "id" serial PRIMARY KEY NOT NULL, | ||
| "name" varchar(100) NOT NULL, | ||
| "email" varchar(255) NOT NULL, | ||
| "company" varchar(100), | ||
| "phone" varchar(50), | ||
| "interests" text, | ||
| "budgetRange" "SponsorBudgetRange" DEFAULT 'EXPLORING', | ||
| "goals" text, | ||
| "status" "SponsorInquiryStatus" DEFAULT 'PENDING' NOT NULL, | ||
| "createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the drizzle directory structure
find . -type f -name "*.sql" | head -20Repository: codu-code/codu
Length of output: 777
🏁 Script executed:
# Now examine the specific migration file
wc -l drizzle/0017_lowly_gunslinger.sqlRepository: codu-code/codu
Length of output: 95
🏁 Script executed:
# Read the migration file, focusing on the lines mentioned and looking for foreign keys
cat -n drizzle/0017_lowly_gunslinger.sql | sed -n '175,291p'Repository: codu-code/codu
Length of output: 4977
🏁 Script executed:
# Search for foreign key references across the entire migration to see which tables reference which
rg "REFERENCES|FOREIGN KEY" drizzle/0017_lowly_gunslinger.sql -A 2 -B 2Repository: codu-code/codu
Length of output: 12156
🏁 Script executed:
# Check if there are any comments in the file explaining the dual schema
cat drizzle/0017_lowly_gunslinger.sql | grep -i "legacy\|migration\|parallel\|both\|dual"Repository: codu-code/codu
Length of output: 466
🏁 Script executed:
# Look at other migration files to understand the pattern
ls -la drizzle/*.sql | tail -10Repository: codu-code/codu
Length of output: 928
🏁 Script executed:
# Check the broader context of the migration file - what's before and after the mentioned lines
head -174 drizzle/0017_lowly_gunslinger.sql | tail -50Repository: codu-code/codu
Length of output: 1850
🏁 Script executed:
# Check what comes after line 291
tail -n +292 drizzle/0017_lowly_gunslinger.sql | head -50Repository: codu-code/codu
Length of output: 12365
Dual schema requires explicit documentation of migration strategy.
Two parallel table ecosystems exist with separate foreign key hierarchies: feed_sources/post_votes/posts reference each other, while FeedSource/PostVote/Post form an independent hierarchy. New rows will end up in only one ecosystem unless application logic enforces synchronization. Add explicit comments explaining which tables are "old" vs "new", which the application actively writes to, and the migration plan to converge them. Without this clarity, future developers risk creating data inconsistencies or duplicating logic across both systems.
| DO $$ BEGIN ALTER TABLE "session" ADD PRIMARY KEY ("sessionToken"); EXCEPTION WHEN duplicate_object THEN null; WHEN invalid_table_definition THEN null; END $$;--> statement-breakpoint | ||
| ALTER TABLE "Post" ADD COLUMN IF NOT EXISTS "upvotes" integer DEFAULT 0 NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "Post" ADD COLUMN IF NOT EXISTS "downvotes" integer DEFAULT 0 NOT NULL;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "AggregatedArticle" ADD CONSTRAINT "AggregatedArticle_sourceId_FeedSource_id_fk" FOREIGN KEY ("sourceId") REFERENCES "public"."FeedSource"("id") ON DELETE no action ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "AggregatedArticleBookmark" ADD CONSTRAINT "AggregatedArticleBookmark_articleId_AggregatedArticle_id_fk" FOREIGN KEY ("articleId") REFERENCES "public"."AggregatedArticle"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "AggregatedArticleBookmark" ADD CONSTRAINT "AggregatedArticleBookmark_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "AggregatedArticleTag" ADD CONSTRAINT "AggregatedArticleTag_articleId_AggregatedArticle_id_fk" FOREIGN KEY ("articleId") REFERENCES "public"."AggregatedArticle"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "AggregatedArticleTag" ADD CONSTRAINT "AggregatedArticleTag_tagId_Tag_id_fk" FOREIGN KEY ("tagId") REFERENCES "public"."Tag"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "AggregatedArticleVote" ADD CONSTRAINT "AggregatedArticleVote_articleId_AggregatedArticle_id_fk" FOREIGN KEY ("articleId") REFERENCES "public"."AggregatedArticle"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "AggregatedArticleVote" ADD CONSTRAINT "AggregatedArticleVote_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "bookmarks" ADD CONSTRAINT "bookmarks_post_id_posts_id_fk" FOREIGN KEY ("post_id") REFERENCES "public"."posts"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "bookmarks" ADD CONSTRAINT "bookmarks_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "comment_votes" ADD CONSTRAINT "comment_votes_comment_id_comments_id_fk" FOREIGN KEY ("comment_id") REFERENCES "public"."comments"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "comment_votes" ADD CONSTRAINT "comment_votes_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "comments" ADD CONSTRAINT "comments_post_id_posts_id_fk" FOREIGN KEY ("post_id") REFERENCES "public"."posts"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "comments" ADD CONSTRAINT "comments_author_id_user_id_fk" FOREIGN KEY ("author_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "comments" ADD CONSTRAINT "comments_parent_id_fkey" FOREIGN KEY ("parent_id") REFERENCES "public"."comments"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "Content" ADD CONSTRAINT "Content_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "Content" ADD CONSTRAINT "Content_sourceId_FeedSource_id_fk" FOREIGN KEY ("sourceId") REFERENCES "public"."FeedSource"("id") ON DELETE set null ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentBookmark" ADD CONSTRAINT "ContentBookmark_contentId_Content_id_fk" FOREIGN KEY ("contentId") REFERENCES "public"."Content"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentBookmark" ADD CONSTRAINT "ContentBookmark_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentReport" ADD CONSTRAINT "ContentReport_contentId_Content_id_fk" FOREIGN KEY ("contentId") REFERENCES "public"."Content"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentReport" ADD CONSTRAINT "ContentReport_discussionId_Discussion_id_fk" FOREIGN KEY ("discussionId") REFERENCES "public"."Discussion"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentReport" ADD CONSTRAINT "ContentReport_reporterId_user_id_fk" FOREIGN KEY ("reporterId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentReport" ADD CONSTRAINT "ContentReport_reviewedById_user_id_fk" FOREIGN KEY ("reviewedById") REFERENCES "public"."user"("id") ON DELETE set null ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentTag" ADD CONSTRAINT "ContentTag_contentId_Content_id_fk" FOREIGN KEY ("contentId") REFERENCES "public"."Content"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentTag" ADD CONSTRAINT "ContentTag_tagId_Tag_id_fk" FOREIGN KEY ("tagId") REFERENCES "public"."Tag"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentVote" ADD CONSTRAINT "ContentVote_contentId_Content_id_fk" FOREIGN KEY ("contentId") REFERENCES "public"."Content"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "ContentVote" ADD CONSTRAINT "ContentVote_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "Discussion" ADD CONSTRAINT "Discussion_contentId_Content_id_fk" FOREIGN KEY ("contentId") REFERENCES "public"."Content"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "Discussion" ADD CONSTRAINT "Discussion_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "Discussion" ADD CONSTRAINT "Discussion_parentId_fkey" FOREIGN KEY ("parentId") REFERENCES "public"."Discussion"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "DiscussionVote" ADD CONSTRAINT "DiscussionVote_discussionId_Discussion_id_fk" FOREIGN KEY ("discussionId") REFERENCES "public"."Discussion"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "DiscussionVote" ADD CONSTRAINT "DiscussionVote_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "feed_sources" ADD CONSTRAINT "feed_sources_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE set null ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "post_tags" ADD CONSTRAINT "post_tags_post_id_posts_id_fk" FOREIGN KEY ("post_id") REFERENCES "public"."posts"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "post_tags" ADD CONSTRAINT "post_tags_tag_id_Tag_id_fk" FOREIGN KEY ("tag_id") REFERENCES "public"."Tag"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "post_votes" ADD CONSTRAINT "post_votes_post_id_posts_id_fk" FOREIGN KEY ("post_id") REFERENCES "public"."posts"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "post_votes" ADD CONSTRAINT "post_votes_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "PostVote" ADD CONSTRAINT "PostVote_postId_Post_id_fk" FOREIGN KEY ("postId") REFERENCES "public"."Post"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "PostVote" ADD CONSTRAINT "PostVote_userId_user_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE cascade; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "posts" ADD CONSTRAINT "posts_author_id_user_id_fk" FOREIGN KEY ("author_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "posts" ADD CONSTRAINT "posts_source_id_feed_sources_id_fk" FOREIGN KEY ("source_id") REFERENCES "public"."feed_sources"("id") ON DELETE set null ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "reports" ADD CONSTRAINT "reports_post_id_posts_id_fk" FOREIGN KEY ("post_id") REFERENCES "public"."posts"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "reports" ADD CONSTRAINT "reports_comment_id_comments_id_fk" FOREIGN KEY ("comment_id") REFERENCES "public"."comments"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "reports" ADD CONSTRAINT "reports_reporter_id_user_id_fk" FOREIGN KEY ("reporter_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint | ||
| DO $$ BEGIN ALTER TABLE "reports" ADD CONSTRAINT "reports_reviewed_by_id_user_id_fk" FOREIGN KEY ("reviewed_by_id") REFERENCES "public"."user"("id") ON DELETE set null ON UPDATE no action; EXCEPTION WHEN duplicate_object THEN null; END $$;--> statement-breakpoint |
There was a problem hiding this comment.
Idempotency gap: missing-table errors aren't handled ("session" PK, "Post" columns, FKs referencing non-existent tables).
These statements will fail with relation does not exist (error code 42P01) if prerequisite tables aren't present—common in CI when migrations are re-ordered, partially applied, or squashed. The idempotency concern is valid.
For the DO...EXCEPTION blocks, use WHEN undefined_object THEN null (not undefined_table, which is not a standard PostgreSQL exception name). For direct ALTER TABLE statements, prefer ALTER TABLE IF EXISTS as it's cleaner for DDL.
Recommended fix (use IF EXISTS for clarity; or catch undefined_object in DO blocks)
-DO $$ BEGIN ALTER TABLE "session" ADD PRIMARY KEY ("sessionToken"); EXCEPTION WHEN duplicate_object THEN null; WHEN invalid_table_definition THEN null; END $$;--> statement-breakpoint
-ALTER TABLE "Post" ADD COLUMN IF NOT EXISTS "upvotes" integer DEFAULT 0 NOT NULL;--> statement-breakpoint
-ALTER TABLE "Post" ADD COLUMN IF NOT EXISTS "downvotes" integer DEFAULT 0 NOT NULL;--> statement-breakpoint
+DO $$ BEGIN
+ ALTER TABLE IF EXISTS "session" ADD PRIMARY KEY ("sessionToken");
+EXCEPTION
+ WHEN duplicate_object THEN null
+ WHEN invalid_table_definition THEN null
+ WHEN undefined_object THEN null
+END $$;--> statement-breakpoint
+
+ALTER TABLE IF EXISTS "Post" ADD COLUMN IF NOT EXISTS "upvotes" integer DEFAULT 0 NOT NULL;--> statement-breakpoint
+ALTER TABLE IF EXISTS "Post" ADD COLUMN IF NOT EXISTS "downvotes" integer DEFAULT 0 NOT NULL;--> statement-breakpointFor FK additions, consider broadening exception handling to catch undefined_object for referenced table references (especially for "Post", "Tag", and "user").
🤖 Prompt for AI Agents
In @drizzle/0017_lowly_gunslinger.sql around lines 293 - 339, The migration has
idempotency gaps: direct ALTERs (e.g., adding "upvotes"/"downvotes" to "Post")
and DO blocks adding constraints (e.g.,
"AggregatedArticle_sourceId_FeedSource_id_fk", "bookmarks_post_id_posts_id_fk",
"PostVote_postId_Post_id_fk", and the "session" primary key) can fail if
referenced tables don't exist; change direct ALTER TABLE statements to use ALTER
TABLE IF EXISTS for tables like "Post" and wrap constraint/PK additions' DO
blocks to catch WHEN undefined_object THEN null (instead of/in addition to
duplicate_object) so missing-table (42P01) errors are ignored and migrations
become safe in CI/reordered runs.
1 participant
Fixes to responsive styling