Conversation
📝 Walkthrough功能概述本PR增强了expiresAt字段的处理机制,区分显式清除过期时间(设为null)与不进行更新(字段未提供)的场景。在验证层、操作层和UI层添加了相应的逻辑判断和测试覆盖。 改动详情
代码审查工作量估计🎯 3 (中等) | ⏱️ ~20 分钟 Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求修复了用户和密钥过期时间清除后保存不生效的错误。该修复通过优化 UpdateUserSchema 和 editKey 函数中的数据处理逻辑,确保正确处理过期时间的显式清除操作,并防止在进行局部更新时意外修改过期时间。同时,还增加了全面的测试以保证这些更改的健壮性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of the expiresAt field in key and user update operations to introduce clearer semantics. Specifically, it distinguishes between explicitly clearing an expiration date (by providing null or an empty string) and not updating it at all (by omitting the field). The editKey action now checks for the explicit presence of the expiresAt field to prevent accidental clearing during partial updates and includes new validation for invalid date formats. The UpdateUserSchema was updated to reflect these new expiresAt preprocessing rules, allowing null as a valid value. The changes are thoroughly covered by new unit tests for both the keys and users actions, as well as the validation schemas. Additionally, new UI tests were added for EditKeyForm and UserForm to ensure that the 'Clear Date' functionality correctly triggers the backend's updated logic. A review comment suggests extracting duplicated test helper functions (loadMessages, render, clickButtonByText) into a shared utility file to improve code reusability and maintainability across UI tests.
| function loadMessages() { | ||
| const base = path.join(process.cwd(), "messages/en"); | ||
| const read = (name: string) => JSON.parse(fs.readFileSync(path.join(base, name), "utf8")); | ||
|
|
||
| return { | ||
| common: read("common.json"), | ||
| errors: read("errors.json"), | ||
| quota: read("quota.json"), | ||
| ui: read("ui.json"), | ||
| dashboard: read("dashboard.json"), | ||
| forms: read("forms.json"), | ||
| }; | ||
| } | ||
|
|
||
| function render(node: ReactNode) { | ||
| const container = document.createElement("div"); | ||
| document.body.appendChild(container); | ||
| const root = createRoot(container); | ||
|
|
||
| act(() => { | ||
| root.render(node); | ||
| }); | ||
|
|
||
| return { | ||
| container, | ||
| unmount: () => { | ||
| act(() => root.unmount()); | ||
| container.remove(); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| function clickButtonByText(text: string) { | ||
| const buttons = Array.from(document.body.querySelectorAll("button")); | ||
| const btn = buttons.find((b) => (b.textContent || "").includes(text)); | ||
| if (!btn) { | ||
| throw new Error(`未找到按钮: ${text}`); | ||
| } | ||
| btn.dispatchEvent(new MouseEvent("click", { bubbles: true })); | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/dashboard/user-form-expiry-clear-ui.test.tsx (1)
79-82: 建议使用afterEach进行清理以避免测试污染当前
unmount()在测试末尾调用,但如果断言失败会导致清理代码无法执行,可能影响后续测试。建议使用afterEach确保清理始终执行。🔎 建议的修改
describe("UserForm: 清除 expiresAt 后应提交 null", () => { + let cleanup: (() => void) | null = null; + beforeEach(() => { vi.clearAllMocks(); }); + + afterEach(() => { + cleanup?.(); + cleanup = null; + });然后在测试中:
- const { unmount } = render( + const { unmount } = cleanup = render(或者直接保存引用:
const { unmount } = render(...); + cleanup = unmount;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (7)
src/actions/keys.tssrc/lib/validation/schemas.tstests/unit/actions/keys-edit-key-expires-at-clear.test.tstests/unit/actions/users-edit-user-expires-at-clear.test.tstests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsxtests/unit/dashboard/user-form-expiry-clear-ui.test.tsxtests/unit/validation/user-schemas-expires-at-clear.test.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2-space indentation in all code files
Files:
tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsxtests/unit/actions/keys-edit-key-expires-at-clear.test.tstests/unit/validation/user-schemas-expires-at-clear.test.tssrc/actions/keys.tstests/unit/actions/users-edit-user-expires-at-clear.test.tssrc/lib/validation/schemas.tstests/unit/dashboard/user-form-expiry-clear-ui.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use double quotes for strings instead of single quotes
Use trailing commas in multi-line structures
Enforce maximum line length of 100 characters
Use path alias@/*to reference files from./src/*directory
**/*.{ts,tsx,js,jsx}: Use Biome for linting and formatting with 2-space indent, double quotes, trailing commas, and 100 character max line length
Use path alias@/*to reference files in./src/*directory
Files:
tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsxtests/unit/actions/keys-edit-key-expires-at-clear.test.tstests/unit/validation/user-schemas-expires-at-clear.test.tssrc/actions/keys.tstests/unit/actions/users-edit-user-expires-at-clear.test.tssrc/lib/validation/schemas.tstests/unit/dashboard/user-form-expiry-clear-ui.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode for type safety
Use readonly or const assertions for immutable data structures
Files:
tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsxtests/unit/actions/keys-edit-key-expires-at-clear.test.tstests/unit/validation/user-schemas-expires-at-clear.test.tssrc/actions/keys.tstests/unit/actions/users-edit-user-expires-at-clear.test.tssrc/lib/validation/schemas.tstests/unit/dashboard/user-form-expiry-clear-ui.test.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for unit testing with Node environment, coverage thresholds: 50% lines/functions, 40% branches
Files:
tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsxtests/unit/actions/keys-edit-key-expires-at-clear.test.tstests/unit/validation/user-schemas-expires-at-clear.test.tstests/unit/actions/users-edit-user-expires-at-clear.test.tstests/unit/dashboard/user-form-expiry-clear-ui.test.tsx
**/*.{tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use next-intl for internationalization with 5 locales: en, ja, ru, zh-CN, zh-TW
Files:
tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsxtests/unit/dashboard/user-form-expiry-clear-ui.test.tsx
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Ensure test database names contain 'test' keyword for safety validation
Files:
tests/unit/actions/keys-edit-key-expires-at-clear.test.tstests/unit/validation/user-schemas-expires-at-clear.test.tstests/unit/actions/users-edit-user-expires-at-clear.test.ts
src/actions/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/actions/**/*.ts: Validate all user inputs with Zod schemas before processing
Use Server Actions innext-safe-actionwith OpenAPI generation for admin API endpoints
Use Next.js API Routes and Server Actions for admin operations and REST endpoints
Files:
src/actions/keys.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Hash API keys using SHA-256 before storing in database, never store plaintext keys
Mask API keys and sensitive data in application logs
Validate required environment variables at startup with clear error messages
Files:
src/actions/keys.tssrc/lib/validation/schemas.ts
src/{repository,actions}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Avoid N+1 queries by using eager loading and batch queries for statistics
Files:
src/actions/keys.ts
src/lib/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use connection pooling for database and Redis connections
Files:
src/lib/validation/schemas.ts
🧠 Learnings (4)
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/**/*{guard,auth}*.ts : Use constant-time comparison for API key validation to prevent timing attacks
Applied to files:
tests/unit/actions/keys-edit-key-expires-at-clear.test.tssrc/actions/keys.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/actions/**/*.ts : Validate all user inputs with Zod schemas before processing
Applied to files:
tests/unit/validation/user-schemas-expires-at-clear.test.tssrc/lib/validation/schemas.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to **/*.test.ts : Ensure test database names contain 'test' keyword for safety validation
Applied to files:
tests/unit/validation/user-schemas-expires-at-clear.test.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to **/*.test.{ts,tsx} : Use Vitest for unit testing with Node environment, coverage thresholds: 50% lines/functions, 40% branches
Applied to files:
tests/unit/dashboard/user-form-expiry-clear-ui.test.tsx
🧬 Code graph analysis (6)
tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsx (1)
src/app/[locale]/dashboard/_components/user/forms/edit-key-form.tsx (1)
EditKeyForm(48-358)
tests/unit/actions/keys-edit-key-expires-at-clear.test.ts (1)
src/actions/keys.ts (1)
editKey(281-477)
tests/unit/validation/user-schemas-expires-at-clear.test.ts (1)
src/lib/validation/schemas.ts (2)
UpdateUserSchema(147-265)CreateUserSchema(16-142)
src/actions/keys.ts (1)
src/lib/utils/error-messages.ts (1)
ERROR_CODES(110-117)
tests/unit/actions/users-edit-user-expires-at-clear.test.ts (1)
src/actions/users.ts (1)
editUser(1066-1202)
tests/unit/dashboard/user-form-expiry-clear-ui.test.tsx (1)
src/app/[locale]/dashboard/_components/user/forms/user-form.tsx (1)
UserForm(60-447)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Greptile Review
- GitHub Check: check-codex-status
- GitHub Check: pr-review
- GitHub Check: pr-label
- GitHub Check: pr-description
- GitHub Check: 🌐 API Tests
- GitHub Check: Docker Build Test
- GitHub Check: dev-build-deploy
🔇 Additional comments (13)
src/lib/validation/schemas.ts (2)
200-248: LGTM! expiresAt 清除语义实现正确预处理逻辑清晰地区分了三种语义:
undefined:不更新该字段null或"":显式清除过期时间- 有效日期:设置新的过期时间
添加
.nullable()(line 228) 与 Zod 4 的 nullable 日期验证对齐,允许显式传递 null 值。注释说明清晰,逻辑正确。
250-250: 移除 dailyResetMode 默认值的更改是正确的这是符合更新语义的正确实现。UpdateUserSchema 用于 PATCH 操作(部分更新),此时 undefined 表示"不修改该字段"。验证结果显示:
- CreateUserSchema(第 124 行)保留了
.default("fixed")供新用户创建使用 ✓- UpdateUserSchema(第 250 行)正确地移除了默认值,避免无意中覆盖现有值 ✓
- 数据库更新逻辑(src/repository/user.ts 第 348 行)正确实现了选择性更新模式:仅当
userData.dailyResetMode !== undefined时才更新该字段 ✓- editUser 和其他调用方正确处理可选的 dailyResetMode 参数 ✓
tests/unit/actions/users-edit-user-expires-at-clear.test.ts (1)
1-49: LGTM! 测试覆盖清除 expiresAt 的核心场景测试结构清晰,正确验证了:
- 传入
expiresAt: null时editUser返回成功updateUser被调用一次且 payload 包含expiresAt: nullMock 设置完整,符合单元测试最佳实践。
tests/unit/validation/user-schemas-expires-at-clear.test.ts (2)
4-48: LGTM! UpdateUserSchema 测试覆盖全面测试用例完整验证了 expiresAt 的三种语义:
- 清除:
null和""均解析为null- 不更新:缺省字段保持
undefined- 设置值:ISO 字符串正确解析为
Date边界条件和错误场景覆盖充分(非法字符串、非法 Date、非法类型、超过 10 年限制)。
50-94: LGTM! CreateUserSchema 测试确保向后兼容性测试验证了 CreateUserSchema 的不同语义:
null视为未设置(解析为undefined)- 支持 Date 对象和 ISO 字符串(必须为未来时间)
- 正确拒绝过去时间和非法输入
与 UpdateUserSchema 的语义区分清晰,符合创建/更新场景的不同需求。
tests/unit/actions/keys-edit-key-expires-at-clear.test.ts (1)
40-146: LGTM! editKey 测试覆盖全面且准确4 个测试用例完整验证了 expiresAt 的条件更新逻辑:
- 不携带字段:验证
expires_at不在 payload 中(避免误清空)- 显式清除:
expiresAt: undefined→expires_at: null- 设置新值:有效日期字符串 →
Date对象- 错误处理:非法字符串 →
INVALID_FORMATMock 数据完整,断言精确(使用
Object.hasOwn、toBeInstanceOf、errorCode检查),符合 PR 目标。tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsx (1)
78-137: LGTM! UI 测试验证端到端清除流程测试完整模拟了用户清除过期时间的操作流程:
- 渲染带有 expiresAt 值的 EditKeyForm
- 点击日期选择器按钮
- 点击 "Clear Date" 清除日期
- 提交表单
关键断言(line 133)验证了 PR 修复的核心:提交时必须显式携带
expiresAt字段,后端才能识别为"清除"操作。辅助函数(
loadMessages、render、clickButtonByText)设计合理,正确使用act()处理 React 更新。tests/unit/dashboard/user-form-expiry-clear-ui.test.tsx (2)
99-105: 测试依赖硬编码的日期字符串和按钮文本测试中使用硬编码的
"2026-01-04"和"Clear Date"来定位按钮。如果日期格式或国际化文本发生变化,测试可能会失败。不过对于当前的单元测试场景,这是可接受的做法。
110-117: 测试逻辑正确,验证了 expiresAt 清除功能测试正确验证了点击清除日期后,
editUser被调用且expiresAt参数为null。这符合 PR 的修复目标。src/actions/keys.ts (4)
346-348: 使用Object.hasOwn检测字段存在性的做法正确这种方式能够区分"未提供字段"和"显式提供字段(即使值为 null/undefined)",解决了局部更新误清空的问题。
434-440: 无效日期验证逻辑正确该检查能够正确捕获
new Date("invalid-string")产生的无效日期。但请注意,如果上述null转换问题存在,epoch 日期(1970-01-01)会被认为是有效日期而通过验证。
447-463: 条件展开实现了正确的局部更新语义使用
...(hasExpiresAtField ? { expires_at: expiresAt } : {})确保仅在显式提供expiresAt时才更新该字段,避免了局部更新时误清空到期时间的问题。
424-432: Schema设计已防止此问题发生——无需修改
KeyFormSchema中expiresAt字段的 transform 逻辑保证了该字段只会输出undefined或字符串值,永不输出null:expiresAt: z .string() .optional() .default("") .transform((val) => (val === "" ? undefined : val))因此当前代码检查
validatedData.expiresAt === undefined是正确且充分的,不存在null值导致 epoch 日期的问题。
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. Changes appear focused on making expiresAt clear/update semantics explicit for both Key edits and User updates, with unit/UI tests added to lock the behavior.
PR Size: L
- Lines changed: 581
- Files changed: 7
- Split suggestions: Consider splitting into (1)
UpdateUserSchemasemantics + related tests and (2)editKeyexpiresAtsemantics + related tests.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
- Identified PR
#533(fix/expire-clear-save→dev), computed sizeL(581 LOC changed across 7 files), and applied labelsize/L. - Reviewed the full diff across
src/actions/keys.tsandsrc/lib/validation/schemas.tsplus the new unit/UI tests; no issues met the ≥80 confidence reporting threshold, so no inline diff comments were posted. - Submitted the summary review to PR
#533viagh pr review --comment(includes required split suggestion for a sizeLPR).
Summary
Fixes a bug where clearing the expiration date (expiresAt) for Users/Keys via the dashboard UI would not persist to the database.
修复用户/Key 到期时间清除后保存不生效的问题。
Problem
When using the "Clear Date" button in the dashboard to remove the expiration date for a User or Key, the change was not being saved to the database. This occurred because:
UpdateUserSchemawas convertingnull/empty string toundefined, losing the explicit "clear" semanticexpiresAtfield would inadvertently clear the expiration date在仪表板中使用"清除日期"按钮移除用户或密钥的到期时间时,更改未能保存到数据库:
Related to:
Solution
Implemented proper update semantics for the
expiresAtfield:UpdateUserSchema (
src/lib/validation/schemas.ts):null/empty string as explicit clear signal (changed from converting toundefined).nullable()to allow explicit null valuesundefined= don't update field,null/""= clear expiration (set to null)editKey (
src/actions/keys.ts):expires_atwhenexpiresAtfield is explicitly present in request dataObject.hasOwn(data, "expiresAt")to detect field presenceINVALID_FORMATerror)修复方案:
Changes
Core Changes
src/lib/validation/schemas.ts(+6/-2): UpdatedUpdateUserSchema.expiresAtpreprocessing logicsrc/actions/keys.ts(+22/-4): Added field presence detection and conditional update forexpires_atTest Coverage
Added 7 new test files covering all scenarios:
Backend Logic Tests:
tests/unit/actions/keys-edit-key-expires-at-clear.test.ts(+146): Tests editKey behavior with/without expiresAt fieldtests/unit/actions/users-edit-user-expires-at-clear.test.ts(+49): Tests editUser with null expiresAttests/unit/validation/user-schemas-expires-at-clear.test.ts(+94): Tests UpdateUserSchema preprocessing logicUI Tests:
tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsx(+137): Tests "Clear Date" button submits expiresAt fieldtests/unit/dashboard/user-form-expiry-clear-ui.test.tsx(+121): Tests user form clear date behaviorTesting
Automated Tests
Manual Testing Scenarios
Local Verification
All checks passed ✓
Files Changed
src/actions/keys.ts(+22/-4)src/lib/validation/schemas.ts(+6/-2)tests/unit/actions/keys-edit-key-expires-at-clear.test.ts(+146/-0)tests/unit/actions/users-edit-user-expires-at-clear.test.ts(+49/-0)tests/unit/dashboard/edit-key-form-expiry-clear-ui.test.tsx(+137/-0)tests/unit/dashboard/user-form-expiry-clear-ui.test.tsx(+121/-0)tests/unit/validation/user-schemas-expires-at-clear.test.ts(+94/-0)Description enhanced by Claude AI