fix: ratelimit date input and time display logs #2717
Conversation
Improve ratelimits filter date pickers and how we display times on ratelimit logs
ogzhanolguncu
requested review from
perkinsjr,
chronark and
mcstepp
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
Warning Rate limit exceeded@ogzhanolguncu has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant updates to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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 (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
github-actions
bot
added
Bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
apps/dashboard/components/date-time-picker.tsx (2)
9-17: Consider enhancing type safety for date propsThe props interface could be improved to ensure better type safety and validation:
- Consider adding validation for the date prop to ensure it's not null
- Add JSDoc comments to document the expected format and behavior
type DateTimePickerProps = { + /** The currently selected date. Must be a valid Date object */ date: Date; + /** Callback fired when the date changes. Provides the new Date value */ onDateChange: (date: Date) => void; calendarProps?: React.ComponentProps<typeof Calendar>; timeInputProps?: React.ComponentProps<typeof Input>; popoverContentProps?: React.ComponentProps<typeof PopoverContent>; timeInputLabel?: string; disabled?: boolean; };
31-40: Improve null date handling in handleSelectThe early return for null dates could be more explicit about why the selection was invalid.
const handleSelect = (selected: Date) => { if (!selected) { - return; + console.warn('Date selection was cancelled or invalid'); + return; } const hours = selectedDateTime.getHours(); const minutes = selectedDateTime.getMinutes(); const newDate = setMinutes(setHours(selected, hours), minutes); setSelectedDateTime(newDate); setDate(newDate); };apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/filter.tsx (2)
62-79: Consider combining state updates for better consistencyThe separate state updates for after and before could lead to temporary inconsistencies. Consider combining them into a single update when appropriate.
- const [after, setAfter] = useQueryState( - "after", - parseAsTimestamp.withOptions({ - history: "push", - shallow: false, - clearOnDefault: true, - }), - ); - - const [before, setBefore] = useQueryState( - "before", - parseAsTimestamp.withOptions({ - history: "push", - shallow: false, - clearOnDefault: true, - }), - ); + const [dateRange, setDateRange] = useQueryState( + ["after", "before"], + parseAsTimestamp.withOptions({ + history: "push", + shallow: false, + clearOnDefault: true, + }), + );
Line range hint
244-254: Optimize reset functionalityThe reset functionality could be more efficient by combining the state updates and removing the empty transition callback.
<Button className="flex-shrink-0" size="icon" variant="secondary" onClick={() => { - setTimeRangeVisible(false); - setAfter(null); - setBefore(null); - startTransition(() => {}); + startTransition(() => { + setTimeRangeVisible(false); + setAfter(null); + setBefore(null); + }); }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/filter.tsx(6 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx(2 hunks)apps/dashboard/components/date-time-picker.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/dashboard/components/date-time-picker.tsx
[error] 68-68: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
🔇 Additional comments (1)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx (1)
163-165: LGTM: Improved timestamp display
The use of TimestampInfo component provides a consistent and user-friendly way to display timestamps. The hover effect and formatting improvements enhance the user experience.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/filter.tsx
Outdated
Show resolved
Hide resolved
What does this PR do?
Fixes how we display dates on logs page, improves how we display date on ratelimit logs table
Fixes #2715, #2706
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
DateTimePickercomponent for enhanced date and time selection.Improvements
TimestampInfocomponent in the audit log.Bug Fixes