fix: Ensure last ratelimit usage is shown #2721
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@Flo4604 is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe pull request introduces several changes across multiple files, primarily focusing on the Changes
Assessment against linked issues
Suggested reviewers
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 (
|
github-actions
bot
added
Bug
|
Thank you for following the naming conventions for pull request titles! 🙏 |
Flo4604
changed the title
Flo4604
requested review from
perkinsjr,
mcstepp,
ogzhanolguncu and
MichaelUnkey
as code owners
|
Missed a place where I didn't pass in limit sorry |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/dashboard/app/(app)/ratelimits/card.tsx (1)
Line range hint
71-73: Replace hardcoded datetime with dynamic valueThe
timeelement contains a hardcoded datetime value "2024-03-11T19:38:06.192Z" which should be replaced with the actuallastUsedtimestamp.Apply this diff to fix the issue:
- <time dateTime="2024-03-11T19:38:06.192Z" title="20:38:06 CET"> + <time dateTime={new Date(lastUsed).toISOString()} title={new Date(lastUsed).toLocaleTimeString()}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/dashboard/app/(app)/ratelimits/card.tsx(1 hunks)
🔇 Additional comments (1)
apps/dashboard/app/(app)/ratelimits/card.tsx (1)
29-33: LGTM! The bug fix implementation looks correct.
The addition of limit: 1 parameter ensures we fetch only the most recent entry, fixing the issue where the last used timestamp was incorrectly showing the first entry instead of the most recent one.
| .latest({ | ||
| workspaceId: namespace.workspaceId, | ||
| namespaceId: namespace.id, | ||
| limit: 1, | ||
| }) |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent usage of limit parameter across .latest() calls needs attention
The verification reveals that the limit: 1 parameter is only used in ratelimit-related components:
apps/dashboard/app/(app)/ratelimits/card.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx
Other components using .latest() either:
- Don't use the
limitparameter and rely on array access (.at(0)) to get the latest entry - Have different parameter requirements (like the overrides table)
This inconsistency in implementation could lead to similar timestamp-related issues in other components.
🔗 Analysis chain
LGTM! The fix correctly implements the last used timestamp retrieval.
The addition of limit: 1 ensures we get only the most recent entry from the ratelimits table, fixing the bug where the first timestamp was being shown instead of the last used time.
Let's verify that this change is consistently implemented across all components that use the latest method:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all components using clickhouse.ratelimits.latest include the limit parameter
# Search for all usages of the latest method
rg -A 3 "clickhouse\.ratelimits\.latest\(" --type ts --type tsx
Length of output: 93
Script:
#!/bin/bash
# Search for all usages of the latest method without file type restrictions
rg -A 3 "\.latest\("
Length of output: 2563
What does this PR do?
Fixes #2720
Cleanups go.mod and go.sum for the chproxy
Removes a file that wasn't in use and threw an error as the
@unkey/clickhouse-zodpackage does not exists.Type of change
How should this be tested?
Send a ratelimit request, check the last change time, wait a minute and send new requests.
Check that the limit does not update.
With this branch the correct last time is shown
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
LastUsedcomponent to retrieve only the most recent entry for specified identifiers.RatelimitNamespacePageto fetch the latest ratelimit data with a limit of 1.RatelimitCardcomponent to fetch the latest ratelimit data with a limit of 1.Bug Fixes
ratelimitEvents, ensuring correct structure for rendering.Chores
last_used.tsfile, which contained unused functionality.