feat(api): migrate personal pages into default note and drop note_pages (#823)#831
Conversation
…es (#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>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR migrates all personal pages (previously ChangesIssue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a single membership model for pages by migrating legacy personal pages into users' default notes, promoting pages.note_id to NOT NULL, and removing the note_pages link table. The changes include a database migration, the deprecation of the general pages listing endpoint in favor of note-scoped routes, and simplified authorization logic that relies on note roles. Review feedback identifies opportunities to optimize search queries by pre-fetching default note IDs instead of using SQL subqueries and suggests removing redundant array type checks on Drizzle ORM query results.
| AND p.note_id = ( | ||
| SELECT n.id FROM notes n | ||
| WHERE n.owner_id = ${userId} | ||
| AND n.is_default = true | ||
| AND n.is_deleted = false | ||
| LIMIT 1 | ||
| ) |
There was a problem hiding this comment.
scope=own の検索において、note_id を特定するために SQL 内でサブクエリを実行していますが、ハンドラーの冒頭で getDefaultNoteOrNull を使用して ID を取得し、それをパラメータとして渡す方が効率的でコードも読みやすくなります。また、他のエンドポイント（syncPages.ts など）と同様に ensureDefaultNote を使用して、未作成の場合に作成を保証するアプローチも検討に値します（ただし GET リクエストでの副作用を許容する場合）。
There was a problem hiding this comment.
対応しました。scope=own ではハンドラ冒頭で getDefaultNoteOrNull を呼び、既定ノートの id を SQL にバインドする形に変更しています。GET での副作用を避けるため ensureDefaultNote は使わず、既定ノート未作成時は空配列で返します。
| .limit(1); | ||
|
|
||
| const firstMember = member[0]; | ||
| const firstMember = (Array.isArray(member) ? member : [])[0]; |
There was a problem hiding this comment.
Drizzle ORM の select() は常に配列を返すため、Array.isArray(member) によるチェックは冗長です。他の箇所（pageSnapshots.ts や syncPages.ts など）でも同様のチェックが追加されていますが、型定義とライブラリの挙動を信頼して簡略化することを検討してください。
| const firstMember = (Array.isArray(member) ? member : [])[0]; | |
| const firstMember = member[0]; |
There was a problem hiding this comment.
ご指摘どおり、Drizzle の select 戻りは常に配列として扱う前提に戻し、Array.isArray のガードを helpers / pageAccessService / pageSnapshots / syncPages から削除しました。
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fa80538c8
ℹ️ 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".
| app.get("/", authRequired, async (c) => { | ||
| const userId = c.get("userId"); | ||
| const db = c.get("db"); | ||
|
|
||
| // クエリパラメータは整数として明示的にパースする。`Number("abc")` だと NaN が SQL に渡るため。 | ||
| // Parse query params as integers — `Number("abc")` would propagate NaN into SQL. | ||
| const limit = Math.min(Math.max(parseInt(c.req.query("limit") ?? "20", 10) || 20, 1), 100); | ||
| const offset = Math.max(parseInt(c.req.query("offset") ?? "0", 10) || 0, 0); | ||
| const scope = c.req.query("scope") === "shared" ? "shared" : "own"; | ||
|
|
||
| // アクセス制御だけを変数化して SELECT 文の重複を避ける。 | ||
| // `shared` は `services/pageAccessService.ts` と同じ正規の認可モデルを採用: | ||
| // - notes が未削除であること | ||
| // - note_members.status = 'accepted' (招待を受諾済み) であること | ||
| // - note_members / note_pages が未削除であること | ||
| // 大規模データセットでもプランナーが効きやすい EXISTS + JOIN を使う。 | ||
| // Vary only the access predicate to avoid duplicating the SELECT. | ||
| // `shared` mirrors the canonical authorization model from `services/pageAccessService.ts`: | ||
| // the linked note must be active, the membership must be accepted, and the join rows | ||
| // must not be soft-deleted. EXISTS + JOIN keeps the planner happy on large datasets. | ||
| // `own` スコープは個人ページ（`pages.note_id IS NULL`）のみを返す。 | ||
| // ノートネイティブページ（issue #713）は、ノート画面または `scope=shared` | ||
| // 経由でのみアクセスする。`shared` 経由の場合は (a) note_members 経由の | ||
| // メンバーシップ、または (b) `note_pages -> notes.owner_id = userId` 経由の | ||
| // オーナーシップで含まれる。オーナー経路を note-native page だけに限定すると、 | ||
| // linked personal page が listing から消えて `assertPageViewAccess` と非対称になる。 | ||
| // `getNoteRole` の解決順 (owner → member → ...) と listing predicate を揃える。 | ||
| // | ||
| // The `own` scope returns personal pages only (`pages.note_id IS NULL`). | ||
| // Note-native pages (issue #713) are accessed via the note view or | ||
| // `scope=shared`. `shared` includes them either through (a) `note_members` | ||
| // membership or (b) note ownership reached through `note_pages`. That owner | ||
| // branch must cover linked personal pages too; otherwise owners could open | ||
| // them via `assertPageViewAccess` while the listing hides them. | ||
| const accessFilter = | ||
| scope === "shared" | ||
| ? sql`( | ||
| (p.owner_id = ${userId} AND p.note_id IS NULL) | ||
| OR EXISTS ( | ||
| SELECT 1 FROM note_pages np | ||
| JOIN notes n ON n.id = np.note_id | ||
| JOIN note_members nm ON nm.note_id = np.note_id | ||
| JOIN "user" u ON u.email = nm.member_email | ||
| WHERE np.page_id = p.id | ||
| AND u.id = ${userId} | ||
| AND nm.status = 'accepted' | ||
| AND nm.is_deleted = false | ||
| AND np.is_deleted = false | ||
| AND n.is_deleted = false | ||
| ) | ||
| OR EXISTS ( | ||
| SELECT 1 FROM note_pages np | ||
| JOIN notes n ON n.id = np.note_id | ||
| WHERE np.page_id = p.id | ||
| AND np.is_deleted = false | ||
| AND n.owner_id = ${userId} | ||
| AND n.is_deleted = false | ||
| ) | ||
| )` | ||
| : sql`p.owner_id = ${userId} AND p.note_id IS NULL`; | ||
|
|
||
| // Wiki の内部システムページ（`special_kind` が `__index__` / `__log__`、 | ||
| // および `is_schema = true` のスキーマページ）は通常一覧から除外する。 | ||
| // クライアントがそれらを編集するための専用 UI が別にあるため、`/api/pages` | ||
| // で返すと NotFound 化したり、ヘッダ付きカードの中に編集不能な行が混ざる。 | ||
| // include_special=true を指定したクライアントのみオプトインで取得できる。 | ||
| // Hide internal/system pages (special_kind set or is_schema=true) from the | ||
| // generic listing; clients that need them can opt in with include_special=true. | ||
| const includeSpecial = c.req.query("include_special") === "true"; | ||
| const specialKindFilter = includeSpecial | ||
| ? sql`TRUE` | ||
| : sql`p.special_kind IS NULL AND p.is_schema = false`; | ||
|
|
||
| // `note_id` を返すことで、`scope=shared` で混在 listing を受け取った | ||
| // クライアントが個人ページ（`note_id IS NULL`）とノートネイティブページを | ||
| // 区別できる。MCP の `zedi_list_pages` ツールはこれに依存している。 | ||
| // Surface `note_id` so callers receiving mixed `scope=shared` results (e.g. | ||
| // the `zedi_list_pages` MCP tool) can distinguish personal vs note-native. | ||
| const result = await db.execute(sql` | ||
| SELECT p.id, p.title, p.content_preview, p.updated_at, p.note_id | ||
| FROM pages p | ||
| WHERE p.is_deleted = false | ||
| AND ${specialKindFilter} | ||
| AND ${accessFilter} | ||
| ORDER BY p.updated_at DESC | ||
| LIMIT ${limit} | ||
| OFFSET ${offset} | ||
| `); | ||
|
|
||
| return c.json({ pages: result.rows }); | ||
| c.header("Deprecation", "true"); | ||
| return c.json( | ||
| { | ||
| message: | ||
| "GET /api/pages is gone (issue #823). Use GET /api/notes/me then GET /api/notes/:noteId/pages.", |
There was a problem hiding this comment.
Returning 410 for GET /api/pages here breaks in-repo consumers that still rely on this contract, such as MCP listPages() in server/mcp/src/client/httpClient.ts (it still calls /api/pages and expects a { pages: [...] } payload). Because this commit does not migrate those callers in the same change, zedi_list_pages and other legacy clients will start failing as soon as this API change is deployed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ご指摘のとおり 410 だけだと MCP の listPages が壊れるため、GET /api/pages を 200 の後方互換シムとして復元しました (Deprecation: true ヘッダ付き)。応答形は { pages } のまま、フィルタは Issue #823 の pages.note_id モデルに合わせています。
| let resolvedNoteId = | ||
| typeof body.note_id === "string" && body.note_id.trim() !== "" ? body.note_id.trim() : null; | ||
| if (!resolvedNoteId) { | ||
| const defaultNote = await ensureDefaultNote(db, userId); | ||
| resolvedNoteId = defaultNote.id; |
There was a problem hiding this comment.
This accepts any non-empty note_id from the request and uses it as-is, but never checks whether the caller can access or edit that note. A user can therefore create pages under another user's note by supplying its id (or trigger unhandled FK failures for invalid ids), which is an authorization and data-integrity regression; this path should validate note permissions before insert.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
POST /api/pages でリクエストが note_id を明示した場合は、getNoteRole と canEdit で編集権を検証してから insert するようにしました。note_id 省略時は従来どおり ensureDefaultNote によるマイノート解決のみです。
… 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>
|
レビューコメントへの対応をコミットしました（6cedb500）。GET /api/search の scope=own 最適化、GET /api/pages の後方互換シム、POST /api/pages の note_id 認可、Array.isArray 整理を含みます。再レビューをお願いします。 @coderabbitai review |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
server/api/src/routes/search.ts (1)
71-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExclude schema/special pages from generic search.
This query now matches every non-deleted page in the note. In the same PR,
wikiSchemaand__index__creation paths started attaching those system pages to the owner's default note, so/api/searchwill now surface them in normal results, including to collaborators on a shared default note. If those rows are meant to stay behind their dedicated APIs, filter them out here too.🔧 Suggested fix
WHERE p.is_deleted = false + AND p.is_schema = false + AND p.special_kind IS NULL AND ( EXISTS ( SELECT 1 FROM notes n @@ WHERE p.is_deleted = false + AND p.is_schema = false + AND p.special_kind IS NULL AND p.note_id = ${defaultNote.id} AND ( p.title ILIKE ${pattern}Also applies to: 102-118
🤖 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/routes/search.ts` around lines 71 - 99, The search query in db.execute(sql`...`) currently returns system/schema pages; update the WHERE clause for the pages table (alias p) to explicitly exclude schema/special pages by adding predicates such as p.is_schema = false (or p.schema IS NULL / NOT IN ('wikiSchema', ...)) and exclude known special paths (e.g. p.path != '__index__'), so these rows are filtered out from generic search results; apply the same predicate changes to the other search query block referenced around lines 102-118 to keep behavior consistent.server/api/src/services/titleRenamePropagationService.ts (1)
222-235:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSkip deleted source pages during ghost-link promotion.
The new note-scoped candidate query joins
pagesbut never filters out soft-deleted sources. A rename can therefore promote stale ghost links from deleted pages back into reallinksrows.🔧 Suggested fix
.where( and( sql`LOWER(TRIM(${ghostLinks.linkText})) = LOWER(TRIM(${newTitle}))`, ne(ghostLinks.sourcePageId, renamedPageId), + eq(pages.isDeleted, false), eq(pages.noteId, scope.noteId), ), );🤖 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/titleRenamePropagationService.ts` around lines 222 - 235, The candidates query can promote ghost links from soft-deleted source pages because it joins pages but doesn't exclude deleted rows; update the where clause for the query that builds candidates (the select from ghostLinks joined with pages using ghostLinks.sourcePageId) to add a condition excluding soft-deleted pages — e.g. require the pages deletion marker to be null (pages.deletedAt IS NULL or pages.deleted_at IS NULL / pages.isDeleted = false depending on your schema) so that ghostLinks from deleted source pages are skipped when promoting to links.
🧹 Nitpick comments (4)
server/api/src/routes/notes/crud.ts (1)
338-339: ⚡ Quick winAdd the English half of this new inline doc.
This new
note_idcomment is Japanese-only right now. Please add the matching English line or remove the comment if it is not needed.As per coding guidelines "Comments and documentation should include both Japanese and English".
🤖 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/routes/notes/crud.ts` around lines 338 - 339, The inline comment above the object property note_id (value p.noteId) is currently Japanese-only; add the matching English comment line (e.g., "Belonging note ID (from Issue `#823` onward this note ID is always used).") directly above or beside the existing Japanese comment so both languages are present, or remove the comment entirely if unnecessary; update the comment near the note_id: p.noteId property to comply with the bilingual guideline.server/api/src/__tests__/routes/syncPages.test.ts (1)
20-35: ⚡ Quick winMake the new test comments bilingual.
The new block comment at Line 20 and the helper doc at Line 44 only carry one language each. Please add the missing English/Japanese half or drop the comments so this file stays consistent with the repo’s documentation convention.
As per coding guidelines "Comments and documentation should include both Japanese and English".
Also applies to: 44-53
🤖 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/syncPages.test.ts` around lines 20 - 35, The inline comment above the mock for ensureDefaultNote and the helper doc later in syncPages.test.ts are only in one language; update those comments to include both Japanese and English (or remove them) to match the repository convention. Locate the comment near the vi.mock for ensureDefaultNote and the helper doc block that documents test behavior, and either add the corresponding English/Japanese translation so both languages are present or delete the comment blocks entirely so the file stays consistent with the bilingual comment guideline.server/api/src/schema/pages.ts (1)
111-116: Consider indexing the new live-note read path.A lot of the PR now reads
pageswithWHERE note_id = ? AND is_deleted = false ORDER BY updated_at DESC(for example note detail and search).idx_pages_note_idhelps the filter, but a partial/composite index such as(note_id, updated_at DESC) WHERE is_deleted = falsewould match the post-note_pageshot path much more closely.🤖 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/schema/pages.ts` around lines 111 - 116, The existing index "idx_pages_note_id" on table.noteId should be replaced or supplemented with a composite/partial index tailored to the hot read path that queries pages WHERE note_id = ? AND is_deleted = false ORDER BY updated_at DESC; update the schema in pages.ts to create a composite index covering (note_id, updated_at DESC) with a partial filter is_deleted = false (or an equivalent DB-specific partial index expression) so the queries on pages (note_id, is_deleted, updated_at) use the index for both filtering and ordering.server/api/src/routes/syncPages.ts (1)
205-223: ⚡ Quick winKeep
ExistingRow.noteIdnon-null to match the migrated schema.
pages.noteIdis now mandatory, but this local type still allowsnull. That weakens the new invariant in exactly the route that is enforcing the default-note model, and it lets stale fixtures/branches compile unnoticed.♻️ Proposed fix
type ExistingRow = { id: string; ownerId: string; - noteId: string | null; + noteId: string; updatedAt: Date; };As per coding guidelines "TypeScript strict mode;
anyis forbidden, explicitly declare types".🤖 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/routes/syncPages.ts` around lines 205 - 223, ExistingRow.noteId is declared nullable but pages.noteId is now non-nullable; update the local type and related handling so the invariant matches the migrated schema. Change ExistingRow.noteId from string | null to string, ensure any code that consumes existingRows (e.g., logic after existingRaw assignment in syncPages.ts) no longer expects null and remove/null-guard branches if present, and fix any fixtures/tests that produced null noteId values so existingRaw assignment (which selects pages.noteId) always maps to the non-null string type.
🤖 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/0023_migrate_personal_pages_drop_note_pages.sql`:
- Around line 16-29: The INSERT into "notes" that creates a user's default note
should be made idempotent by adding explicit conflict handling: modify the
INSERT INTO "notes" (the statement that selects u."id", COALESCE(u."name", '')
|| 'のノート', ... and tests NOT EXISTS for n."is_default") to include an ON
CONFLICT ... DO NOTHING clause (targeting the unique constraint that would
conflict for a user's default note—e.g., the unique/index on owner_id +
is_default or the notes primary key) so reruns or races won't error; update the
statement at the statement-breakpoint accordingly.
In `@server/api/src/__tests__/routes/media.test.ts`:
- Line 50: The test file media.test.ts contains new English-only inline comments
(e.g., the "/* stub — ownership probe on /confirm */" comment around the
/confirm test and another at line 93); update those comments to include Japanese
equivalents alongside the English so they follow the existing JP/EN
paired-comment convention used throughout the file—locate the English-only
comment text and add a Japanese translation immediately before or after it
(matching nearby comment formatting and style).
In `@server/api/src/routes/notes/search.ts`:
- Around line 6-14: The route is wrapped with authRequired which blocks
unauthenticated callers, so either update the route wrapper to authOptional to
allow resolving a guest role via getNoteRole (change the middleware from
authRequired to authOptional on the search route) or, if you want to keep
requiring authentication, tighten the doc comment to remove “guest” as an
allowed resolver; locate the search route that currently uses authRequired and
either replace that middleware with authOptional or update the comment block
above (and any similar occurrences) to state that only authenticated roles are
supported.
---
Outside diff comments:
In `@server/api/src/routes/search.ts`:
- Around line 71-99: The search query in db.execute(sql`...`) currently returns
system/schema pages; update the WHERE clause for the pages table (alias p) to
explicitly exclude schema/special pages by adding predicates such as p.is_schema
= false (or p.schema IS NULL / NOT IN ('wikiSchema', ...)) and exclude known
special paths (e.g. p.path != '__index__'), so these rows are filtered out from
generic search results; apply the same predicate changes to the other search
query block referenced around lines 102-118 to keep behavior consistent.
In `@server/api/src/services/titleRenamePropagationService.ts`:
- Around line 222-235: The candidates query can promote ghost links from
soft-deleted source pages because it joins pages but doesn't exclude deleted
rows; update the where clause for the query that builds candidates (the select
from ghostLinks joined with pages using ghostLinks.sourcePageId) to add a
condition excluding soft-deleted pages — e.g. require the pages deletion marker
to be null (pages.deletedAt IS NULL or pages.deleted_at IS NULL /
pages.isDeleted = false depending on your schema) so that ghostLinks from
deleted source pages are skipped when promoting to links.
---
Nitpick comments:
In `@server/api/src/__tests__/routes/syncPages.test.ts`:
- Around line 20-35: The inline comment above the mock for ensureDefaultNote and
the helper doc later in syncPages.test.ts are only in one language; update those
comments to include both Japanese and English (or remove them) to match the
repository convention. Locate the comment near the vi.mock for ensureDefaultNote
and the helper doc block that documents test behavior, and either add the
corresponding English/Japanese translation so both languages are present or
delete the comment blocks entirely so the file stays consistent with the
bilingual comment guideline.
In `@server/api/src/routes/notes/crud.ts`:
- Around line 338-339: The inline comment above the object property note_id
(value p.noteId) is currently Japanese-only; add the matching English comment
line (e.g., "Belonging note ID (from Issue `#823` onward this note ID is always
used).") directly above or beside the existing Japanese comment so both
languages are present, or remove the comment entirely if unnecessary; update the
comment near the note_id: p.noteId property to comply with the bilingual
guideline.
In `@server/api/src/routes/syncPages.ts`:
- Around line 205-223: ExistingRow.noteId is declared nullable but pages.noteId
is now non-nullable; update the local type and related handling so the invariant
matches the migrated schema. Change ExistingRow.noteId from string | null to
string, ensure any code that consumes existingRows (e.g., logic after
existingRaw assignment in syncPages.ts) no longer expects null and
remove/null-guard branches if present, and fix any fixtures/tests that produced
null noteId values so existingRaw assignment (which selects pages.noteId) always
maps to the non-null string type.
In `@server/api/src/schema/pages.ts`:
- Around line 111-116: The existing index "idx_pages_note_id" on table.noteId
should be replaced or supplemented with a composite/partial index tailored to
the hot read path that queries pages WHERE note_id = ? AND is_deleted = false
ORDER BY updated_at DESC; update the schema in pages.ts to create a composite
index covering (note_id, updated_at DESC) with a partial filter is_deleted =
false (or an equivalent DB-specific partial index expression) so the queries on
pages (note_id, is_deleted, updated_at) use the index for both filtering and
ordering.
🪄 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: 1c470cf6-0b90-47d4-bf08-740556c00313
📒 Files selected for processing (33)
server/api/drizzle/0023_migrate_personal_pages_drop_note_pages.sqlserver/api/drizzle/meta/_journal.jsonserver/api/src/__tests__/routes/media.test.tsserver/api/src/__tests__/routes/notes/crud.test.tsserver/api/src/__tests__/routes/notes/pages.test.tsserver/api/src/__tests__/routes/notes/search.test.tsserver/api/src/__tests__/routes/notes/setup.tsserver/api/src/__tests__/routes/pageSnapshots.test.tsserver/api/src/__tests__/routes/pages.test.tsserver/api/src/__tests__/routes/search.test.tsserver/api/src/__tests__/routes/syncPages.test.tsserver/api/src/__tests__/services/pageAccessService.test.tsserver/api/src/lib/clipAndCreate.tsserver/api/src/lib/welcomePageService.tsserver/api/src/routes/notes/crud.tsserver/api/src/routes/notes/helpers.tsserver/api/src/routes/notes/pages.tsserver/api/src/routes/notes/search.tsserver/api/src/routes/notes/types.tsserver/api/src/routes/pageSnapshots.tsserver/api/src/routes/pages.tsserver/api/src/routes/search.tsserver/api/src/routes/syncPages.tsserver/api/src/routes/wikiSchema.tsserver/api/src/schema/index.tsserver/api/src/schema/notes.tsserver/api/src/schema/pages.tsserver/api/src/schema/relations.tsserver/api/src/services/defaultNoteService.tsserver/api/src/services/indexBuilder.tsserver/api/src/services/pageAccessService.tsserver/api/src/services/titleRenamePropagationService.tsserver/api/src/types/dbOrTx.ts
💤 Files with no reviewable changes (1)
- server/api/src/schema/index.ts
…s, 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>
|
CodeRabbit 追従コミットを push しました（005f0d33）。0023 の ON CONFLICT、media.test の JP/EN コメント、notes/search の JSDoc 認証整理です。 |
1 participant
概要
Issue #823 / PR 1b: 既存個人ページ (
pages.note_id IS NULL) をすべて所有者のデフォルトノートに移行し、pages.note_idを NOT NULL に昇格、note_pages中間テーブルを廃止する。これでフロント以外のすべての層が「ページは必ずちょうど 1 つのノートに属する」モデルに揃う（A 案統合の API 側準備）。PR #821 (PR 1a) で導入した
notes.is_defaultの上に乗せた破壊的変更で、フロント (/home) は PR 2 (#824 配下の #825〜#830) で別途置き換える。変更点
server/api/drizzle/0023_migrate_personal_pages_drop_note_pages.sql: 孤児個人ページの物理削除 → 安全弁の既定ノート作成 → 既定ノートへバックフィル →pages.note_idを NOT NULL →DROP TABLE IF EXISTS note_pages。手動適用済み環境でも壊れないようIF NOT EXISTS/NOT EXISTSガード付き。meta/_journal.jsonに idx 22 のエントリを追記。server/api/src/schema/notes.ts:notePagesテーブル定義とNotePage/NewNotePage型 export を削除。pages.ts:noteIdを.notNull()に。JSDoc を「単一ノート所属モデル」へ書き換え。index.ts/relations.ts:notePages/notePagesRelationsの export を撤去（notes.pages = many(pages)は維持）。server/api/src/routes/pages.tsGET /api/pagesを 410 Gone（Deprecation: trueヘッダ）。POST /api/pagesでnote_id必須、未指定時はensureDefaultNoteで解決。notes/pages.tsPOST /:noteId/pagesは title 経路のみ（page_idリンクは 400）。PUT /:noteId/pagesは noop（updated_at DESCが順序）。copy-from-personal/copy-to-personalを削除。GET /:noteId/pagesをpages.noteId = :noteId直接 SELECT に書き換え。notes/crud.tsGET /:noteIdのページ一覧をpages.noteId = :noteIdで取得。NotePageApiItemからsort_order/added_by_*を削除。notes/helpers.tsgetActivePageCountsをpages.note_idベースに。getNoteRoleのselect結果を防御的にArray.isArrayで扱う。notes/search.tsnote_pagesJOIN を削除しWHERE p.note_id = :noteIdに置換。search.tsscope=ownは呼び出し元のデフォルトノートに絞る。scope=sharedはnote_pagesJOIN を撤去し owner / accepted member / domain access の EXISTS 群に再構築。pageSnapshots.tsselect結果を防御的に扱う。syncPages.tsnoteIdを埋める。select結果をArray.isArrayでガード。wikiSchema.tsnoteId = ensureDefaultNote(...).idを埋める。server/api/src/services/pageAccessService.ts:pageRow.noteId IS NULL分岐とリンク済み個人ページ経由の lookup を削除。assertPageViewAccess/assertPageEditAccessは 常にgetNoteRole(pages.note_id)経由で判定。titleRenamePropagationService.ts:isNull(pages.noteId)個人スコープ分岐を撤去し、常にeq(pages.noteId, scope.noteId)。defaultNoteService.ts/indexBuilder.ts: 共有用のDbOrTx型を新規types/dbOrTx.tsに切り出し。server/api/src/lib/welcomePageService.ts: ウェルカムページ作成時にnoteId = ensureDefaultNote(tx, userId).idを埋める。clipAndCreate.ts: クリップ起点のページ作成でも同様にnoteIdを埋める。server/api/src/__tests__/setup.tsのcreateMockPageRowからsortOrder/addedByUserIdを削除しnoteIdをデフォルトに追加。createMockPageListRowからsort_order/added_byを削除。routes/notes/pages.test.tsを全面書き換え（title 経路のみ・page_id400・PUT noop・DELETE soft-delete・コピー系 404）。routes/pages.test.tsを 410 /ensureDefaultNoteモック / アクセス用 SELECT 連鎖前提に書き換え。routes/search.test.ts/routes/notes/search.test.tsを新 SQL（p.note_id = :noteId/note_pages不在）前提に更新。routes/notes/crud.test.tsからsort_order/added_by_*検証を削除。routes/syncPages.test.tsでensureDefaultNoteをvi.mock、noteIdを含めて検証。routes/pageSnapshots.test.tsをassertPageViewAccessの 3 段アクセスチェック（pages / users / note）に合わせて再構成。services/pageAccessService.test.tsを「すべてノートロール経由」シナリオに書き換え。routes/media.test.tsの@aws-sdk/client-s3モックにHeadObjectCommandを追加しmockS3Sendの既定値を整備。変更の種類
テスト方法
cd server/api && bun installbun run typecheck— 通ること。bun run test:run— Vitest 全件パス（90 ファイル / 1150 件）。bunx drizzle-kit migrateを実行 →note_pagesテーブルが消え、すべてのpages行でnote_id IS NOT NULLであることを確認。GET /api/notes/me→GET /api/notes/:noteId/pagesで旧個人ページがデフォルトノートの中身として返ることを確認。GET /api/pagesが 410 Gone（Deprecation: trueヘッダ）を返すことを確認。POST /api/notes/:noteId/pagesをpage_id付きで叩くと 400 が返ること、titleのみで叩くとページが作成されることを確認。PUT /api/notes/:noteId/pages（並び替え）が 200 noop でnotes.updated_atのみ進むことを確認。チェックリスト
bun run test:run)bun run lint/bun run format:check)server/api/drizzle/0023_*.sqlと_journal.jsonのエントリを追加したスクリーンショット（UI 変更がある場合）
UI 変更は無し（フロントは PR 2 で別途実施）。
関連 Issue
Closes #823
Related to #821 (PR 1a) / #824 (PR 2 meta) / #825〜#830 (PR 2 子 issue 群)
Summary by CodeRabbit
Release Notes
Chores
/api/pageslist endpoint as deprecated.Tests