-
Notifications
You must be signed in to change notification settings - Fork 11
Add support for deleting roadmap board #87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds bordered vote-button styling in roadmap item views and modal. Adds a Delete Board workflow in roadmap settings that deletes a board, creates an audit log, and redirects. Removes two PL/pgSQL vote helper functions and adds an initializer to seed default roadmap categories. Tightens several TypeScript handler parameter types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SettingsPage
participant ConfirmDialog as ConfirmDeleteDialog
participant DB as roadmap_boards
participant Audit as createAuditLog
participant Router as Router
User->>SettingsPage: Click "Delete Board"
SettingsPage->>ConfirmDialog: open=true (highRiskAction)
alt User confirms
ConfirmDialog->>SettingsPage: onConfirm()
SettingsPage->>SettingsPage: isDeletingBoard=true
SettingsPage->>DB: delete(board.id)
alt Delete succeeds
SettingsPage->>Audit: createAuditLog({page_id, actor_id, action, changes})
SettingsPage->>Router: push(/pages/{page_id}/roadmap)
SettingsPage->>ConfirmDialog: open=false
else Delete fails
SettingsPage->>SettingsPage: isDeletingBoard=false
SettingsPage->>User: alert(error)
SettingsPage->>ConfirmDialog: open=false
end
else User cancels
ConfirmDialog->>SettingsPage: onCancel()
SettingsPage->>ConfirmDialog: open=false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (2)
258-272: Add accessibility states to the vote button (card).Expose toggle and busy states for assistive tech; also set an explicit type.
-<button +<button + type="button" onClick={(e) => { e.stopPropagation(); handleVote(item.id); }} disabled={votingItems.has(item.id)} + aria-pressed={Boolean(votes[item.id]?.voted)} + aria-busy={votingItems.has(item.id) || undefined} + aria-label={`${votes[item.id]?.voted ? "Remove vote" : "Vote"} for ${item.title} (${(votes[item.id]?.count ?? (item.vote_count || 0))} total)`} className={`flex items-center text-xs px-2 py-1 rounded border transition-colors ${
416-431: Mirror the same accessibility on the modal vote button.Keep the modal control equally accessible.
-<button +<button + type="button" onClick={() => selectedItem && handleVote(selectedItem.id) } disabled={ !selectedItem || votingItems.has(selectedItem.id) } + aria-pressed={Boolean(selectedItem && votes[selectedItem.id]?.voted)} + aria-busy={Boolean(selectedItem && votingItems.has(selectedItem.id)) || undefined} + aria-label={ + selectedItem + ? `${votes[selectedItem.id]?.voted ? "Remove vote" : "Vote"} for ${selectedItem.title} (${(votes[selectedItem.id]?.count ?? (selectedItem.vote_count || 0))} total)` + : "Vote" + } className={`flex items-center text-sm px-3 py-1.5 rounded-lg border transition-colors ${apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (1)
415-417: Fix: adding first stage on an empty board computes -Infinity.Math.max on an empty array returns -Infinity.
- const maxPosition = - Math.max(...boardColumns.map((col) => col.position)) + 1; + const maxPosition = + boardColumns.length > 0 + ? Math.max(...boardColumns.map((col) => col.position || 0)) + 1 + : 1;
🧹 Nitpick comments (4)
apps/web/components/roadmap/RoadmapItem.tsx (1)
89-98: LGTM: vote-count pill styling is consistent.Matches the new bordered button look on the roadmap page.
Small nit: consider stabilizing digit widths to avoid minor layout shift when counts change.
Apply:
-<div className="flex items-center text-xs text-gray-500 dark:text-gray-400 border border-gray-200 dark:border-gray-600 rounded-md px-2 py-1 hover:border-gray-300 dark:hover:border-gray-500 transition-colors"> +<div className="flex items-center text-xs text-gray-500 dark:text-gray-400 border border-gray-200 dark:border-gray-600 rounded-md px-2 py-1 hover:border-gray-300 dark:hover:border-gray-500 transition-colors tabular-nums">apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (3)
363-407: Prefer consistent confirmation UX over window.confirm for category delete.Re-use ConfirmDeleteDialog for a uniform, accessible flow.
486-530: Prefer consistent confirmation UX for stage delete; also block when items exist.Current alerts/confirms work but are inconsistent with the new dialog.
1092-1101: Optional: require typing the slug instead of title.Slug is less error-prone for users with long/emoji-laden titles.
- riskVerificationText={board.title} + riskVerificationText={board.slug}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx(2 hunks)apps/web/components/roadmap/RoadmapItem.tsx(1 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (2)
apps/web/utils/auditLog.ts (1)
createAuditLog(10-24)apps/web/components/dialogs/confirm-delete-dialog.component.tsx (1)
ConfirmDeleteDialog(9-155)
🔇 Additional comments (6)
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (6)
11-11: LGTM: brings in the shared confirmation dialog.
157-160: LGTM: deletion dialog and loading state wiring added.
311-361: LGTM: explicit string typing for category update handler.
446-484: LGTM: explicit string typing for column update handler.
806-815: LGTM: clear CTA and correctly type="button" inside a form.
615-645: Cascading deletes and RLS verified — no action requiredpackages/supabase/migrations/18_roadmap.sql defines ON DELETE CASCADE for roadmap_boards → roadmap_columns/roadmap_categories/roadmap_items and roadmap_items → roadmap_votes, and RLS is enabled with policies that tie access to pages (pages RLS enforces auth.uid()), so deletes should not leave orphans and are restricted to page owners.
Summary by CodeRabbit