fix: repair audit log action i18n labels#1041
Conversation
📝 WalkthroughWalkthrough将 auditLogs 的 Changes
Estimated code review effort🎯 3 (中等复杂度) | ⏱️ ~25 分钟 🚥 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 |
There was a problem hiding this comment.
Code Review
This pull request refactors the audit log translation structure by converting flat dotted keys into nested objects across all supported locales. It introduces helper functions for label retrieval and adds unit tests to ensure translation keys are synchronized and cover all emitted audit actions. The review feedback suggests improving the codebase's consistency by using English for comments, refining the test scanner to exclude test files, and enhancing regex patterns to support both single and double quotes.
| } | ||
|
|
||
| export function getAuditActionLabel(t: AuditLogTranslator, actionType: string): string { | ||
| // next-intl 缺失文案时会返回完整 key 字符串,因此这里必须先用 has() 判断。 |
There was a problem hiding this comment.
The comment is in Chinese, while the rest of the codebase and PR description are in English. For consistency and better maintainability in an international project, it's recommended to use English for code comments.
| // next-intl 缺失文案时会返回完整 key 字符串,因此这里必须先用 has() 判断。 | |
| // next-intl returns the full key string when a translation is missing, so we use has() to check existence and provide a cleaner fallback. |
|
|
||
| function collectEmittedAuditActionTypes(): string[] { | ||
| const srcRoot = path.join(REPO_ROOT, "src"); | ||
| const files = walkFiles(srcRoot).filter((file) => file.endsWith(".ts") || file.endsWith(".tsx")); |
There was a problem hiding this comment.
The file walker currently includes all .ts and .tsx files, including test files. If a test file contains dummy calls to emitActionAudit or createAuditLogAsync, it will incorrectly require those dummy keys to be present in the translation files. It's better to exclude test files from this scan.
| const files = walkFiles(srcRoot).filter((file) => file.endsWith(".ts") || file.endsWith(".tsx")); | |
| const files = walkFiles(srcRoot).filter((file) => (file.endsWith(".ts") || file.endsWith(".tsx")) && !file.includes(".test.") && !file.includes(".spec.")); |
| /emitActionAudit\(\s*\{[\s\S]*?action:\s*"([^"]+)"/g, | ||
| /createAuditLogAsync\(\s*\{[\s\S]*?actionType:\s*"([^"]+)"/g, |
There was a problem hiding this comment.
The regex patterns only match double-quoted strings. In many TypeScript projects, single quotes are also used. Improving the regex to support both quote types will make the test more robust against style changes.
| /emitActionAudit\(\s*\{[\s\S]*?action:\s*"([^"]+)"/g, | |
| /createAuditLogAsync\(\s*\{[\s\S]*?actionType:\s*"([^"]+)"/g, | |
| /emitActionAudit\(\s*\{[\s\S]*?action:\s*["']([^"']+)["']/g, | |
| /createAuditLogAsync\(\s*\{[\s\S]*?actionType:\s*["']([^"']+)["']/g, |
| act(() => { | ||
| root.render(node); | ||
| }); |
There was a problem hiding this comment.
Consider
@testing-library/react instead of raw createRoot
The test uses createRoot + act directly for rendering and relies on inspecting document.body.textContent. If @testing-library/react is already a project dependency, render + screen.getByText would give more targeted assertions, cleaner teardown, and easier-to-read test output — without the manual container.remove() teardown dance.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsx
Line: 91-93
Comment:
**Consider `@testing-library/react` instead of raw `createRoot`**
The test uses `createRoot` + `act` directly for rendering and relies on inspecting `document.body.textContent`. If `@testing-library/react` is already a project dependency, `render` + `screen.getByText` would give more targeted assertions, cleaner teardown, and easier-to-read test output — without the manual `container.remove()` teardown dance.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| test("store actions in nested objects instead of dotted leaf keys", () => { | ||
| for (const locale of LOCALES) { | ||
| const dottedKeys = getDirectDottedKeys(readLocaleActionTree(locale)); | ||
| expect(dottedKeys, `${locale} should not contain dotted direct keys under actions`).toEqual( | ||
| [] | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| test("stay in sync across locales and cover every emitted audit action type", () => { | ||
| const canonicalKeys = flattenLeafKeys(readLocaleActionTree("en")).sort(); | ||
| const emittedKeys = collectEmittedAuditActionTypes(); |
There was a problem hiding this comment.
Sync test direction only catches missing translations, not extra ones
expect(canonicalKeys).toEqual(emittedKeys) does enforce bidirectional coverage, which is good. However, collectEmittedAuditActionTypes only matches string-literal arguments to emitActionAudit / createAuditLogAsync. Any action type constructed dynamically (e.g. via template literal or concatenation) would be silently missed, leaving the test with a false sense of full coverage. A comment noting this limitation would help future maintainers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/i18n/audit-log-actions-messages.test.ts
Line: 79-90
Comment:
**Sync test direction only catches missing translations, not extra ones**
`expect(canonicalKeys).toEqual(emittedKeys)` does enforce bidirectional coverage, which is good. However, `collectEmittedAuditActionTypes` only matches string-literal arguments to `emitActionAudit` / `createAuditLogAsync`. Any action type constructed dynamically (e.g. via template literal or concatenation) would be silently missed, leaving the test with a false sense of full coverage. A comment noting this limitation would help future maintainers.
How can I resolve this? If you propose a fix, please make it concise.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/app/[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx (1)
24-24: 请使用@/路径别名导入共享 helper。新增 import 仍是相对路径,和当前仓库规范不一致。
建议修改
-import { getAuditActionLabel, getAuditCategoryLabel } from "./audit-log-labels"; +import { + getAuditActionLabel, + getAuditCategoryLabel, +} from "@/app/[locale]/dashboard/audit-logs/_components/audit-log-labels";As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use path alias@/mapped to ./src/ for imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx at line 24, The import for getAuditActionLabel and getAuditCategoryLabel uses a relative path; update it to use the project path alias by importing these helpers via '@/...' instead of a relative path so it follows the repository convention (replace the current "./audit-log-labels" import with an alias import that resolves to the same module containing getAuditActionLabel and getAuditCategoryLabel).src/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsx (1)
63-63: 请用@/路径别名导入被测组件。新增测试里的组件 import 也应遵循仓库统一 import 规范。
建议修改
-import { AuditLogDetailSheet } from "./audit-log-detail-sheet"; +import { AuditLogDetailSheet } from "@/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet";As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use path alias@/mapped to ./src/ for imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsx at line 63, 测试文件中直接使用相对路径导入组件,应改为使用仓库约定的 `@/` 路径别名;在 src/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsx 中把 import { AuditLogDetailSheet } from "./audit-log-detail-sheet"; 替换为使用 `@/` 别名的导入(导入 AuditLogDetailSheet 组件),确保引入路径以 `@/` 开头并映射到 ./src/ 下,使测试遵循项目统一的 import 规范。src/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.tsx (1)
12-12: 请把新增 helper import 改成@/路径别名。这里新增的相对路径导入和仓库 import 规范不一致。
建议修改
-import { getAuditActionLabel, getAuditCategoryLabel } from "./audit-log-labels"; +import { + getAuditActionLabel, + getAuditCategoryLabel, +} from "@/app/[locale]/dashboard/audit-logs/_components/audit-log-labels";As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use path alias@/mapped to ./src/ for imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.tsx at line 12, The import for helpers getAuditActionLabel and getAuditCategoryLabel uses a relative path; change it to use the repository path alias (@" mapped to ./src/) so it follows project conventions — replace the relative import from "./audit-log-labels" with an alias-based import like "@/app/[locale]/dashboard/audit-logs/_components/audit-log-labels" (keeping the same exported symbols getAuditActionLabel and getAuditCategoryLabel) to conform to the import rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/app/`[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsx:
- Line 63: 测试文件中直接使用相对路径导入组件,应改为使用仓库约定的 `@/` 路径别名;在
src/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsx
中把 import { AuditLogDetailSheet } from "./audit-log-detail-sheet"; 替换为使用 `@/`
别名的导入(导入 AuditLogDetailSheet 组件),确保引入路径以 `@/` 开头并映射到 ./src/ 下,使测试遵循项目统一的 import
规范。
In
`@src/app/`[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.tsx:
- Line 12: The import for helpers getAuditActionLabel and getAuditCategoryLabel
uses a relative path; change it to use the repository path alias (@" mapped to
./src/) so it follows project conventions — replace the relative import from
"./audit-log-labels" with an alias-based import like
"@/app/[locale]/dashboard/audit-logs/_components/audit-log-labels" (keeping the
same exported symbols getAuditActionLabel and getAuditCategoryLabel) to conform
to the import rules.
In `@src/app/`[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx:
- Line 24: The import for getAuditActionLabel and getAuditCategoryLabel uses a
relative path; update it to use the project path alias by importing these
helpers via '@/...' instead of a relative path so it follows the repository
convention (replace the current "./audit-log-labels" import with an alias import
that resolves to the same module containing getAuditActionLabel and
getAuditCategoryLabel).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 50c68550-1c72-4925-b8e3-6647cb0de312
📒 Files selected for processing (10)
messages/en/auditLogs.jsonmessages/ja/auditLogs.jsonmessages/ru/auditLogs.jsonmessages/zh-CN/auditLogs.jsonmessages/zh-TW/auditLogs.jsonsrc/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsxsrc/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.tsxsrc/app/[locale]/dashboard/audit-logs/_components/audit-log-labels.tssrc/app/[locale]/dashboard/audit-logs/_components/audit-logs-view.tsxtests/unit/i18n/audit-log-actions-messages.test.ts
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. Changes restructure auditLogs.actions into nested objects and update the audit log UI to resolve dotted action keys via t.has() before translating. New unit tests guard against locale drift and missing action translations.
PR Size: L
- Lines changed: 617
- Files changed: 10
- Split suggestions: (1) i18n message key reshaping + UI helper adoption, (2) tests (UI translation + locale/key sync)
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.
- Reviewed PR
#1041(“fix: repair audit log action i18n labels”) against the diff ingh pr diff. - Computed size as L (
468additions +149deletions =617lines;10files) and applied labelsize/L. - Posted the required summary review via
gh pr review --comment; no inline comments were submitted because no diff-line issues met the >=80 confidence threshold.
There was a problem hiding this comment.
Code Review Summary
Clean, well-scoped i18n bug fix. Restructures audit log action translations from flat dotted keys to nested objects compatible with next-intl resolution, centralizes translation helpers with proper t.has() guards, and adds regression tests for both UI rendering and cross-locale drift detection.
PR Size: L
- Lines changed: 617 (468 additions, 149 deletions)
- Files changed: 10
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 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Coverage
- Logic and correctness - Verified action key resolution, fallback behavior, and cross-locale consistency across all 5 locales
- Security (OWASP Top 10) - No security surface; UI-only i18n rendering
- Error handling -
t.has()guard correctly replaces prior try/catch; no silent failures - Type safety -
AuditLogTranslatorstructural type is compatible withuseTranslationsreturn - Documentation accuracy - Comment in
audit-log-labels.tsaccurately explainsnext-intlkey-resolution behavior - Test coverage - Component test covers translated and fallback paths; drift test validates all emitted action types (verified via source audit: login, user, provider, provider_group, system_settings, key, notification, sensitive_word, model_price) match i18n keys across all 5 locales
- Code clarity - Shared helpers eliminate duplicated try/catch blocks in both UI surfaces
Automated review by Claude AI
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/app/[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx (2)
255-260: 细节建议:避免对同一 action 计算两次标签。
actionLabel(log.actionType)在title和内容中分别调用了一次,每次都会走一次t.has+t查找。可在行渲染开头计算一次局部变量后复用,虚拟列表下对成千上万行的重复查找可略微减负。非阻塞。♻️ 建议改动
const operator = log.operatorUserName ?? t("adminTokenOperator"); + const actionText = actionLabel(log.actionType); ... <div className="flex-[1.2] min-w-[140px] font-mono text-xs px-1.5 truncate" - title={actionLabel(log.actionType)} + title={actionText} > - {actionLabel(log.actionType)} + {actionText} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx around lines 255 - 260, 在渲染行时避免重复调用 actionLabel(log.actionType):在该行渲染开始处为 actionLabel(log.actionType) 计算一次局部变量(例如 const label = actionLabel(log.actionType)),然后在 title 和内容处复用该变量,替换掉两处对 actionLabel 的重复调用(参见组件中的 actionLabel 和 log 用法);这样在虚拟列表大量行时能减少重复的 t.has/t 查找开销。
115-121: LGTM,可选小优化。
categoryLabel/actionLabel目前只是对 helper 的薄封装,直接在渲染处调用getAuditCategoryLabel(t, ...)/getAuditActionLabel(t, ...)也可(或将其用useCallback记忆化以与组件内其他回调保持一致)。当前写法无功能问题,属于可延后的代码整洁性微调。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx around lines 115 - 121, The wrappers categoryLabel and actionLabel are thin passthroughs around getAuditCategoryLabel(t, ...) and getAuditActionLabel(t, ...); replace their usage in the render with direct calls to getAuditCategoryLabel(t, cat) and getAuditActionLabel(t, actionType) or, if you prefer to keep named callbacks, memoize them with React's useCallback (e.g., const categoryLabel = useCallback((cat) => getAuditCategoryLabel(t, cat), [t]) and similarly for actionLabel) so the code is either simplified or consistent with other memoized callbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/`[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx:
- Around line 255-260: 在渲染行时避免重复调用 actionLabel(log.actionType):在该行渲染开始处为
actionLabel(log.actionType) 计算一次局部变量(例如 const label =
actionLabel(log.actionType)),然后在 title 和内容处复用该变量,替换掉两处对 actionLabel 的重复调用(参见组件中的
actionLabel 和 log 用法);这样在虚拟列表大量行时能减少重复的 t.has/t 查找开销。
- Around line 115-121: The wrappers categoryLabel and actionLabel are thin
passthroughs around getAuditCategoryLabel(t, ...) and getAuditActionLabel(t,
...); replace their usage in the render with direct calls to
getAuditCategoryLabel(t, cat) and getAuditActionLabel(t, actionType) or, if you
prefer to keep named callbacks, memoize them with React's useCallback (e.g.,
const categoryLabel = useCallback((cat) => getAuditCategoryLabel(t, cat), [t])
and similarly for actionLabel) so the code is either simplified or consistent
with other memoized callbacks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a22307de-37a0-42c2-9c55-0aa6bb594aea
📒 Files selected for processing (3)
src/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsxsrc/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.tsxsrc/app/[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsx
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Summary
auditLogs.actionsto nested locale objects thatnext-intlcan resolvet.has(...)before translatingVerification
bun run buildbun run lintbun run lint:fixbun run typecheckbun run testbunx vitest run 'src/app/[locale]/dashboard/audit-logs/_components/audit-log-detail-sheet.test.tsx' 'tests/unit/i18n/audit-log-actions-messages.test.ts'UI Verification
AuditLogDetailSheetrendering withlogin.successsample data and production locale messages.DSN/REDIS_URLruntime configured.Greptile Summary
This PR fixes a bug where
next-intlcould not resolve audit-log action labels stored as flat dotted keys (e.g."login.success": "...") by restructuring all five locale files into proper nested objects and routing both UI surfaces through a new sharedgetAuditActionLabel/getAuditCategoryLabelhelper that guards witht.has()before translating. New tests cover correct translation, raw-action fallback, and cross-locale key sync.Confidence Score: 5/5
This PR is safe to merge — the fix is correct, well-tested, and all five locales are consistent.
The root cause (flat dotted keys unresolvable by next-intl) is addressed correctly by nesting the messages and using t.has() guards. All five locale files are updated consistently, both UI surfaces route through the shared helper, and new tests cover translation, fallback, and cross-locale drift. No P0 or P1 findings were identified.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[actionType from DB] --> B[getAuditActionLabel] B --> C[Build key with actions prefix] C --> D{t.has check} D -- found --> E[Return translated string] D -- missing --> F[Return raw actionType] E --> G[UI renders label] F --> GReviews (2): Last reviewed commit: "fix: align audit log imports with alias ..." | Re-trigger Greptile