fix: audit log filters use search params #2769
Conversation
chronark
requested review from
perkinsjr,
mcstepp and
ogzhanolguncu
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
📝 WalkthroughWalkthroughThis pull request introduces significant changes to the audit logs functionality in the dashboard application. The modifications primarily focus on refactoring the audit log components, updating type definitions, and improving the user interface for selecting and filtering audit logs. The changes include renaming parameters, simplifying database queries, and restructuring components to provide a more streamlined approach to displaying and interacting with audit log data. Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/dashboard/app/(app)/audit/actions.ts (1)
Line range hint
55-67: Consider extracting the default bucket name to a constantThe hardcoded value "unkey_mutations" is used as a default. This should be centralized with other constants like
DEFAULT_BUCKET_NAMEthat's imported in filters/index.tsx.- bucketName: bucketParser.withDefault("unkey_mutations").parseServerSide(params.bucketName), + bucketName: bucketParser.withDefault(DEFAULT_BUCKET_NAME).parseServerSide(params.bucketName),
🧹 Nitpick comments (6)
apps/dashboard/app/(app)/audit/page.tsx (2)
1-5: Use caution with external icon library imports.
The usage of "@unkey/icons" is fine here, but be mindful of potential increases in bundle size if many icons are imported. Consider only importing the needed icons or treeshaking the library if possible.
40-51: Provide actionable suggestions in EmptyPlaceholder.
Currently, the "No logs" message suggests creating a key or resource. Consider linking directly to relevant pages, or providing a CTA button to guide users more effectively (e.g. "Create Key").apps/dashboard/app/(app)/audit/components/filters/bucket-select.tsx (1)
30-43: Refine labeling for clarity.
Currently, the label "Ratelimit: {b.name}" is shown for buckets other than "unkey_mutations". If these buckets no longer relate specifically to rate limiting, adapt the label to reflect their true purpose within the audit logs.- {b.name === "unkey_mutations" ? "System" : `Ratelimit: ${b.name}`} + {b.name === "unkey_mutations" ? "System" : `Audit Bucket: ${b.name}`}apps/dashboard/app/(app)/audit/actions.ts (1)
11-16: Consider adding an index for the orderBy clauseThe query now includes an orderBy clause on
createdAt. For optimal performance, ensure there's an index on this column.apps/dashboard/app/(app)/audit/components/filters/index.tsx (2)
19-19: Consider adding prop validationSince
selectedBucketNameis required and should match available bucket names, consider adding runtime validation.if (!workspace.auditLogBuckets.some(b => b.name === selectedBucketName)) { console.warn(`Invalid bucket name: ${selectedBucketName}`); }Also applies to: 23-23
Line range hint
29-48: Consider extracting event options to a constantThe hardcoded ratelimit events could be moved to a constant for better maintainability.
const RATELIMIT_EVENTS = [ { value: "ratelimit.success", label: "Ratelimit success" }, { value: "ratelimit.denied", label: "Ratelimit denied" } ] as const;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/dashboard/app/(app)/audit/[bucket]/components/filters/bucket-select.tsx(0 hunks)apps/dashboard/app/(app)/audit/[bucket]/page.tsx(0 hunks)apps/dashboard/app/(app)/audit/actions.ts(4 hunks)apps/dashboard/app/(app)/audit/components/filters/bucket-select.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/filters/filter.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/filters/index.tsx(3 hunks)apps/dashboard/app/(app)/audit/components/filters/root-key-filter.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/filters/user-filter.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/table/audit-log-table-client.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/table/table-details.tsx(1 hunks)apps/dashboard/app/(app)/audit/page.tsx(1 hunks)apps/dashboard/lib/trpc/routers/audit/fetch.ts(6 hunks)
💤 Files with no reviewable changes (2)
- apps/dashboard/app/(app)/audit/[bucket]/page.tsx
- apps/dashboard/app/(app)/audit/[bucket]/components/filters/bucket-select.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/dashboard/app/(app)/audit/components/filters/filter.tsx
🔇 Additional comments (12)
apps/dashboard/app/(app)/audit/page.tsx (2)
10-11: Be aware of forced dynamic rendering and edge runtime.
Using dynamic = "force-dynamic" and runtime = "edge" may impact SSR caching and performance. Verify that these align with your caching and scaling strategies.
Would you like to run a script to check how often these flags appear in your codebase and confirm usage intent?
30-39: Clarify table client dependencies.
While the conditional logic for rendering is straightforward, ensure that necessary filters or data are passed to the table if you plan to handle user interactions or advanced sorting/pagination later.
apps/dashboard/app/(app)/audit/components/filters/root-key-filter.tsx (1)
8-9: Reassess deleted and expired keys filtering.
Previously, keys might have been filtered to exclude deleted or expired ones. Now, the code no longer checks that. Confirm whether you still need to exclude those keys to avoid confusing or invalid results.
apps/dashboard/app/(app)/audit/components/filters/user-filter.tsx (1)
10-10: Ensure tenant ID logic is consistent.
The conditional check for tenantId.startsWith("user_") might skip revealing certain organization members if the ID is unexpected. Confirm that all valid organization IDs do not start with "user_".
apps/dashboard/app/(app)/audit/components/filters/bucket-select.tsx (1)
17-25: Verify default bucket logic.
The default value "unkey_mutations" might be a legacy placeholder. Ensure it remains relevant for new or reorganized audit log structures.
apps/dashboard/app/(app)/audit/actions.ts (1)
42-42: Verify the type change impact across the codebase
The type change from optional bucket to required bucketName in ParsedParams could affect existing code. Ensure all consumers handle this non-nullable requirement correctly.
Also applies to: 51-51
apps/dashboard/app/(app)/audit/components/filters/index.tsx (1)
3-3: LGTM! Type safety improvements
Good use of explicit type imports and schema definitions.
Also applies to: 15-15
apps/dashboard/app/(app)/audit/components/table/table-details.tsx (1)
6-6: LGTM! Import path simplified
The import path has been simplified by removing unnecessary parent directory traversal.
apps/dashboard/app/(app)/audit/components/table/audit-log-table-client.tsx (1)
22-22: LGTM: Parameter rename maintains functionality
The rename from bucket to bucketName is consistent with the schema changes in the TRPC router. The optional chaining and undefined fallback are handled correctly.
apps/dashboard/lib/trpc/routers/audit/fetch.ts (3)
14-14: LGTM: Schema update with proper default
The schema correctly defines bucketName with a default value of DEFAULT_BUCKET_NAME, maintaining backward compatibility.
27-27: LGTM: Consistent parameter destructuring
The destructuring pattern matches the updated schema definition.
132-133: Verify query performance with renamed field
The query condition has been updated to use bucketName, but we should verify the performance impact.
This moves the audit logs from
/audit/[bucket]to/audit?bucket=to make use of unified tooling via nuqsSummary by CodeRabbit
New Features
BucketSelectcomponent for selecting audit log buckets.AuditPagecomponent for displaying audit logs with enhanced data fetching and conditional rendering.Bug Fixes
Filtercomponent from "Events" to "Search".Refactor
buckettobucketNameacross various components and functions for clarity.Chores