feat: inline-editable boards page header with DB persistence#128
Merged
ryota-murakami merged 4 commits intomainfrom Feb 18, 2026
Merged
feat: inline-editable boards page header with DB persistence#128ryota-murakami merged 4 commits intomainfrom
ryota-murakami merged 4 commits intomainfrom
Conversation
Add click-to-edit functionality for the /boards page title and subtitle, persisted in a new user_settings table. Users can customize "My Boards" heading and description, with changes stored per-user via upsert. - Create user_settings table with RLS policies (one row per user) - Add Zod validation schemas (title 50 chars, subtitle 100 chars) - Add server actions with rate limiting for title/subtitle mutations - Create BoardsPageHeader client component using InlineEditableText - Parallel fetch boards + settings in page.tsx via Promise.all - Fix InlineEditableText to use editValue for optimistic display - Add MSW handlers for Storybook, seed data, and E2E helpers - Add 13 E2E test cases covering edit, save, cancel, blur, DB verify
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds per-user boards page header customization: DB table and types, server actions to persist title/subtitle with rate limiting and validation, a client BoardsPageHeader component using InlineEditableText with optimistic display, mocks and E2E tests/helpers, migration/seed updates, and minor tooling changes. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser as Browser (React)
participant Server as Server (Next.js server actions)
participant DB as Database (Supabase)
User->>Browser: Click title/subtitle to edit
Browser->>Browser: Enter edit mode, show input (optimistic editValue)
User->>Browser: Type and press Enter or blur
Browser->>Server: call updateBoardsPageTitle / updateBoardsPageSubtitle
Server->>Server: Validate via schema, convert "" → NULL
Server->>DB: Upsert into user_settings (onConflict: user_id)
DB-->>Server: Success
Server-->>Browser: ActionResult success
Browser->>Browser: Exit edit mode, display saved value
Browser->>User: Render updated header
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
==========================================
+ Coverage 71.40% 71.46% +0.05%
==========================================
Files 144 145 +1
Lines 4330 4335 +5
Branches 1130 1130
==========================================
+ Hits 3092 3098 +6
+ Misses 1217 1216 -1
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/lib/supabase/types.ts (1)
276-299: Consider adding a convenience type alias foruser_settings.Other tables have aliases at the bottom of this file (e.g.,
type Board = Tables<'board'>). AUserSettingsalias would keep things consistent.♻️ Add convenience alias
type UserLinkPreset = Tables<'user_link_presets'> +type UserSettings = Tables<'user_settings'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/supabase/types.ts` around lines 276 - 299, Add a convenience TypeScript alias for the user_settings table similar to other aliases in this file: create a type named UserSettings that equals Tables<'user_settings'> (matching the pattern used for Board/Other aliases). Place it alongside the other table aliases at the bottom of the file so consumers can import UserSettings instead of referencing Tables<'user_settings'> directly.src/supabase/migrations/20260219000000_add_user_settings.sql (1)
6-7: Consider adding CHECK constraints for max length as defense-in-depth.The Zod schemas enforce max 50/100 chars at the app layer, but a DB-level constraint prevents rogue writes (e.g., direct DB access, other clients). Optional but cheap insurance.
🛡️ Add CHECK constraints
- boards_page_title TEXT DEFAULT NULL, - boards_page_subtitle TEXT DEFAULT NULL, + boards_page_title TEXT DEFAULT NULL CHECK (char_length(boards_page_title) <= 50), + boards_page_subtitle TEXT DEFAULT NULL CHECK (char_length(boards_page_subtitle) <= 100),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/supabase/migrations/20260219000000_add_user_settings.sql` around lines 6 - 7, Add DB-level length checks for the two new columns to enforce the same limits as your Zod schemas: update the migration that defines boards_page_title and boards_page_subtitle to include CHECK constraints (e.g., CHECK (char_length(boards_page_title) <= 50) and CHECK (char_length(boards_page_subtitle) <= 100)) or change the column types to varchar(50)/varchar(100). Ensure you reference the exact column names boards_page_title and boards_page_subtitle and give each CHECK a clear name (e.g., chk_user_settings_boards_page_title_len) so the constraint can be managed later.src/lib/actions/user-settings.ts (1)
73-106: Subtitle action mirrors the title action — consistent pattern.If more per-user settings are added down the road, consider extracting a generic
upsertUserSetting(column, schema, value)helper to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/actions/user-settings.ts` around lines 73 - 106, Extract a generic helper upsertUserSetting(column, schema, value) and have updateBoardsPageSubtitle call it: the helper should safeParse the value using the passed schema, convert empty string to null for DB storage, call withAuthResultRateLimit and perform supabase.from('user_settings').upsert({ user_id: user.id, [column]: dbValue, updated_at: new Date().toISOString() }, { onConflict: 'user_id' }), capture and Sentry.captureException any supabase error with context including column and user.id and rethrow a friendly Error, and return the parsed value mapped back to the original shape; then replace the body of updateBoardsPageSubtitle to delegate validation and upsert work to upsertUserSetting('boards_page_subtitle', boardsPageSubtitleSchema, subtitle) and return { subtitle: parsedValue } as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mocks/handlers/supabase.ts`:
- Around line 616-619: The GET handler registered via http.get for
`${SUPABASE_URL}/rest/v1/user_settings` ignores the request Accept header (so
.maybeSingle() behavior is not honored); update the handler inside the http.get
callback to read the incoming request (e.g., destructure { request } or use the
handler args) and inspect request.headers.get('accept') for the maybeSingle
pattern, returning HttpResponse.json(mockUserSettings) when a normal Accept is
present and HttpResponse.json(null) (or an empty result) when the Accept
indicates maybeSingle; keep the same route and variable names (http.get,
SUPABASE_URL, mockUserSettings, HttpResponse.json, maybeSingle) so tests using
.maybeSingle() behave correctly.
- Around line 626-644: The POST handler is mutating the shared mockUserSettings
object directly causing test pollution; add an INITIAL_MOCK_USER_SETTINGS
constant (copy of mockUserSettings initial state) and update resetMockData() to
reassign mockUserSettings = deepClone(INITIAL_MOCK_USER_SETTINGS) so each
test/story starts with a fresh object, and in the POST handler (the http.post
handler in mocks/handlers/supabase.ts) update the in-memory value via that
shared variable rather than mutating an imported constant; ensure deep cloning
is used to avoid reference leakage (refer to mockUserSettings,
INITIAL_MOCK_BOARDS, and resetMockData in mocks/handlers/data.ts).
---
Duplicate comments:
In `@mocks/handlers/data.ts`:
- Around line 622-628: Replace the immutable export of mockUserSettings with a
mutable, resettable pattern: create an INITIAL_MOCK_USER_SETTINGS constant
mirroring the current object, change export const mockUserSettings to export let
mockUserSettings = { ...INITIAL_MOCK_USER_SETTINGS }, and update/reset logic in
resetMockData() to reassign mockUserSettings = { ...INITIAL_MOCK_USER_SETTINGS }
so tests can mutate and be reset (follow the same approach used by mockBoards).
---
Nitpick comments:
In `@src/lib/actions/user-settings.ts`:
- Around line 73-106: Extract a generic helper upsertUserSetting(column, schema,
value) and have updateBoardsPageSubtitle call it: the helper should safeParse
the value using the passed schema, convert empty string to null for DB storage,
call withAuthResultRateLimit and perform supabase.from('user_settings').upsert({
user_id: user.id, [column]: dbValue, updated_at: new Date().toISOString() }, {
onConflict: 'user_id' }), capture and Sentry.captureException any supabase error
with context including column and user.id and rethrow a friendly Error, and
return the parsed value mapped back to the original shape; then replace the body
of updateBoardsPageSubtitle to delegate validation and upsert work to
upsertUserSetting('boards_page_subtitle', boardsPageSubtitleSchema, subtitle)
and return { subtitle: parsedValue } as before.
In `@src/lib/supabase/types.ts`:
- Around line 276-299: Add a convenience TypeScript alias for the user_settings
table similar to other aliases in this file: create a type named UserSettings
that equals Tables<'user_settings'> (matching the pattern used for Board/Other
aliases). Place it alongside the other table aliases at the bottom of the file
so consumers can import UserSettings instead of referencing
Tables<'user_settings'> directly.
In `@src/supabase/migrations/20260219000000_add_user_settings.sql`:
- Around line 6-7: Add DB-level length checks for the two new columns to enforce
the same limits as your Zod schemas: update the migration that defines
boards_page_title and boards_page_subtitle to include CHECK constraints (e.g.,
CHECK (char_length(boards_page_title) <= 50) and CHECK
(char_length(boards_page_subtitle) <= 100)) or change the column types to
varchar(50)/varchar(100). Ensure you reference the exact column names
boards_page_title and boards_page_subtitle and give each CHECK a clear name
(e.g., chk_user_settings_boards_page_title_len) so the constraint can be managed
later.
🧪 E2E Coverage Report (Sharded: 12 parallel jobs)
📊 Full report available in workflow artifacts |
- GET handler now respects Accept header for .maybeSingle() behavior - Add INITIAL_MOCK_USER_SETTINGS for reset between tests - Include mockUserSettings reset in resetMockData()
🤖 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.
🧹 Nitpick comments (1)
mocks/handlers/supabase.ts (1)
616-623: Optional: GET mock can't simulate the "row doesn't exist" path for.maybeSingle().
mockUserSettingsis always initialised, so.maybeSingle()always returns an object — nevernull. The production code path where nouser_settingsrow exists yet (first-ever load before any save) is untestable with this mock. Currently harmless becauseboards_page_title: null+boards_page_subtitle: nullinINITIAL_MOCK_USER_SETTINGSproduces the same rendered defaults as anullrow. Worth noting if "row absent vs row with nulls" logic diverges later.♻️ Approach to support the null path when needed
Add a toggle in
data.ts:+export let mockUserSettingsExists = true export let mockUserSettings = { ...INITIAL_MOCK_USER_SETTINGS } export function resetMockData(): void { mockBoards = INITIAL_MOCK_BOARDS.map((b) => ({ ...b })) mockUserSettings = { ...INITIAL_MOCK_USER_SETTINGS } + mockUserSettingsExists = true }Then in the GET handler:
- if (acceptHeader.includes('application/vnd.pgrst.object+json')) { - return HttpResponse.json(mockUserSettings) - } - return HttpResponse.json([mockUserSettings]) + if (acceptHeader.includes('application/vnd.pgrst.object+json')) { + return HttpResponse.json(mockUserSettingsExists ? mockUserSettings : null) + } + return HttpResponse.json(mockUserSettingsExists ? [mockUserSettings] : [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mocks/handlers/supabase.ts` around lines 616 - 623, The GET handler for `${SUPABASE_URL}/rest/v1/user_settings` always returns mockUserSettings so `.maybeSingle()` can never produce null; add a toggle (e.g., export let returnNullUserSettings = false) in the test data module (data.ts) and update the http.get handler that checks Accept for 'application/vnd.pgrst.object+json' to return HttpResponse.json(null) when that toggle is true (otherwise return mockUserSettings as before), ensuring symbols to edit are the exported toggle in data.ts and the existing mockUserSettings / http.get handler that services maybeSingle requests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@mocks/handlers/supabase.ts`:
- Around line 630-648: No code change required: confirm that resetMockData
reassigns the exported let mockUserSettings to INITIAL_MOCK_USER_SETTINGS and
keep the POST handler for /rest/v1/user_settings (the anonymous http.post
handler) that conditionally updates mockUserSettings.boards_page_title,
mockUserSettings.boards_page_subtitle, sets mockUserSettings.updated_at, and
returns the representation when request.headers.get('Prefer') includes
'return=representation'; if resetMockData does not reassign mockUserSettings,
change it to reassign the variable rather than mutating its properties.
- Around line 616-623: The GET handler for
`${SUPABASE_URL}/rest/v1/user_settings` already correctly honors the Accept
header by checking for 'application/vnd.pgrst.object+json' (see the http.get
callback using request.headers.get('Accept') and the acceptHeader branch that
returns HttpResponse.json(mockUserSettings) vs
HttpResponse.json([mockUserSettings])); no code change is required—leave the
existing acceptHeader logic and response branches as-is.
---
Nitpick comments:
In `@mocks/handlers/supabase.ts`:
- Around line 616-623: The GET handler for
`${SUPABASE_URL}/rest/v1/user_settings` always returns mockUserSettings so
`.maybeSingle()` can never produce null; add a toggle (e.g., export let
returnNullUserSettings = false) in the test data module (data.ts) and update the
http.get handler that checks Accept for 'application/vnd.pgrst.object+json' to
return HttpResponse.json(null) when that toggle is true (otherwise return
mockUserSettings as before), ensuring symbols to edit are the exported toggle in
data.ts and the existing mockUserSettings / http.get handler that services
maybeSingle requests.
- Increase slash command menu visibility timeout to 10s - Add 300ms stabilization delay after Slate editor clear operations - Add pre-slash verification step (type/verify/clear) to ensure editor is accepting input - Increase heading autoformat assertion timeout from 5s to 10s
🤖 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.
🧹 Nitpick comments (1)
e2e/logged-in/note-modal.spec.ts (1)
263-268: Optional: add a guard after the second clear before typing/.After clearing
xat lines 263–264, there's no verification the editor is actually empty before/is typed. The 300 ms fixed wait is best-effort; on an extremely slow runner, residualxcould be selected by the slash-command logic instead of a clean empty-editor state. The existingtoPasspattern (lines 248–252) already works well here — applying the same after the second clear makes this fully deterministic.♻️ Proposed fix
await page.keyboard.press(`${MOD}+a`) await page.keyboard.press('Backspace') + // Poll until 'x' is gone, matching the same pattern used above + await expect(async () => { + const text = await editorContent.textContent() + expect(text?.replace(/\s/g, '')).toBe('') + }).toPass({ timeout: 3000 }) // eslint-disable-next-line playwright/no-wait-for-timeout await page.waitForTimeout(300)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/logged-in/note-modal.spec.ts` around lines 263 - 268, After the second clear (the sequence using await page.keyboard.press(`${MOD}+a`) and await page.keyboard.press('Backspace') followed by waitForTimeout) add the same deterministic guard used earlier (the existing toPass pattern) to assert the editor is actually empty before calling await page.keyboard.type('/'); in other words, wait-for-or-poll the editor content to be empty (using the same locator/check used in the earlier toPass block) and only then proceed to type '/' so the test cannot race on slow runners.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e/logged-in/note-modal.spec.ts`:
- Around line 263-268: After the second clear (the sequence using await
page.keyboard.press(`${MOD}+a`) and await page.keyboard.press('Backspace')
followed by waitForTimeout) add the same deterministic guard used earlier (the
existing toPass pattern) to assert the editor is actually empty before calling
await page.keyboard.type('/'); in other words, wait-for-or-poll the editor
content to be empty (using the same locator/check used in the earlier toPass
block) and only then proceed to type '/' so the test cannot race on slow
runners.
Slate/Plate editor contenteditable DOM mutations are significantly slower on GitHub Actions Linux runners. Mark all editor-intensive tests with test.slow() to triple their timeout and increase stabilization delay to 1000ms for slash command test.
🤖 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/boardspage title ("My Boards") and subtitle, persisted in a newuser_settingstableuser_settingsDB table with RLS policies (one row per user, upsert on first edit)InlineEditableTextto useeditValuestate for optimistic display after save (was using stale prop)BOARDS_PAGE_DEFAULTSfrom'use server'module to shared validation file (non-function exports are inaccessible from client components)Test plan
pnpm typecheck— cleanpnpm lint— cleanpnpm test— 1286/1286 passedpnpm build— successpnpm e2e:parallel— 11/12 shards (1 pre-existing flaky in NoteModal)boards-page-header-inline-edit.spec.ts— all passSummary by CodeRabbit
New Features
Tests