fix(e2e): restore projectinfo rows after CASCADE DELETE in resetRepoCards()#121
Conversation
Theme selection was duplicated between the Settings page grid and the sidebar ThemeToggle dropdown. Remove the Settings page theme UI to align with SPEC.md which defines Settings as Display-only. Theme is now exclusively accessed via the sidebar ThemeToggle. - Remove ThemeButton component and theme card grid from SettingsClient - Remove "Change Theme" command from CommandPalette - Rewrite theme E2E tests to use sidebar dropdown instead of Settings - Update loading skeleton to match Display-only layout - Update CLAUDE.md to prefer pnpm e2e:parallel
…ards() resetRepoCards() deletes all repocards then re-inserts them, but the DELETE triggers ON DELETE CASCADE on projectinfo.repo_card_id, silently removing projectinfo rows (IDs 401-404). Downstream helpers like resetProjectInfoComments() and resetProjectInfoLinks() then fail with "UPDATE matched 0 rows" when run after resetRepoCards() in the same shard. Add projectinfo upsert after repocard restoration, matching seed.sql values exactly. Card5 intentionally has no projectinfo per seed design.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReplaces theme selection UI on Settings with a sidebar ThemeToggle; Settings now only exposes display preferences. E2E tests updated to select themes via the sidebar and restore projectinfo seed rows after repo card resets. Command Palette theme command removed. CLAUDE.md recommends running E2E in parallel first. Changes
Sequence DiagramsequenceDiagram
participant User
participant Sidebar
participant ThemeToggle
participant Settings
rect rgba(200, 150, 255, 0.5)
Note over User,Settings: Previous Flow (removed)
User->>Settings: Navigate to /settings
Settings->>User: Render theme options
User->>Settings: Select theme
Settings->>Settings: Apply & persist theme
end
rect rgba(100, 200, 255, 0.5)
Note over User,Sidebar: New Flow (current)
User->>Sidebar: Open sidebar
Sidebar->>ThemeToggle: Reveal theme control
User->>ThemeToggle: Select theme from dropdown
ThemeToggle->>ThemeToggle: Apply & persist theme (localStorage)
ThemeToggle->>User: UI reflects applied theme
end
rect rgba(150, 200, 100, 0.5)
Note over User,Settings: Settings Page (simplified)
User->>Settings: Navigate to /settings
Settings->>User: Display only display settings (compact, metadata)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
🤖 Morph Preview Test⚡ Looks like you hit your rate limits! Please upgrade your limits here, or wait a few minutes and try again. If you need help, reach out to support@morphllm.com. Automated testing by Morph |
There was a problem hiding this comment.
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 (2)
src/app/settings/SettingsClient.stories.tsx (1)
26-28:⚠️ Potential issue | 🟡 MinorStale doc comment still references theme selection.
Line 27 says "theme selection and display preferences" but lines 5–6 explicitly state theme is handled via sidebar. Update to match.
Proposed fix
-/** Default settings page showing theme selection and display preferences */ +/** Default settings page showing display preferences */e2e/logged-in/settings.spec.ts (1)
14-16:⚠️ Potential issue | 🟡 MinorMissing
networkidlewait afterpage.goto.All
page.goto('/settings')calls in this file lackwaitForLoadState('networkidle'). The other theme E2E files consistently call it. As per coding guidelines: "Usenetworkidlewait strategy for page loads."Example fix for the first test
await page.goto('/settings') + await page.waitForLoadState('networkidle')
🤖 Fix all issues with AI agents
In `@e2e/logged-in/theme-visual-application.spec.ts`:
- Around line 51-56: The test currently waits for a possibly stale dropdown
(existingMenu locator) to auto-close which can flake; instead, actively dismiss
it before proceeding by sending a keyboard Escape (e.g.,
page.keyboard.press('Escape')) or clicking outside until
existingMenu.isVisible() returns false; update the block that checks
existingMenu to catch errors, press Escape (or click outside) and loop a couple
times with short waits to ensure existingMenu is not visible before continuing,
using the existingMenu locator name to find and verify closure.
🧹 Nitpick comments (3)
src/app/settings/SettingsClient.tsx (2)
123-132: Hydration placeholder doesn't match theloading.tsxskeleton.The inline fallback uses a generic pulse div, while
loading.tsxrenders a structured skeleton with header + card + toggle rows. Consider reusing or aligning with the loading skeleton for visual consistency during mount.
48-79: Replace custom Toggle with shadcn Switch.The custom Toggle component duplicates the functionality of
Switchfrom@/components/ui. Both acceptcheckedandonCheckedChangewith identical interfaces. The shadcn Switch provides better accessibility through the Radix UI primitive, including proper focus-visible ring styling and disabled state handling.- import Toggle from custom + import { Switch } from '@/components/ui/switch' - <Toggle checked={...} onCheckedChange={...} /> + <Switch checked={...} onCheckedChange={...} />e2e/logged-in/theme-persistence.spec.ts (1)
16-29: ExtractselectThemeFromSidebarto shared helper.This function is duplicated in
theme-persistence.spec.tsandtheme-visual-application.spec.tswith divergent implementations. The version intheme-visual-application.spec.tsincludes defensive dropdown-close logic (lines 51–55) and post-selection wait (lines 65–66) that prevents flakiness;theme-persistence.spec.tslacks these guards. Extract toe2e/helpers/theme.tsso both files use the same robust implementation and stay in sync.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
- Coverage 72.28% 72.21% -0.08%
==========================================
Files 143 143
Lines 4236 4214 -22
Branches 1135 1097 -38
==========================================
- Hits 3062 3043 -19
+ Misses 1153 1150 -3
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🧪 E2E Coverage Report (Sharded: 12 parallel jobs)
📊 Full report available in workflow artifacts |
Press Escape to deterministically close any open dropdown menu rather than passively waiting for it to auto-close, preventing flaky test failures in theme visual application E2E tests.
🤖 Morph Preview Test⚡ Looks like you hit your rate limits! Please upgrade your limits here, or wait a few minutes and try again. If you need help, reach out to support@morphllm.com. Automated testing by Morph |
Summary
resetRepoCards()to restore projectinfo rows that are silently deleted by PostgreSQL'sON DELETE CASCADEwhen repocards are deletedRoot Cause
Test plan
pnpm typecheck— cleanpnpm lint— 0 warningspnpm build— successpnpm test— 1282 passedpnpm e2efull suite — 318 passed, 3 failed (pre-existingadd-repository-paginationflakiness, unrelated)Summary by CodeRabbit
Changes
Documentation
Tests