fix: date select issue on logs page #2764
Conversation
ogzhanolguncu
requested review from
perkinsjr,
chronark,
mcstepp and
MichaelUnkey
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive enhancement to the audit log functionality in the dashboard application. The changes span multiple files and components, focusing on creating a more robust and user-friendly audit log experience. The implementation includes new components for displaying log details, utility functions for event type categorization, and improvements to data fetching and pagination mechanisms for audit logs. Changes
Sequence DiagramsequenceDiagram
participant User
participant AuditTable
participant AuditLogRouter
participant Database
User->>AuditTable: Request audit logs
AuditTable->>AuditLogRouter: Fetch logs with parameters
AuditLogRouter->>Database: Query audit logs
Database-->>AuditLogRouter: Return log entries
AuditLogRouter-->>AuditTable: Provide log data with pagination
AuditTable->>User: Display audit log table
Possibly related PRs
Suggested Labels
Suggested Reviewers
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: 5
🧹 Nitpick comments (28)
apps/dashboard/app/(app)/logs/components/filters/components/custom-date-filter.tsx (1)
Line range hint
43-71: Consider adding date/time validation checksThe
handleFinalDatefunction could benefit from additional validation:
- Ensure end time is after start time
- Consider adding validation for dates in the past if that's a business requirement
Example validation:
const handleFinalDate = (interimDate: DateRange | undefined) => { setOpen(false); if (interimDate?.from) { let mergedFrom = setHours(interimDate.from, Number(startTime.HH)); mergedFrom = setMinutes(mergedFrom, Number(startTime.mm)); mergedFrom = setSeconds(mergedFrom, Number(startTime.ss)); let mergedTo: Date; if (interimDate.to) { mergedTo = setHours(interimDate.to, Number(endTime.HH)); mergedTo = setMinutes(mergedTo, Number(endTime.mm)); mergedTo = setSeconds(mergedTo, Number(endTime.ss)); } else { mergedTo = setHours(interimDate.from, Number(endTime.HH)); mergedTo = setMinutes(mergedTo, Number(endTime.mm)); mergedTo = setSeconds(mergedTo, Number(endTime.ss)); } + // Validate that end time is after start time + if (mergedTo <= mergedFrom) { + // Handle invalid time range (e.g., show error message) + return; + } setFinalDate({ from: mergedFrom, to: mergedTo }); setSearchParams({ startTime: mergedFrom.getTime(), endTime: mergedTo.getTime(), }); }apps/dashboard/app/(app)/logs/components/filters/components/time-split.tsx (1)
Line range hint
143-182: Consider enhancing input validationWhile the function validates numeric ranges, it doesn't prevent non-numeric input. Consider adding input sanitization.
Example enhancement:
function handleOnChange(value: string, valueType: TimeType) { + // Only allow numeric input + if (!/^\d*$/.test(value)) { + return; + } + const payload = { HH: time.HH, mm: time.mm, ss: time.ss, };apps/dashboard/lib/trpc/routers/audit/fetch.ts (5)
26-38: Ensure proper error management for workspace retrieval.
A fallback or custom logging before throwing TRPCError could help in debugging issues where the workspace retrieval fails. Consider capturing context like user ID andbucketin a log statement for operational monitoring.
46-49: Revisit retention strategy for different subscription plans.
The code currently defaults to 30 days retention for the "free" plan or 90 days for others. Validate that these defaults match business requirements or whether they should be configurable.
53-72: Add an index or partitioning strategy for large log tables.
Your query filters logs by time, event, and actorId. If the table grows substantially, consider adding a composite index or partitioning strategy on these columns to keep query times fast.
74-79: Consider a more descriptive error message.
“Audit log bucket not found” could provide more actionable information for users or support staff, e.g., instructing them to check whether the bucket name is correct or has been deleted.
81-86: Document slicing logic.
The comment clarifies the reason for fetching limit + 1 logs to determine if more pages exist. This is a subtle but important detail. Consider expanding documentation to ensure maintainers understand this approach.apps/dashboard/lib/utils.ts (1)
31-130: Potential memory leak if “func” captures large references.
While unlikely, note that if “func” references large closures, they’ll stay in memory untilthrottled.cancel()is called or the throttle is flushed. Document usage best practices in code comments to avoid misuse.internal/db/src/schema/audit_logs.ts (2)
Line range hint
39-46: Workspace relation naming.
You’ve set a custom “relationName: workspace_audit_log_bucket_relation”. This is fine but ensure it’s consistently recognized across your codebase.
Line range hint
48-80: Confirm consistent plan for timestamps.
“time” is a bigint set to Date.now(). For some DB systems or potential migrations, consider storing UTC timestamps or standard DB date/time fields to facilitate time-based indexing more readily.apps/dashboard/app/(app)/audit/[bucket]/components/table/index.tsx (2)
27-36: Potential performance overhead in useEffect.
The effect sets the cursor if and only if we haveinitialDataand nocursorId. For extremely large datasets, consider deferring or optimizing. For now, it should be fine.
118-127: Add optional load more indicator.
If many logs are returned, showing a spinner or “Loading more…” feedback near the bottom can boost user experience.apps/dashboard/components/virtual-table.tsx (8)
2-7: Consider adding a descriptive comment for new imports.
Importing the "throttle" method from "@/lib/utils" is justified; however, a quick inline comment or JSDoc-style note explaining its usage can help orient future maintainers.
38-39: Magic numbers for constants.
"350" is a suitable delay, but consider adding a short comment or referencing a shared constants file for reusability. Similarly, document the rationale for "HEADER_HEIGHT = 40" to ease future maintenance.
64-69: Scroll lock caution.
Locking scrolling in "#layout-wrapper" can occasionally cause unexpected user experience if the user tries to scroll outside the intended container. Confirm this behavior is desired on smaller screens or in complex nested layouts.
70-98: ResizeObserver usage.
Great approach for dynamic sizing! Keep in mind older browsers do not fully support ResizeObserver. If wide browser coverage is required, consider a fallback or polyfill.
99-117: Trailing throttle consideration.
By specifying "{ leading: true, trailing: false }", calls at the tail end of user scrolling may be missed if the user stops scrolling abruptly. Consider enabling trailing calls to capture the final scroll position.
162-167: Animated pulse accessibility.
The loading row uses "animate-pulse," which is visually appealing. For accessibility, consider adding "aria-busy" or relevant roles to communicate a loading state to screen readers.
173-189: Reusable table header.
"TableHeader" is a neat extraction. If multiple tables share a similar structure, consider further modularizing it into a shared component or hooking it into a design system to ensure consistent UI across the app.
295-295: CSS class layering.
The stacked classes "text-[13px] leading-[14px] ..." are very specific. Make sure these inline utilities don’t conflict with global design tokens or theming, or consider factoring them into a shared utility class.apps/dashboard/app/(app)/audit/[bucket]/components/log-header.tsx (1)
11-26: Minimal styling with flexible layout.
The header layout looks simple and flexible, but confirm that icons or text do not overflow on smaller screens. If there's a risk of text-truncation, consider responsive breakpoints.apps/dashboard/app/(app)/audit/[bucket]/query-state.ts (2)
17-24: Consider adding validation for query parameters.While the basic parameter parsing is in place, consider adding validation for:
- Maximum array lengths for events, users, and rootKeys to prevent excessive query sizes
- Time range validation for cursorTime
export const auditLogParamsPayload = { bucket: parseAsString, - events: parseAsArrayOf(parseAsString).withDefault([]), + events: parseAsArrayOf(parseAsString).withDefault([]).withValidation((arr) => arr.length <= 50), - users: parseAsArrayOf(parseAsString).withDefault([]), + users: parseAsArrayOf(parseAsString).withDefault([]).withValidation((arr) => arr.length <= 50), - rootKeys: parseAsArrayOf(parseAsString).withDefault([]), + rootKeys: parseAsArrayOf(parseAsString).withDefault([]).withValidation((arr) => arr.length <= 50), cursorTime: parseAsInteger, cursorId: parseAsString, };
26-41: Consider adding error handling for setCursor.The
setCursorfunction should handle potential edge cases when setting cursor values.const setCursor = (cursor?: Cursor) => { + if (cursor && (isNaN(cursor.time) || !cursor.id)) { + console.warn('Invalid cursor values provided'); + return; + } setSearchParams({ cursorTime: cursor?.time ?? null, cursorId: cursor?.id ?? null, }); };apps/dashboard/app/(app)/audit/[bucket]/components/table/constants.ts (1)
1-1: Consider making DEFAULT_FETCH_COUNT configurable.The hard-coded fetch count might need adjustment based on performance metrics or user requirements.
Consider moving this to an environment variable or configuration file:
export const DEFAULT_FETCH_COUNT = process.env.AUDIT_LOG_FETCH_COUNT ? parseInt(process.env.AUDIT_LOG_FETCH_COUNT, 10) : 50;apps/dashboard/app/(app)/audit/[bucket]/components/table/table-details.tsx (1)
65-68: Consider adding prop types validation for memoized component.The memoization comparison could be more robust with proper type checking.
export const LogDetails = memo( _LogDetails, - (prev, next) => prev.log?.auditLog.id === next.log?.auditLog.id, + (prev, next) => { + if (!prev.log && !next.log) return true; + if (!prev.log || !next.log) return false; + return prev.log.auditLog.id === next.log.auditLog.id; + }, );apps/dashboard/app/(app)/audit/[bucket]/components/table/columns.tsx (1)
26-44: Simplify nested conditional renderingThe actor rendering logic uses nested ternaries which could be simplified for better maintainability.
Consider extracting the logic into a separate component or using a mapping:
const actorIcons = { user: null, key: <KeySquare className="w-4 h-4" />, default: <FunctionSquare className="w-4 h-4" /> }; const getActorDisplay = (log: Data) => { if (log.auditLog.actor.type === "user" && log.user) { return { icon: actorIcons.user, text: `${log.user.firstName ?? ""} ${log.user.lastName ?? ""}` }; } return { icon: actorIcons[log.auditLog.actor.type] ?? actorIcons.default, text: log.auditLog.actor.id }; };apps/dashboard/app/(app)/audit/[bucket]/components/log-footer.tsx (1)
37-58: Consider extracting actor rendering logic to a separate componentThe actor details rendering logic is complex and could benefit from being extracted into a dedicated component for better maintainability.
Consider creating an
ActorDetailscomponent:type ActorDetailsProps = { actor: { type: string; id: string }; user?: { imageUrl?: string; username?: string; firstName?: string; lastName?: string }; }; const ActorDetails: React.FC<ActorDetailsProps> = ({ actor, user }) => { if (actor.type === "user" && user?.imageUrl) { return ( <div className="flex justify-end items-center w-full gap-2 max-sm:m-0 max-sm:gap-1 max-sm:text-xs md:flex-grow"> <Avatar className="w-6 h-6"> <AvatarImage src={user.imageUrl} /> <AvatarFallback>{user?.username?.slice(0, 2)}</AvatarFallback> </Avatar> <span className="text-sm text-content whitespace-nowrap"> {`${user?.firstName ?? ""} ${user?.lastName ?? ""}`} </span> </div> ); } const Icon = actor.type === "key" ? KeySquare : FunctionSquare; return ( <div className="flex items-center w-full gap-2 max-sm:m-0 max-sm:gap-1 max-sm:text-xs md:flex-grow"> <Icon className="w-4 h-4" /> <span className="font-mono text-xs text-content">{actor.id}</span> </div> ); };apps/dashboard/app/(app)/audit/[bucket]/filter.tsx (1)
Line range hint
85-91: Potential accessibility improvement neededThe div wrapping the CommandItem is keyboard-accessible but could benefit from proper ARIA attributes and role definition.
Add appropriate ARIA attributes:
<div key={option.value} + role="option" + aria-selected={isSelected} + tabIndex={0} onClick={() => handleSelection(option.value, isSelected)} onKeyDown={(e) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/dashboard/app/(app)/audit/[bucket]/components/log-footer.tsx(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/components/log-header.tsx(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/components/table/columns.tsx(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/components/table/constants.ts(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/components/table/index.tsx(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/components/table/table-details.tsx(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/components/table/types.ts(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/components/table/utils.ts(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/filter.tsx(2 hunks)apps/dashboard/app/(app)/audit/[bucket]/page.tsx(9 hunks)apps/dashboard/app/(app)/audit/[bucket]/query-state.ts(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/row.tsx(0 hunks)apps/dashboard/app/(app)/layout.tsx(1 hunks)apps/dashboard/app/(app)/logs/components/filters/components/custom-date-filter.tsx(1 hunks)apps/dashboard/app/(app)/logs/components/filters/components/time-split.tsx(4 hunks)apps/dashboard/app/(app)/logs/components/table/log-details/components/request-response-details.tsx(1 hunks)apps/dashboard/app/(app)/logs/page.tsx(1 hunks)apps/dashboard/components/virtual-table.tsx(8 hunks)apps/dashboard/lib/trpc/routers/audit/fetch.ts(1 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/utils.ts(1 hunks)internal/db/src/schema/audit_logs.ts(1 hunks)packages/api/package.json(1 hunks)packages/hono/package.json(1 hunks)packages/nextjs/package.json(1 hunks)packages/ratelimit/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/app/(app)/audit/[bucket]/row.tsx
✅ Files skipped from review due to trivial changes (5)
- packages/ratelimit/package.json
- packages/hono/package.json
- apps/dashboard/app/(app)/layout.tsx
- packages/api/package.json
- packages/nextjs/package.json
🔇 Additional comments (34)
apps/dashboard/app/(app)/logs/components/filters/components/custom-date-filter.tsx (1)
13-13: LGTM: Import statement updated correctly
The change from default import to named import aligns with the updated export in the TimeSplitInput component.
apps/dashboard/app/(app)/logs/components/filters/components/time-split.tsx (2)
27-27: LGTM: Export declaration updated correctly
The change to named export follows modern React practices and matches the import in custom-date-filter.tsx.
231-231: LGTM: Event handler cleanup
The onBlur handler has been simplified by directly referencing the function instead of using arrow functions, which is a good optimization.
Also applies to: 253-253
apps/dashboard/lib/trpc/routers/audit/fetch.ts (3)
7-19: Validate complex default array handling.
Currently, arrays default to empty arrays (e.g., events, users, rootKeys). If these inputs come from dynamically generated data, confirm that empty arrays are indeed safe defaults, especially for search or filter functionalities.
Would you like a script that checks downstream usage to confirm that empty arrays won’t cause unexpected behavior?
21-23: Good integration of rate-limited procedure.
Leveraging the rateLimitedProcedure for the fetchAuditLog is a neat way to reduce overhead from excessive calls. This approach appears sound and aligns with best practices.
87-96: Well-structured pagination return.
Returning both items and nextCursor in a single object is concise and user-friendly for the InfiniteQuery usage. Good job!
apps/dashboard/lib/utils.ts (1)
26-29: Throttling options align with standard approaches.
Exposing optional leading/trailing flags is excellent for flexible usage.
internal/db/src/schema/audit_logs.ts (3)
Line range hint 17-38: Validate name uniqueness at the application layer.
The unique index ensures no duplicate (workspaceId, name) pairs. Confirm that the application surfaces a clear error when collisions occur so users can fix naming conflicts quickly.
Line range hint 82-115: Composite primary key approach.
Having “auditLogId” + “id” as a composite primary key is a solid approach for guaranteeing uniqueness. Be mindful if you plan to reference “id” alone in the future.
Line range hint 117-120: Clear typed exports.
Exporting “SelectAuditLog” and “SelectAuditLogTarget” fosters strongly typed usage in the codebase, reducing potential mismatches.
apps/dashboard/app/(app)/audit/[bucket]/components/table/index.tsx (6)
17-25: Offer a fallback for empty user objects.
The component expects a record of user objects keyed by ID. If users[l.actorId] is missing, you provide “undefined.” Confirm that the UI gracefully handles missing user data in all cases.
38-80: Ensure no concurrency issues with infinite queries.
When fetchNextPage is called repeatedly in rapid succession, concurrency can be tricky. Currently, the code looks robust, but ensure the consumer doesn’t inadvertently cause race conditions.
81-116: Flattened data transformation is well-organized.
The approach for mapping each log into a user-augmented structure is straightforward and maintainable. Good job!
129-136: Event-driven styling is helpful.
Color-coded rows for create/update/delete events are a nice UX touch.
152-166: Robust error fallback.
Rendering the EmptyPlaceholder with specific instructions is appreciated from a user’s perspective.
168-186: Integration with VirtualTable.
Props like onRowClick and renderDetails are nicely integrated, making the table more versatile. Great job.
apps/dashboard/components/virtual-table.tsx (2)
30-30: Property name alignment.
The new property "isFetchingNextPage" is clear, but verify that this naming convention matches similar flags in the rest of the codebase for consistency (e.g., "isLoadingNextPage" or "loadingNextPage").
130-144: Ensure robust boundary checks.
Infinite scroll logic checks if the last item is near the bottom to load more data. Confirm that boundary conditions (e.g., small dataset, or usage in different containers) are tested to avoid UI freeze or redundant calls.
apps/dashboard/app/(app)/audit/[bucket]/components/table/types.ts (1)
1-30: Validate the wide scope of the "Data" type.
"Data" is quite comprehensive. Verify fields like "meta: unknown" do not mask crucial properties. If possible, refine its type for stronger runtime safety and type hints.
apps/dashboard/app/(app)/audit/[bucket]/components/log-header.tsx (1)
1-3: Leverage consistent UI library nomenclature.
Although "Badge" and "Button" are self-explanatory, ensuring consistent import paths and matching naming in the rest of the code fosters maintainability.
apps/dashboard/app/(app)/audit/[bucket]/query-state.ts (1)
3-15: LGTM! Well-structured type definitions.
The types are clearly defined and properly capture the structure of audit log query parameters.
apps/dashboard/app/(app)/audit/[bucket]/components/table/constants.ts (1)
3-59: Verify event group completeness and consider maintainability improvements.
The event groups are well-organized but could benefit from some improvements:
- Some events might be missing corresponding operations (e.g.,
secret.deleteis missing) - Consider using an enum or const object for event names to prevent typos
✅ Verification successful
Event groups match schema definition, but consider type safety improvements
After verifying against the schema definition in internal/schema/src/auditlog.ts, all events in the groups are properly defined in the unkeyAuditLogEvents enum. The missing secret.delete event was a false concern as it's not part of the official schema.
Recommendations for improvement:
- Import and use the
unkeyAuditLogEventsenum from@unkey/schemainstead of maintaining a separate string array to ensure type safety - Consider adding JSDoc comments to document the purpose of each event group
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for audit log event emitting code to verify completeness
rg -t typescript -t javascript "emit.*audit.*event" -A 2
Length of output: 93
Script:
#!/bin/bash
# Search for audit event patterns in the codebase
rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' -t ts -t js 'emitAuditEvent|auditEvent|AUDIT_EVENT' -A 2
Length of output: 115
Script:
#!/bin/bash
# Search for event type definitions
fd -e ts -e tsx -e js -e jsx --exec rg -l 'type.*Event.*=' {}
Length of output: 123
Script:
#!/bin/bash
# Look for event-related constants or enums
fd -e ts -e tsx -e js -e jsx --exec rg -l 'export (const|enum).*Event' {}
Length of output: 226
Script:
#!/bin/bash
# Search for any files containing audit-related terms
fd -e ts -e tsx -e js -e jsx --exec rg -l 'audit|event' {}
Length of output: 10048
Script:
#!/bin/bash
# Let's check the audit log schema and event types
cat ./internal/schema/src/auditlog.ts
Length of output: 2462
Script:
#!/bin/bash
# Check the event types in the events package
cat ./internal/events/src/index.ts
Length of output: 719
Script:
#!/bin/bash
# Check audit-related code in the API
cat ./apps/api/src/pkg/audit.ts
Length of output: 3919
apps/dashboard/app/(app)/audit/[bucket]/components/table/table-details.tsx (1)
23-25: Consider adjusting debounce delay based on performance metrics.
The 150ms debounce delay might need tuning based on user experience and performance metrics.
✅ Verification successful
Let me search for the constant definition to understand the current delay value.
Debounce delay is consistent with existing implementations
The current 150ms debounce delay for panel width resizing is already consistent across the codebase, being used in both audit and logs components. The only other debounce implementation (350ms) serves a different purpose for query param reset. Given this consistency and the specific use case for UI resizing, the current delay is appropriate.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other debounce implementations in the codebase for consistency
rg "useDebounceCallback.*\d+" -g "*.{ts,tsx}"
Length of output: 175
Script:
#!/bin/bash
# Search for PANEL_WIDTH_SET_DELAY constant
rg "PANEL_WIDTH_SET_DELAY" -A 1 -g "*.{ts,tsx}"
Length of output: 764
apps/dashboard/app/(app)/logs/page.tsx (1)
Line range hint 1-89: Verify dependency on PR #2757
The PR objectives mention this should be merged after PR #2757, but there's no clear dependency in the code changes. Please clarify the relationship between these PRs.
apps/dashboard/app/(app)/logs/components/table/log-details/components/request-response-details.tsx (1)
70-70: LGTM! UI refinements look good
The styling changes improve consistency with a uniform bottom border and better label padding.
Also applies to: 75-75
apps/dashboard/app/(app)/audit/[bucket]/components/table/columns.tsx (1)
11-19: Consider flexible width for timestamp column
The fixed width of "130px" might truncate timestamps in certain formats or locales.
Consider using a minimum width instead:
- width: "130px",
+ width: "min-content",
+ minWidth: "130px",apps/dashboard/app/(app)/audit/[bucket]/components/log-footer.tsx (1)
19-21: Verify TimestampInfo component behavior with audit log timestamps
The TimestampInfo component is used for displaying the audit log time. Ensure it handles all possible timestamp formats from the audit log correctly.
✅ Verification successful
Let me check the implementation of TimestampInfo to verify its behavior.
Let me try a different approach to find the TimestampInfo component implementation.
Let me check the timestamp formatter functions used in the component to ensure they handle all formats correctly.
Let me check the isUnixMicro and unixMicroToDate helper functions to complete the verification.
TimestampInfo component correctly handles audit log timestamps
The implementation is robust and handles all timestamp formats correctly:
- Supports both Unix microseconds timestamps (16-digit) and standard date strings
- Provides local, UTC, and relative time formatting using date-fns
- Consistent formatting pattern "dd MMM HH:mm:ss" across all usages in logs and audit views
- Proper conversion of microseconds to standard date format
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other TimestampInfo usages to verify consistent formatting
rg -l "TimestampInfo" | xargs rg "TimestampInfo.*value="
Length of output: 476
Script:
#!/bin/bash
# Search for TimestampInfo component implementation
ast-grep --pattern 'export $_ function TimestampInfo($_) {
$$$
}'
# Also check for any type definitions related to the component
rg "type.*TimestampInfo" -A 3
Length of output: 101
Script:
#!/bin/bash
# Find the file containing TimestampInfo component
fd TimestampInfo
# Search for the component implementation with more context
rg -B 2 -A 10 "export.*TimestampInfo"
Length of output: 972
Script:
#!/bin/bash
# Search for timestamp formatter implementations
rg -B 2 -A 5 "timestampLocalFormatter|timestampUtcFormatter|timestampRelativeFormatter"
# Also check for any test files related to timestamp formatting
fd "timestamp.*test"
Length of output: 3236
Script:
#!/bin/bash
# Search for the timestamp helper functions
rg -B 2 -A 5 "isUnixMicro|unixMicroToDate"
# Also check for any imports of date formatting libraries
rg "from ['\"](date-fns|dayjs|moment)" apps/dashboard/components/timestamp-info.tsx
Length of output: 2668
apps/dashboard/lib/trpc/routers/index.ts (1)
125-126: Verify audit log fetch implementation
The new audit router looks good, but ensure the fetchAuditLog implementation handles date-based queries correctly since this PR aims to fix date selection issues.
✅ Verification successful
Let me gather more information about the date handling by checking the input type definition and other related files.
Let me check one more thing to see if date filtering is handled through the cursor mechanism or if there are any date-related queries in the implementation.
Date handling in audit logs is properly implemented
The implementation correctly handles date-based queries through:
- A retention period filter using
retentionCutoffUnixMilli - Cursor-based pagination using the
timefield - Proper time comparison operators (
gte,lt) in the database query
The code ensures that:
- Logs are filtered based on workspace-specific retention periods (30 or 90 days)
- Results are ordered by time in descending order
- Pagination is handled through time-based cursors
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for date handling in fetchAuditLog implementation
rg -A 10 "fetchAuditLog.*=" ./apps/dashboard/lib/trpc/routers/audit/
Length of output: 1036
Script:
#!/bin/bash
# Search for getAuditLogsInput definition and any date-related fields
rg -A 10 "getAuditLogsInput" ./apps/dashboard/lib/trpc/routers/
# Also check for any date-related fields in the audit logs query
ast-grep --pattern 'const { $$$, startDate, endDate, $$$ } = input'
Length of output: 1899
Script:
#!/bin/bash
# Check the complete fetch.ts implementation for any date-related filtering
cat ./apps/dashboard/lib/trpc/routers/audit/fetch.ts
# Also check if there are any date-related queries in the codebase for audit logs
rg -A 5 "time.*where" ./apps/dashboard/lib/trpc/routers/audit/
Length of output: 3403
apps/dashboard/app/(app)/audit/[bucket]/filter.tsx (1)
46-73: Verify if date selection issue is addressed
The changes to the Filter component appear to be UI refinements, but I don't see changes addressing date selection functionality. Given that this PR is titled "fix: date select issue on logs page", we should verify if additional changes are needed.
apps/dashboard/app/(app)/audit/[bucket]/page.tsx (5)
18-19: LGTM! Clean separation of concerns.
The introduction of AuditTable component and DEFAULT_FETCH_COUNT constant improves code organization and maintainability.
36-39: LGTM! Good type definition practice.
Exporting the AuditLogWithTargets type enables reuse and maintains consistency across the codebase.
92-92: LGTM! Consistent pagination limit.
Using DEFAULT_FETCH_COUNT standardizes the pagination limit across the application.
153-170: LGTM! Comprehensive empty state handling.
The implementation provides clear feedback and appropriate actions for different empty states:
- Bucket not found
- No logs with active filters
- No logs without filters
Line range hint 74-74: Verify date selection issue resolution.
The PR objectives mention fixing a "date select issue on logs page", but the changes don't show explicit date-related modifications. Please ensure that:
- The date selection issue is properly addressed
- The fix is documented
- Test cases cover the date selection functionality
✅ Verification successful
Based on the search results and code changes, I can now generate the final response:
The date selection issue has been properly addressed with a comprehensive date/time picker implementation.
The codebase shows a robust date selection implementation in the logs and audit pages:
- A custom
DatePickerWithRangecomponent with time selection capabilities - Proper time zone handling and date formatting
- Input validation and error handling for date/time selections
- Recent improvements to the timestamp display using
TimestampInfocomponent
The retention days calculation in the audit page (retentionCutoffUnixMilli = Date.now() - retentionDays * 24 * 60 * 60 * 1000) is correctly implemented and works in conjunction with the date selection UI.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for date-related components and their tests
rg -g '!node_modules' -g '!dist' -A 5 'datepicker|dateselect|date.*select' ./apps/dashboard/
Length of output: 94
Script:
#!/bin/bash
# Let's try a broader search for date-related code changes and components
rg -g '!node_modules' -g '!dist' -A 5 'Date\.|new Date|timestamp|retention.*date' ./apps/dashboard/app/
# Also search for any test files related to audit or logs
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts . ./apps/dashboard/app/ --exec rg -l 'audit|logs'
# Check the git diff for the specific file to see date-related changes
git diff HEAD~1 -- apps/dashboard/app/\(app\)/audit/\[bucket\]/page.tsx
Length of output: 49051
Script:
#!/bin/bash
# Let's search for date-related components and filters in the audit and logs pages
rg -g '!node_modules' -g '!dist' -A 5 'DatePicker|DateSelect|DateFilter|date.*filter' ./apps/dashboard/app/\(app\)/audit/
rg -g '!node_modules' -g '!dist' -A 5 'DatePicker|DateSelect|DateFilter|date.*filter' ./apps/dashboard/app/\(app\)/logs/
# Let's also check for any recent changes in the audit and logs components
git diff HEAD~5 -- apps/dashboard/app/\(app\)/audit/ apps/dashboard/app/\(app\)/logs/
Length of output: 15481
apps/dashboard/app/(app)/logs/components/filters/components/time-split.tsx
Show resolved
Hide resolved
apps/dashboard/app/(app)/audit/[bucket]/components/table/table-details.tsx
Show resolved
Hide resolved
What does this PR do?
Merge this after audit PR
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Release Notes
New Features
AuditTablefor displaying audit logs with pagination.LogHeaderandLogFootercomponents for enhanced log detail presentation.LogDetailscomponent for viewing detailed log entries.Improvements
Bug Fixes
RequestResponseDetailscomponent.Chores