feat(api): add default note ("マイノート") foundation#821
Conversation
各ユーザーに 1 件のデフォルトノート (`<users.name>のノート`) を持たせる
土台を追加する。これは「ホーム廃止 → /notes/me 着地」UX 再設計の Phase 1a で、
既存の個人ページや note_pages テーブルには触れない(後続の Phase 1b で
個人ページをデフォルトノートに移行し、note_pages を廃止する予定)。
Adds the foundation for the default-note ("マイノート") model where every user
owns exactly one note titled `<users.name>のノート`. This is Phase 1a of the
"/home → /notes/me" UX refactor; existing personal pages and the note_pages
link table are intentionally left untouched and will be migrated in a
follow-up.
主な変更点 / Highlights:
- `notes.is_default` カラムを追加し、partial unique index で 1 ユーザー
1 件を担保 (`server/api/drizzle/0020_add_default_note.sql`)
- 既存ユーザー全員にデフォルトノートをバックフィル
- `defaultNoteService` で `ensureDefaultNote` / `getDefaultNoteIdOrNull` /
`formatDefaultNoteTitle` を提供(idempotent、並行安全)
- `GET /api/notes/me` 新設:呼び出し元のデフォルトノートを返す(未作成なら
自動作成)
- `DELETE /api/notes/:id` でデフォルトノートを 400 で拒否
- `NoteApiFields.is_default` を露出してフロントから識別可能に
- `welcomePageService.insertWelcomePage` がウェルカムページをデフォルト
ノートに所属させるよう更新(新規ユーザー向け)
- 単体テストとルートテストを追加(defaultNoteService、/api/notes/me、
デフォルトノート削除拒否)
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements a per-user default note by adding an ChangesDefault Note Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a 'default note' model, where each user has exactly one note marked as 'is_default' (titled '<users.name>のノート'). This replaces the previous 'personal pages' concept, ensuring all pages belong to a note. The changes include a database migration, a new 'defaultNoteService' to handle idempotent note creation, a new '/api/notes/me' endpoint, and logic to prevent the deletion of default notes. The reviewer suggests refactoring 'ensureDefaultNote' to return the full 'Note' object instead of just the ID, which would allow for more efficient data retrieval in routes like 'GET /api/notes/me' by eliminating redundant database queries.
| export async function getDefaultNoteIdOrNull(db: DbOrTx, userId: string): Promise<string | null> { | ||
| const rows = await db | ||
| .select({ id: notes.id }) | ||
| .from(notes) | ||
| .where(and(eq(notes.ownerId, userId), eq(notes.isDefault, true), eq(notes.isDeleted, false))) | ||
| .limit(1); | ||
| return rows[0]?.id ?? null; | ||
| } |
There was a problem hiding this comment.
This helper currently only returns the note ID. To avoid redundant database lookups in calling routes (like GET /api/notes/me), consider refactoring this to return the full Note object. This would allow the service to provide the complete note data in a single step, reducing the total number of queries in the request lifecycle.
| const noteId = await ensureDefaultNote(db, userId); | ||
|
|
||
| const rows = await db.select().from(notes).where(eq(notes.id, noteId)).limit(1); | ||
| const row = rows[0]; |
There was a problem hiding this comment.
There is a redundant database query here. ensureDefaultNote already performs a lookup (and potentially an insert) to ensure the note exists. If ensureDefaultNote is refactored to return the full Note row, this subsequent db.select() call can be removed, improving the efficiency of this frequently used endpoint.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c5f971aea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .insert(pages) | ||
| .values({ | ||
| ownerId: userId, | ||
| noteId, | ||
| title, |
There was a problem hiding this comment.
Insert welcome pages into note_pages when assigning note_id
Now that insertWelcomePage creates a note-native page (note_id is set), it also needs to add a corresponding note_pages row; otherwise the page is effectively invisible in note workflows. Both GET /api/notes/:noteId/pages (server/api/src/routes/notes/pages.ts) and note search (server/api/src/routes/notes/search.ts) read visible pages from note_pages, so newly onboarded users can end up with a default note that appears empty even though the welcome page was created.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
server/api/src/services/defaultNoteService.test.ts (1)
1-22: ⚡ Quick winAdd unit tests for
ensureDefaultNote/getDefaultNoteIdOrNullidempotency paths.This suite currently validates only title formatting, so the service’s core idempotent/concurrency behavior is not unit-pinned here.
As per coding guidelines: Tests serve as a source of truth for specifications alongside implementation code TSDoc/JSDoc.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/services/defaultNoteService.test.ts` around lines 1 - 22, Current test file only covers formatDefaultNoteTitle; add unit tests that exercise ensureDefaultNote and getDefaultNoteIdOrNull idempotency and concurrency paths: write tests calling ensureDefaultNote twice (or concurrently) and asserting the returned/default note id is stable and that getDefaultNoteIdOrNull returns the same id (or null before creation), include cases for concurrent invocations to ensure one created id and others receive identical id, and reference the functions ensureDefaultNote and getDefaultNoteIdOrNull from defaultNoteService.js so the tests assert idempotence and race-safe behavior.server/api/src/__tests__/routes/notes/crud.test.ts (1)
309-323: ⚡ Quick winAssert that default-note delete path does not execute soft-delete mutation.
Nice status/body assertion. To lock behavior, also verify no
updatechain runs when deletion is rejected.Proposed test hardening
- const { app } = createTestApp([ + const { app, chains } = createTestApp([ [defaultNote], // requireNoteOwner → findActiveNoteById (owner) ]); @@ const body = await res.text(); expect(body).toMatch(/default note/i); + expect(chains.some((c) => c.startMethod === "update")).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/__tests__/routes/notes/crud.test.ts` around lines 309 - 323, The test should also assert that no soft-delete update is invoked when deletion is rejected: before calling app.request in the DELETE test for defaultNote, create a Jest spy/mock (e.g., jest.spyOn(prisma.note, "update") or spy on the repository method used by the route such as NoteRepository.update or the function name findActiveNoteById → update chain) and after the request assert the spy was not called; then restore the spy. This ensures the update mutation path (the soft-delete) did not run when the handler returned 400 for defaultNote.server/api/src/services/defaultNoteService.ts (1)
75-85: 💤 Low valueConsider specifying the conflict target for explicitness.
The
onConflictDoNothing()without a target will catch any unique constraint violation. While this works correctly here (the only realistic conflict is the partial unique index), explicitly targeting the index would make the intent clearer and prevent accidental silent failures if new constraints are added later.♻️ Optional: Explicit conflict target
const inserted = await db .insert(notes) .values({ ownerId: userId, title, visibility: "private", editPermission: "owner_only", isDefault: true, }) - .onConflictDoNothing() + .onConflictDoNothing({ + target: notes.ownerId, + where: sql`${notes.isDefault} = true AND ${notes.isDeleted} = false`, + }) .returning({ id: notes.id });Note: You'd need to import
sqlfromdrizzle-ormif adopting this change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/services/defaultNoteService.ts` around lines 75 - 85, The insert currently calls onConflictDoNothing() without a target (in the db.insert(notes).values(...) block that sets isDefault: true and returns inserted), which can silently swallow unrelated unique-constraint violations; change it to explicitly target the partial-unique index that enforces one default note per user by using onConflictDoNothing({ target: sql`...` }) (you'll need to import sql from drizzle-orm) so the conflict is limited to the specific index/columns (the ownerId + isDefault partial index) rather than any unique constraint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/api/src/__tests__/routes/notes/crud.test.ts`:
- Around line 309-323: The test should also assert that no soft-delete update is
invoked when deletion is rejected: before calling app.request in the DELETE test
for defaultNote, create a Jest spy/mock (e.g., jest.spyOn(prisma.note, "update")
or spy on the repository method used by the route such as NoteRepository.update
or the function name findActiveNoteById → update chain) and after the request
assert the spy was not called; then restore the spy. This ensures the update
mutation path (the soft-delete) did not run when the handler returned 400 for
defaultNote.
In `@server/api/src/services/defaultNoteService.test.ts`:
- Around line 1-22: Current test file only covers formatDefaultNoteTitle; add
unit tests that exercise ensureDefaultNote and getDefaultNoteIdOrNull
idempotency and concurrency paths: write tests calling ensureDefaultNote twice
(or concurrently) and asserting the returned/default note id is stable and that
getDefaultNoteIdOrNull returns the same id (or null before creation), include
cases for concurrent invocations to ensure one created id and others receive
identical id, and reference the functions ensureDefaultNote and
getDefaultNoteIdOrNull from defaultNoteService.js so the tests assert
idempotence and race-safe behavior.
In `@server/api/src/services/defaultNoteService.ts`:
- Around line 75-85: The insert currently calls onConflictDoNothing() without a
target (in the db.insert(notes).values(...) block that sets isDefault: true and
returns inserted), which can silently swallow unrelated unique-constraint
violations; change it to explicitly target the partial-unique index that
enforces one default note per user by using onConflictDoNothing({ target:
sql`...` }) (you'll need to import sql from drizzle-orm) so the conflict is
limited to the specific index/columns (the ownerId + isDefault partial index)
rather than any unique constraint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2ff58351-f2ec-4243-b071-400e9795e798
📒 Files selected for processing (16)
server/api/drizzle/0020_add_default_note.sqlserver/api/drizzle/meta/_journal.jsonserver/api/src/__tests__/routes/notes/crud.test.tsserver/api/src/__tests__/routes/notes/me.test.tsserver/api/src/__tests__/routes/notes/setup.tsserver/api/src/lib/welcomePageService.tsserver/api/src/routes/notes/crud.tsserver/api/src/routes/notes/helpers.tsserver/api/src/routes/notes/index.tsserver/api/src/routes/notes/me.tsserver/api/src/routes/notes/types.tsserver/api/src/schema/notes.tsserver/api/src/schema/pages.tsserver/api/src/schema/relations.tsserver/api/src/services/defaultNoteService.test.tsserver/api/src/services/defaultNoteService.ts
…-structure-MUQCh # Conflicts: # server/api/drizzle/meta/_journal.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/api/drizzle/0022_add_default_note.sql`:
- Around line 1-11: Update the stale migration number in the header comments:
replace the two occurrences of the prefix "0020:" (the English header line
starting with "0020: Default note (additive)..." and the Japanese header line
starting with "0020: デフォルトノート(追加のみ)...") with "0022:" so the comment matches the
filename/journal tag `0022_add_default_note`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee17a840-9e1b-4ab2-87e6-b0b27b255679
📒 Files selected for processing (3)
server/api/drizzle/0022_add_default_note.sqlserver/api/drizzle/meta/_journal.jsonserver/api/src/schema/pages.ts
✅ Files skipped from review due to trivial changes (1)
- server/api/src/schema/pages.ts
レビューコメント対応 / Address review comments on PR #821: - Codex P1: ウェルカムページの作成挙動を旧来どおり個人ページ (`note_id = NULL`) に戻す。`note_pages` 行を作らずに `note_id` だけ 埋めると、`GET /api/notes/:id/pages` と note search が `note_pages` 経由でページを引くため、ウェルカムページがデフォルトノート画面でも `/home` でも見えなくなる回帰になる。PR 1b で個人ページ全体をデフォルト ノートへ移行する際に一緒に扱う。 Revert welcome page assignment to default note. Setting `note_id` without also creating a `note_pages` row hid the page from `GET /api/notes/:id/pages`, note search, and the legacy `/home` listing. PR 1b will migrate personal pages — including welcome pages — into the default note in one step. - Gemini medium: `ensureDefaultNote` / `getDefaultNoteIdOrNull` を `Note` 全体を返す形にリファクタ(`getDefaultNoteOrNull`)。これにより `GET /api/notes/me` の重複 SELECT を削除して 1 クエリ少なくする。 Refactor the service to return the full `Note` (rename to `getDefaultNoteOrNull`). `GET /api/notes/me` no longer needs a follow-up SELECT after `ensureDefaultNote`, dropping one query per request. - 関連テスト (me.test.ts) を新しいクエリ計画に合わせて更新。 Updated `me.test.ts` to match the new query plan. https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
CodeRabbit review feedback: マイグレーションファイルを 0020 → 0022 に リネームした際、本文のヘッダコメント (1行目と7行目) の "0020:" を 更新し漏れていた。ファイル名と journal タグに合わせて "0022:" に修正。 Address CodeRabbit review note: the migration was renamed 0020 → 0022 to avoid colliding with develop's `0020_seed_thumbnail_tier_quotas`, but the header comment lines kept the old "0020:" prefix. Sync them with the filename and journal tag (`0022_add_default_note`). https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
3 件の CodeRabbit nitpick (Quick win / Optional) に対応: 1. `defaultNoteService.ts`: `onConflictDoNothing()` に明示的な target + where を指定し、無関係なユニーク制約衝突を黙って飲み込まないように。 partial unique index `idx_notes_unique_default_per_owner` の述語と揃える。 2. `defaultNoteService.test.ts`: タイトル整形のみのテストに加え、 `ensureDefaultNote` と `getDefaultNoteOrNull` の冪等性 / 並行ハンドリング / エラー経路のテストを追加(3 → 10 件)。 3. `crud.test.ts`: デフォルトノート削除拒否テストで、`update` チェーンが 走っていないことも併せて検証。「拒否したのに更新は適用された」を防ぐ 契約として固定。 Address three CodeRabbit nitpick suggestions: 1. `defaultNoteService.ts`: explicitly target the partial unique index in `onConflictDoNothing()` so unrelated unique-constraint violations are not silently swallowed. 2. `defaultNoteService.test.ts`: extend the suite from formatter-only to cover idempotency, race-recovery, 404, and 500 paths of `ensureDefaultNote` / `getDefaultNoteOrNull` (3 → 10 tests). 3. `crud.test.ts`: tighten the default-note delete test to assert that no `update` chain fires when deletion is rejected. https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
…es (#823) (#831) * feat(api): migrate personal pages into default note and drop note_pages (#823) PR 1b: backfill all `pages.note_id IS NULL` rows into each owner's default note, promote `pages.note_id` to NOT NULL, and drop the `note_pages` junction table. After this PR every page belongs to exactly one note via `pages.note_id`, so authorization, search, and sync all flow through the note model uniformly. API: - `GET /api/pages` returns 410 Gone with `Deprecation: true` header. - `POST /api/pages` requires `note_id`; falls back to `ensureDefaultNote`. - `POST /api/notes/:noteId/pages` accepts only the title path; `page_id` linking is rejected with 400. - `PUT /api/notes/:noteId/pages` is now a noop (order is `updated_at DESC`). - `copy-from-personal` / `copy-to-personal` routes are removed. - Note search / global search drop the `note_pages` join and use `pages.note_id` directly. - `assertPageViewAccess` / `assertPageEditAccess` resolve roles solely through `getNoteRole` on `pages.note_id` (no personal-page branch). - `welcomePageService`, `clipAndCreate`, `wikiSchema`, `indexBuilder`, `syncPages`, etc. all set `noteId` from `ensureDefaultNote`. Schema / migration: - New SQL migration `0023_migrate_personal_pages_drop_note_pages.sql` with safety inserts, default-note backfill, NOT NULL promotion, and `DROP TABLE note_pages` (idempotent guards). - `schema/notes.ts` removes `notePages`; `schema/pages.ts` makes `noteId` non-null; `schema/relations.ts` drops `notePagesRelations`. Tests: - Rewrites for `pageAccessService`, route tests for pages / notes pages / search / pageSnapshots / syncPages / media to match the new contract (note-role chains, `HeadObjectCommand` mock, `ensureDefaultNote` mock, removal of `sort_order` / `added_by` fields). Refs: #823 (PR 1b), #821 (PR 1a), #824 (PR 2 meta). Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #831 review (list pages shim, POST note guard, search prefetch) - Restore GET /api/pages as a deprecation shim (200 + Deprecation header) using pages.note_id and the same own/shared access model as search, so MCP listPages keeps working. - POST /api/pages: require getNoteRole + canEdit when the client supplies note_id. - GET /api/search scope=own: resolve default note via getDefaultNoteOrNull instead of a SQL subquery; early-empty when missing. - Drop redundant Array.isArray guards on Drizzle select results in helpers, pageAccessService, pageSnapshots, and syncPages. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #831 CodeRabbit follow-up (migration idempotency, docs, comments) - 0023 migration: append ON CONFLICT DO NOTHING targeting the partial unique index on owner_id for default-note inserts. - notes/search.ts: clarify JSDoc that authRequired applies; guest role is for authenticated callers with note access. - media.test.ts: restore JP/EN paired comments on HeadObject stub and beforeEach. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
各ユーザーに 1 件のデフォルトノート (
<users.name>のノート) を持たせる土台を追加する。これは「ホーム廃止 → /notes/me 着地」UX 再設計の Phase 1a で、
既存の個人ページや note_pages テーブルには触れない(後続の Phase 1b で
個人ページをデフォルトノートに移行し、note_pages を廃止する予定)。
Adds the foundation for the default-note ("マイノート") model where every user
owns exactly one note titled
<users.name>のノート. This is Phase 1a of the"/home → /notes/me" UX refactor; existing personal pages and the note_pages
link table are intentionally left untouched and will be migrated in a
follow-up.
主な変更点 / Highlights:
notes.is_defaultカラムを追加し、partial unique index で 1 ユーザー1 件を担保 (
server/api/drizzle/0020_add_default_note.sql)defaultNoteServiceでensureDefaultNote/getDefaultNoteIdOrNull/formatDefaultNoteTitleを提供(idempotent、並行安全)GET /api/notes/me新設:呼び出し元のデフォルトノートを返す(未作成なら自動作成)
DELETE /api/notes/:idでデフォルトノートを 400 で拒否NoteApiFields.is_defaultを露出してフロントから識別可能にwelcomePageService.insertWelcomePageがウェルカムページをデフォルトノートに所属させるよう更新(新規ユーザー向け)
デフォルトノート削除拒否)
https://claude.ai/code/session_01QjBBYn2DSouRkUUkLPZXxx
Summary by CodeRabbit