Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughRemoved filter-based gating from multiple LogsRefresh components so the RefreshButton is always enabled. In several components, integrated useQueryTime to call refreshQueryTime before invalidating queries and refreshing. No exported signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant LR as LogsRefresh Component
participant QT as QueryTime Provider
participant Q as Query Cache
participant R as Router
U->>LR: Click Refresh
alt Components using QueryTime
LR->>QT: refreshQueryTime()
end
LR->>Q: invalidate relevant queries
LR->>R: router.refresh()
Note right of LR: RefreshButton is always enabled
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsx (2)
11-15: Return a Promise from onRefresh (await query-time + invalidations) to avoid UI racesIf
RefreshButtonshows a loading state based on a Promise, returning/awaiting here prevents stale flashes and batches invalidations predictably.Apply:
- const handleRefresh = () => { - refreshQueryTime(); - logs.queryLogs.invalidate(); - logs.queryTimeseries.invalidate(); - }; + const handleRefresh = async () => { + // Support both sync/async implementations + await Promise.resolve(refreshQueryTime()); + await Promise.all([ + logs.queryLogs.invalidate(), + logs.queryTimeseries.invalidate(), + ]); + };
7-15: Optional: memoize handler to reduce re-renders downstreamIf
RefreshButtonor siblings are memoized, consideruseCallbackforhandleRefreshto stabilize the prop.+import { useCallback } from "react"; ... - const handleRefresh = async () => { + const handleRefresh = useCallback(async () => { await Promise.resolve(refreshQueryTime()); await Promise.all([ logs.queryLogs.invalidate(), logs.queryTimeseries.invalidate(), ]); - }; + }, [refreshQueryTime, logs]);apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-refresh.tsx (1)
9-14: Make refresh flow async and batch invalidationsAwaiting both the query-time tick and invalidations improves determinism and user feedback.
- const handleRefresh = () => { - refreshQueryTime(); - ratelimit.overview.logs.query.invalidate(); - ratelimit.overview.logs.queryRatelimitLatencyTimeseries.invalidate(); - ratelimit.logs.queryRatelimitTimeseries.invalidate(); - }; + const handleRefresh = async () => { + await Promise.resolve(refreshQueryTime()); + await Promise.all([ + ratelimit.overview.logs.query.invalidate(), + ratelimit.overview.logs.queryRatelimitLatencyTimeseries.invalidate(), + ratelimit.logs.queryRatelimitTimeseries.invalidate(), + ]); + };apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-refresh.tsx (1)
11-15: Async onRefresh + Promise.all for consistent refresh UXSame rationale as other files: return a Promise to the button and avoid interleaving state updates.
- const handleRefresh = () => { - refreshQueryTime(); - ratelimit.logs.query.invalidate(); - ratelimit.logs.queryRatelimitTimeseries.invalidate(); - }; + const handleRefresh = async () => { + await Promise.resolve(refreshQueryTime()); + await Promise.all([ + ratelimit.logs.query.invalidate(), + ratelimit.logs.queryRatelimitTimeseries.invalidate(), + ]); + };apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-refresh.tsx (1)
9-14: Await query-time tick and API invalidationsPrevents stale flashes and lets the button reflect an in-progress state.
- const handleRefresh = () => { - refreshQueryTime(); - api.keys.query.invalidate(); - api.keys.timeseries.invalidate(); - api.keys.activeKeysTimeseries.invalidate(); - }; + const handleRefresh = async () => { + await Promise.resolve(refreshQueryTime()); + await Promise.all([ + api.keys.query.invalidate(), + api.keys.timeseries.invalidate(), + api.keys.activeKeysTimeseries.invalidate(), + ]); + };apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-refresh.tsx (1)
9-14: Convert to async and batch invalidationsSame change suggested elsewhere for consistency and to support loading states.
- const handleRefresh = () => { - refreshQueryTime(); - api.keys.query.invalidate(); - api.keys.timeseries.invalidate(); - api.keys.activeKeysTimeseries.invalidate(); - }; + const handleRefresh = async () => { + await Promise.resolve(refreshQueryTime()); + await Promise.all([ + api.keys.query.invalidate(), + api.keys.timeseries.invalidate(), + api.keys.activeKeysTimeseries.invalidate(), + ]); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-refresh.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-refresh.tsx(1 hunks)apps/dashboard/app/(app)/apis/_components/controls/components/logs-refresh.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/controls/components/logs-refresh.tsx(1 hunks)apps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-refresh.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-refresh.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-refresh.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Use Biome for formatting and linting in TypeScript/JavaScript projects
Prefer named exports over default exports in TypeScript/JavaScript, except for Next.js pages
Files:
apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/audit/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/_components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-refresh.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow strict TypeScript configuration
Use Zod for runtime validation in TypeScript projects
Files:
apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/audit/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/_components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-refresh.tsx
🧠 Learnings (2)
📚 Learning: 2025-06-24T13:29:10.129Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
Applied to files:
apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/audit/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/apis/_components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-refresh.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-refresh.tsx
📚 Learning: 2024-10-23T16:21:47.395Z
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/search.tsx:16-20
Timestamp: 2024-10-23T16:21:47.395Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, refactoring type definitions into an interface is not necessary at this time.
Applied to files:
apps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsx
🧬 Code graph analysis (3)
apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-refresh.tsx (3)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-refresh.tsx (1)
LogsRefresh(5-17)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-refresh.tsx (1)
LogsRefresh(6-20)apps/dashboard/providers/query-time-provider.tsx (2)
useQueryTime(74-80)refreshQueryTime(8-12)
apps/dashboard/app/(app)/audit/components/controls/components/logs-refresh.tsx (2)
apps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsx (1)
LogsRefresh(6-20)apps/dashboard/providers/query-time-provider.tsx (2)
useQueryTime(74-80)refreshQueryTime(8-12)
apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-refresh.tsx (1)
internal/ui/src/components/buttons/refresh-button.tsx (1)
RefreshButton(80-80)
⏰ 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). (3)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (16)
apps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsx (1)
18-18: Always-enabled RefreshButton fixes #3107’s disabled-on-load bugMaking
isEnabledunconditional aligns with the objective to allow refresh on initial load without re-selecting relative filters. Good call.apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-refresh.tsx (2)
16-16: Always-enabled RefreshButton: aligned with PR goalUn-gating via
isEnabledensures refresh is usable on first load. Matches the bugfix intent.
1-17: AI summary inconsistency: this file still invalidates TRPC queriesThe summary claims “refresh handler simplified to trigger a query-time refresh only,” but the code also invalidates three ratelimit queries. Summary should be corrected for traceability.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-refresh.tsx (2)
18-18: Un-gated refresh with live toggle is correct for logs UXAlways-enabled plus
isLive/toggleLivewiring makes sense and keeps the live mode control discoverable.
7-8: Optional: disable manual refresh while live is on (if not already handled in RefreshButton)If
RefreshButtondoesn’t internally gate manual refresh during live mode, consider passing a computedisEnabled={!isLive}to prevent redundant requests. If it already handles this, ignore.apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-refresh.tsx (1)
16-16: RefreshButton always enabled: consistent with pattern across pagesMatches the standardized approach in this PR.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-refresh.tsx (1)
16-16: Always-enabled refresh: aligns with fix for disabled initial stateConsistent with the rest of the PR—good.
apps/dashboard/app/(app)/apis/_components/controls/components/logs-refresh.tsx (1)
14-14: LGTM: Refresh button now always enabled as intended.The change successfully addresses the issue where the refresh button was initially disabled. The hardcoded
isEnabledprop ensures the button is always interactive, aligning with the PR objective to fix the disabled state issue.apps/dashboard/app/(app)/audit/components/controls/components/logs-refresh.tsx (4)
2-2: LGTM: Added useQueryTime for consistent refresh behavior.The import of
useQueryTimeenables the component to synchronize query time state across components, which is essential for proper refresh functionality.
7-7: LGTM: Hook usage follows React patterns correctly.The destructuring of
refreshQueryTimefrom theuseQueryTimehook is implemented correctly and follows React hook conventions.
9-12: LGTM: Enhanced refresh logic with query time synchronization.The addition of
refreshQueryTime()before cache invalidation ensures that the query time state is updated consistently across all components that depend on it. This provides a more reliable refresh experience.
14-14: LGTM: Refresh button now always enabled.The hardcoded
isEnabledprop ensures the refresh button is always interactive, which directly addresses the core issue described in the PR objectives where the button was initially disabled despite having default relative time filters.apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-refresh.tsx (4)
2-2: LGTM: Consistent implementation with query time provider.The import of
useQueryTimealigns with the pattern established across other LogsRefresh components in this PR, ensuring consistent refresh behavior across the application.
7-7: LGTM: Proper hook usage pattern.The destructuring of
refreshQueryTimefollows the established pattern seen in other components likeapps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-refresh.tsx(Lines 4-16).
11-15: LGTM: Complete refresh flow implementation.The refresh sequence correctly:
- Updates query time state via
refreshQueryTime()- Invalidates specific cache entries for ratelimit data
- Triggers router refresh
This follows the same pattern as other LogsRefresh components and ensures comprehensive data refresh.
17-17: LGTM: Addresses the core issue with always-enabled refresh button.The hardcoded
isEnabledprop successfully resolves the reported issue where refresh buttons were initially disabled even with default relative time selections. This change ensures consistent user experience across all log pages.
apps/dashboard/app/(app)/logs/components/controls/components/logs-refresh.tsx
Show resolved
Hide resolved
|
@mcstepp @perkinsjr we need a 2nd review |
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (08/27/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
3 participants
What does this PR do?
This PR allows people to refresh on log pages. Previously, it was blocked because we had no way to refresh the list without users first selecting the relative filters, but now thanks to
refreshQueryTimewe can easily do that.Fixes #3107
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