feat: Roadmaps #83
feat: Roadmaps #83
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds a complete roadmap feature: DB migrations and RLS, TypeScript DB typings, server APIs for votes and bulk vote queries with visitor cookie handling, SSR public and admin roadmap pages (voting, modal, drag‑and‑drop, CRUD), UI components/hooks, theme/tailwind updates, and related utilities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Browser
participant Next as Next.js SSR
participant PublicPage as Roadmap Page (SSR)
participant API_VOTES as POST /api/roadmap/votes
participant API_VOTE as POST /api/roadmap/vote
participant DB as Supabase
Note over Next,DB: SSR fetch for initial render
User->>Browser: Open roadmap URL
Browser->>Next: SSR request
Next->>DB: fetch page, settings, roadmaps, board, columns, items
Next-->>PublicPage: render props (SSR)
Note over PublicPage,API_VOTES: client boot — fetch bulk votes
PublicPage->>API_VOTES: POST { item_ids }
API_VOTES->>DB: query counts and user votes
API_VOTES-->>Browser: { votes map } (may set cp_pa_vid)
Note over PublicPage: voting interaction
User->>PublicPage: click vote
PublicPage->>PublicPage: optimistic UI update
PublicPage->>API_VOTE: POST { item_id }
API_VOTE->>DB: toggle vote for visitor_id, return vote_count
API_VOTE-->>PublicPage: { vote_count } (may set cp_pa_vid)
PublicPage->>PublicPage: reconcile optimistic state
sequenceDiagram
autonumber
participant Next as Next.js SSR
participant fetchRenderData as fetchRenderData(site)
participant DB as Supabase
participant Header as PageHeader
participant Browser
Next->>fetchRenderData: fetchRenderData(site)
par Concurrent
fetchRenderData->>DB: get page, settings
and
fetchRenderData->>DB: get public roadmap boards
end
fetchRenderData-->>Next: { page, settings, roadmaps }
Next-->>Header: render header with roadmaps prop
Header->>Browser: render Changelog / Roadmaps nav (link or dropdown)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✨ Finishing Touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/components/layout/page.component.tsx (1)
155-161: Select defaultValue mismatch (uses tab.name but option values are href).This prevents correct initial selection on small screens. Use the href for defaultValue.
- defaultValue={tabs.find((tab) => tab.current)?.name} + defaultValue={tabs.find((tab) => tab.current)?.href}
🧹 Nitpick comments (41)
apps/web/tailwind.config.js (1)
9-24: Replace long safelist with patterns to reduce maintenance and dupesManually listing classes is brittle and already has duplicates (e.g., dark:bg-indigo-900). Use Tailwind’s safelist patterns with variants.
- safelist: [ - // Roadmap category colors - 'bg-blue-100', 'text-blue-800', 'border-blue-200', 'dark:bg-blue-900', 'dark:text-blue-200', 'dark:border-blue-800', - 'bg-indigo-100', 'text-indigo-800', 'border-indigo-200', 'dark:bg-indigo-900', 'dark:text-indigo-200', 'dark:border-indigo-800', - 'bg-purple-100', 'text-purple-800', 'border-purple-200', 'dark:bg-purple-900', 'dark:text-purple-200', 'dark:border-purple-800', - 'bg-pink-100', 'text-pink-800', 'border-pink-200', 'dark:bg-pink-900', 'dark:text-pink-200', 'dark:border-pink-800', - 'bg-red-100', 'text-red-800', 'border-red-200', 'dark:bg-red-900', 'dark:text-red-200', 'dark:border-red-800', - 'bg-orange-100', 'text-orange-800', 'border-orange-200', 'dark:bg-orange-900', 'dark:text-orange-200', 'dark:border-orange-800', - 'bg-yellow-100', 'text-yellow-800', 'border-yellow-200', 'dark:bg-yellow-900', 'dark:text-yellow-200', 'dark:border-yellow-800', - 'bg-green-100', 'text-green-800', 'border-green-200', 'dark:bg-green-900', 'dark:text-green-200', 'dark:border-green-800', - 'bg-emerald-100', 'text-emerald-800', 'border-emerald-200', 'dark:bg-emerald-900', 'dark:text-emerald-200', 'dark:border-emerald-800', - 'bg-cyan-100', 'text-cyan-800', 'border-cyan-200', 'dark:bg-cyan-900', 'dark:text-cyan-200', 'dark:border-cyan-800', - 'bg-blue-500', 'bg-indigo-500', 'bg-purple-500', 'bg-pink-500', 'bg-red-500', 'bg-orange-500', 'bg-yellow-500', 'bg-green-500', 'bg-emerald-500', 'bg-cyan-500', - // Drag and drop indigo colors - 'bg-indigo-50', 'bg-indigo-400', 'bg-indigo-600', 'text-indigo-600', 'text-indigo-400', 'dark:bg-indigo-900', 'dark:bg-indigo-600', 'dark:text-indigo-400', - ], + safelist: [ + { + pattern: /(bg|text|border)-(blue|indigo|purple|pink|red|orange|yellow|green|emerald|cyan)-(100|200|800|900|500)/, + variants: ['dark'], + }, + // DnD-specific indigo shades + { pattern: /(bg|text)-(indigo)-(50|400|600)/, variants: ['dark'] }, + ],apps/page/pages/api/roadmap/votes.ts (2)
12-18: Set Secure on the cookie when over HTTPSAvoid sending the visitor cookie over HTTP in prod.
- if (!visitor_id) { + if (!visitor_id) { visitor_id = v4(); - res.setHeader( - "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; HttpOnly; SameSite=Lax; Max-Age=31536000` - ); + const isSecure = + (req.headers["x-forwarded-proto"] || "").toString().includes("https") || + process.env.NODE_ENV === "production"; + const secureAttr = isSecure ? "; Secure" : ""; + res.setHeader( + "Set-Cookie", + `cp_pa_vid=${visitor_id}; Path=/; HttpOnly; SameSite=Lax; Max-Age=31536000${secureAttr}` + ); }
48-51: Fix catch type per Biome and improve loggingUse unknown for catch variable.
- } catch (e: Error | any) { - console.log("getRoadmapItemVotes [Error]", e); + } catch (e: unknown) { + console.error("getRoadmapItemVotes [Error]", e); res.status(200).json({ ok: true, vote_count: 0, user_voted: false }); }apps/page/pages/api/roadmap/votes/bulk.ts (3)
41-47: Add Secure to cookie when HTTPSMirror the single-item endpoint hardening.
- if (!visitor_id) { + if (!visitor_id) { visitor_id = v4(); - res.setHeader( - "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; HttpOnly; SameSite=Lax; Max-Age=31536000` - ); + const isSecure = + (req.headers["x-forwarded-proto"] || "").toString().includes("https") || + process.env.NODE_ENV === "production"; + const secureAttr = isSecure ? "; Secure" : ""; + res.setHeader( + "Set-Cookie", + `cp_pa_vid=${visitor_id}; Path=/; HttpOnly; SameSite=Lax; Max-Age=31536000${secureAttr}` + ); }
100-103: Biome: prefer unknown in catch and use error loggingMinor cleanup.
- } catch (e: Error | any) { - console.log("getBulkRoadmapItemVotes [Error]", e); + } catch (e: unknown) { + console.error("getBulkRoadmapItemVotes [Error]", e); res.status(500).json({ ok: false, votes: {} }); }
18-21: Consider aligning error semantics with votes.tsSingle-item returns 200 with ok:true even on internal errors; bulk returns 500. Pick one policy for API consumers.
apps/page/components/subscribe-prompt.tsx (2)
187-189: Tailwind class 'font-base' doesn’t existUse font-medium (or font-normal) instead.
- "flex w-full items-center justify-center rounded-r-full border border-transparent bg-slate-600 py-1.5 px-5 text-base font-base text-white hover:bg-slate-700 focus:outline-none focus:ring-2 focus:ring-slate-500 focus:ring-offset-2", + "flex w-full items-center justify-center rounded-r-full border border-transparent bg-slate-600 py-1.5 px-5 text-base font-medium text-white hover:bg-slate-700 focus:outline-none focus:ring-2 focus:ring-slate-500 focus:ring-offset-2",
98-128: DRY: extract RSS/Atom link row into a tiny componentSame markup appears twice; consolidating avoids divergence.
Also applies to: 66-91
packages/utils/roadmap/index.ts (2)
66-70: Harden color lookup against typos and nulls.Do a runtime guard instead of relying on a cast + falsy check.
Apply:
-export function getCategoryColorClasses(color: string = 'blue') { - const colorKey = color as RoadmapColor; - const colorConfig = ROADMAP_COLORS[colorKey] || ROADMAP_COLORS.blue; - return `${colorConfig.light} ${colorConfig.dark}`; -} +export function getCategoryColorClasses(color?: string | null): string { + const key = (color && Object.prototype.hasOwnProperty.call(ROADMAP_COLORS, color) + ? (color as RoadmapColor) + : 'blue') as RoadmapColor; + const { light, dark } = ROADMAP_COLORS[key]; + return `${light} ${dark}`; +}
72-78: Strengthen typing of options’ value.Expose the union instead of plain string.
Apply:
-export function getCategoryColorOptions() { - return Object.entries(ROADMAP_COLORS).map(([key, value]) => ({ - value: key, +export function getCategoryColorOptions(): Array<{ value: RoadmapColor; label: string; preview: string }> { + return Object.entries(ROADMAP_COLORS).map(([key, value]) => ({ + value: key as RoadmapColor, label: value.name, preview: value.preview, })); }apps/page/components/page-header.tsx (2)
10-15: Avoid duplicating the RoadmapBoard type; derive from Supabase types.Prevents drift with DB schema and handles nullability correctly.
Apply:
-import OptionalLink from "./optional-link"; - -type RoadmapBoard = { - id: string; - title: string; - slug: string; - description?: string; -}; +import OptionalLink from "./optional-link"; +import type { Tables } from "@changes-page/supabase/types"; +type RoadmapBoard = Pick<Tables<"roadmap_boards">, "id" | "title" | "slug" | "description">;
17-25: “Changelog” link hardcodes “/”; may break on tenant routes.Make the base URL configurable to work for “/_sites/[site]” and custom domains.
Apply:
-export default function PageHeader({ - page, - settings, - roadmaps = [], -}: { +export default function PageHeader({ + page, + settings, + roadmaps = [], + homeHref = "/", +}: { page: IPage; settings: IPageSettings; - roadmaps?: RoadmapBoard[]; + roadmaps?: RoadmapBoard[]; + homeHref?: string; }) { - const router = useRouter(); + const router = useRouter(); const currentPath = router.asPath; const isRoadmapPage = currentPath.includes("/roadmap/");- <Link - href="/" + <Link + href={homeHref} className={`py-2 px-1 border-b-2 font-medium text-sm ${Verification: ensure callers pass the proper homeHref (e.g.,
/_sites/${site}) on multi-tenant pages.Also applies to: 26-29, 92-101
apps/page/tailwind.config.js (1)
26-35: Duplicate “theme” keys; second one overrides the first.Merge into a single theme block.
Apply:
- theme: { - extend: {}, - }, - theme: { - extend: { - colors: { - gray: neutral, - }, - }, - }, + theme: { + extend: { + colors: { + gray: neutral, + }, + }, + },packages/supabase/types/index.ts (2)
503-560: Add ordering/filter indexes for items.Common queries will sort/filter by column/board and position.
Migration:
CREATE INDEX IF NOT EXISTS ix_roadmap_items_column_position ON public.roadmap_items (column_id, position); CREATE INDEX IF NOT EXISTS ix_roadmap_items_board ON public.roadmap_items (board_id);
437-470: Constrain category color to known tokens.Either a Postgres ENUM or a CHECK against allowed values to match ROADMAP_COLORS.
Example:
-- or a separate enum type ALTER TABLE public.roadmap_categories ADD CONSTRAINT chk_category_color CHECK (color IS NULL OR color IN ('blue','indigo','purple','pink','red','orange','yellow','green','emerald','cyan'));packages/supabase/migrations/18_roadmap.sql (2)
37-39: Input/data quality checks and indexes (optional).
- Add CHECKs: positive positions and non-empty titles.
- Add indexes for common lookups.
-- Positions must be >= 1 alter table roadmap_columns add constraint position_positive check (position >= 1); alter table roadmap_items add constraint position_positive check (position >= 1); -- Slug format (lowercase kebab) alter table roadmap_boards add constraint slug_format check (slug ~ '^[a-z0-9]+(?:-[a-z0-9]+)*$'); -- Helpful indexes create index if not exists idx_roadmap_items_board on roadmap_items(board_id); create index if not exists idx_roadmap_items_category on roadmap_items(category_id); create index if not exists idx_roadmap_votes_item on roadmap_votes(item_id);Also applies to: 88-89, 60-62, 13-15, 83-85
143-154: Idempotent initialization to avoid unique constraint errors.If init functions run twice, they fail on unique constraints. Use ON CONFLICT DO NOTHING.
- INSERT INTO roadmap_columns (board_id, name, position) VALUES + INSERT INTO roadmap_columns (board_id, name, position) VALUES (board_id, 'Backlog', 1), (board_id, 'Planned', 2), (board_id, 'In Progress', 3), - (board_id, 'Done', 4); + (board_id, 'Done', 4) + ON CONFLICT (board_id, name) DO NOTHING; - INSERT INTO roadmap_categories (board_id, name) VALUES + INSERT INTO roadmap_categories (board_id, name) VALUES (board_id, 'Feature'), (board_id, 'Bug Fix'), - (board_id, 'Improvement'); + (board_id, 'Improvement') + ON CONFLICT (board_id, name) DO NOTHING;Also applies to: 156-166
apps/web/pages/pages/[page_id]/index.tsx (1)
119-133: Make tab selection dynamic instead of hardcoding current flags.Compute current based on the route to avoid divergence across pages.
- const viewTabs = useMemo( - () => [ - { - name: "Changelog", - current: true, - href: `/pages/${page_id}`, - }, - { - name: "Roadmap", - current: false, - href: `/pages/${page_id}/roadmap`, - }, - ], - [page_id] - ); + const viewTabs = useMemo(() => { + const changelogHref = `/pages/${page_id}`; + const roadmapHref = `/pages/${page_id}/roadmap`; + const path = router.asPath; + return [ + { name: "Changelog", current: path === changelogHref, href: changelogHref }, + { name: "Roadmap", current: path.startsWith(roadmapHref), href: roadmapHref }, + ]; + }, [page_id, router.asPath]);apps/web/components/layout/page.component.tsx (1)
149-190: Minor: simplify conditional rendering, keep checks consistent.You use
tabs?.lengthhere buttabs.lengthabove; since default is[], optional chaining isn’t needed.- {!!title && tabs?.length > 0 ? ( + {!!title && tabs.length > 0 ? (apps/page/pages/api/roadmap/vote.ts (3)
31-37: Select only needed columns when checking existing vote.Reduce payload and cost.
- const { data: existingVote } = await supabaseAdmin - .from("roadmap_votes") - .select("*") + const { data: existingVote } = await supabaseAdmin + .from("roadmap_votes") + .select("id") .eq("item_id", item_id) .eq("visitor_id", visitor_id) .maybeSingle();
79-83: Fix catch clause typing to satisfy Biome/TS rules.Remove the type or use unknown.
- } catch (e: Error | any) { + } catch (e: unknown) { console.log("voteOnRoadmapItem [Error]", e); res.status(500).json({ ok: false, error: 'Internal server error' }); }
1-83: Abuse prevention: add basic rate limiting and bot checks.Public, cookie-based voting is trivial to game by rotating visitor IDs. Add IP-based rate limiting (e.g., Upstash Redis) and/or CAPTCHA before calling this endpoint.
apps/page/pages/_sites/[site]/index.tsx (2)
17-24: Avoid duplicating the RoadmapBoard type; derive from DB types or centralize.Prevents drift with schema changes.
- type RoadmapBoard = { - id: string; - title: string; - slug: string; - description?: string; - }; + import type { Database } from "@changes-page/supabase/types"; + type RoadmapBoard = Pick< + Database["public"]["Tables"]["roadmap_boards"]["Row"], + "id" | "title" | "slug" | "description" + >;
167-167: Remove console log in SSR.Leaking internals into logs and noisy in prod.
- console.log("handle site ->", site); + // console.debug("handle site ->", site);apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (7)
230-232: Avoid full page reload after saving board.Use router.replace to refresh data without nuking app state.
- // Refresh the page to show updated settings - window.location.reload(); + // Refresh the data without a full reload + router.replace(router.asPath);
175-177: Clarify validation message.Message says “less than 50” but code allows 50. Say “at most 50.”
- return "Slug must be less than 50 characters"; + return "Slug must be at most 50 characters";
198-201: Type event params to avoid implicit any and DOM type issues.Tighten types on handlers; helps with e.relatedTarget, etc.
- const handleUpdateBoard = async (e) => { + const handleUpdateBoard = async (e: React.FormEvent<HTMLFormElement>) => { - const handleAddCategory = async (e) => { + const handleAddCategory = async (e: React.FormEvent<HTMLFormElement>) => { - const handleAddColumn = async (e) => { + const handleAddColumn = async (e: React.FormEvent<HTMLFormElement>) => { - const handleColumnDragStart = (e, column, index) => { + const handleColumnDragStart = ( + e: React.DragEvent<HTMLDivElement>, + column, + index: number + ) => { - const handleColumnDragOver = (e, index) => { + const handleColumnDragOver = (e: React.DragEvent<HTMLDivElement>, index: number) => { - const handleColumnDragLeave = (e) => { + const handleColumnDragLeave = (e: React.DragEvent<HTMLDivElement>) => { - const handleColumnDrop = async (e, dropIndex) => { + const handleColumnDrop = async (e: React.DragEvent<HTMLDivElement>, dropIndex: number) => {Also applies to: 241-246, 335-337, 423-427, 428-433, 434-439, 441-444
258-261: Use functional state updates to avoid stale closures.Safer when multiple rapid updates occur.
- setBoardCategories([...boardCategories, data]); + setBoardCategories((prev) => [...prev, data]); ... - setBoardColumns([...boardColumns, data]); + setBoardColumns((prev) => [...prev, data]);Also applies to: 355-357
39-45: Select only needed columns server-side to reduce payload.Limit selects to used fields.
- .select("*") + .select("id,page_id,title,description,slug,is_public,created_at") ... - .select("*") + .select("id,board_id,name,position,created_at") ... - .select("*") + .select("id,board_id,name,color,created_at")Also applies to: 55-58, 65-69
456-465: Make reordering atomic.Multiple updates without a transaction can leave inconsistent positions on partial failure. Prefer a single RPC that updates all positions in one transaction.
486-489: Avoid false “Access denied” flash while user loads.If user is initially undefined, render a loader until user is resolved.
apps/web/pages/pages/[page_id]/roadmap/index.tsx (1)
27-32: Reduce payload from Supabase.Select only fields used by the UI.
- const { data: boards, error: boardsError } = await supabase - .from("roadmap_boards") - .select("*") + const { data: boards, error: boardsError } = await supabase + .from("roadmap_boards") + .select("id,title,description,is_public,created_at")apps/web/pages/pages/[page_id]/roadmap/new.tsx (1)
142-149: Handle RPC errors during post-create initialization.If either RPC fails, you’ll silently proceed. Capture and react to errors or perform initialization in a single transactional RPC.
- await supabase.rpc("initialize_roadmap_columns", { board_id: board.id }); + const { error: initColsError } = await supabase.rpc("initialize_roadmap_columns", { board_id: board.id }); + if (initColsError) throw initColsError; - await supabase.rpc("initialize_roadmap_categories", { - board_id: board.id, - }); + const { error: initCatsError } = await supabase.rpc("initialize_roadmap_categories", { board_id: board.id }); + if (initCatsError) throw initCatsError;apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (4)
415-431: Batch updates should be atomic; consider a DB-side reorder RPC.Multiple updates without a transaction risk inconsistent state on failure/races. Wrap reorders in a single SQL function (running in a transaction) and call via supabase.rpc for both same-column and cross-column moves.
I can sketch a Postgres function (reorder within a column and move across columns) if you want.
Also applies to: 453-479
64-85: Avoid fetching all vote rows; fetch counts instead.Selecting roadmap_votes(id) for every item inflates payload and may hit RLS. Prefer an aggregate count (view/RPC) and return vote_count only.
133-147: Tighten types for local state.Use explicit types for item, category, form, and errors to avoid any/undefined pitfalls.
-import { Fragment, useMemo, useState, type JSX } from "react"; +import { Fragment, useMemo, useState, type JSX } from "react"; +import type { Database } from "@changes-page/supabase/types"; +type DbItem = Database["public"]["Tables"]["roadmap_items"]["Row"] & { + roadmap_categories?: { id: string; name: string; color: string } | null; + roadmap_votes?: Array<{ id: string }> | null; +}; +type ItemForm = { title: string; description: string; category_id: string }; +type FormErrors = { title?: string; general?: string }; @@ -const [selectedItem, setSelectedItem] = useState(null); +const [selectedItem, setSelectedItem] = useState<DbItem | null>(null); @@ -const [editingItem, setEditingItem] = useState(null); +const [editingItem, setEditingItem] = useState<DbItem | null>(null); @@ -const [itemForm, setItemForm] = useState({ +const [itemForm, setItemForm] = useState<ItemForm>({ title: "", description: "", category_id: "", }); @@ -const [formErrors, setFormErrors] = useState({}); +const [formErrors, setFormErrors] = useState<FormErrors>({}); @@ -const [boardItems, setBoardItems] = useState(items); -const [boardCategories] = useState(categories); +const [boardItems, setBoardItems] = useState<DbItem[]>(items); +const [boardCategories] = useState(categories);Also applies to: 149-156
640-644: Click handler sets selectedItem but nothing uses it.Either open a details view or remove the unused state to avoid dead code.
Also applies to: 134-136
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (4)
86-110: Optimistic vote count ignores initial SSR/bulk counts; causes flicker.Use a baseCount that falls back to existing count when local state is empty.
- const currentVoteState = votes[itemId]; - const newVotedState = !currentVoteState?.voted; - const newCount = currentVoteState?.count - ? newVotedState - ? currentVoteState.count + 1 - : currentVoteState.count - 1 - : newVotedState - ? 1 - : 0; + const currentVoteState = votes[itemId]; + const newVotedState = !currentVoteState?.voted; + const ssrCount = items.find((i) => i.id === itemId)?.vote_count ?? 0; + const baseCount = currentVoteState?.count ?? ssrCount; + const newCount = Math.max(0, baseCount + (newVotedState ? 1 : -1));
111-121: Harden fetch handling: check response.ok before parsing JSON.Prevents crashes on non-JSON responses and surfaces HTTP errors.
- const response = await fetch("/api/roadmap/vote", { + const response = await fetch("/api/roadmap/vote", { method: "POST", headers: { "Content-Type": "application/json", }, body: JSON.stringify({ item_id: itemId }), }); - - const data = await response.json(); + if (!response.ok) throw new Error(`HTTP ${response.status}`); + const data = await response.json(); @@ - const response = await fetch("/api/roadmap/votes/bulk", { + const response = await fetch("/api/roadmap/votes/bulk", { method: "POST", headers: { "Content-Type": "application/json", }, body: JSON.stringify({ item_ids: items.map((item) => item.id), }), }); - - const data = await response.json(); + if (!response.ok) throw new Error(`HTTP ${response.status}`); + const data = await response.json();Also applies to: 166-178
452-453: Remove dev console.log or guard it.Avoid leaking internals in server logs; gate behind NODE_ENV.
-console.log("handle roadmap ->", site, roadmap_slug); +if (process.env.NODE_ENV !== "production") { + console.log("handle roadmap ->", site, roadmap_slug); +}
521-528: Optional: include vote counts in SSR to avoid extra roundtrip.You can return vote_count per item from SSR (via a view/RPC) and skip the initial /votes/bulk fetch for better TTFB and fewer flashes.
Happy to propose a Postgres view or function that returns items with vote_count aggregated.
Also applies to: 233-251
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
apps/page/components/page-header.tsx(2 hunks)apps/page/components/subscribe-prompt.tsx(4 hunks)apps/page/pages/_sites/[site]/index.tsx(3 hunks)apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx(1 hunks)apps/page/pages/api/roadmap/vote.ts(1 hunks)apps/page/pages/api/roadmap/votes.ts(1 hunks)apps/page/pages/api/roadmap/votes/bulk.ts(1 hunks)apps/page/tailwind.config.js(1 hunks)apps/web/components/layout/page.component.tsx(3 hunks)apps/web/pages/pages/[page_id]/index.tsx(2 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx(1 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx(1 hunks)apps/web/pages/pages/[page_id]/roadmap/index.tsx(1 hunks)apps/web/pages/pages/[page_id]/roadmap/new.tsx(1 hunks)apps/web/styles/global.css(1 hunks)apps/web/tailwind.config.js(1 hunks)packages/supabase/migrations/18_roadmap.sql(1 hunks)packages/supabase/types/index.ts(4 hunks)packages/utils/index.ts(1 hunks)packages/utils/roadmap/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
apps/web/pages/pages/[page_id]/roadmap/new.tsx (7)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClient(14-37)apps/web/utils/useSSR.ts (1)
getPage(3-12)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-229)apps/web/utils/useUser.tsx (1)
useUserData(101-107)apps/web/utils/hooks/usePageSettings.ts (1)
usePageSettings(10-62)apps/web/components/layout/page.component.tsx (1)
Page(11-209)apps/web/components/layout/auth-layout.component.tsx (1)
AuthLayout(3-16)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (9)
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (1)
getServerSideProps(20-86)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClient(14-37)apps/web/utils/useSSR.ts (1)
getPage(3-12)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-229)apps/web/utils/useUser.tsx (1)
useUserData(101-107)apps/web/utils/hooks/usePageSettings.ts (1)
usePageSettings(10-62)apps/web/components/layout/page.component.tsx (1)
Page(11-209)apps/web/components/core/buttons.component.tsx (1)
SecondaryButton(148-174)packages/utils/roadmap/index.ts (1)
getCategoryColorClasses(66-70)
apps/web/pages/pages/[page_id]/roadmap/index.tsx (7)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (1)
getServerSideProps(21-109)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClient(14-37)apps/web/utils/useSSR.ts (1)
getPage(3-12)apps/web/components/layout/page.component.tsx (1)
Page(11-209)apps/web/data/routes.data.ts (1)
ROUTES(1-26)apps/web/components/core/buttons.component.tsx (1)
PrimaryRouterButton(7-50)apps/web/components/layout/auth-layout.component.tsx (1)
AuthLayout(3-16)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (6)
packages/supabase/types/page.ts (2)
IPage(3-3)IPageSettings(4-5)apps/page/components/page-header.tsx (1)
PageHeader(17-170)packages/utils/roadmap/index.ts (1)
getCategoryColorClasses(66-70)apps/page/components/footer.tsx (1)
Footer(18-186)apps/page/lib/data.ts (1)
BLACKLISTED_SLUGS(8-71)packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (7)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClient(14-37)apps/web/utils/useSSR.ts (1)
getPage(3-12)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-229)apps/web/utils/useUser.tsx (1)
useUserData(101-107)apps/web/utils/hooks/usePageSettings.ts (1)
usePageSettings(10-62)apps/web/components/layout/page.component.tsx (1)
Page(11-209)packages/utils/roadmap/index.ts (3)
getCategoryColorOptions(72-78)ROADMAP_COLORS(1-62)getCategoryColorClasses(66-70)
apps/page/pages/api/roadmap/votes.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/page/pages/_sites/[site]/index.tsx (3)
packages/supabase/types/page.ts (3)
IPage(3-3)IPageSettings(4-5)IPost(6-6)apps/page/components/page-header.tsx (1)
PageHeader(17-170)packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/page/pages/api/roadmap/votes/bulk.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/page/components/page-header.tsx (1)
packages/supabase/types/page.ts (2)
IPage(3-3)IPageSettings(4-5)
apps/page/pages/api/roadmap/vote.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/page/components/subscribe-prompt.tsx (1)
packages/ui/components/Spinner.tsx (1)
Spinner(3-32)
🪛 Biome (2.1.2)
apps/page/pages/api/roadmap/votes.ts
[error] 48-48: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
apps/page/pages/api/roadmap/votes/bulk.ts
[error] 100-100: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
apps/page/pages/api/roadmap/vote.ts
[error] 79-79: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
🔇 Additional comments (9)
apps/web/styles/global.css (1)
91-94: NProgress bar height change looks fine4px improves visibility and shouldn’t affect layout. Keep an eye on overlap with any fixed headers; adjust z-index if you notice clipping.
packages/utils/index.ts (1)
3-3: Re-export looks good.Keeps roadmap utils available at the package root.
apps/page/components/page-header.tsx (1)
133-158: Dropdown accessibility/state is fine.Headless UI usage and active styles look correct.
packages/supabase/types/index.ts (1)
783-806: No call sites for page_view_ or post_reactions_aggregate—no action needed.* Verified that the repo contains no usages of these RPCs or their result fields outside the type definitions; no call-site updates are required.packages/supabase/migrations/18_roadmap.sql (1)
23-27: Verified: thetrigger_set_timestamp()function is defined inpackages/supabase/migrations/0_schema.sql:36; no further action needed.apps/web/pages/pages/[page_id]/index.tsx (1)
306-306: LGTM: tabs prop wired into Page layout.Tabs integration matches the updated Page component API.
apps/page/pages/_sites/[site]/index.tsx (3)
84-85: LGTM: passing roadmaps to PageHeader.Matches the new header API.
192-199: Query looks good; consider stable ordering for UX.If you later expose manual ordering, switch to that column. For now, created_at ASC is fine.
200-207: Verify null-safety for roadmaps.Defensive default OK. Ensure PageHeader props mark roadmaps optional with a default.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/page/pages/api/post/react.ts (1)
28-36: Whitelist reaction keys and drop the ts-ignoreValidate reaction against an allowlist and type the dynamic key to avoid invalid column writes.
- const { error } = await supabaseAdmin.from("post_reactions").upsert([ - { - id: data?.id ?? v4(), - post_id: String(post_id), - visitor_id: visitor_id ?? v4(), - // @ts-ignore - [reaction]: data ? !data[reaction] : true, - }, - ]); + const allowed = new Set(["thumbs_up", "thumbs_down", "heart", "sad", "rocket"]); + if (typeof reaction !== "string" || !allowed.has(reaction)) { + return res.status(400).json({ ok: false }); + } + type ReactionKey = "thumbs_up" | "thumbs_down" | "heart" | "sad" | "rocket"; + const key = reaction as ReactionKey; + const { error } = await supabaseAdmin.from("post_reactions").upsert([ + { + id: data?.id ?? v4(), + post_id: String(post_id), + visitor_id: visitor_id ?? v4(), + [key]: data ? !Boolean((data as Record<ReactionKey, boolean>)[key]) : true, + }, + ]);apps/page/pages/api/post/reactions.ts (1)
9-11: Validate post_id (reject arrays/missing) before queryingGuard against array params and empty values to avoid odd String(array) coercions hitting the DB.
- let { post_id } = req.query; + let { post_id } = req.query; + if (Array.isArray(post_id) || !post_id) { + return res.status(400).json({ ok: false, aggregate: null, user: null }); + }
♻️ Duplicate comments (13)
apps/page/pages/api/roadmap/votes.ts (1)
45-49: Fix undercount and bandwidth by using head: true and response.countCurrent logic uses data.length with count: 'exact' which still fetches rows (limited) and can undercount. Use PostgREST head counting and read count. (Previously flagged.)
- const { data: voteCount, error: countError } = await supabaseAdmin + const { count, error: countError } = await supabaseAdmin .from("roadmap_votes") - .select("id", { count: 'exact' }) + .select("id", { count: "exact", head: true }) .eq("item_id", item_id); @@ - vote_count: voteCount?.length || 0, + vote_count: count ?? 0,Also applies to: 66-70
apps/page/components/subscribe-prompt.tsx (1)
160-197: Label/id association fixed—thanks for addressing.htmlFor and id now match (email).
packages/supabase/migrations/18_roadmap.sql (4)
92-95: Same RLS ownership bug on items.Restrict access to the page owner.
-create policy "Can view own roadmap items." on roadmap_items for select using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can insert own roadmap items." on roadmap_items for insert with check (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can update own roadmap items." on roadmap_items for update using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can delete own roadmap items." on roadmap_items for delete using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); +create policy "Can view own roadmap items." on roadmap_items for select using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_items.board_id and p.user_id = auth.uid() + ) +); +create policy "Can insert own roadmap items." on roadmap_items for insert with check ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_items.board_id and p.user_id = auth.uid() + ) +); +create policy "Can update own roadmap items." on roadmap_items for update using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_items.board_id and p.user_id = auth.uid() + ) +); +create policy "Can delete own roadmap items." on roadmap_items for delete using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_items.board_id and p.user_id = auth.uid() + ) +);
18-21: RLS allows anyone with a valid page_id to CRUD boards. Scope to owner via auth.uid().Replace predicates with EXISTS join to pages.user_id.
-create policy "Can view own page roadmap boards." on roadmap_boards for select using (page_id in (select id from pages)); -create policy "Can insert own page roadmap boards." on roadmap_boards for insert with check (page_id in (select id from pages)); -create policy "Can update own page roadmap boards." on roadmap_boards for update using (page_id in (select id from pages)); -create policy "Can delete own page roadmap boards." on roadmap_boards for delete using (page_id in (select id from pages)); +create policy "Can view own page roadmap boards." on roadmap_boards + for select using ( + exists (select 1 from pages p where p.id = roadmap_boards.page_id and p.user_id = auth.uid()) + ); +create policy "Can insert own page roadmap boards." on roadmap_boards + for insert with check ( + exists (select 1 from pages p where p.id = roadmap_boards.page_id and p.user_id = auth.uid()) + ); +create policy "Can update own page roadmap boards." on roadmap_boards + for update using ( + exists (select 1 from pages p where p.id = roadmap_boards.page_id and p.user_id = auth.uid()) + ); +create policy "Can delete own page roadmap boards." on roadmap_boards + for delete using ( + exists (select 1 from pages p where p.id = roadmap_boards.page_id and p.user_id = auth.uid()) + );
41-45: Same RLS ownership bug on columns.Tighten policies to the board’s page owner.
-create policy "Can view own roadmap columns." on roadmap_columns for select using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can insert own roadmap columns." on roadmap_columns for insert with check (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can update own roadmap columns." on roadmap_columns for update using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can delete own roadmap columns." on roadmap_columns for delete using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); +create policy "Can view own roadmap columns." on roadmap_columns for select using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_columns.board_id and p.user_id = auth.uid() + ) +); +create policy "Can insert own roadmap columns." on roadmap_columns for insert with check ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_columns.board_id and p.user_id = auth.uid() + ) +); +create policy "Can update own roadmap columns." on roadmap_columns for update using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_columns.board_id and p.user_id = auth.uid() + ) +); +create policy "Can delete own roadmap columns." on roadmap_columns for delete using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_columns.board_id and p.user_id = auth.uid() + ) +);
65-68: Same RLS ownership bug on categories.Align with columns fix.
-create policy "Can view own roadmap categories." on roadmap_categories for select using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can insert own roadmap categories." on roadmap_categories for insert with check (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can update own roadmap categories." on roadmap_categories for update using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); -create policy "Can delete own roadmap categories." on roadmap_categories for delete using (board_id in (select id from roadmap_boards where page_id in (select id from pages))); +create policy "Can view own roadmap categories." on roadmap_categories for select using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_categories.board_id and p.user_id = auth.uid() + ) +); +create policy "Can insert own roadmap categories." on roadmap_categories for insert with check ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_categories.board_id and p.user_id = auth.uid() + ) +); +create policy "Can update own roadmap categories." on roadmap_categories for update using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_categories.board_id and p.user_id = auth.uid() + ) +); +create policy "Can delete own roadmap categories." on roadmap_categories for delete using ( + exists ( + select 1 from roadmap_boards b join pages p on p.id = b.page_id + where b.id = roadmap_categories.board_id and p.user_id = auth.uid() + ) +);apps/page/pages/api/roadmap/vote.ts (3)
21-27: Secure cookie added—nice.Secure + HttpOnly + SameSite=Lax is correct.
29-42: Public-board validation before toggle—good guard.Prevents toggling votes on private items when using service role.
81-86: Switched to head+count—efficient.Avoids loading rows to count.
apps/page/pages/api/roadmap/votes/bulk.ts (1)
49-61: Aggregate counts in DB; current approach fetches all rows and can break at >1k rowsReplace the full-row select + client-side reduce with a grouped aggregate so the DB returns one row per item_id. Also rename variables accordingly.
- const [voteCountResult, userVoteResult] = await Promise.all([ - supabaseAdmin - .from("roadmap_votes") - .select("item_id") - .in("item_id", item_ids), + const [voteAggResult, userVoteResult] = await Promise.all([ + supabaseAdmin + .from("roadmap_votes") + .select("item_id, count:id", { head: false }) + .in("item_id", distinctItemIds) + .group("item_id"), supabaseAdmin .from("roadmap_votes") - .select("item_id") - .in("item_id", item_ids) + .select("item_id", { head: false }) + .in("item_id", distinctItemIds) .eq("visitor_id", visitor_id), ]); @@ - if (voteCountResult.error) { + if (voteAggResult.error) { console.error( "getBulkRoadmapItemVotes [Count Error]", - voteCountResult.error + voteAggResult.error ); return res.status(500).json({ ok: false, votes: {} }); } @@ - const voteCountsMap = (voteCountResult.data || []).reduce((acc, vote) => { - acc[vote.item_id] = (acc[vote.item_id] || 0) + 1; - return acc; - }, {} as Record<string, number>); + const voteCountsMap = Object.fromEntries( + (voteAggResult.data || []).map( + (row: { item_id: string; count: number }) => [row.item_id, Number(row.count)] + ) + ) as Record<string, number>;Also applies to: 62-69, 78-85
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (3)
21-35: Block unauthorized access in SSR (owner check missing).Anyone with a URL can fetch this internal board. Enforce owner check.
-export async function getServerSideProps({ req, res, params }) { +export async function getServerSideProps({ req, res, params }) { const { page_id, board_id } = params; - const { supabase } = await getSupabaseServerClient({ req, res }); + const { supabase, user } = await getSupabaseServerClient({ req, res }); const page = await getPage(supabase, page_id).catch((e) => { console.error("Failed to get page", e); return null; }); if (!page) { return { notFound: true, }; } + + // Only owners should access the internal board view + if (!user || page.user_id !== user.id) { + return { notFound: true }; + }
433-531: Cross-column move: resequence source column to avoid gaps/driftAfter moving to a new column you update target positions but leave holes in the source positions.
} else { // Moving between different columns let newPosition = 1; @@ const { error } = await supabase .from("roadmap_items") .update({ column_id: targetColumnId, position: newPosition, }) .eq("id", draggedItem.id); if (error) throw error; - // Update local state + // Resequence source column positions after removal + const remainingSourceItems = sourceColumnItems + .filter((i) => i.id !== draggedItem.id) + .sort((a, b) => (a.position || 0) - (b.position || 0)); + const sourceFinalUpdates = remainingSourceItems.map((item, index) => + supabase + .from("roadmap_items") + .update({ position: index + 1 }) + .eq("id", item.id) + ); + await Promise.all(sourceFinalUpdates); + + // Update local state setBoardItems((prev) => prev.map((item) => { if (item.id === draggedItem.id) { return { ...item, column_id: targetColumnId, position: newPosition, }; } // Update positions of shifted items in target column if (item.column_id === targetColumnId && currentDragOverPosition) { @@ } + // Normalize positions of remaining source items + if (item.column_id === sourceColumnId) { + const idx = remainingSourceItems.findIndex((ri) => ri.id === item.id); + if (idx !== -1) return { ...item, position: idx + 1 }; + } return item; }) ); }
553-561: Fix SecondaryButton usage: no “icon” prop; use router.push not location.hrefThis currently breaks TS and causes a full reload.
- <SecondaryButton - label="Settings" - icon={<DotsVerticalIcon className="-ml-1 mr-2 h-5 w-5" />} - onClick={() => - (window.location.href = `/pages/${page_id}/roadmap/${board.id}/settings`) - } - /> + <SecondaryButton + label={ + <span className="inline-flex items-center"> + <DotsVerticalIcon className="-ml-1 mr-2 h-5 w-5" /> + Settings + </span> + } + onClick={() => router.push(`/pages/${page_id}/roadmap/${board.id}/settings`)} + />
🧹 Nitpick comments (16)
apps/page/pages/api/post/react.ts (1)
12-18: Add Secure conditionally to avoid breaking local dev cookie settingSecure prevents cookies over http://localhost; you’ll regen visitor_id on every request in dev. Gate it by NODE_ENV or centralize via a helper.
- res.setHeader( - "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age=31536000` - ); + const secureAttr = process.env.NODE_ENV === "production" ? " Secure;" : ""; + res.setHeader( + "Set-Cookie", + `cp_pa_vid=${visitor_id}; Path=/;${secureAttr} HttpOnly; SameSite=Lax; Max-Age=31536000` + );If you prefer, I can extract a shared setVisitorCookie(req, res, visitorId) utility used across all three endpoints.
apps/page/pages/api/post/reactions.ts (1)
12-18: Add Secure conditionally to avoid breaking local dev cookie settingSame concern as other endpoints: Secure blocks cookies on http://localhost.
- res.setHeader( - "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age=31536000` - ); + const secureAttr = process.env.NODE_ENV === "production" ? " Secure;" : ""; + res.setHeader( + "Set-Cookie", + `cp_pa_vid=${visitor_id}; Path=/;${secureAttr} HttpOnly; SameSite=Lax; Max-Age=31536000` + );apps/page/pages/api/pa/view.ts (1)
25-31: Add Secure conditionally to avoid breaking local dev cookie settingMirror the conditional used elsewhere to ensure the cookie persists during local testing.
- res.setHeader( - "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age=31536000` - ); + const secureAttr = process.env.NODE_ENV === "production" ? " Secure;" : ""; + res.setHeader( + "Set-Cookie", + `cp_pa_vid=${visitor_id}; Path=/;${secureAttr} HttpOnly; SameSite=Lax; Max-Age=31536000` + );apps/page/pages/api/roadmap/votes.ts (3)
35-41: Add Secure conditionally to avoid breaking local dev cookie settingSame cookie behavior as the other endpoints.
- res.setHeader( - "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age=31536000` - ); + const secureAttr = process.env.NODE_ENV === "production" ? " Secure;" : ""; + res.setHeader( + "Set-Cookie", + `cp_pa_vid=${visitor_id}; Path=/;${secureAttr} HttpOnly; SameSite=Lax; Max-Age=31536000` + );
71-73: Align catch typing with Biome ruleChange Error | any to unknown (or any) to satisfy the linter.
- } catch (e: Error | any) { + } catch (e: unknown) {
66-71: Optional: set no-store and vary by cookie to avoid stale counts per userCounts and user_voted depend on Cookie; hint caches accordingly.
- res.status(200).json({ + res.setHeader("Cache-Control", "private, no-store, max-age=0, must-revalidate"); + res.setHeader("Vary", "Cookie"); + res.status(200).json({apps/page/components/subscribe-prompt.tsx (4)
95-129: Add noopener for external links.Use rel="noopener noreferrer" for best practice; currently only "noreferrer".
Apply:
- rel="noreferrer" + rel="noopener noreferrer"
132-158: Add role="alert" to error panel.Improves screen-reader announcement for errors.
Apply:
- <div + <div + role="alert"
186-189: Tailwind class typo: replace font-base.font-base isn’t a Tailwind class; use font-medium (or desired weight).
- "flex w-full items-center justify-center rounded-r-full border border-transparent bg-slate-600 py-1.5 px-5 text-base font-base text-white hover:bg-slate-700 focus:outline-none focus:ring-2 focus:ring-slate-500 focus:ring-offset-2", + "flex w-full items-center justify-center rounded-r-full border border-transparent bg-slate-600 py-1.5 px-5 text-base font-medium text-white hover:bg-slate-700 focus:outline-none focus:ring-2 focus:ring-slate-500 focus:ring-offset-2",
40-58: Unify error logging and ensure loading reset via finally.Drop duplicate console.log, use finally to always clear loading.
onSubmit: async (values) => { - setLoading(true); - try { + setLoading(true); + try { await httpPost({ url: "/api/notifications/subscribe-email", data: values, }); setShowSuccess(true); - setLoading(false); } catch (e: any) { - console.log("Error", e); if (e?.message) { setErrorMessage(e?.message); } else { setErrorMessage("Something went wrong"); } - - setLoading(false); - console.error("/api/notification/email: error", e); + console.error("/api/notifications/subscribe-email error", e); + } finally { + setLoading(false); } },packages/supabase/migrations/18_roadmap.sql (1)
28-39: Optional: add updated_at on columns and useful indexes.Consider updated_at + set_timestamp on roadmap_columns; add indexes for common lookups.
-- indexes create index if not exists idx_roadmap_columns_board_id on roadmap_columns(board_id); create index if not exists idx_roadmap_categories_board_id on roadmap_categories(board_id); create index if not exists idx_roadmap_items_board_id on roadmap_items(board_id);Also applies to: 60-62, 88-89
apps/page/pages/api/roadmap/vote.ts (2)
13-17: Validate UUID shape for item_id.Reject obvious invalid IDs early.
- const { item_id } = req.body; + const { item_id } = req.body; + const isUuid = typeof item_id === "string" && /^[0-9a-fA-F-]{36}$/.test(item_id); - if (!item_id) { + if (!isUuid) { return res.status(400).json({ ok: false, error: "Missing item_id" }); }
95-98: Use unknown in catch per Biome rule.Simplify the type and keep logging.
-} catch (e: Error | any) { +} catch (e: unknown) { console.log("voteOnRoadmapItem [Error]", e); res.status(500).json({ ok: false, error: "Internal server error" }); }apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (1)
64-82: Avoid fetching all votes per item in SSR; use counts or defer to bulk APIThe nested roadmap_votes select returns every vote row and can balloon payloads. Prefer a count aggregate in the select or drop votes from SSR and rely on client-side bulk fetch.
- const { data: items, error: itemsError } = await supabase + const { data: items, error: itemsError } = await supabase .from("roadmap_items") - .select( - ` - *, - roadmap_categories ( - id, - name, - color - ), - roadmap_votes ( - id - ) - ` - ) + .select(` + *, + roadmap_categories ( id, name, color ) + `)Option A (DB aggregate): include vote counts via
vote_count:roadmap_votes(count)and adjust types. Option B: show 0 and let the client fetch counts in bulk (simpler).apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (2)
161-201: Stabilize bulk vote fetch: de-duplicate IDs and abort on unmountPrevents redundant work and avoids setState on unmounted component.
- useEffect(() => { - const fetchVotes = async () => { + useEffect(() => { + const controller = new AbortController(); + const fetchVotes = async () => { if (items.length === 0) return; try { const response = await fetch("/api/roadmap/votes/bulk", { method: "POST", headers: { "Content-Type": "application/json", }, body: JSON.stringify({ - item_ids: items.map((item) => item.id), + item_ids: Array.from(new Set(items.map((item) => item.id))), }), + signal: controller.signal, }); @@ - fetchVotes(); - }, [items]); + fetchVotes(); + return () => controller.abort(); + }, [items]);
509-524: Optional: seed SSR with vote counts to avoid initial “0” flashIf desired, fetch counts server-side using an aggregate instead of returning all vote rows.
- const { data: items, error: itemsError } = await supabaseAdmin - .from("roadmap_items") - .select( - ` - *, - roadmap_categories ( - id, - name, - color - ) - ` - ) + const { data: items, error: itemsError } = await supabaseAdmin + .from("roadmap_items") + .select(` + *, + roadmap_categories ( id, name, color ), + vote_count:roadmap_votes(count) + `) + .group("id, roadmap_categories.id")Note: adjust RoadmapItem to accept vote_count: number | null and UI to read that field.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
apps/page/components/subscribe-prompt.tsx(4 hunks)apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx(1 hunks)apps/page/pages/api/pa/view.ts(1 hunks)apps/page/pages/api/post/react.ts(1 hunks)apps/page/pages/api/post/reactions.ts(1 hunks)apps/page/pages/api/roadmap/vote.ts(1 hunks)apps/page/pages/api/roadmap/votes.ts(1 hunks)apps/page/pages/api/roadmap/votes/bulk.ts(1 hunks)apps/web/components/layout/page.component.tsx(4 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx(1 hunks)package.json(1 hunks)packages/supabase/migrations/18_roadmap.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/layout/page.component.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
apps/page/pages/api/roadmap/votes.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (9)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClient(14-37)apps/web/utils/useSSR.ts (1)
getPage(3-12)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-229)apps/web/utils/useUser.tsx (1)
useUserData(101-107)apps/web/utils/hooks/usePageSettings.ts (1)
usePageSettings(10-62)apps/web/components/layout/page.component.tsx (1)
Page(11-209)apps/web/components/core/buttons.component.tsx (1)
SecondaryButton(148-174)packages/utils/roadmap/index.ts (1)
getCategoryColorClasses(66-70)apps/web/components/layout/auth-layout.component.tsx (1)
AuthLayout(3-16)
apps/page/components/subscribe-prompt.tsx (1)
packages/ui/components/Spinner.tsx (1)
Spinner(3-32)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (6)
packages/supabase/types/index.ts (1)
Database(9-836)packages/supabase/types/page.ts (2)
IPage(3-3)IPageSettings(4-5)apps/page/components/page-header.tsx (1)
PageHeader(17-170)packages/utils/roadmap/index.ts (1)
getCategoryColorClasses(66-70)apps/page/lib/data.ts (3)
BLACKLISTED_SLUGS(8-71)fetchRenderData(357-357)isSubscriptionActive(358-358)packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/page/pages/api/roadmap/vote.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/page/pages/api/roadmap/votes/bulk.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
🪛 Biome (2.1.2)
apps/page/pages/api/roadmap/votes.ts
[error] 71-71: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
apps/page/pages/api/roadmap/vote.ts
[error] 95-95: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
apps/page/pages/api/roadmap/votes/bulk.ts
[error] 100-100: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
🔇 Additional comments (8)
package.json (1)
9-9: LGTM: convenience script for multi-app devThe brace filter with quotes is portable enough across shells; this should streamline running both apps concurrently.
apps/page/pages/api/roadmap/votes.ts (2)
12-16: Nice: Method guard with Allow headerGET-only with proper 405/Allow is correct for a read endpoint.
18-31: Good input validation for item_idUUID regex + array guard cover common attack/shape issues.
apps/page/components/subscribe-prompt.tsx (3)
3-8: Icon imports look good.No issues with added heroicons usage.
25-25: Good UX addition: local state to gate the form.The showForm toggle is a clean approach.
66-91: RSS-only rendering path reads clearly.Scoped links and container sizing look correct.
packages/supabase/migrations/18_roadmap.sql (2)
102-112: No RLS policies on votes—confirm intent (server-only access).If clients won’t read/write roadmap_votes directly, keeping it restricted is fine. Otherwise, add minimal SELECT/INSERT scoped to public boards and visitor_id.
1-11: Verify trigger function and UUID generator availability.Ensure trigger_set_timestamp() exists earlier in migrations and that uuid_generate_v4() extension is enabled (or switch to gen_random_uuid()).
Also applies to: 23-27, 70-74, 97-101
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (3)
25-36: Block unauthorized access in SSR (owner/public check missing).Anyone with a URL can fetch private board data. Enforce access server-side: allow if board.is_public, otherwise require ownership.
- const { supabase } = await getSupabaseServerClient({ req, res }); + const { supabase, user } = await getSupabaseServerClient({ req, res }); @@ if (boardError || !board) { console.error("Failed to fetch roadmap board", boardError); return { notFound: true, }; } + + // Enforce access: owner or public board + const isOwner = !!user && page.user_id === user.id; + if (!board.is_public && !isOwner) { + return { notFound: true }; + }Also applies to: 39-52
561-569: Fix SecondaryButton usage (no icon prop) and avoid full reload.Remove unsupported icon prop and use router.push.
- <SecondaryButton - label="Settings" - icon={<DotsVerticalIcon className="-ml-1 mr-2 h-5 w-5" />} - onClick={() => - (window.location.href = `/pages/${page_id}/roadmap/${board.id}/settings`) - } - /> + <SecondaryButton + label={ + <span className="inline-flex items-center"> + <DotsVerticalIcon className="-ml-1 mr-2 h-5 w-5" /> + Settings + </span> + } + onClick={() => router.push(`/pages/${page_id}/roadmap/${board.id}/settings`)} + />
441-539: Cross-column move leaves gaps in source column; resequence remaining items.After moving an item to another column, normalize positions in the source column both in DB and local state.
// Update the dragged item const { error } = await supabase .from("roadmap_items") .update({ column_id: targetColumnId, position: newPosition, }) .eq("id", draggedItem.id); if (error) throw error; - // Update local state + // Resequence source column positions after removal + const remainingSourceItems = sourceColumnItems + .filter((i) => i.id !== draggedItem.id) + .sort((a, b) => (a.position || 0) - (b.position || 0)); + const sourceFinalUpdates = remainingSourceItems.map((item, index) => + supabase.from("roadmap_items").update({ position: index + 1 }).eq("id", item.id) + ); + await Promise.all(sourceFinalUpdates); + + // Update local state setBoardItems((prev) => prev.map((item) => { if (item.id === draggedItem.id) { return { ...item, column_id: targetColumnId, position: newPosition, }; } // Update positions of shifted items in target column if (item.column_id === targetColumnId && currentDragOverPosition) { const targetItem = targetColumnItems.find( (ti) => ti.id === currentDragOverPosition.itemId ); if (targetItem) { if ( currentDragOverPosition.position === "before" && item.position >= targetItem.position ) { return { ...item, position: item.position + 1 }; } if ( currentDragOverPosition.position === "after" && item.position > targetItem.position ) { return { ...item, position: item.position + 1 }; } } } + // Normalize positions of remaining source items + if (item.column_id === sourceColumnId) { + const idx = remainingSourceItems.findIndex((ri) => ri.id === item.id); + if (idx !== -1) return { ...item, position: idx + 1 }; + } return item; }) );
🧹 Nitpick comments (4)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (4)
422-440: Reduce N×2 update churn on same-column reorder (use RPC/transaction).Batching via “temp” and “final” updates doubles writes and risks contention. Prefer a single SQL function to compute row_number() over the desired order and update in one transaction.
121-133: Avoid duplicate context reads.Pull both user and supabase from a single useUserData() call.
- const router = useRouter(); - const { user } = useUserData(); + const router = useRouter(); + const { user, supabase } = useUserData(); @@ - const { supabase } = useUserData();
147-148: Type formErrors to prevent implicit any.Small TS polish.
- const [formErrors, setFormErrors] = useState({}); + const [formErrors, setFormErrors] = useState<Record<string, string>>({}); @@ - const validateForm = () => { - const errors = {}; + const validateForm = () => { + const errors: Record<string, string> = {};Also applies to: 212-217
134-139: selectedItem is set but never used.Remove the state or wire it to an item details view to avoid dead code.
Also applies to: 646-656
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx(1 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (9)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClient(14-37)apps/web/utils/useSSR.ts (1)
getPage(3-12)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-229)apps/web/utils/useUser.tsx (1)
useUserData(101-107)apps/web/utils/hooks/usePageSettings.ts (1)
usePageSettings(10-62)apps/web/components/layout/page.component.tsx (1)
Page(11-209)apps/web/components/core/buttons.component.tsx (1)
SecondaryButton(148-174)packages/utils/roadmap/index.ts (1)
getCategoryColorClasses(66-70)apps/web/components/layout/auth-layout.component.tsx (1)
AuthLayout(3-16)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (4)
22-26: Block unauthorized access in SSR (owner check still missing).Enforce ownership on the server to prevent non-owners from reading private boards via direct URL. Retrieve
userand gate onpage.user_id.export async function getServerSideProps({ req, res, params }) { const { page_id, board_id } = params; - const { supabase } = await getSupabaseServerClient({ req, res }); + const { supabase, user } = await getSupabaseServerClient({ req, res }); const page = await getPage(supabase, page_id).catch((e) => { console.error("Failed to get page", e); return null; }); if (!page) { return { notFound: true, }; } + // Owner-only internal board view + if (!user || page.user_id !== user.id) { + return { notFound: true }; + }Also applies to: 31-35, 47-52
194-210: Deletion doesn’t resequence column positions; normalize after delete.Gaps break ordering semantics. Resequence both DB and local state.
- const handleDeleteItem = async (itemId: string) => { + const handleDeleteItem = async (itemToDelete: { id: string; column_id: string }) => { if (!confirm("Are you sure you want to delete this item?")) return; try { const { error } = await supabase .from("roadmap_items") .delete() - .eq("id", itemId); + .eq("id", itemToDelete.id); if (error) throw error; - setBoardItems((prev) => prev.filter((item) => item.id !== itemId)); + // Resequence remaining items in the same column + const remaining = boardItems + .filter((i) => i.column_id === itemToDelete.column_id && i.id !== itemToDelete.id) + .sort((a, b) => (a.position || 0) - (b.position || 0)); + const updates = remaining.map((ri, idx) => + supabase.from("roadmap_items").update({ position: idx + 1 }).eq("id", ri.id) + ); + await Promise.all(updates); + + setBoardItems((prev) => + prev + .filter((i) => i.id !== itemToDelete.id) + .map((i) => { + if (i.column_id !== itemToDelete.column_id) return i; + const idx = remaining.findIndex((ri) => ri.id === i.id); + return idx !== -1 ? { ...i, position: idx + 1 } : i; + }) + ); } catch (error) { console.error("Error deleting item:", error); alert("Failed to delete item"); } };And update the call site:
- handleDeleteItem(item.id); + handleDeleteItem({ id: item.id, column_id: item.column_id });Also applies to: 670-673
441-538: Cross-column move leaves gaps in source column; resequence source items.After moving across columns, source positions drift. Normalize DB and local state for the source column.
if (error) throw error; - // Update local state + // Resequence source column positions after removal + const remainingSourceItems = sourceColumnItems + .filter((i) => i.id !== draggedItem.id) + .sort((a, b) => (a.position || 0) - (b.position || 0)); + const sourceFinalUpdates = remainingSourceItems.map((item, index) => + supabase.from("roadmap_items").update({ position: index + 1 }).eq("id", item.id) + ); + await Promise.all(sourceFinalUpdates); + + // Update local state setBoardItems((prev) => prev.map((item) => { if (item.id === draggedItem.id) { return { ...item, column_id: targetColumnId, position: newPosition, }; } // Update positions of shifted items in target column if (item.column_id === targetColumnId && currentDragOverPosition) { const targetItem = targetColumnItems.find( (ti) => ti.id === currentDragOverPosition.itemId ); if (targetItem) { if ( currentDragOverPosition.position === "before" && item.position >= targetItem.position ) { return { ...item, position: item.position + 1 }; } if ( currentDragOverPosition.position === "after" && item.position > targetItem.position ) { return { ...item, position: item.position + 1 }; } } } + // Normalize positions of remaining source items + if (item.column_id === sourceColumnId) { + const idx = remainingSourceItems.findIndex((ri) => ri.id === item.id); + if (idx !== -1) return { ...item, position: idx + 1 }; + } return item; }) );
561-569: Fix SecondaryButton props and avoid full reload.
SecondaryButtonhas noiconprop (TS error) andwindow.location.hrefcauses a hard reload. Inline the icon inlabeland userouter.push.- <SecondaryButton - label="Settings" - icon={<DotsVerticalIcon className="-ml-1 mr-2 h-5 w-5" />} - onClick={() => - (window.location.href = `/pages/${page_id}/roadmap/${board.id}/settings`) - } - /> + <SecondaryButton + label={ + <span className="inline-flex items-center"> + <DotsVerticalIcon className="-ml-1 mr-2 h-5 w-5" /> + Settings + </span> + } + onClick={() => router.push(`/pages/${page_id}/roadmap/${board.id}/settings`)} + />
🧹 Nitpick comments (4)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (4)
310-315: Ensure drag works in Safari: setData is required.Some browsers (Safari) require setting data on dragstart.
const handleDragStart = (e, item) => { if (!isPageOwner) return; setDraggedItem(item); e.dataTransfer.effectAllowed = "move"; + try { e.dataTransfer.setData("text/plain", String(item.id)); } catch (_) {} e.target.style.opacity = "0.5"; };
121-133: Consolidate useUserData calls.Fetch
userandsupabaseonce.- const router = useRouter(); - const { user } = useUserData(); + const router = useRouter(); + const { user, supabase } = useUserData(); - - const { supabase } = useUserData();
65-86: Avoid fetching full vote rows when only counts are rendered.Selecting
roadmap_votes(id)pulls many rows; prefer a count (view/RPC or nested count) to reduce payload.
- Option A: add a materialized view or column
votes_countand select it.- Option B: use a Supabase RPC to return items with
votes_countfor a board.Also applies to: 693-706
422-440: Make reordering atomic to prevent race conditions.Two-step updates and many round-trips can leave inconsistent positions on failures. Prefer a single SQL function (transaction) that reorders within a column or moves across columns (shift target, resequence source, update dragged) atomically.
Sketch: create RPC
reorder_roadmap_items(src_column uuid, dst_column uuid, dragged uuid, before uuid|null, place text)that performs all updates in one transaction, then refetch affected columns.Also applies to: 466-487
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx(1 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (10)
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (1)
getServerSideProps(20-86)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClient(14-37)apps/web/utils/useSSR.ts (1)
getPage(3-12)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-229)apps/web/utils/useUser.tsx (1)
useUserData(101-107)apps/web/utils/hooks/usePageSettings.ts (1)
usePageSettings(10-62)apps/web/components/layout/page.component.tsx (1)
Page(11-209)apps/web/components/core/buttons.component.tsx (1)
SecondaryButton(148-174)packages/utils/roadmap/index.ts (1)
getCategoryColorClasses(66-70)apps/web/components/layout/auth-layout.component.tsx (1)
AuthLayout(3-16)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/page/pages/api/roadmap/votes/bulk.ts (2)
56-67: Aggregate in DB and use distinct IDs; current approach is heavy and can miscount >1k rows.
Switch to grouped counts and query with distinctItemIds to avoid row scans and PostgREST row limits. Also adjust the mapper. This mirrors prior feedback.- const [voteCountResult, userVoteResult] = await Promise.all([ - supabaseAdmin - .from("roadmap_votes") - .select("item_id") - .in("item_id", item_ids), + const [voteAggResult, userVoteResult] = await Promise.all([ + supabaseAdmin + .from("roadmap_votes") + .select("item_id, count", { head: false }) + .in("item_id", distinctItemIds) + .group("item_id"), supabaseAdmin .from("roadmap_votes") .select("item_id") - .in("item_id", item_ids) + .in("item_id", distinctItemIds) .eq("visitor_id", visitor_id), ]); @@ - if (voteCountResult.error) { + if (voteAggResult.error) { console.error( "getBulkRoadmapItemVotes [Count Error]", - voteCountResult.error + voteAggResult.error ); return res.status(500).json({ ok: false, votes: {} }); } @@ - const voteCountsMap = (voteCountResult.data || []).reduce((acc, vote) => { - acc[vote.item_id] = (acc[vote.item_id] || 0) + 1; - return acc; - }, {} as Record<string, number>); + const voteCountsMap = Object.fromEntries( + (voteAggResult.data || []).map( + (row: { item_id: string; count: number | string }) => [ + row.item_id, + Number(row.count) || 0, + ] + ) + ) as Record<string, number>; @@ - item_ids.forEach((itemId: string) => { + distinctItemIds.forEach((itemId: string) => { votes[itemId] = { vote_count: voteCountsMap[itemId] || 0, user_voted: userVotedSet.has(itemId), }; });Also applies to: 69-75, 85-89, 96-101
107-109: Fix Biome error and use safe logging in catch.
Change the catch type to unknown and narrow before logging.- } catch (e: Error | any) { - console.log("getBulkRoadmapItemVotes [Error]", e); + } catch (e: unknown) { + const err = e instanceof Error ? e : new Error(String(e)); + console.error("getBulkRoadmapItemVotes [Error]", err); res.status(500).json({ ok: false, votes: {} }); }
🧹 Nitpick comments (2)
apps/page/pages/api/roadmap/votes/bulk.ts (2)
42-46: Nice: de-dup + early return.
Consider also using distinctItemIds in the DB queries below (see next comment).
48-54: Secure cookie breaks on localhost; gate by env.
Use Secure only in production so dev can set/read the cookie.if (!visitor_id) { visitor_id = v4(); + const secureAttr = process.env.NODE_ENV === "production" ? " Secure;" : ""; res.setHeader( "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age=31536000` + `cp_pa_vid=${visitor_id}; Path=/;${secureAttr} HttpOnly; SameSite=Lax; Max-Age=31536000` ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/page/pages/api/roadmap/votes/bulk.ts(1 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/page/pages/api/roadmap/votes/bulk.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
🪛 Biome (2.1.2)
apps/page/pages/api/roadmap/votes/bulk.ts
[error] 107-107: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
🔇 Additional comments (2)
apps/page/pages/api/roadmap/votes/bulk.ts (2)
18-22: Good 405 handling with Allow header.
28-35: Solid input validation (shape, limits, UUIDs).Also applies to: 37-41
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/supabase/types/page.ts (1)
69-69: Fix slug regex: missing escape for\w(currently matches literal "w").
Use a regex literal or escape the backslash in the string.Apply minimal fix:
-export const URL_SLUG_REGEX = new RegExp("^[a-zA-Z0-9]([w-]*[a-zA-Z0-9])*$"); +export const URL_SLUG_REGEX = new RegExp("^[a-zA-Z0-9]([\\w-]*[a-zA-Z0-9])*$");Alternatively, prefer a literal to avoid double-escaping:
-export const URL_SLUG_REGEX = new RegExp("^[a-zA-Z0-9]([w-]*[a-zA-Z0-9])*$"); +export const URL_SLUG_REGEX = /^[a-zA-Z0-9]([\w-]*[a-zA-Z0-9])*$/;If underscores shouldn’t be allowed, tighten the middle class to
[a-zA-Z0-9-].
♻️ Duplicate comments (2)
apps/page/pages/api/roadmap/votes.ts (1)
117-119: Fix Biome catch typing and log level; use console.errorMatches linter rules and improves error visibility.
- } catch (e: Error | any) { - console.log("getBulkRoadmapItemVotes [Error]", e); + } catch (e: unknown) { + console.error("getBulkRoadmapItemVotes [Error]", e);apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (1)
12-19: Block unauthorized access in SSR (owner check missing).You fetch the page but never validate the current user owns it. A non-owner with a guessed URL can read private board data. Gate on user.id before returning props.
Apply:
-export async function getServerSideProps({ req, res, params }) { +export async function getServerSideProps({ req, res, params }) { const { page_id, board_id } = params; - const { supabase } = await getSupabaseServerClient({ + const { supabase, user } = await getSupabaseServerClient({ req, res, }); const page = await getPage(supabase, page_id).catch((e) => { console.error("Failed to get page", e); return null; }); if (!page) { return { notFound: true, }; } + if (!user || page.user_id !== user.id) { + return { notFound: true }; + }Also applies to: 24-29
🧹 Nitpick comments (17)
apps/page/pages/api/roadmap/votes.ts (5)
48-54: Make Secure cookie conditional in dev (and prefer proper serialization)Secure cookies won’t set on http://localhost, breaking local testing. Gate Secure by env (or proto), and consider using a serializer to avoid subtle header issues.
Apply:
- res.setHeader( - "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age=31536000` - ); + const isProd = process.env.NODE_ENV === "production"; + res.setHeader( + "Set-Cookie", + `cp_pa_vid=${visitor_id}; Path=/; ${isProd ? "Secure; " : ""}HttpOnly; SameSite=Lax; Max-Age=31536000` + );Optional (preferred): use cookie serialization.
+import { serialize } from "cookie"; @@ - res.setHeader( - "Set-Cookie", - `cp_pa_vid=${visitor_id}; Path=/; ${isProd ? "Secure; " : ""}HttpOnly; SameSite=Lax; Max-Age=31536000` - ); + res.setHeader("Set-Cookie", serialize("cp_pa_vid", visitor_id, { + path: "/", + httpOnly: true, + sameSite: "lax", + secure: isProd, + maxAge: 60 * 60 * 24 * 365, + }));
82-89: Fix “externally-controlled format string” warningAvoid %s formatting with untrusted values; prefer template strings.
- console.error( - "getBulkRoadmapItemVotes [Count Error for %s]", - distinctItemIds[i], - voteCountResults[i].error - ); + console.error( + `getBulkRoadmapItemVotes [Count Error for ${distinctItemIds[i]}]`, + voteCountResults[i].error + );
96-99: Prefer nullish coalescing over || for countsEnsures 0 is preserved without fallback semantics.
- voteCountsMap[itemId] = voteCountResults[index].count || 0; + voteCountsMap[itemId] = voteCountResults[index].count ?? 0;
113-116: Add anti-miscache headers (user-specific response)Response varies by Cookie (user_voted). Prevent CDN/proxy caching mistakes.
- res.status(200).json({ + res.setHeader("Vary", "Cookie"); + res.setHeader("Cache-Control", "private, no-store"); + res.status(200).json({ ok: true, votes, });
57-73: Use a batch RPC or a GROUP BY query to collapse N HEAD requests into one
The existing single-item RPCroadmap_item_votes_count(itemid: uuid)is defined inpackages/supabase/migrations/18_roadmap.sql(lines 115–117) and typed inpackages/supabase/types/index.ts(817–823), but it only handles one ID per call. To avoid spawning up to N round-trips:• Expose a new RPC that accepts
itemids: uuid[]and returns a set of{ item_id, vote_count }, then call:const { data, error } = await supabaseAdmin.rpc( 'roadmap_items_votes_counts', { itemids: distinctItemIds } );• Or use a single GROUP BY on the votes table:
const { data, error } = await supabaseAdmin .from('roadmap_votes') .select('item_id, vote_count:count(id)', { count: 'exact' }) .in('item_id', distinctItemIds) .group('item_id'); data?.forEach(({ item_id, vote_count }) => { voteCountsMap[item_id] = vote_count ?? 0; });This reduces latency and rate-limit risk.
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (1)
32-59: Consider ordering in SQL, not JS.Reduce payload and work in Node by ordering related rows in the query instead of sorting in-memory.
const { data: board, error: boardError } = await supabase .from("roadmap_boards") .select( ` *, roadmap_columns (*), roadmap_items ( *, roadmap_categories (id,name,color), roadmap_votes (id) ), roadmap_categories (*) ` ) .eq("id", board_id) .eq("page_id", page_id) + .order("position", { foreignTable: "roadmap_columns", ascending: true }) + .order("position", { foreignTable: "roadmap_items", ascending: true }) + .order("created_at", { foreignTable: "roadmap_categories", ascending: true }) .single(); - -const columns = (board.roadmap_columns || []).sort((a,b)=>(a.position||0)-(b.position||0)); -const items = (board.roadmap_items || []).sort((a,b)=>(a.position||0)-(b.position||0)); -const categories = (board.roadmap_categories || []).sort((a,b)=>new Date(a.created_at).getTime()-new Date(b.created_at).getTime()); +const columns = board.roadmap_columns || []; +const items = board.roadmap_items || []; +const categories = board.roadmap_categories || [];Also applies to: 67-76
apps/web/components/roadmap/RoadmapItem.tsx (1)
53-73: Add accessible labels to action buttons.Title helps, but aria-label improves screen reader support.
- <button + <button + aria-label="Edit item" onClick={(e) => { e.stopPropagation(); onEdit(item); }} className="p-2 text-gray-400 hover:text-gray-600 dark:hover:text-gray-300 hover:bg-gray-100 dark:hover:bg-gray-700 rounded-md transition-colors" title="Edit item" > <PencilIcon className="h-4 w-4" /> </button> <button + aria-label="Delete item" onClick={(e) => { e.stopPropagation(); onDelete(item.id); }} className="p-2 text-gray-400 hover:text-red-600 dark:hover:text-red-400 hover:bg-red-50 dark:hover:bg-red-900/20 rounded-md transition-colors" title="Delete item" > <TrashIcon className="h-4 w-4" /> </button>apps/web/components/roadmap/RoadmapItemModal.tsx (2)
112-131: Improve form accessibility (label + autofocus).Bind a label to the title input and focus it when modal opens.
- <div> - <input + <div> + <label + htmlFor="item-title" + className="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1" + > + Title + </label> + <input + id="item-title" type="text" value={itemForm.title} onChange={(e) => setItemForm((prev) => ({ ...prev, title: e.target.value, })) } + autoFocus className="w-full text-xl font-semibold leading-6 text-gray-900 dark:text-gray-100 bg-white dark:bg-gray-700 border border-gray-300 dark:border-gray-600 rounded-md px-3 py-2 shadow-sm focus:border-indigo-500 focus:ring-indigo-500 focus:outline-none mb-6" placeholder="Enter item title..." />
136-157: Associate label with category select.Use a real label and id for better a11y; keep visual style the same.
- <div className="flex items-center justify-between"> - <span className="text-sm font-medium text-gray-500 dark:text-gray-400"> - Category - </span> - <select - value={itemForm.category_id} + <div className="flex items-center justify-between"> + <label + htmlFor="item-category" + className="text-sm font-medium text-gray-500 dark:text-gray-400" + > + Category + </label> + <select + id="item-category" + value={itemForm.category_id} onChange={(e) => setItemForm((prev) => ({ ...prev, category_id: e.target.value, })) } className="text-sm rounded-md border-gray-300 dark:border-gray-600 shadow-sm focus:border-indigo-500 focus:ring-indigo-500 dark:bg-gray-700 dark:text-gray-300" >apps/web/components/roadmap/types.ts (1)
27-29: Tighten FormErrors typing.Constrain to known keys used (“general”, “title”) to catch typos early.
-export interface FormErrors { - [key: string]: string; -} +export type FormErrors = Partial<{ + general: string; + title: string; +}>;apps/web/components/roadmap/RoadmapBoard.tsx (2)
28-36: OK to sort client-side; consider server ordering when lists grow.Current memoization is fine. If columns/items scale, push ordering to the query for less JS work.
38-49: Reordering concurrency and write amplification.Same-column reorder issues many updates; cross-column shifts O(n). Under concurrent users this can clash without DB constraints/transactions.
- Add a unique partial index (board_id, column_id, position) and use two-phase “temp then final” updates (you already do for same-column).
- Consider “fractional indexing” (e.g., positions as decimals or using ordinals with gaps) to reduce mass updates on insert/reorder.
- If feasible, wrap related updates in a Postgres function with a single RPC to keep consistency.
apps/web/components/roadmap/RoadmapColumn.tsx (1)
47-57: Add ARIA roles/attributes to improve DnD accessibility.Mark the column as a droppable region and announce its state for assistive tech.
- <div - className={`flex-shrink-0 w-80 flex flex-col transition-colors ${ + <div + role="list" + aria-dropeffect={dragOverColumn === column.id ? "move" : undefined} + className={`flex-shrink-0 w-80 flex flex-col transition-colors ${ dragOverColumn === column.id ? "bg-indigo-50 dark:bg-indigo-900/20 rounded-lg" : "" }`}apps/web/components/roadmap/hooks/useRoadmapItems.ts (1)
127-134: Surface a user-facing error when columnId is missing (edge).Early-return silently exits; keep the modal open and show an error.
- if (!selectedColumnId) return; + if (!selectedColumnId) { + setFormErrors({ general: "Please select a column." }); + return; + }apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts (2)
20-28: Avoid inline style mutations; rely on isDragged styling.Inline opacity tweaks can target nested elements and conflict with the isDragged prop. Let UI derive from state only.
const handleDragStart = (e: React.DragEvent, item: RoadmapItemWithRelations) => { setDraggedItem(item); e.dataTransfer.effectAllowed = "move"; - (e.target as HTMLElement).style.opacity = "0.5"; }; const handleDragEnd = (e: React.DragEvent) => { - (e.target as HTMLElement).style.opacity = "1"; setDraggedItem(null); setDragOverColumn(null); setDragOverPosition(null); };
202-224: Normalize positions in the source column after moving out (consistency).You update target column positions but leave a gap in the source column. Not strictly wrong, but normalizing keeps ordering dense and simplifies future ops.
setBoardItems(prev => prev.map(item => { if (item.id === draggedItem.id) { return { ...item, column_id: targetColumnId, position: newPosition, }; } + // Compact positions in the source column + if (item.column_id === draggedItem.column_id) { + const sourceItems = itemsByColumn[draggedItem.column_id] || []; + const dragged = sourceItems.find(si => si.id === draggedItem.id); + if (dragged && (item.position || 0) > (dragged.position || 0)) { + return { ...item, position: (item.position || 0) - 1 }; + } + } if (item.column_id === targetColumnId && currentDragOverPosition) { const targetItem = targetColumnItems.find(ti => ti.id === currentDragOverPosition.itemId); if (targetItem) { if (currentDragOverPosition.position === "before" && item.position >= targetItem.position) { return { ...item, position: item.position + 1 }; } if (currentDragOverPosition.position === "after" && item.position > targetItem.position) { return { ...item, position: item.position + 1 }; } } } return item; }) );Follow up with a matching DB update for the decremented source items, ideally in a single ordered batch.
packages/supabase/types/page.ts (1)
3-15: Type aliases viaTables<...>look solid; add Insert/Update aliases if needed.
If you create/patch records frequently, consider exporting corresponding Insert/Update aliases for DX consistency (only if such helpers exist in your generated types).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/page/pages/api/roadmap/votes.ts(1 hunks)apps/web/components/roadmap/RoadmapBoard.tsx(1 hunks)apps/web/components/roadmap/RoadmapColumn.tsx(1 hunks)apps/web/components/roadmap/RoadmapItem.tsx(1 hunks)apps/web/components/roadmap/RoadmapItemModal.tsx(1 hunks)apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts(1 hunks)apps/web/components/roadmap/hooks/useRoadmapItems.ts(1 hunks)apps/web/components/roadmap/types.ts(1 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx(1 hunks)packages/supabase/types/page.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-06T10:44:41.092Z
Learnt from: arjunkomath
PR: techulus/changes-page#83
File: apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx:194-211
Timestamp: 2025-09-06T10:44:41.092Z
Learning: When deleting items with position fields, gaps in position numbering are acceptable if they don't affect user experience or functionality. The sorting and drag-and-drop logic can handle non-sequential positions without issues.
Applied to files:
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx
🧬 Code graph analysis (10)
apps/web/components/roadmap/types.ts (1)
packages/supabase/types/page.ts (3)
IRoadmapItem(14-14)IRoadmapCategory(13-13)IRoadmapVote(15-15)
apps/web/components/roadmap/RoadmapItemModal.tsx (2)
apps/web/components/roadmap/types.ts (3)
ItemForm(16-20)FormErrors(27-29)RoadmapItemWithRelations(7-10)packages/supabase/types/page.ts (2)
IRoadmapCategory(13-13)IRoadmapBoard(11-11)
apps/web/components/roadmap/RoadmapItem.tsx (2)
apps/web/components/roadmap/types.ts (1)
RoadmapItemWithRelations(7-10)packages/utils/roadmap/index.ts (1)
getCategoryColorClasses(66-70)
apps/web/components/roadmap/hooks/useRoadmapItems.ts (2)
apps/web/components/roadmap/RoadmapBoard.tsx (1)
RoadmapBoard(14-125)apps/web/components/roadmap/types.ts (4)
ItemsByColumn(12-14)RoadmapItemWithRelations(7-10)ItemForm(16-20)FormErrors(27-29)
apps/web/components/roadmap/RoadmapColumn.tsx (3)
packages/supabase/types/page.ts (1)
IRoadmapColumn(12-12)apps/web/components/roadmap/types.ts (2)
RoadmapItemWithRelations(7-10)DragOverPosition(22-25)apps/web/components/roadmap/RoadmapItem.tsx (1)
RoadmapItem(15-105)
apps/web/components/roadmap/RoadmapBoard.tsx (7)
packages/supabase/types/page.ts (3)
IRoadmapBoard(11-11)IRoadmapColumn(12-12)IRoadmapCategory(13-13)apps/web/components/roadmap/types.ts (2)
RoadmapItemWithRelations(7-10)ItemsByColumn(12-14)apps/web/utils/useUser.tsx (1)
useUserData(101-107)apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts (1)
useRoadmapDragDrop(11-239)apps/web/components/roadmap/hooks/useRoadmapItems.ts (1)
useRoadmapItems(19-192)apps/web/components/roadmap/RoadmapColumn.tsx (1)
RoadmapColumn(29-148)apps/web/components/roadmap/RoadmapItemModal.tsx (1)
RoadmapItemModal(23-200)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (7)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClient(14-37)apps/web/utils/useSSR.ts (1)
getPage(3-12)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-229)apps/web/components/layout/page.component.tsx (1)
Page(11-209)apps/web/components/core/buttons.component.tsx (1)
SecondaryButton(148-174)apps/web/components/roadmap/RoadmapBoard.tsx (1)
RoadmapBoard(14-125)apps/web/components/layout/auth-layout.component.tsx (1)
AuthLayout(3-16)
apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts (1)
apps/web/components/roadmap/types.ts (3)
ItemsByColumn(12-14)RoadmapItemWithRelations(7-10)DragOverPosition(22-25)
packages/supabase/types/page.ts (1)
packages/supabase/types/index.ts (1)
Tables(842-869)
apps/page/pages/api/roadmap/votes.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
🪛 Biome (2.1.2)
apps/page/pages/api/roadmap/votes.ts
[error] 117-117: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
(parse)
🔇 Additional comments (9)
apps/page/pages/api/roadmap/votes.ts (2)
18-22: Method guard looks good405 with Allow: POST is correct.
24-41: Input validation is solidArray/type/UUID checks and max length are appropriate.
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (1)
114-120: Settings button routing looks good.Uses router.push and valid SecondaryButton API.
apps/web/components/roadmap/RoadmapBoard.tsx (1)
66-69: Delete handler is lightweight and aligns with product choice.Gaps after deletions are acceptable per earlier guidance; local state update is correct.
apps/web/components/roadmap/RoadmapColumn.tsx (1)
84-135: LGTM on DnD hit zones and visuals.Per-item “before/after” drop zones and the column-level fallback are wired correctly and reflect state as intended.
apps/web/components/roadmap/hooks/useRoadmapItems.ts (2)
129-144: Concurrency caveat on computing position.Using max(position)+1 is fine for single-user flows; in multi-user scenarios it can race. Consider server-side sequencing (RPC) if concurrent inserts become common.
Would you like an RPC snippet to atomically allocate next position per (board_id, column_id)?
60-79: LGTM on optimistic delete with rollback via error alert.Flow is correct and updates local state after remote success.
apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts (1)
129-144: Good call on two-phase reorder within a column.The temp bump then normalize avoids unique conflicts and keeps local/UI state responsive.
packages/supabase/types/page.ts (1)
1-1: Good shift to importingTableshelper.
Reduces verbosity and keeps aliases resilient to schema changes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (2)
109-116: Use server’s voted flag to avoid client/server mismatch.If the API rejects/de-dupes, local voted may be wrong. Prefer server truth when available.
Apply:
- setVotes((prev) => ({ - ...prev, - [itemId]: { - count: data.vote_count || 0, - voted: newVotedState, - }, - })); + setVotes((prev) => ({ + ...prev, + [itemId]: { + count: typeof data.vote_count === "number" ? data.vote_count : 0, + voted: + typeof data.user_voted === "boolean" + ? data.user_voted + : newVotedState, + }, + }));
376-386: Remove ts-ignore and pass an explicit sanitize schema (or drop rehypeRaw).Avoid raw HTML footguns and typed-ignore.
Apply:
+import { defaultSchema } from "hast-util-sanitize"; @@ - <ReactMarkdown - rehypePlugins={[ - rehypeRaw, - // @ts-ignore - rehypeSanitize, - ]} - remarkPlugins={[remarkGfm]} - > + <ReactMarkdown + rehypePlugins={[ + rehypeRaw, + [ + rehypeSanitize, + { + ...defaultSchema, + attributes: { + ...defaultSchema.attributes, + a: [ + ...(defaultSchema.attributes?.a || []), + ["target"], + ["rel"], + ], + code: [ + ...(defaultSchema.attributes?.code || []), + ["className"], + ], + span: [ + ...(defaultSchema.attributes?.span || []), + ["className"], + ], + }, + }, + ], + ]} + remarkPlugins={[remarkGfm]} + >If raw HTML isn’t needed, remove rehypeRaw entirely for a stricter posture.
🧹 Nitpick comments (3)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (2)
137-167: Harden vote-fetch parsing; guard against unexpected API shape.Avoid runtime errors if data or data.votes is missing.
- const data = await httpPost({ + const data = await httpPost({ url: "/api/roadmap/votes", data: { item_ids: items.map((item) => item.id) }, }); - - // Transform API response to match expected frontend structure + if (!data || typeof data !== "object" || !data.votes) { + setVotes({}); + return; + } + // Transform API response to match expected frontend structure const transformedVotes: Record< string, { count: number; voted: boolean } > = {}; Object.entries(data.votes).forEach( ([itemId, voteData]: [string, any]) => { transformedVotes[itemId] = { - count: voteData.vote_count, - voted: voteData.user_voted, + count: Number(voteData?.vote_count) || 0, + voted: Boolean(voteData?.user_voted), }; } );
262-277: Improve accessibility on vote button.Expose toggle/busy state for assistive tech.
- <button + <button onClick={(e) => { e.stopPropagation(); handleVote(item.id); }} disabled={votingItems.has(item.id)} + aria-pressed={!!votes[item.id]?.voted} + aria-busy={votingItems.has(item.id)} className={`flex items-center text-xs px-2 py-1 rounded transition-colors ${apps/page/lib/data.ts (1)
422-441: Normalize items to an array to simplify consumers.Returning undefined/null forces extra coalescing downstream.
- const { data: items, error: itemsError } = await supabaseAdmin + const { data: items, error: itemsError } = await supabaseAdmin @@ - if (itemsError) { + if (itemsError) { console.error("Failed to fetch items", itemsError); } - return { board, columns, items }; + return { board, columns, items: items ?? [] };If you keep the union type, update the return type accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/page/lib/data.ts(7 hunks)apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (6)
apps/page/lib/data.ts (3)
RoadmapItemWithCategory(383-389)PageRoadmap(80-83)BLACKLISTED_SLUGS(15-78)packages/supabase/types/page.ts (4)
IPage(3-3)IPageSettings(4-4)IRoadmapBoard(11-11)IRoadmapColumn(12-12)apps/page/hooks/usePageTheme.ts (1)
usePageTheme(4-12)apps/web/utils/http.ts (1)
httpPost(3-21)apps/page/components/page-header.tsx (1)
PageHeader(10-163)packages/utils/roadmap/index.ts (1)
getCategoryColorClasses(66-70)
apps/page/lib/data.ts (2)
packages/supabase/types/page.ts (5)
IRoadmapBoard(11-11)IPage(3-3)IPageSettings(4-4)IRoadmapItem(14-14)IRoadmapColumn(12-12)packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
🔇 Additional comments (2)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (1)
29-47: Types alignment looks good.Props now use PageRoadmap[] and RoadmapItemWithCategory-based item type. This resolves earlier friction with server payloads.
apps/page/lib/data.ts (1)
399-410: Board fetch error handling: OK.On failure you log and short-circuit before using board.id. Good.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/page/tailwind.config.js (1)
10-20: Safelist patterns LGTM and align with prior suggestion.Pattern-based safelist with dark variants is a clean improvement.
🧹 Nitpick comments (3)
apps/web/pages/pages/index.tsx (2)
141-145: Mark decorative icons as hidden from screen readers.Prevents redundant announcements.
- <UserGroupIcon className="h-4 w-4" /> + <UserGroupIcon className="h-4 w-4" aria-hidden="true" />- <svg + <svg + aria-hidden="true" + focusable="false" className="h-5 w-5 text-gray-400 group-hover:text-indigo-500 transition-colors" fill="currentColor" viewBox="0 0 20 20" >Also applies to: 172-183
116-135: De-duplicate type badge classes with a small map.Reduces condition churn and future color tweaks.
// near top of file const TYPE_BADGE: Record<PageType, string> = { [PageType.announcements]: "bg-blue-100 dark:bg-blue-700 text-blue-500 dark:text-blue-100", [PageType.changelogs]: "bg-teal-100 dark:bg-teal-700 text-teal-500 dark:text-teal-100", [PageType.releases]: "bg-rose-100 dark:bg-rose-700 text-rose-500 dark:text-rose-100", [PageType.updates]: "bg-amber-100 dark:bg-amber-700 text-amber-500 dark:text-amber-100", };- <span - className={classNames( - page.type === PageType.announcements && - "bg-blue-100 dark:bg-blue-700", - page.type === PageType.changelogs && - "bg-teal-100 dark:bg-teal-700", - page.type === PageType.releases && - "bg-rose-100 dark:bg-rose-700", - page.type === PageType.updates && - "bg-amber-100 dark:bg-amber-700", - page.type === PageType.announcements && - "text-blue-500 dark:text-blue-100", - page.type === PageType.changelogs && - "text-teal-500 dark:text-teal-100", - page.type === PageType.releases && - "text-rose-500 dark:text-rose-100", - page.type === PageType.updates && - "text-amber-500 dark:text-amber-100", - "inline-flex px-2 py-1 text-xs font-bold rounded-md" - )} - > + <span className={classNames(TYPE_BADGE[page.type], "inline-flex px-2 py-1 text-xs font-bold rounded-md")}>apps/page/tailwind.config.js (1)
21-30: Remove duplicatethemekeys.Only the latter
themetakes effect; collapse into one.- theme: { - extend: {}, - }, - theme: { + theme: { extend: { colors: { gray: neutral, }, }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx(1 hunks)apps/page/tailwind.config.js(1 hunks)apps/web/components/layout/page.component.tsx(3 hunks)apps/web/pages/pages/index.tsx(1 hunks)apps/web/tailwind.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/tailwind.config.js
- apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx
- apps/web/components/layout/page.component.tsx
🔇 Additional comments (1)
apps/web/pages/pages/index.tsx (1)
158-162: Ensure Tailwind line-clamp utilities are available.You use
line-clamp-2; verify the plugin is enabled for apps/web.#!/bin/bash # Check if line-clamp plugin is configured in apps/web rg -n "line-clamp" apps/web/tailwind.config.js || trueIf absent, add:
// apps/web/tailwind.config.js plugins: [require("@tailwindcss/typography"), require("@tailwindcss/line-clamp")],
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts (1)
146-192: Resolved: descending sequential shifts avoid uniqueness collisionsNice fix adopting descending, per‑row updates for target column shifts; this removes the transient (column_id, position) collision risk raised previously.
🧹 Nitpick comments (4)
apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts (4)
20-24: UsecurrentTargetinstead oftargetfor styling
e.targetcan be a child; usee.currentTargetto ensure you style the element with the handler.- (e.target as HTMLElement).style.opacity = "0.5"; + (e.currentTarget as HTMLElement).style.opacity = "0.5"; @@ - (e.target as HTMLElement).style.opacity = "1"; + (e.currentTarget as HTMLElement).style.opacity = "1";Also applies to: 26-31
43-48: GuardrelatedTargetnulls in dragleaveOn some browsers
relatedTargetcan be null; add a null guard to avoid false clears.- if (!e.currentTarget.contains(e.relatedTarget as Node)) { + const next = (e.relatedTarget as Node | null); + if (!next || !e.currentTarget.contains(next)) { setDragOverColumn(null); setDragOverPosition(null); }
151-154: Compact source column positions after cross‑column move (optional)After removing an item from its source column, decrement positions of items below it to keep the sequence dense. This helps future inserts and simplifies sorting.
const handleCrossColumnMove = async ( @@ ) => { - if (!draggedItem) return; + if (!draggedItem) return; + const sourceColumnId = draggedItem.column_id; + const sourcePosition = draggedItem.position || 0; @@ - if (error) throw error; + if (error) throw error; + + // Compact source column positions (items below the removed one) + const sourceColumnItems = itemsByColumn[sourceColumnId] || []; + const toCompact = sourceColumnItems + .filter(it => (it.position || 0) > sourcePosition) + .sort((a, b) => (a.position || 0) - (b.position || 0)); + for (const it of toCompact) { + const { error: decErr } = await supabase + .from("roadmap_items") + .update({ position: (it.position || 0) - 1 }) + .eq("id", it.id); + if (decErr) throw decErr; + } @@ - setBoardItems(prev => + setBoardItems(prev => prev.map(item => { if (item.id === draggedItem.id) { return { ...item, column_id: targetColumnId, position: newPosition, }; } + // Decrement positions in source column below the removed item + if (item.column_id === sourceColumnId && (item.position || 0) > sourcePosition) { + return { ...item, position: (item.position || 0) - 1 }; + } if (item.column_id === targetColumnId && currentDragOverPosition) { const targetItem = targetColumnItems.find(ti => ti.id === currentDragOverPosition.itemId); if (targetItem) { if (currentDragOverPosition.position === "before" && item.position >= targetItem.position) { return { ...item, position: item.position + 1 }; } if (currentDragOverPosition.position === "after" && item.position > targetItem.position) { return { ...item, position: item.position + 1 }; } } } return item; }) );Also applies to: 202-227
88-145: Consider a single DB RPC to move/reorder atomicallyMultiple client‑side updates remain vulnerable to races across clients and can be slow on high latency. Prefer a Postgres function (called via Supabase RPC) that, in one transaction, locks rows in the affected columns, shifts ranges, and sets the moved item’s column/position.
Also applies to: 146-227
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts (1)
apps/web/components/roadmap/types.ts (3)
ItemsByColumn(12-14)RoadmapItemWithRelations(7-10)DragOverPosition(22-25)
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores