Conversation
📝 WalkthroughWalkthroughThis PR introduces notification event history tracking and management capabilities, adds development environment configuration for Cloudflare bindings, updates project dependencies to newer versions, and removes documentation comments from existing API routes. Changes
Sequence DiagramsequenceDiagram
actor User
participant SettingsClient as Settings UI
participant API as /api/notification-events
participant DB as Database
rect rgba(100, 150, 200, 0.5)
Note over User,DB: Fetch Notification Events
User->>SettingsClient: Click "View Events"
SettingsClient->>API: GET /api/settings/notification-events
API->>API: Authenticate session
API->>DB: Query events (50 limit, joined with subscriptions/channels)
DB-->>API: Return event records
API-->>SettingsClient: { items: [...] }
SettingsClient->>SettingsClient: Render event table in dialog
SettingsClient-->>User: Display events
end
rect rgba(200, 100, 100, 0.5)
Note over User,DB: Delete Events by Age
User->>SettingsClient: Enter months, confirm cleanup
SettingsClient->>API: DELETE (JSON: { months })
API->>API: Validate months (Zod schema)
alt Validation Failed
API-->>SettingsClient: { error: 400 }
else Validation Passed
API->>API: Calculate cutoff date (now - months)
API->>DB: Select event IDs for user
DB-->>API: Return candidate IDs
API->>DB: Delete events (ID in candidates AND createdAt < cutoff)
DB-->>API: Return deletion count
API-->>SettingsClient: { success: true, deletedCount }
end
SettingsClient->>SettingsClient: Toast notification
SettingsClient->>SettingsClient: Refresh event records
SettingsClient-->>User: Updated event list
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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 introduces a notification event tracking system, allowing users to view recent notification history and perform cleanup of old records. It includes new API endpoints for managing these events, UI components for display and interaction, and updates to the database schema and migration journal. Additionally, several dependencies were updated, and the Cloudflare configuration was expanded to include a development environment. A significant performance and scalability issue was identified in the event cleanup logic, where fetching all IDs into memory before deletion could lead to memory exhaustion or database parameter limits; a single-query subquery approach is recommended instead.
| const ids = await db | ||
| .select({ id: notificationEvents.id }) | ||
| .from(notificationEvents) | ||
| .innerJoin(subscriptions, eq(notificationEvents.subscriptionId, subscriptions.id)) | ||
| .where(eq(subscriptions.userId, session.userId)); | ||
|
|
||
| const deletableIds = ids.map((item) => item.id); | ||
| if (deletableIds.length === 0) { | ||
| return NextResponse.json({ success: true, deletedCount: 0 }); | ||
| } | ||
|
|
||
| const deleted = await db | ||
| .delete(notificationEvents) | ||
| .where( | ||
| and( | ||
| inArray(notificationEvents.id, deletableIds), | ||
| lt(notificationEvents.createdAt, cutoff) | ||
| ) | ||
| ) | ||
| .returning({ id: notificationEvents.id }); |
There was a problem hiding this comment.
The current cleanup logic is inefficient and does not scale well. It fetches all notification event IDs for the user into application memory (without even filtering by the cutoff date) and then performs a deletion using an inArray filter. This approach can lead to high memory usage and may exceed SQLite's parameter limit (typically 999) if a user has many events.
It is recommended to perform the deletion in a single query using a subquery. This avoids fetching unnecessary data and reduces database round-trips.
const deleted = await db
.delete(notificationEvents)
.where(
and(
lt(notificationEvents.createdAt, cutoff),
inArray(
notificationEvents.subscriptionId,
db.select({ id: subscriptions.id })
.from(subscriptions)
.where(eq(subscriptions.userId, session.userId))
)
)
)
.returning({ id: notificationEvents.id });![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wrangler.jsonc (1)
1-8:⚠️ Potential issue | 🟡 MinorDuplicate JSDoc comment block.
Lines 1–4 and 5–8 are identical. Drop one copy.
♻️ Proposed fix
-/** - * For more details on how to configure Wrangler, refer to: - * https://developers.cloudflare.com/workers/wrangler/configuration/ - */ /** * For more details on how to configure Wrangler, refer to: * https://developers.cloudflare.com/workers/wrangler/configuration/ */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wrangler.jsonc` around lines 1 - 8, Remove the duplicate JSDoc header at the top of wrangler.jsonc by keeping a single copy of the comment block "For more details on how to configure Wrangler, refer to: https://developers.cloudflare.com/workers/wrangler/configuration/" and deleting the repeated block so only one header comment remains.
🧹 Nitpick comments (4)
components/settings-client.tsx (2)
201-214: Error responses parse body before checkingres.ok, swallowing server error messages.
await res.json()is called unconditionally, thenif (!res.ok) throw new Error()throws with no message, and the genericcatchshows a hardcoded"加载通知记录失败". If the server returns{ error: "…" }with a non-2xx status (e.g., auth expired), the actual reason is discarded.Consider surfacing server-provided error text when present:
♻️ Suggested refactor
const fetchEventRecords = async () => { setEventRecordsLoading(true); try { const res = await fetch("/api/settings/notification-events"); - const data = await res.json() as { items?: NotificationEventRecord[] }; - if (!res.ok) throw new Error(); + const data = await res.json() as { items?: NotificationEventRecord[]; error?: string }; + if (!res.ok) throw new Error(data.error); setEventRecords(data.items ?? []); setEventDialogOpen(true); - } catch { - toast.error("加载通知记录失败"); + } catch (err) { + toast.error(err instanceof Error && err.message ? err.message : "加载通知记录失败"); } finally { setEventRecordsLoading(false); } };The same pattern exists in
handleCleanupEvents(Lines 407–433) — apply the same treatment there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/settings-client.tsx` around lines 201 - 214, fetchEventRecords currently calls await res.json() before checking res.ok and throws a generic Error, which discards server error details; change the flow in fetchEventRecords to first check res.ok and if false parse the response body (await res.text() or await res.json()) to extract any server-provided message (e.g., body.error or text) and include that in the thrown Error or pass it to toast.error so users see the real reason; apply the same fix to handleCleanupEvents so both functions surface server-provided error text instead of always showing the hardcoded "加载通知记录失败" message and so setEventRecords is only called on success.
762-783: Confirm dialog shows stalecleanupMonthsif user clears the input after opening.Minor UX: if the user opens the confirm dialog, then edits
cleanupMonthsbehind it (the dialog doesn't block the underlying input necessarily, and the input's state is shared), the dialog text updates live to{cleanupMonths || "0"}. If blank, it reads "0 个月前" which is confusing sincemonths < 1is rejected insidehandleCleanupEvents. Consider either:
- Snapshotting
cleanupMonthsinto a local state when opening the confirm, or- Disabling the "清理记录" trigger button when the current input is invalid (so the confirm dialog can never be opened with an invalid value).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/settings-client.tsx` around lines 762 - 783, The confirm dialog reads the live cleanupMonths value and can show a stale/invalid value if the user edits the input after opening; fix by snapshotting the value when the dialog is opened: add a local state (e.g., confirmCleanupMonths) and, when opening the dialog (via setClearConfirmOpen or the onClick that sets clearConfirmOpen), set confirmCleanupMonths = cleanupMonths (or parsed numeric value); update the dialog text to use confirmCleanupMonths || "0" and have handleCleanupEvents consume that snapshot (or accept it as an argument) instead of the live cleanupMonths; alternatively, if you prefer the other suggestion, disable the "清理记录" trigger button when cleanupMonths is invalid so the dialog cannot be opened with an invalid value.wrangler.jsonc (2)
47-66: Committed development resource IDs are specific to one Cloudflare account.The
database_id(ca032790-…) and KVid(19ce58a4b…) underenv.developmentare hard-coded to resources that only exist in the PR author's Cloudflare account. Any other contributor runningwrangler dev --env developmentwill hit binding errors unless they either:
- Create their own resources and modify the file locally, or
- Follow a documented setup step to substitute these IDs.
Consider either (a) adding a
CONTRIBUTING.md/README note instructing contributors to replace these with their own IDs, or (b) using placeholder tokens (like the existing__D1_ID__/__KV_ID__) and loading real values via.env.development/ a setup script, consistent with the top-level bindings.Note: these IDs are not secrets (Cloudflare identifiers alone don't grant access without an API token), so this is a DX concern, not a security one.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wrangler.jsonc` around lines 47 - 66, The env.development block contains hard-coded Cloudflare resource IDs (database_id and kv namespace id) which break other contributors; replace the literal IDs in env.development with the existing placeholder tokens (e.g., __D1_ID__, __KV_ID__) used at the top-level bindings and update the project to read real values from a .env.development or a setup script (or add CONTRIBUTING.md instructions) so that the D1 binding "DB" (database_id) and kv_namespaces entry for "KV" use configurable values rather than the specific UUIDs; ensure the images binding remains unchanged and document the env setup step in CONTRIBUTING.md if you opt for developer instructions instead of placeholders.
62-65: Trailing comma after theimagesblock.The
,at the end of line 64 (after}) is a trailing comma with no following property in thedevelopmentobject. JSONC tolerates this, but some tooling does not — and it's inconsistent with how the rest of the file is formatted. Recommend removing it.♻️ Proposed fix
"images": { "binding": "IMAGES" - }, + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wrangler.jsonc` around lines 62 - 65, Remove the trailing comma after the "images" block inside the development object in wrangler.jsonc — locate the "images" binding entry (key "images": { "binding": "IMAGES" }) and delete the comma following its closing brace so the JSONC stays valid and consistent with the rest of the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.development.example:
- Line 5: The file ends without a trailing newline after the line "#
CLOUDFLARE_ENV=development"; add a single blank line (newline character) at the
end of the file so the file terminates with a trailing newline to satisfy
dotenv-linter.
In `@app/api/settings/notification-events/route.ts`:
- Around line 48-49: The current cutoff computation in route.ts uses
Date.setMonth on the cutoff variable which can produce end-of-month rollovers
(e.g., Mar 31 -> Mar 3); update the logic that constructs cutoff to normalize to
a stable boundary—either set cutoff to the first day of the month before
subtracting months (e.g., cutoff.setDate(1) then setMonth(... -
parsed.data.months)) or use a day-based subtraction (compute parsed.data.months
* 30 or use a library/date arithmetic to subtract exact months) so the retention
window is deterministic; adjust the code around the cutoff variable in the
notification-events route accordingly.
- Around line 41-46: The handler currently does await request.json() which
throws on malformed/empty JSON, causing a 500; modify the code in the route
handler to read the raw request text (e.g., await request.text()) and then try
to JSON.parse it inside a try/catch, or wrap await request.json() in a
try/catch, and on parse errors return NextResponse.json({ error: 'Invalid JSON'
}, { status: 400 }) before calling cleanupSchema.safeParse(body); keep
references to body, parsed, cleanupSchema and ensure any thrown SyntaxError is
caught and mapped to a 400 response instead of letting it bubble up.
- Around line 51-70: The code currently selects all notificationEvents IDs into
deletableIds and uses inArray(notificationEvents.id, deletableIds) which loads
many IDs into memory and breaks on DB parameter limits; instead remove the
initial ids query and the inArray check and change the delete's WHERE to use an
exists() subquery that checks subscriptions where eq(subscriptions.id,
notificationEvents.subscriptionId) and eq(subscriptions.userId, session.userId),
combined with lt(notificationEvents.createdAt, cutoff); update the delete call
that references notificationEvents, subscriptions, session.userId, and cutoff to
use this server-side ownership check.
In `@components/settings-client.tsx`:
- Around line 686-718: Add explicit form associations: give the cleanup input an
id (e.g. "cleanup-months") and set the corresponding Label's htmlFor to that id
so the Label and the Input (value={cleanupMonths}, onChange={setCleanupMonths})
are programmatically linked; likewise, for the "最近 50 条发送记录" block, add an id to
the Label and reference it from the View Records Button (either via
aria-labelledby on the Button or aria-describedby depending on intent) so screen
readers announce the relationship while keeping existing handlers like
fetchEventRecords and props like eventRecordsLoading untouched.
---
Outside diff comments:
In `@wrangler.jsonc`:
- Around line 1-8: Remove the duplicate JSDoc header at the top of
wrangler.jsonc by keeping a single copy of the comment block "For more details
on how to configure Wrangler, refer to:
https://developers.cloudflare.com/workers/wrangler/configuration/" and deleting
the repeated block so only one header comment remains.
---
Nitpick comments:
In `@components/settings-client.tsx`:
- Around line 201-214: fetchEventRecords currently calls await res.json() before
checking res.ok and throws a generic Error, which discards server error details;
change the flow in fetchEventRecords to first check res.ok and if false parse
the response body (await res.text() or await res.json()) to extract any
server-provided message (e.g., body.error or text) and include that in the
thrown Error or pass it to toast.error so users see the real reason; apply the
same fix to handleCleanupEvents so both functions surface server-provided error
text instead of always showing the hardcoded "加载通知记录失败" message and so
setEventRecords is only called on success.
- Around line 762-783: The confirm dialog reads the live cleanupMonths value and
can show a stale/invalid value if the user edits the input after opening; fix by
snapshotting the value when the dialog is opened: add a local state (e.g.,
confirmCleanupMonths) and, when opening the dialog (via setClearConfirmOpen or
the onClick that sets clearConfirmOpen), set confirmCleanupMonths =
cleanupMonths (or parsed numeric value); update the dialog text to use
confirmCleanupMonths || "0" and have handleCleanupEvents consume that snapshot
(or accept it as an argument) instead of the live cleanupMonths; alternatively,
if you prefer the other suggestion, disable the "清理记录" trigger button when
cleanupMonths is invalid so the dialog cannot be opened with an invalid value.
In `@wrangler.jsonc`:
- Around line 47-66: The env.development block contains hard-coded Cloudflare
resource IDs (database_id and kv namespace id) which break other contributors;
replace the literal IDs in env.development with the existing placeholder tokens
(e.g., __D1_ID__, __KV_ID__) used at the top-level bindings and update the
project to read real values from a .env.development or a setup script (or add
CONTRIBUTING.md instructions) so that the D1 binding "DB" (database_id) and
kv_namespaces entry for "KV" use configurable values rather than the specific
UUIDs; ensure the images binding remains unchanged and document the env setup
step in CONTRIBUTING.md if you opt for developer instructions instead of
placeholders.
- Around line 62-65: Remove the trailing comma after the "images" block inside
the development object in wrangler.jsonc — locate the "images" binding entry
(key "images": { "binding": "IMAGES" }) and delete the comma following its
closing brace so the JSONC stays valid and consistent with the rest of the file.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2ea8768d-39c5-4716-9594-abb0d10d1767
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.env.development.exampleapp/api/settings/account/route.tsapp/api/settings/notification-events/route.tscomponents/dashboard-client.tsxcomponents/settings-client.tsxdrizzle/meta/_journal.jsonlib/notifications/dispatcher.tspackage.jsonpnpm-workspace.yamlwrangler.jsonc
| CLOUDFLARE_TOKEN= No newline at end of file | ||
| CLOUDFLARE_TOKEN= | ||
| # 本地开发时取消下面的注释 | ||
| # CLOUDFLARE_ENV=development No newline at end of file |
There was a problem hiding this comment.
Add a trailing newline.
Per the dotenv-linter hint, the file is missing an ending blank line.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 5-5: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.development.example at line 5, The file ends without a trailing newline
after the line "# CLOUDFLARE_ENV=development"; add a single blank line (newline
character) at the end of the file so the file terminates with a trailing newline
to satisfy dotenv-linter.
| const body = await request.json() as unknown; | ||
| const parsed = cleanupSchema.safeParse(body); | ||
|
|
||
| if (!parsed.success) { | ||
| return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 }); | ||
| } |
There was a problem hiding this comment.
Guard against malformed JSON bodies.
await request.json() throws SyntaxError on invalid/empty bodies, which will surface as a 500 instead of the intended 400 validation response. Wrap the parse or read the raw text first.
🛡️ Proposed fix
- const body = await request.json() as unknown;
- const parsed = cleanupSchema.safeParse(body);
+ let body: unknown;
+ try {
+ body = await request.json();
+ } catch {
+ return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 });
+ }
+ const parsed = cleanupSchema.safeParse(body);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const body = await request.json() as unknown; | |
| const parsed = cleanupSchema.safeParse(body); | |
| if (!parsed.success) { | |
| return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 }); | |
| } | |
| let body: unknown; | |
| try { | |
| body = await request.json(); | |
| } catch { | |
| return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }); | |
| } | |
| const parsed = cleanupSchema.safeParse(body); | |
| if (!parsed.success) { | |
| return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/settings/notification-events/route.ts` around lines 41 - 46, The
handler currently does await request.json() which throws on malformed/empty
JSON, causing a 500; modify the code in the route handler to read the raw
request text (e.g., await request.text()) and then try to JSON.parse it inside a
try/catch, or wrap await request.json() in a try/catch, and on parse errors
return NextResponse.json({ error: 'Invalid JSON' }, { status: 400 }) before
calling cleanupSchema.safeParse(body); keep references to body, parsed,
cleanupSchema and ensure any thrown SyntaxError is caught and mapped to a 400
response instead of letting it bubble up.
| const cutoff = new Date(); | ||
| cutoff.setMonth(cutoff.getMonth() - parsed.data.months); |
There was a problem hiding this comment.
Date.setMonth has end-of-month pitfalls.
Subtracting months from a date like Mar 31 yields Mar 3 (since Feb has no 31st), so the cutoff can end up slightly inside the current month. Typically harmless for a retention/cleanup window, but worth being aware of; consider normalizing to day 1 or using a day-based offset if exact semantics matter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/settings/notification-events/route.ts` around lines 48 - 49, The
current cutoff computation in route.ts uses Date.setMonth on the cutoff variable
which can produce end-of-month rollovers (e.g., Mar 31 -> Mar 3); update the
logic that constructs cutoff to normalize to a stable boundary—either set cutoff
to the first day of the month before subtracting months (e.g., cutoff.setDate(1)
then setMonth(... - parsed.data.months)) or use a day-based subtraction (compute
parsed.data.months * 30 or use a library/date arithmetic to subtract exact
months) so the retention window is deterministic; adjust the code around the
cutoff variable in the notification-events route accordingly.
| const ids = await db | ||
| .select({ id: notificationEvents.id }) | ||
| .from(notificationEvents) | ||
| .innerJoin(subscriptions, eq(notificationEvents.subscriptionId, subscriptions.id)) | ||
| .where(eq(subscriptions.userId, session.userId)); | ||
|
|
||
| const deletableIds = ids.map((item) => item.id); | ||
| if (deletableIds.length === 0) { | ||
| return NextResponse.json({ success: true, deletedCount: 0 }); | ||
| } | ||
|
|
||
| const deleted = await db | ||
| .delete(notificationEvents) | ||
| .where( | ||
| and( | ||
| inArray(notificationEvents.id, deletableIds), | ||
| lt(notificationEvents.createdAt, cutoff) | ||
| ) | ||
| ) | ||
| .returning({ id: notificationEvents.id }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Drizzle ORM inArray subquery support and Cloudflare D1 SQLITE_MAX_VARIABLE_NUMBER bound parameter limit
💡 Result:
Drizzle ORM's inArray does not directly support subqueries as the second argument. Users have reported issues attempting this, with no TypeScript errors but runtime failures. Instead, Drizzle supports subqueries in WHERE clauses via exists, notExists, or raw sqlcolumn IN (subquery) templates. For example: ts const userIds = db.select({ id: posts.userId }).from(posts).as('user_ids'); await db.select.from(users).where(inArray(users.id, sql(SELECT id FROM user_ids))); // or use exists However, the standard workaround for large arrays in inArray is manual chunking due to Cloudflare D1's limit. Cloudflare D1 enforces a maximum of 100 bound parameters per query (SQLITE_MAX_VARIABLE_NUMBER equivalent), as stated in official docs. This affects Drizzle's inArray, where each array element becomes a bound parameter (e.g., WHERE id IN (?, ?, ...) with N parameters for N IDs). For inArray with >100 IDs on D1, queries fail with "too many SQL variables" errors. Drizzle does not auto-chunk; users must manually chunk arrays and execute multiple queries or use db.batch. Examples from issues: ts const BATCH_SIZE = 90; // Conservative, accounting for other params const chunks = chunk(ids, BATCH_SIZE); for (const chunk of chunks) { await db.select.from(table).where(inArray(table.id, chunk)); } Subqueries avoid parameter limits entirely since they execute separately, making them ideal for large/dynamic sets on D1. Use subqueries or chunking to work around the 100-parameter limit.
Citations:
- 1: https://www.answeroverflow.com/m/1230640009720238121
- 2: https://stackoverflow.com/questions/77803745/drizzle-orm-how-to-use-subquery-inside-where
- 3: https://orm.drizzle.team/docs/select
- 4: [BUG]: D1 too many SQL variables at offset drizzle-team/drizzle-orm#2479
- 5: https://developer.cloudflare.com/d1/platform/limits/
- 6: https://developers.cloudflare.com/d1/platform/limits
- 7: https://www.mintlify.com/drizzle-team/drizzle-orm/queries/subqueries
- 8: [BUG]: cannot insert many sql variables when using cloudflare d1 drizzle-team/drizzle-orm#2659
🏁 Script executed:
head -50 app/api/settings/notification-events/route.tsRepository: Merack/subflare-vinext
Length of output: 1727
🏁 Script executed:
rg "exists|inArray|sql\(" app/api/settings/notification-events/route.ts -A 2Repository: Merack/subflare-vinext
Length of output: 354
🏁 Script executed:
rg "from drizzle|import.*drizzle" app/api/settings/notification-events/route.tsRepository: Merack/subflare-vinext
Length of output: 123
Use a subquery with exists() to filter owned events instead of fetching all IDs into memory.
The current approach loads every notification event ID belonging to the user into a JavaScript array and passes it to inArray. For users with many events, this is unnecessarily expensive and on Cloudflare D1 (which enforces a 100 bound-parameter limit) will cause the query to fail with "too many SQL variables" errors—each array element becomes a bound parameter, so 101+ IDs will break the query.
Instead, use a subquery with exists() to filter by subscription ownership directly in the WHERE clause. This avoids loading IDs into memory and executes the ownership check server-side.
♻️ Proposed refactor
+import { exists } from "drizzle-orm";
import { and, desc, eq, inArray, lt } from "drizzle-orm";
- const ids = await db
- .select({ id: notificationEvents.id })
- .from(notificationEvents)
- .innerJoin(subscriptions, eq(notificationEvents.subscriptionId, subscriptions.id))
- .where(eq(subscriptions.userId, session.userId));
-
- const deletableIds = ids.map((item) => item.id);
- if (deletableIds.length === 0) {
- return NextResponse.json({ success: true, deletedCount: 0 });
- }
-
const deleted = await db
.delete(notificationEvents)
.where(
and(
- inArray(notificationEvents.id, deletableIds),
+ exists(
+ db.select({ id: subscriptions.id })
+ .from(subscriptions)
+ .where(
+ and(
+ eq(subscriptions.userId, session.userId),
+ eq(subscriptions.id, notificationEvents.subscriptionId)
+ )
+ )
+ ),
lt(notificationEvents.createdAt, cutoff)
)
)
.returning({ id: notificationEvents.id });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/settings/notification-events/route.ts` around lines 51 - 70, The code
currently selects all notificationEvents IDs into deletableIds and uses
inArray(notificationEvents.id, deletableIds) which loads many IDs into memory
and breaks on DB parameter limits; instead remove the initial ids query and the
inArray check and change the delete's WHERE to use an exists() subquery that
checks subscriptions where eq(subscriptions.id,
notificationEvents.subscriptionId) and eq(subscriptions.userId, session.userId),
combined with lt(notificationEvents.createdAt, cutoff); update the delete call
that references notificationEvents, subscriptions, session.userId, and cutoff to
use this server-side ownership check.
| <div className="space-y-1"> | ||
| <Label>最近 50 条发送记录</Label> | ||
| <p className="text-xs text-muted-foreground"> | ||
| 查看最近通知发送结果，用于排查发送状态和错误信息 | ||
| </p> | ||
| </div> | ||
| <Button variant="outline" onClick={() => void fetchEventRecords()} disabled={eventRecordsLoading}> | ||
| {eventRecordsLoading ? "加载中..." : "查看最近记录"} | ||
| </Button> | ||
| </div> | ||
|
|
||
| <Separator /> | ||
|
|
||
| <div className="flex items-start justify-between gap-4"> | ||
| <div className="space-y-1"> | ||
| <Label>清理历史记录</Label> | ||
| <p className="text-xs text-muted-foreground"> | ||
| 删除指定月份之前的通知发送记录，此操作不可恢复 | ||
| </p> | ||
| </div> | ||
| <div className="flex items-center gap-2"> | ||
| <Input | ||
| type="number" | ||
| min={1} | ||
| value={cleanupMonths} | ||
| onChange={(e) => setCleanupMonths(e.target.value)} | ||
| className="w-24" | ||
| /> | ||
| <span className="text-sm text-muted-foreground">个月前</span> | ||
| <Button variant="destructive" onClick={() => setClearConfirmOpen(true)} disabled={cleaningEvents}> | ||
| 清理记录 | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
Label is not programmatically associated with its input.
The <Label>清理历史记录</Label> at line 701 describes the cleanup block, but the <Input> at lines 707–713 has no id and the Label has no htmlFor. Screen readers won't announce the relationship, and clicking the label won't focus the input. Same consideration for the "最近 50 条发送记录" label vs the "查看最近记录" button (button doesn't need a label association, but the pattern here is inconsistent with the timezone/hour inputs above).
♿ Suggested fix for the cleanup input
- <Label>清理历史记录</Label>
+ <Label htmlFor="cleanup-months">清理历史记录</Label>
...
- <Input
+ <Input
+ id="cleanup-months"
type="number"
min={1}
value={cleanupMonths}
onChange={(e) => setCleanupMonths(e.target.value)}
className="w-24"
+ aria-label="清理指定月份前的记录"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="space-y-1"> | |
| <Label>最近 50 条发送记录</Label> | |
| <p className="text-xs text-muted-foreground"> | |
| 查看最近通知发送结果，用于排查发送状态和错误信息 | |
| </p> | |
| </div> | |
| <Button variant="outline" onClick={() => void fetchEventRecords()} disabled={eventRecordsLoading}> | |
| {eventRecordsLoading ? "加载中..." : "查看最近记录"} | |
| </Button> | |
| </div> | |
| <Separator /> | |
| <div className="flex items-start justify-between gap-4"> | |
| <div className="space-y-1"> | |
| <Label>清理历史记录</Label> | |
| <p className="text-xs text-muted-foreground"> | |
| 删除指定月份之前的通知发送记录，此操作不可恢复 | |
| </p> | |
| </div> | |
| <div className="flex items-center gap-2"> | |
| <Input | |
| type="number" | |
| min={1} | |
| value={cleanupMonths} | |
| onChange={(e) => setCleanupMonths(e.target.value)} | |
| className="w-24" | |
| /> | |
| <span className="text-sm text-muted-foreground">个月前</span> | |
| <Button variant="destructive" onClick={() => setClearConfirmOpen(true)} disabled={cleaningEvents}> | |
| 清理记录 | |
| </Button> | |
| </div> | |
| <div className="space-y-1"> | |
| <Label>最近 50 条发送记录</Label> | |
| <p className="text-xs text-muted-foreground"> | |
| 查看最近通知发送结果，用于排查发送状态和错误信息 | |
| </p> | |
| </div> | |
| <Button variant="outline" onClick={() => void fetchEventRecords()} disabled={eventRecordsLoading}> | |
| {eventRecordsLoading ? "加载中..." : "查看最近记录"} | |
| </Button> | |
| </div> | |
| <Separator /> | |
| <div className="flex items-start justify-between gap-4"> | |
| <div className="space-y-1"> | |
| <Label htmlFor="cleanup-months">清理历史记录</Label> | |
| <p className="text-xs text-muted-foreground"> | |
| 删除指定月份之前的通知发送记录，此操作不可恢复 | |
| </p> | |
| </div> | |
| <div className="flex items-center gap-2"> | |
| <Input | |
| id="cleanup-months" | |
| type="number" | |
| min={1} | |
| value={cleanupMonths} | |
| onChange={(e) => setCleanupMonths(e.target.value)} | |
| className="w-24" | |
| aria-label="清理指定月份前的记录" | |
| /> | |
| <span className="text-sm text-muted-foreground">个月前</span> | |
| <Button variant="destructive" onClick={() => setClearConfirmOpen(true)} disabled={cleaningEvents}> | |
| 清理记录 | |
| </Button> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/settings-client.tsx` around lines 686 - 718, Add explicit form
associations: give the cleanup input an id (e.g. "cleanup-months") and set the
corresponding Label's htmlFor to that id so the Label and the Input
(value={cleanupMonths}, onChange={setCleanupMonths}) are programmatically
linked; likewise, for the "最近 50 条发送记录" block, add an id to the Label and
reference it from the View Records Button (either via aria-labelledby on the
Button or aria-describedby depending on intent) so screen readers announce the
relationship while keeping existing handlers like fetchEventRecords and props
like eventRecordsLoading untouched.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores