Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
📝 WalkthroughWalkthroughThis change introduces a new modular, multi-step dialog system for creating API keys within the dashboard application. The previous monolithic key creation page and related components are removed and replaced with a dialog-based workflow, leveraging new reusable React components for each configuration section (general, rate limit, credits, expiration, metadata). Validation schemas are refactored and centralized using Zod, and a new form persistence hook is added. The dialog supports step-by-step navigation, persistent form state, granular validation, and displays a success dialog with key details upon creation. Supporting UI components, icons, and utilities are also introduced or updated for consistency and enhanced user experience. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateKeyDialog
participant FormSectionComponent
participant usePersistedForm
participant useCreateKey
participant API
User->>CreateKeyDialog: Open dialog
CreateKeyDialog->>usePersistedForm: Initialize/load form state
loop For each section
CreateKeyDialog->>FormSectionComponent: Render section UI
User->>FormSectionComponent: Input data/toggle features
FormSectionComponent->>usePersistedForm: Update form state
end
User->>CreateKeyDialog: Submit form
CreateKeyDialog->>useCreateKey: Trigger key creation mutation
useCreateKey->>API: Call createKey endpoint
API-->>useCreateKey: Return key data
useCreateKey-->>CreateKeyDialog: onSuccess callback
CreateKeyDialog->>User: Show success dialog with key details
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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:
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: 0
🧹 Nitpick comments (1)
internal/ui/src/components/date-time/components/calendar.tsx (1)
149-173: Consider memoizing the disabled date matcher function.The
getDisabledMatcherfunction creates new arrays and objects on each render, which could impact performance if the component re-renders frequently or if there are many disabled date ranges.- const getDisabledMatcher = (): Matcher | Matcher[] | undefined => { + const getDisabledMatcher = React.useMemo((): Matcher | Matcher[] | undefined => { const matchers: Matcher[] = []; if (minDate) { matchers.push({ before: minDate }); } if (maxDate) { matchers.push({ after: maxDate }); } if (disabledDates && disabledDates.length > 0) { disabledDates.forEach((dateRange) => { if (dateRange.from && dateRange.to) { matchers.push({ from: dateRange.from, to: dateRange.to }); } else if (dateRange.before) { matchers.push({ before: dateRange.before }); } else if (dateRange.after) { matchers.push({ after: dateRange.after }); } }); } return matchers.length > 0 ? matchers : undefined; - }; + }, [minDate, maxDate, disabledDates]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/expiration-setup.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/_components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/logs/components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/components/logs/datetime/datetime-popover.tsx(7 hunks)internal/ui/src/components/date-time/components/calendar.tsx(4 hunks)internal/ui/src/components/date-time/date-time.tsx(3 hunks)internal/ui/src/components/form/form-input.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-datetime/index.tsx
- internal/ui/src/components/form/form-input.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx
- apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/expiration-setup.tsx
- apps/dashboard/components/logs/datetime/datetime-popover.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
internal/ui/src/components/date-time/date-time.tsx (1)
internal/ui/src/components/date-time/components/time-split.tsx (1)
TimeUnit(10-14)
internal/ui/src/components/date-time/components/calendar.tsx (2)
apps/dashboard/components/ui/calendar.tsx (1)
CalendarProps(10-10)internal/ui/src/components/date-time/date-time.tsx (1)
useDateTimeContext(123-123)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Go API Local / Test (Shard 6/8)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
apps/dashboard/app/(app)/logs/components/controls/components/logs-datetime/index.tsx (1)
31-31: Good addition of date validation constraintAdding
maxDate={new Date()}improves the user experience by preventing users from selecting future dates for logs, which wouldn't contain any data. This is a sensible constraint that aligns with the data availability.apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-datetime/index.tsx (1)
32-32: Good addition of date validation constraintAdding
maxDate={new Date()}appropriately restricts date selection to past and current dates, which is consistent with the available log data. This prevents users from selecting invalid future dates.apps/dashboard/app/(app)/audit/components/controls/components/logs-datetime/index.tsx (1)
31-31: Good addition of date validation constraintAdding
maxDate={new Date()}is a good user experience improvement that prevents users from selecting future dates in the audit logs, where no data would exist.apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-datetime/index.tsx (1)
31-31: Good addition of date validation constraintAdding
maxDate={new Date()}ensures users can only select dates up to the current time, which is appropriate for rate limit logs since future data doesn't exist yet.apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-datetime/index.tsx (1)
31-31: Good addition of date constraint.Adding the
maxDateprop limits date selection to the current date and earlier, which is a logical constraint for log viewing.apps/dashboard/app/(app)/apis/_components/controls/components/logs-datetime/index.tsx (1)
31-31: Good addition of date constraint.Adding the
maxDateprop limits date selection to the current date and earlier, which is a logical constraint for log viewing.internal/ui/src/components/date-time/date-time.tsx (4)
3-4: Linter comment update looks good.The updated comment format is appropriate.
11-12: Good rename for better semantic clarity.Renaming from
minDateRange/maxDateRangetominDate/maxDateimproves naming clarity, as these represent individual date constraints rather than ranges.
48-55: API update for DateTime component looks good.The function signature now properly includes the new props, maintaining consistency with the interface changes.
102-104: Context value updated correctly.The context provider value properly uses the renamed props.
internal/ui/src/components/date-time/components/calendar.tsx (5)
6-12: Improved import statement structure.The named imports provide better clarity and the addition of the
Matchertype is necessary for the new disabled date functionality.
76-76: Improved styling for disabled days.The styling specification makes disabled days more visually distinct while maintaining consistency with the overall design language.
85-91: Good enhancement with flexible date disabling API.The new
disabledDatesarray allows for specifying multiple date ranges or boundaries to be disabled, which significantly improves the component's flexibility.
101-101: Correct context usage with renamed props.The destructuring now properly uses the renamed
minDateandmaxDateprops from context.
185-185: Good implementation of disabled date handling.Using the aggregated matcher from the new function ensures all date constraints are properly applied.
|
We are ready for round 2 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/expiration-setup.tsx (5)
72-72: Inconsistent comments about minimum time constraintThere's a discrepancy between the comment on line 96 which says "at least 2 minutes in the future" and the actual implementation on line 72 which sets
minValidDateto 10 minutes from now.- // Check if the date is valid (at least 2 minutes in the future) + // Check if the date is valid (at least 10 minutes in the future)Also applies to: 96-96
95-102: Improve user feedback for invalid date selectionsWhen a user selects a date too close to the current time, the code silently adjusts it to the minimum valid date without notifying the user. This could be confusing.
Consider adding explicit user feedback when dates are auto-adjusted:
if (newDate < minValidDate) { // If date is too soon, set it to minimum valid date setValue("expiration.data", minValidDate); + // You could add a toast notification here: + // toast.info("Expiration date must be at least 10 minutes in the future. Adjusted automatically."); } else { setValue("expiration.data", newDate); }
50-50: Ensure selected title synchronizationThe component initializes
selectedTitleto "1 day", but doesn't update it when the form initializes with a different value. This could cause UI inconsistency.Consider synchronizing the initial title with the actual form value:
- const [selectedTitle, setSelectedTitle] = useState<string>("1 day"); + const [selectedTitle, setSelectedTitle] = useState<string>( + currentExpiryDate ? + (EXPIRATION_OPTIONS.find(opt => { + if (opt.value === undefined) return false; // Skip custom option + const now = new Date(); + let optionDate = new Date(); + switch (opt.value) { + case "1d": optionDate = addDays(now, 1); break; + case "7d": optionDate = addDays(now, 7); break; + case "30d": optionDate = addDays(now, 30); break; + } + return Math.abs(optionDate.getTime() - currentExpiryDate.getTime()) < 60000; // Within a minute + })?.display || "Custom") + : "1 day" + );Also applies to: 153-153
133-133: Clarify timezone handling in UIThe description mentions UTC, but there's no explicit timezone handling in the code. This could lead to confusion for users in different timezones.
Consider making the timezone handling more explicit in both the code and UI. For example:
- return "The key will be automatically disabled at the specified date and time (UTC)."; + return "The key will be automatically disabled at the specified date and time in UTC timezone.";You could also consider showing both local and UTC time to prevent confusion.
152-159: Consider adding error handling for date selectionThe DatetimePopover component doesn't have explicit error handling if something goes wrong during date selection.
Consider adding error handling for the DatetimePopover:
<DatetimePopover initialTitle={selectedTitle} initialTimeValues={getInitialTimeValues()} onSuggestionChange={setSelectedTitle} onDateTimeChange={handleDateTimeChange} + onError={(error) => { + console.error("Error in date selection:", error); + // Optionally show error to user + }} customOptions={EXPIRATION_OPTIONS} customHeader={<ExpirationHeader />} singleDateMode minDate={minValidDate} // Set minimum date to 2 minutes from now >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/expiration-setup.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Go API Local / Test (Shard 6/8)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/expiration-setup.tsx (2)
11-40: LGTM! Well-structured expiration optionsThe expiration options are well-defined with clear labels and descriptions.
42-61: Good use of react-hook-form and form state managementThe component correctly uses react-hook-form hooks and properly manages form state. The toggle switch implementation is clean and handles state changes appropriately.
Also applies to: 136-145
| const isExpiringVerySoon = | ||
| currentExpiryDate && | ||
| new Date(currentExpiryDate).getTime() - new Date().getTime() < 60 * 60 * 1000; | ||
|
|
||
| const getExpiryDescription = () => { | ||
| if (isExpiringVerySoon) { | ||
| return "This key will expire very soon (less than 1 hour). Consider setting a longer expiration time."; | ||
| } | ||
| return "The key will be automatically disabled at the specified date and time (UTC)."; | ||
| }; |
There was a problem hiding this comment.
Fix expiration warning when expiration is disabled
Per PR comments, an expiration warning appears even when the date field is disabled. This is because isExpiringVerySoon can be true even when expirationEnabled is false.
// Calculate date for showing warning about close expiry (less than 1 hour)
const isExpiringVerySoon =
- currentExpiryDate &&
+ expirationEnabled && currentExpiryDate &&
new Date(currentExpiryDate).getTime() - new Date().getTime() < 60 * 60 * 1000;
const getExpiryDescription = () => {
if (isExpiringVerySoon) {
return "This key will expire very soon (less than 1 hour). Consider setting a longer expiration time.";
}
return "The key will be automatically disabled at the specified date and time (UTC).";
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isExpiringVerySoon = | |
| currentExpiryDate && | |
| new Date(currentExpiryDate).getTime() - new Date().getTime() < 60 * 60 * 1000; | |
| const getExpiryDescription = () => { | |
| if (isExpiringVerySoon) { | |
| return "This key will expire very soon (less than 1 hour). Consider setting a longer expiration time."; | |
| } | |
| return "The key will be automatically disabled at the specified date and time (UTC)."; | |
| }; | |
| const isExpiringVerySoon = | |
| expirationEnabled && currentExpiryDate && | |
| new Date(currentExpiryDate).getTime() - new Date().getTime() < 60 * 60 * 1000; | |
| const getExpiryDescription = () => { | |
| if (isExpiringVerySoon) { | |
| return "This key will expire very soon (less than 1 hour). Consider setting a longer expiration time."; | |
| } | |
| return "The key will be automatically disabled at the specified date and time (UTC)."; | |
| }; |
| <Controller | ||
| control={control} | ||
| name="expiration.data" | ||
| render={({ field }) => ( | ||
| <DatetimePopover | ||
| initialTitle={selectedTitle} | ||
| initialTimeValues={getInitialTimeValues()} | ||
| onSuggestionChange={setSelectedTitle} | ||
| onDateTimeChange={handleDateTimeChange} | ||
| customOptions={EXPIRATION_OPTIONS} | ||
| customHeader={<ExpirationHeader />} | ||
| singleDateMode | ||
| minDate={minValidDate} // Set minimum date to 2 minutes from now | ||
| > | ||
| <FormInput | ||
| label="Expiry Date" | ||
| description={getExpiryDescription()} | ||
| readOnly | ||
| disabled={!expirationEnabled} | ||
| value={formatExpiryDate(field.value)} | ||
| className="cursor-pointer w-full" | ||
| variant={expirationEnabled && isExpiringVerySoon ? "warning" : undefined} | ||
| error={errors.expiration?.data?.message} | ||
| /> | ||
| </DatetimePopover> | ||
| )} | ||
| /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate and clear form errors correctly
Per PR comments, the date field is automatically marked as invalid and selecting a valid date doesn't clear the validation error. This may be related to how form validation is handled.
Check if the form validation is properly updated when dates change. Consider logging the validation in the handleDateTimeChange function:
🏁 Script executed:
#!/bin/bash
# Search for form validation schema for expiration
rg -A 10 -B 10 "expiration.*schema|schema.*expiration" --glob="*.{ts,tsx}"Length of output: 76
🏁 Script executed:
#!/bin/bash
# Locate where handleDateTimeChange is defined/used
rg -n "handleDateTimeChange" -A 5 -B 5 --glob="*.{ts,tsx}"
# Check if clearErrors is ever called in relation to expiration
rg -n "clearErrors" -A 5 -B 5 --glob="*.{ts,tsx}"Length of output: 6363
🏁 Script executed:
#!/bin/bash
# Inspect handleDateTimeChange implementation in expiration-setup.tsx and related form methods
file="apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/expiration-setup.tsx"
# Show the function body
rg -n -A30 -B0 "const handleDateTimeChange" -- "$file"
# Check how the form state is updated
rg -n "setValue" -- "$file"
# Verify if errors are cleared or set
rg -n "clearErrors" -- "$file"
rg -n "setError" -- "$file"Length of output: 1890
Clear or re-validate the expiry field when the date changes
The handleDateTimeChange helper updates expiration.data via setValue but never clears or re-validates the field, so any prior validation error remains. To fix this, update the logic in expiration-setup.tsx as follows:
• Destructure and call clearErrors("expiration.data") inside handleDateTimeChange immediately after you call setValue for a valid date.
• Alternatively, invoke setValue("expiration.data", newDate, { shouldValidate: true }) to re-trigger validation and clear errors automatically.
• You can also leverage the Controller’s field.onChange callback instead of setValue directly, so React Hook Form runs its built-in validation flow.
Locations to update:
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/expiration-setup.tsx
–handleDateTimeChange(lines 75–103)
perkinsjr
left a comment
perkinsjr
left a comment
There was a problem hiding this comment.
Approved. Let's merge on Tuesday
|
I'll let you merge tomorrow. I'll rebase my other PR from this one 🫡 |
2 participants
What does this PR do?
This PR adds new key creation multi step dialog, refactors dialog component for better composability and styles
Selectcomponent with our own colors similar toInputandInputArea.Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores