Conversation
**Issue #431**: 修复删除禁用 key 时的错误提示 - 问题：删除已禁用的 key 时，系统错误地提示"至少需要保留一个可用密钥" - 原因：未区分要删除的 key 是启用还是禁用状态 - 修复：只有删除启用的 key 时才检查剩余启用 key 数量 - 位置：src/actions/keys.ts:478-488 **Issue #438**: 修复删除 key 后前端未刷新的问题 - 问题：删除 key 后，前端页面没有实时刷新，已删除的 key 仍然显示 - 原因：只调用了 router.refresh()，但没有使 React Query 缓存失效 - 修复：在删除成功后调用 queryClient.invalidateQueries({ queryKey: ["users"] }) - 位置：src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx:107,151 **变更内容**： - src/actions/keys.ts: 添加 key.isEnabled 条件判断，只对启用的 key 进行数量检查 - src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx: - 导入 useQueryClient hook - 在 handleDeleteKey 中添加缓存失效逻辑 **符合原则**： - SOLID: 单一职责，每个检查都有明确的职责 - KISS: 逻辑简单清晰，易于理解 - DRY: 与 toggleKeyEnabled 函数保持一致的验证逻辑
Summary of ChangesHello @NightYuYyy, 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! 此拉取请求旨在解决与密钥删除功能相关的两个关键缺陷。首先，它修正了后端逻辑，允许用户在仍有其他启用密钥的情况下删除禁用密钥，从而提升了密钥管理的灵活性。其次，它通过在前端引入缓存失效机制，确保了删除操作后的用户界面能够即时反映最新的数据状态，显著改善了用户体验。 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| if (key.isEnabled) { | ||
| const activeKeyCount = await countActiveKeysByUser(key.userId); | ||
| if (activeKeyCount <= 1) { | ||
| return { | ||
| ok: false, | ||
| error: "该用户至少需要保留一个可用的密钥，无法删除最后一个密钥", | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
此更改正确地解决了删除禁用密钥的问题。但是，这里存在一个潜在的竞态条件（race condition）。如果在 countActiveKeysByUser 检查和 deleteKey 执行之间，有另一个并发请求禁用了用户的其他密钥，这可能会导致用户最终没有可用的密钥，从而违反了业务规则。
为了确保操作的原子性并防止此竞态条件，整个检查和删除逻辑应该被包装在一个数据库事务中，这与此文件中的 batchUpdateKeys 函数处理类似问题的方式一致。在事务中，您应该：
- （可选，但推荐）为要更新的行加锁。
- 执行活动密钥数量检查。
- 执行删除操作。
这将保证检查和删除操作之间数据的一致性。
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }; | ||
| // 只有删除启用的密钥时，才需要检查是否是最后一个启用的密钥 | ||
| // 删除禁用的密钥不会影响用户的可用密钥数量 | ||
| if (key.isEnabled) { |
There was a problem hiding this comment.
[HIGH] [TEST-MISSING-CRITICAL] removeKey 新增分支缺少覆盖用例
Evidence: src/actions/keys.ts:480 新增逻辑：
if (key.isEnabled) {
const activeKeyCount = await countActiveKeysByUser(key.userId);
if (activeKeyCount <= 1) {
return {
ok: false,
error: "该用户至少需要保留一个可用的密钥，无法删除最后一个密钥",
};
}
}Why this is a problem: 该分支改变了「删除禁用 key」的行为，但当前测试集中没有断言“删除禁用 key 不应触发最后一个启用 key 检查”。一旦回归，很容易在 CI 中漏掉（vitest.config.ts:32-38 仍开启了 branches: 40 覆盖率阈值）。
Suggested fix (添加一个不依赖 Next 运行时的单元测试，用 mock 固定依赖)：
// tests/unit/actions/remove-key.test.ts
import { expect, it, vi } from "vitest";
vi.mock("@/lib/auth", () => ({
getSession: vi.fn(async () => ({ user: { id: 1, role: "admin" } })),
}));
const repoMock = {
findKeyById: vi.fn(async () => ({ id: 123, userId: 1, isEnabled: false })),
countActiveKeysByUser: vi.fn(async () => 1),
deleteKey: vi.fn(async () => undefined),
findKeyList: vi.fn(async () => []),
};
vi.mock("@/repository/key", () => repoMock);
vi.mock("@/actions/users", () => ({ syncUserProviderGroupFromKeys: vi.fn(async () => undefined) }));
vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
it("removeKey allows deleting disabled key without last-active check", async () => {
const { removeKey } = await import("@/actions/keys");
const res = await removeKey(123);
expect(res.ok).toBe(true);
expect(repoMock.countActiveKeysByUser).not.toHaveBeenCalled();
});
There was a problem hiding this comment.
Code Review Summary
This PR is small and changes key deletion semantics plus the dashboard refresh path. The behavior change in removeKey introduces a new branch that is currently untested and could regress without being caught.
PR Size: XS
- Lines changed: 20
- Files changed: 2
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 1 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
- (90) [TEST-MISSING-CRITICAL] Add coverage for deleting a disabled key without triggering the last-active-key check (
src/actions/keys.ts:480).
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Reviewed PR
#439(fix: 修复删除密钥的两个问题 (#431 #438)), size classified as XS and applied labelsize/XS. - Posted 1 inline review comment on
src/actions/keys.ts:480for [HIGH] [TEST-MISSING-CRITICAL]: the newkey.isEnabledbranch inremoveKeychanges behavior but has no targeted test coverage; included a concretevitestmock-based test snippet suggestion. - Submitted the required PR review summary via
gh pr review --comment.
**问题1**: Key 侧快捷续期时间计算不准确
- 问题：快捷选择 7 天时，实际只延长约 7 天减去当前小时数
- 原因：使用 addDays() 后没有设置时间到当天结束 (23:59:59.999)
- 修复：添加 newDate.setHours(23, 59, 59, 999) 确保整天有效
- 位置：src/app/[locale]/dashboard/_components/user/forms/quick-renew-key-dialog.tsx:104-105
**问题2**: 用户侧快捷续期后前端未实时刷新
- 问题：续期后页面不刷新，显示旧数据
- 原因：只调用 router.refresh()，未使 React Query 缓存失效
- 修复：添加 queryClient.invalidateQueries({ queryKey: ["users"] })
- 位置：src/app/[locale]/dashboard/_components/user/user-management-table.tsx:122,356
**变更内容**：
- quick-renew-key-dialog.tsx: 添加时间设置到当天结束
- user-management-table.tsx:
- 导入 useQueryClient hook
- 在续期成功后使缓存失效
**验证**：
- ✅ TypeScript 类型检查通过
- ✅ 与用户侧 quick-renew-dialog.tsx 保持一致的时间处理逻辑
- ✅ 与 #438 修复保持一致的缓存失效策略
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The changes are minimal, well-targeted bug fixes that follow existing codebase patterns.
PR Size: XS
- Lines changed: 20 (14 additions, 6 deletions)
- Files changed: 2
Change Analysis
src/actions/keys.ts (Issue #431 fix)
- The fix correctly wraps the active key count check inside a
key.isEnabledcondition - Logic is sound: disabled keys don't contribute to the active key count (per
countActiveKeysByUser), so deleting a disabled key shouldn't trigger the "last key" protection - Comments accurately describe the fix rationale
src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx (Issue #438 fix)
- Properly imports and uses
useQueryClienthook from@tanstack/react-query - The component is rendered within
UsersPageClientwhich providesQueryClientProvidercontext - Query key
["users"]matches the existing pattern used throughout the codebase (verified inunified-edit-dialog.tsx,batch-edit-dialog.tsx) - Cache invalidation is correctly placed after successful deletion confirmation
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (existing patterns maintained)
- Type safety - Clean
- Documentation accuracy - Clean (comments match implementation)
- Test coverage - Existing tests are skipped (pending integration test refactor), but the fix logic is straightforward
- Code clarity - Good
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The changes are minimal, well-targeted bug fixes that follow existing codebase patterns.
PR Size: XS
- Lines changed: 20 (14 additions, 6 deletions)
- Files changed: 2
Change Analysis
src/actions/keys.ts (Issue #431 fix)
- The fix correctly wraps the active key count check inside a
key.isEnabledcondition - Logic is sound: disabled keys don't contribute to the active key count (per
countActiveKeysByUser), so deleting a disabled key shouldn't trigger the "last key" protection - Comments accurately describe the fix rationale
src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx (Issue #438 fix)
- Properly imports and uses
useQueryClienthook from@tanstack/react-query - The component is rendered within
UsersPageClientwhich providesQueryClientProvidercontext - Query key
["users"]matches the existing pattern used throughout the codebase (verified inunified-edit-dialog.tsx,batch-edit-dialog.tsx) - Cache invalidation is correctly placed after successful deletion confirmation
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (existing patterns maintained)
- Type safety - Clean
- Documentation accuracy - Clean (comments match implementation)
- Test coverage - Existing tests are skipped (pending integration test refactor), but the fix logic is straightforward
- Code clarity - Good
Automated review by Claude AI
🧪 测试结果
总体结果: ✅ 所有测试通过 |
- Add form submission validation to ensure at least one key remains enabled - Disable Switch and show tooltip when trying to disable the last enabled key - Add i18n translations for all locales (zh-CN, en, ja, ru, zh-TW) Related to issue #431 and key deletion protection improvements
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Previously, when creating a new key through the edit user dialog with isEnabled set to false, the key would still be created as enabled. This was because: 1. addKey() function signature was missing the isEnabled parameter 2. The implementation hardcoded is_enabled to true 3. The frontend was passing the value but it was silently ignored Changes: - Add isEnabled parameter to addKey() function signature - Use data.isEnabled ?? true in createKey() call (defaults to true) - Pass isEnabled: key.isEnabled from unified-edit-dialog Fixes the issue where disabled keys were always created as enabled.
Previously, the error message for CANNOT_DISABLE_LAST_KEY had a hardcoded Chinese fallback text "无法禁用最后一个可用密钥". This violated i18n principles because: 1. All 5 languages (en/ja/ru/zh-CN/zh-TW) already have proper translations in errors.json 2. The hardcoded fallback would show Chinese to non-Chinese users if i18n system failed 3. It's inconsistent with other error messages that don't have fallbacks Changes: - Removed hardcoded fallback from batchUpdateKeys (2 occurrences) - Now relies on proper i18n translations from errors.json Related translations: - en: "Cannot disable the last active key..." - zh-CN: "无法禁用最后一个可用密钥..." - zh-TW: "無法禁用最後一個可用金鑰..."
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
2 participants
问题描述
本 PR 修复了两个与删除密钥相关的 bug：
Issue #431: 删除禁用 key 时的错误提示
重现步骤：
预期行为：只要用户还有其他启用状态的 key，就应该允许删除已禁用的 key
Issue #438: 删除 key 后前端未刷新
问题描述：点击删除后，前端页面没有实时刷新，然后点编辑用户里面已删除的 key 还存在，保存会提示 key 不存在
根本原因分析
#431 原因
在
src/actions/keys.ts:478-484的removeKey函数中，未区分要删除的 key 是启用还是禁用状态，只要用户的启用 key 数量 ≤1，就阻止删除任何 key（包括禁用的 key）。#438 原因
在
src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx的handleDeleteKey中，只调用了router.refresh()，但没有使 React Query 缓存失效，导致前端显示的仍是缓存的旧数据。修复方案
#431 修复
在
src/actions/keys.ts:478-488中添加条件判断：#438 修复
在
src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx中：useQueryClienthookqueryClienthandleDeleteKey中添加缓存失效逻辑：测试场景
代码质量
Closes #431
Closes #438