Conversation
ogzhanolguncu
requested review from
MichaelUnkey,
chronark,
mcstepp and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
📝 WalkthroughWalkthroughThis pull request systematically refactors conditional expressions across the codebase, replacing logical OR ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Component
participant Server/API
participant DB
User->>UI_Component: Triggers action or submits form
UI_Component->>UI_Component: Validates parameters using refactored conditionals
alt Parameters valid
UI_Component->>Server/API: Sends request
Server/API->>Server/API: Validates input using new conditional logic
alt Input valid
Server/API->>DB: Performs operation (query/update)
DB-->>Server/API: Returns result
Server/API-->>UI_Component: Returns success/data
else Input invalid
Server/API-->>UI_Component: Returns error
end
else Parameters invalid
UI_Component-->>User: Shows validation error
end
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (9)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
🔭 Outside diff range comments (1)
tools/local/src/main.ts (1)
68-68: 💡 Verification agent🧩 Analysis chain
Verify impact on workspace ID usage.
This line expects
passedOptions.wsto be a string value, but the argument parsing change will always set it totrue.
🏁 Script executed:
#!/bin/bash # Description: Check for other usages of passedOptions that might expect string values # Expected: Find any code that casts or uses passedOptions values as strings rg -A 3 -B 3 "passedOptions\[.*\].*as.*string" rg -A 3 -B 3 "passedOptions\..*as.*string"Length of output: 578
🏁 Script executed:
#!/bin/bash # Locate CLI parser configuration for the 'ws' option rg -n "option.*['\"]ws['\"]"Length of output: 34
🏁 Script executed:
#!/bin/bash # Display the CLI parsing setup and passedOptions initialization in main.ts sed -n '1,300p' tools/local/src/main.tsLength of output: 2595
Fix CLI parsing to capture
--wsvalue instead of alwaystrueThe current loop in tools/local/src/main.ts ignores any
=valueand sets every flag totrue, sopassedOptions.wscan never be a string. Update the assignment to use the part after=when present:• File: tools/local/src/main.ts
Location: theargs.forEachblock at the topSuggested change:
- args.forEach((arg) => { - const [key, _value] = arg.split("="); - passedOptions[key.replace("--", "")] = true; - }); + args.forEach((arg) => { + const [key, value] = arg.split("="); + passedOptions[key.replace("--", "")] = + value !== undefined ? value : true; + });This ensures
--ws=abc123yieldspassedOptions.ws === "abc123", allowingconst workspaceId = passedOptions.ws as string | undefined;to work as intended.
🧹 Nitpick comments (2)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx (1)
151-153: Approve updated “Important” paragraph messagingThe conditional text now accurately reflects the enable/disable semantics and clarifies that the setting can be toggled again. To reduce duplication and simplify the JSX, you might extract these two strings into descriptive constants at the top of the component.
apps/dashboard/pages/api/v1/github/verify.ts (1)
67-67: Remove duplicate signature check.The condition contains a redundant
signature &&check. This should be simplified for clarity.- if (!(signature && signature && keyId && data)) { + if (!(signature && keyId && data)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (80)
apps/api/src/routes/v1_identities_getIdentity.ts(1 hunks)apps/api/src/routes/v1_identities_updateIdentity.ts(2 hunks)apps/api/src/routes/v1_keys_getVerifications.ts(1 hunks)apps/api/src/routes/v1_migrations_createKey.ts(1 hunks)apps/api/src/routes/v1_ratelimits_deleteOverride.ts(1 hunks)apps/api/src/routes/v1_ratelimits_getOverride.ts(1 hunks)apps/api/src/routes/v1_ratelimits_listOverrides.ts(1 hunks)apps/api/src/routes/v1_ratelimits_setOverride.ts(1 hunks)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/metadata-setup.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.utils.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/actions.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/disable-key.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-external-id/index.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/selection-controls/components/batch-edit-external-id.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/hooks/use-keys-list-query.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.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)/authorization/roles/[roleId]/page.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/app/(app)/settings/billing/client.tsx(1 hunks)apps/dashboard/app/(app)/settings/team/client.tsx(1 hunks)apps/dashboard/app/(app)/settings/team/invite.tsx(1 hunks)apps/dashboard/app/auth/sign-up/email-signup.tsx(1 hunks)apps/dashboard/app/integrations/vercel/callback/client.tsx(1 hunks)apps/dashboard/app/new/create-ratelimit.tsx(1 hunks)apps/dashboard/components/banner.tsx(1 hunks)apps/dashboard/components/logs/chart/index.tsx(2 hunks)apps/dashboard/components/logs/details/request-response-details.tsx(1 hunks)apps/dashboard/components/logs/details/resizable-panel.tsx(1 hunks)apps/dashboard/components/logs/hooks/use-bookmarked-filters.ts(1 hunks)apps/dashboard/components/logs/overview-charts/overview-area-chart.tsx(3 hunks)apps/dashboard/components/logs/overview-charts/overview-bar-chart.tsx(2 hunks)apps/dashboard/components/logs/overview-charts/utils.tsx(1 hunks)apps/dashboard/components/logs/queries/queries-popover.tsx(1 hunks)apps/dashboard/components/logs/validation/utils/nuqs-parsers.ts(1 hunks)apps/dashboard/components/navigation/copyable-id-button.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx(2 hunks)apps/dashboard/components/navigation/sidebar/team-switcher.tsx(2 hunks)apps/dashboard/components/stats-card/components/chart/stats-chart.tsx(1 hunks)apps/dashboard/components/ui/chart.tsx(3 hunks)apps/dashboard/components/ui/form.tsx(1 hunks)apps/dashboard/components/virtual-table/hooks/useVirtualData.ts(2 hunks)apps/dashboard/components/virtual-table/index.tsx(1 hunks)apps/dashboard/hooks/use-keyboard-shortcut.tsx(2 hunks)apps/dashboard/lib/auth.ts(1 hunks)apps/dashboard/lib/auth/local.ts(4 hunks)apps/dashboard/lib/auth/server.ts(1 hunks)apps/dashboard/lib/auth/workos.ts(5 hunks)apps/dashboard/lib/posthog.ts(1 hunks)apps/dashboard/lib/trpc/routers/api/keys/api-query.ts(1 hunks)apps/dashboard/lib/trpc/routers/api/keys/query-active-keys-timeseries/index.ts(1 hunks)apps/dashboard/lib/trpc/routers/api/keys/query-overview-logs/index.ts(1 hunks)apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts(1 hunks)apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts(1 hunks)apps/dashboard/lib/trpc/routers/key/updateRemaining.ts(2 hunks)apps/dashboard/lib/trpc/routers/utils/granularity.ts(1 hunks)apps/dashboard/lib/zod-helper.ts(1 hunks)apps/dashboard/pages/api/v1/github/verify.ts(2 hunks)apps/dashboard/pages/api/v1/workos/webhooks.ts(1 hunks)apps/engineering/app/components/render.tsx(1 hunks)biome.json(1 hunks)internal/clickhouse/src/keys/keys.ts(5 hunks)internal/clickhouse/src/ratelimits.ts(8 hunks)internal/clickhouse/src/verifications.ts(4 hunks)internal/ui/src/components/id.tsx(1 hunks)tools/artillery/main.ts(1 hunks)tools/local/src/cmd/seed.ts(1 hunks)tools/local/src/main.ts(1 hunks)tools/local/src/prepare.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx (2)
Learnt from: MichaelUnkey
PR: unkeyed/unkey#3173
File: apps/docs/security/delete-protection.mdx:21-24
Timestamp: 2025-04-22T17:34:04.438Z
Learning: In the Unkey dashboard UI for enabling delete protection, the button/link to initiate the process is labeled "Enable Delete Protection" while the confirmation button is labeled "Enable API Delete Protection". The documentation should maintain these different labels to match the actual UI.
Learnt from: MichaelUnkey
PR: unkeyed/unkey#3173
File: apps/docs/security/delete-protection.mdx:32-36
Timestamp: 2025-04-22T17:33:28.162Z
Learning: In the Unkey dashboard UI for delete protection, the button/link to initiate the process is labeled "Disable Delete Protection" while the confirmation button is labeled "Disable API Delete Protection". The documentation should maintain these different labels to match the actual UI.
🧬 Code Graph Analysis (4)
apps/dashboard/components/logs/hooks/use-bookmarked-filters.ts (1)
apps/dashboard/lib/utils.ts (1)
isBrowser(8-8)
apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts (1)
internal/clickhouse/src/index.ts (1)
api(191-213)
apps/dashboard/lib/trpc/routers/api/keys/query-overview-logs/index.ts (1)
internal/clickhouse/src/index.ts (1)
api(191-213)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/hooks/use-keys-list-query.ts (1)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/filters.schema.ts (1)
keysListFilterFieldNames(44-44)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (109)
internal/ui/src/components/id.tsx (1)
62-65: Conditional icon rendering inversion looks good
The updated logic now showsTaskCheckedwhenisCopiedis true andTaskUncheckedwhen false, which correctly aligns the UI feedback with the copy status.tools/local/src/cmd/seed.ts (1)
53-58: Refactored conditional to check positive case firstInverted the
if/elseordering so the successfulworkspacepath is handled before the error path, improving readability and aligning with the PR’s De Morgan–style refactoring while preserving behavior.apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx (1)
143-143: Approve updated footer note messagingThe inversion of the footer copy correctly informs the user—when delete protection is currently enabled, they can “re-enable” it later, and when disabled, they can “disable” it at any time.
biome.json (5)
18-21: Strengthened correctness linting rules
EscalatingnoUnusedImportstoerrorand addingnoChildrenProp,useJsxKeyInIterable, andnoUnsafeOptionalChainingwill catch dead code and unsafe prop usage early. Ensure the codebase is updated to comply with exhaustive dependencies and optional chaining safety.
24-24: Enforce safe HTML rendering
Turning onnoDangerouslySetInnerHtmlas an error prevents potential XSS vectors. Please audit or refactor any existingdangerouslySetInnerHtmlusages to safe alternatives.
28-39: Unified style rule enhancements
The new style rules (useConst,noUselessElse,useImportType, etc.) promote more idiomatic and concise code. Verify that the codebase has been refactored to satisfy these checks and that the formatter rules remain consistent.
45-47: Expanded suspicious code detections
EnablingnoDoubleEquals,useIsArray, andnoFallthroughSwitchClausewill reduce subtle bugs related to type coercion and switch fall-through. Confirm that existing patterns have been updated accordingly.
52-59: Tighter complexity controls
The complexity rules (noExtraBooleanCast,noUselessTernary,useOptionalChain, etc.) help simplify logic and improve readability. Ensure that anyforEachuse remains intentional (sincenoForEachis left off) and that other constructs conform to the new guidelines.apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (2)
46-46: LGTM! Logical equivalence maintained with improved clarity.The condition has been refactored using De Morgan's laws:
!(hasChildren && pathname)is logically equivalent to!hasChildren || !pathname. This change improves readability by expressing the condition as a negation of the positive case rather than a disjunction of negations.
79-87: LGTM! Improved logical flow in navigation handling.The reordering of conditional logic improves code organization by handling the simpler case (external links) first, then the more complex case (internal navigation with loading state). This creates a clearer logical flow without changing the functional behavior.
apps/engineering/app/components/render.tsx (1)
31-34: LGTM! Clean application of De Morgan's law.This stylistic change correctly applies De Morgan's law (
!(A || B) = !A && !B) while maintaining the exact same filtering logic. The prop filtering still correctly excludes React internal props that start with either "_" or "$" prefixes, and the change aligns with the broader linter style refactoring effort across the codebase.apps/dashboard/lib/trpc/routers/api/keys/query-overview-logs/index.ts (1)
32-32: LGTM! Clean refactoring using optional chaining.The simplified condition
!api?.keyAuth?.idis functionally equivalent to the previous explicit null check while being more concise. Optional chaining properly handles all null/undefined cases in the chain.apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts (1)
16-16: LGTM! Consistent with the codebase refactoring.This change aligns with the systematic refactoring effort across the API key router files, simplifying the conditional check while maintaining the same error handling behavior.
apps/dashboard/lib/trpc/routers/api/keys/api-query.ts (1)
227-227: LGTM! Consistent refactoring in shared utility function.The simplified condition using optional chaining maintains functional equivalence while improving code clarity. Since this
queryApiKeysfunction is used by multiple router files, applying the same refactoring pattern here ensures consistency across the codebase.apps/dashboard/lib/trpc/routers/api/keys/query-active-keys-timeseries/index.ts (1)
15-15: LGTM! Completes the systematic refactoring.This change completes the consistent application of optional chaining simplification across all API key router files. The refactoring maintains functional equivalence while improving code clarity and reducing redundancy.
apps/dashboard/components/banner.tsx (1)
32-34:Details
✅ Verification successful
Logic looks correct, but verify hook usage compliance.
The conditional logic is appropriate for the component's behavior: using localStorage when
persistChoiceis provided for persistence, and regular state otherwise.However, this pattern conditionally calls different hooks based on props, which could violate React's Rules of Hooks if
persistChoicechanges between renders.Please verify that
persistChoiceremains stable throughout the component's lifecycle:
🏁 Script executed:
#!/bin/bash # Description: Check how persistChoice prop is used across the codebase # Expected: persistChoice should be static/constant in most usage patterns echo "=== Searching for Banner component usage ===" rg -A 5 -B 2 '<Banner.*persistChoice' echo -e "\n=== Searching for Banner import and usage patterns ===" rg -A 10 -B 2 'import.*Banner'Length of output: 3553
Banner hook initialization is safe; no changes needed.
A search across the codebase shows
Bannerisn’t consumed with a dynamicpersistChoiceprop—its only example usage uses a static string literal. The conditional use ofuseLocalStoragevs.useStatetherefore won’t violate React’s Rules of Hooks in practice.apps/dashboard/app/(app)/apis/[apiId]/actions.ts (1)
23-23: LGTM! Correct application of De Morgan's law.The transformation from
if (!apiId || !orgId)toif (!(apiId && orgId))is logically equivalent and improves consistency with the broader codebase style.apps/dashboard/app/(app)/settings/team/invite.tsx (1)
62-62: LGTM! Logical transformation is correct.The change from
if (!user!.orgId || !organization)toif (!(user!.orgId && organization))maintains the same functionality while following the consistent style pattern applied throughout this PR.apps/dashboard/app/(app)/authorization/roles/[roleId]/page.tsx (1)
67-67: LGTM! Effective use of optional chaining.The simplification from
if (!role || !role.workspace)toif (!role?.workspace)maintains the same logic while being more concise and readable through optional chaining.apps/dashboard/components/logs/validation/utils/nuqs-parsers.ts (1)
77-77: LGTM! Consistent style transformation.The change from
if (!column || !direction)toif (!(column && direction))correctly applies De Morgan's law while maintaining the same validation logic and improving consistency with the codebase style.apps/dashboard/lib/auth/server.ts (1)
41-41: LGTM: Correct application of De Morgan's law.The conditional expression has been refactored from a disjunction of negations to a negated conjunction, maintaining the same logical behavior while improving code style consistency. The condition correctly evaluates to
truewhen eitherWORKOS_API_KEYorWORKOS_CLIENT_IDis missing, properly triggering the error message.apps/dashboard/lib/trpc/routers/utils/granularity.ts (1)
65-65: LGTM: Logical equivalence maintained with improved style.The condition has been refactored using De Morgan's law from
!startTime && !endTimeto!(startTime || endTime). This correctly maintains the logic of providing default granularity and time range values when both time parameters are missing, as indicated by the comment on line 64.apps/dashboard/components/logs/details/resizable-panel.tsx (1)
41-41: LGTM: Consistent conditional style with preserved logic.The conditional expression has been refactored using De Morgan's law, changing from
!isDragging || !panelRef.currentto!(isDragging && panelRef.current). This maintains the same early return behavior when either the component is not in dragging state or the panel reference is unavailable.apps/dashboard/app/(app)/settings/billing/client.tsx (1)
80-82: LGTM: Clear free tier logic with consistent style.The
isFreeTiervariable definition has been refactored to use a negated conjunction, which correctly evaluates totruewhen either there's no subscription or the subscription status is not "active" or "trialing". This maintains the same logic for determining free tier status while following the consistent conditional expression style used throughout this PR.apps/dashboard/components/logs/details/request-response-details.tsx (1)
72-72: LGTM: Conditional assignment is logically correct.The ternary expression correctly assigns
undefinedtoonClickwhenfield.skipTooltipis true, which aligns with the component's design pattern of conditionally enabling click-to-copy functionality.apps/dashboard/components/logs/hooks/use-bookmarked-filters.ts (1)
34-34: LGTM: De Morgan's law transformation applied correctly.The condition
!(filters.length && isBrowser)is logically equivalent to the original!filters.length || !isBrowser. This transformation improves readability by making the positive requirements more explicit - the code continues only when both conditions are met.apps/dashboard/app/integrations/vercel/callback/client.tsx (1)
50-53: LGTM: Button disable logic is correctly structured.The condition properly requires both a
projectIdand at least one selected API environment before enabling the save button. The negated conjunction clearly expresses the requirements: the button is disabled when we don't have both a project selection and at least one API environment configured.tools/artillery/main.ts (1)
26-26: LGTM: Environment variable validation correctly transformed.The condition
!(ARTILLERY_CLOUD_API_KEY && UNKEY_KEY)is logically equivalent to the original form and improves readability by expressing the positive requirement - both environment variables must be present for the script to proceed.apps/dashboard/app/new/create-ratelimit.tsx (1)
18-18: LGTM! Correct application of De Morgan's law for style consistency.The logical transformation from
!user?.orgId || !user.roleto!(user?.orgId && user.role)is mathematically equivalent and improves code readability by using a single negated conjunction instead of separate negations.apps/dashboard/lib/auth.ts (1)
35-35:Details
❓ Verification inconclusive
Critical: This changes the authentication logic behavior!
The transformation from
!authResult.orgId || !authResult.roleto!(authResult.orgId || authResult.role)is NOT logically equivalent:
- Original: Redirect if EITHER
orgIdORroleis missing- Current: Redirect if BOTH
orgIdANDroleare missingThis means users with only one of these properties will now proceed instead of being redirected to "/new", which could be a security issue or functional regression.
If the intent was to apply De Morgan's law for style consistency, the correct transformation should be:
-if (!(authResult.orgId || authResult.role)) { +if (!(authResult.orgId && authResult.role)) {Let me verify the original condition in the codebase:
🏁 Script executed:
#!/bin/bash # Search for the original condition pattern in auth-related files rg -A 3 -B 3 "authResult\.(orgId|role)" apps/dashboard/lib/auth* || true rg -A 3 -B 3 "getAuth.*redirect" apps/dashboard/lib/auth* || trueLength of output: 1383
Critical: Authentication conditional logic change alters behavior
I wasn’t able to locate the original
if (!authResult.orgId || !authResult.role)check in the current branch, so please confirm whether the previous logic indeed redirected when eitherorgIdorrolewas missing. As it stands:
- File: apps/dashboard/lib/auth.ts (line 35)
- Current code:
if (!(authResult.orgId || authResult.role)) { redirect("/new"); }- This only redirects when both
orgIdandroleare missing, whereasif (!A || !B)would redirect when either is missing.If the intent was to enforce that both fields must be present before proceeding, the correct De Morgan’s transformation would be:
- if (!(authResult.orgId || authResult.role)) { + if (!(authResult.orgId && authResult.role)) { redirect("/new"); }Please verify the intended behavior and adjust accordingly.
apps/dashboard/components/ui/form.tsx (1)
109-109: LGTM! Improved accessibility implementation.The conditional logic correctly associates form elements with their descriptions and error messages:
- When there's an error: includes both description and error message IDs
- When no error: includes only the description ID
This follows proper accessibility patterns for form controls.
apps/dashboard/components/virtual-table/hooks/useVirtualData.ts (2)
43-43: LGTM! Correct De Morgan's law transformation.The change from
!lastItem || !onLoadMoreto!(lastItem && onLoadMore)is logically equivalent and improves code consistency.
56-56: LGTM! Correct De Morgan's law transformation.The change from
!isLoading && !isFetchingNextPageto!(isLoading || isFetchingNextPage)is logically equivalent and maintains the same behavior while improving readability.apps/dashboard/components/navigation/copyable-id-button.tsx (1)
46-55: LGTM! Clean conditional logic implementation.The handleClick function correctly handles the two scenarios:
- When text is selected - prevents copy action and stops event propagation
- When no text is selected - triggers the programmatic copy action
The logic is clear and maintains the intended behavior.
apps/dashboard/app/auth/sign-up/email-signup.tsx (1)
114-122: LGTM! Proper handling of hydration and loading states.The nested conditional rendering correctly handles:
- Client-side hydration state (
clientLoaded)- Loading state within the client-loaded context
- Fallback display when client hasn't loaded yet
The logic maintains the intended behavior while improving readability through better nesting structure.
apps/dashboard/components/logs/overview-charts/utils.tsx (1)
26-26: LGTM! Correct application of De Morgan's law.The condition
!(currentTimestamp && data?.length)is logically equivalent to the original!currentTimestamp || !data?.length. This refactoring improves consistency with the codebase style while maintaining identical functionality.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-external-id/index.tsx (2)
54-54: LGTM! Correct application of De Morgan's law.The condition
!(isConfirmPopoverOpen || open)is logically equivalent to!isConfirmPopoverOpen && !open. This ensures the dialog closes only when neither the confirm popover nor the dialog itself is open, maintaining the correct behavior.
102-102: LGTM! Proper button state logic.The condition
!(originalIdentityId || selectedIdentityId)correctly disables the button when neither ID is present. This is logically equivalent to!originalIdentityId && !selectedIdentityIdand ensures the button is only enabled when at least one ID is available.apps/dashboard/components/virtual-table/index.tsx (2)
415-415: LGTM: Correct De Morgan's law transformation.The refactoring from
if (!sortable || !onSort)toif (!(sortable && onSort))is logically equivalent and improves code consistency.
419-419: LGTM: Ternary logic refactoring is correct.The ternary expression maintains the same logic flow: no direction → "asc", "asc" → "desc", "desc" → null.
tools/local/src/prepare.ts (1)
17-19: LGTM: Control flow simplification is correct.The restructuring maintains the same logical behavior while simplifying the conditional structure. The success message is appropriately logged when artifacts exist.
apps/dashboard/pages/api/v1/github/verify.ts (1)
55-55: LGTM: Correct De Morgan's law transformation.The environment variable validation is correctly refactored from
!RESEND_API_KEY || !GITHUB_KEYS_URIto!(RESEND_API_KEY && GITHUB_KEYS_URI).apps/dashboard/pages/api/v1/workos/webhooks.ts (2)
12-12: LGTM: Correct multi-variable environment check transformation.The refactoring correctly applies De Morgan's law to the environment variable validation, maintaining logical equivalence while improving consistency.
16-16: LGTM: Correct payload validation transformation.The condition correctly transforms from
!payload || !sigHeaderto!(payload && sigHeader)using De Morgan's law.apps/dashboard/components/navigation/sidebar/team-switcher.tsx (1)
112-123: LGTM! Improved conditional rendering clarity.The refactor from logical AND to explicit ternary operator makes the conditional rendering more readable and follows React best practices. The behavior remains identical - tooltip is shown when sidebar is not collapsed.
apps/dashboard/lib/posthog.ts (1)
10-16: LGTM! Clearer environment variable validation.The refactor improves readability by using positive conditions to check for the presence of both required environment variables. This is more intuitive than checking for their absence and maintains the same logical behavior.
apps/dashboard/hooks/use-keyboard-shortcut.tsx (2)
175-190: LGTM! Improved control flow with early return pattern.The refactor uses an early return pattern for duplicate key detection, making the code more readable and reducing nesting. The key assignment logic outside the conditional is clearer and maintains the same validation behavior.
200-200: LGTM! Clearer validation using De Morgan's law.The condition
!(keyAssigned && combo.key && combo.code)clearly expresses "not all required values are present" which is more intuitive than the equivalent disjunctive form. The logical behavior is preserved.apps/dashboard/components/logs/queries/queries-popover.tsx (1)
43-53: LGTM! Consistent conditional logic pattern.The refactor inverts the keyboard shortcut handler logic to check the "close" condition first, maintaining identical behavior while aligning with the broader conditional expression standardization across the codebase. State management remains unchanged.
apps/api/src/routes/v1_keys_getVerifications.ts (1)
306-306: LGTM! Modern optional chaining improvement.The change from
!data || !data.lengthto!data?.lengthis logically equivalent and uses modern JavaScript optional chaining for better readability and conciseness.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts (1)
212-212: LGTM! Performance optimization using Date.now().Replacing
new Date().getTime()withDate.now()is a good optimization that avoids unnecessary Date object creation while providing the same timestamp value.apps/api/src/routes/v1_ratelimits_getOverride.ts (1)
75-75: LGTM! Improved logical expression clarity.The change from
!namespaceId && !namespaceNameto!(namespaceId || namespaceName)applies De Morgan's law and makes the condition more intuitive to read: "if not (either parameter is provided)".apps/api/src/routes/v1_ratelimits_listOverrides.ts (1)
77-77: LGTM! Consistent logical expression improvement.This change matches the same De Morgan's law refactoring applied in other ratelimit routes, providing consistent and more readable parameter validation across the codebase.
apps/api/src/routes/v1_identities_getIdentity.ts (1)
83-83: LGTM! Correct application of De Morgan's law.The conditional logic refactoring from
!identityId && !externalIdto!(identityId || externalId)is logically equivalent and maintains the same validation behavior while expressing the requirement more explicitly.apps/dashboard/app/(app)/apis/_components/controls/components/logs-datetime/index.tsx (1)
76-76: LGTM! Improved conditional class application.The change from
!titletotitle ? "" : "opacity-50"makes the conditional CSS class application more explicit and correct. This ensures the "opacity-50" class is applied whentitleis falsy, which is the intended behavior.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/expiration-setup.tsx (1)
126-126: LGTM! Better performance and modern approach.Replacing
new Date().getTime()withDate.now()is a good improvement.Date.now()is more direct, slightly more performant (avoids object instantiation), and follows modern JavaScript best practices while maintaining identical functionality.apps/api/src/routes/v1_ratelimits_deleteOverride.ts (1)
69-69: LGTM! Consistent application of De Morgan's law.The conditional logic refactoring from
!namespaceId && !namespaceNameto!(namespaceId || namespaceName)is logically equivalent and maintains the same validation behavior. This change improves readability by expressing the requirement more explicitly and aligns with the systematic refactoring across the codebase.apps/dashboard/app/(app)/audit/components/controls/components/logs-datetime/index.tsx (1)
76-76: LGTM! Improved ternary condition readability.The refactored ternary condition
title ? "" : "opacity-50"maintains the same logic (apply opacity when title is falsy) but is more direct and readable than the negated version. This change aligns with the systematic style improvements across similar datetime components.apps/dashboard/app/(app)/settings/team/client.tsx (1)
55-55: Excellent refactor using De Morgan's law.The condition
!(user && organization && userMemberships && currentOrgMembership)is logically equivalent to the original but more clearly expresses the intent: "if we don't have all required data". This improves readability by focusing on the positive condition (having all data) rather than multiple negations.apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts (1)
49-51: Well-structured filter condition.The refactored condition
!(Array.isArray(value) && ["keyIds", "names", "identities", "outcomes"].includes(field))maintains the same filtering logic but more clearly expresses the intent: "continue unless we have an array value for one of these specific fields". This makes the filter processing logic more explicit and easier to understand.apps/dashboard/components/stats-card/components/chart/stats-chart.tsx (1)
70-70: Improved logical grouping in tooltip condition.The refactored condition
!(active && payload?.length) || payload?.[0]?.payload.total === 0maintains equivalent logic while better grouping related checks. The first part!(active && payload?.length)clearly validates basic tooltip prerequisites, while the second part checks for meaningful data. This enhances code readability and maintainability.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/disable-key.tsx (1)
69-73: LGTM! Logical refactoring improves readability.The conditional logic inversion maintains the same behavior while improving readability by checking the positive condition (
isEnabling) first. This aligns well with the comment indicating that confirmation popover is only shown for disabling keys.apps/dashboard/lib/zod-helper.ts (1)
67-67: Good simplification: UsingDate.now()is more efficient.Replacing
new Date().getTime()withDate.now()is a good optimization that avoids creating an unnecessary Date object while maintaining the same functionality for timestamp retrieval.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (2)
101-101: Correct application of De Morgan's law.The transformation from
!isSelected && !isHoveredto!(isSelected || isHovered)is logically equivalent and improves readability by emphasizing the negation of the entire condition.
282-282: Consistent De Morgan's law transformation.The early exit condition
!(allEnabled || allDisabled)is logically equivalent to the original!allEnabled && !allDisabledand maintains the same control flow while aligning with the codebase-wide refactoring pattern.apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-datetime/index.tsx (1)
76-76: Clean conditional expression for opacity styling.The ternary operator
title ? "" : "opacity-50"clearly expresses the intent to apply opacity styling when no title is present, which aligns with the button's disabled state logic on line 82.apps/dashboard/app/(app)/logs/components/controls/components/logs-datetime/index.tsx (1)
76-76: LGTM: Improved conditional clarity.The refactored ternary operator is logically equivalent to the previous
!titlecondition but makes the intent more explicit. This change aligns with similar updates across other logs-datetime components for consistency.apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-datetime/index.tsx (1)
76-76: LGTM: Consistent conditional style improvement.The ternary operator refactoring maintains the same functional behavior while improving code consistency across similar components. The explicit conditional expression is clearer than the negated version.
apps/dashboard/app/(app)/ratelimits/_components/controls/components/logs-datetime/index.tsx (1)
76-76: LGTM: Completes consistent style refactoring.This change completes the consistent refactoring pattern applied across all logs-datetime components, improving code readability while maintaining identical functional behavior.
apps/api/src/routes/v1_ratelimits_setOverride.ts (1)
84-84: LGTM! Cleaner conditional expression using De Morgan's law.The refactored condition
!(req.namespaceId || req.namespaceName)is more readable than the original!req.namespaceId && !req.namespaceNameas it directly expresses "if neither parameter is provided" rather than "if the first is not provided AND the second is not provided".apps/dashboard/lib/auth/local.ts (4)
152-152: LGTM! Improved parameter validation clarity.The refactored condition
!(name && userId)is more intuitive than!name || !userIdas it directly expresses the requirement that both parameters must be provided.
185-185: LGTM! Consistent parameter validation pattern.The change to
!(id && name)maintains logical equivalence while making the validation requirement clearer.
265-265: LGTM! Clear validation logic.The refactored condition
!(membershipId && role)clearly expresses that both parameters are required.
292-292: LGTM! Consistent validation approach.The change to
!(orgId && email)follows the same clear pattern established in other methods.apps/api/src/routes/v1_migrations_createKey.ts (1)
405-405: LGTM! More intuitive parameter validation.The refactored condition
!(key.hash || key.plaintext)directly expresses the requirement that at least one of these fields must be provided, which aligns better with the error message "provide eitherhashorplaintext".apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/metadata-setup.tsx (1)
103-103: LGTM! Clearer validation logic.The refactored condition
!(value && validateJSON(value))more clearly expresses the validation requirement: "both a value must exist AND it must be valid JSON" for the validation to pass when metadata is enabled.apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-datetime/index.tsx (2)
70-70: LGTM! Improved optional chaining usage.The change from explicit falsy check to optional chaining (
!option.value?.endsWith("m")) is cleaner and handles undefined/null values more safely.
78-78: LGTM! Clearer conditional expression.The inverted ternary operator makes the positive condition more explicit while maintaining the same logical behavior.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-datetime/index.tsx (2)
70-70: LGTM! Consistent optional chaining improvement.Same improvement as in the related component - using optional chaining for safer value access.
78-78: LGTM! Consistent conditional expression clarity.The ternary operator inversion maintains logical equivalence while improving readability, consistent with similar components.
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx (1)
68-80: LGTM! Improved conditional rendering logic.The inversion of the conditional logic makes the code more intuitive - when metadata is empty, show the "No metadata available" message, otherwise show the formatted JSON with copy functionality. The logic is equivalent but more readable.
apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts (1)
217-223: LGTM! Logical reordering of conditional branches.The reordering of conditional logic maintains the same behavior while prioritizing the edge case validation (rate limiting enabled but no rules provided) before handling the disabled state. This makes the error handling more explicit and follows good practice of checking edge cases early.
apps/dashboard/lib/trpc/routers/key/updateRemaining.ts (1)
36-75: LGTM! Logic correctly implements limit enable/disable functionality.The conditional structure properly handles both scenarios:
- When limits are enabled: Sets appropriate refill values and remaining count
- When limits are disabled: Clears all limit-related fields to null
The logic is clear and aligns well with the comment on line 35.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/selection-controls/components/batch-edit-external-id.tsx (1)
50-52: LGTM! De Morgan's law refactoring maintains logical equivalence.The change from
!isConfirmPopoverOpen && !opento!(isConfirmPopoverOpen || open)is logically equivalent and improves style consistency across the codebase.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/hooks/use-keys-list-query.ts (1)
27-29: LGTM! Conditional refactoring maintains correct filter logic.The change applies De Morgan's law correctly:
!(A && B)is equivalent to!A || !B. The filter processing logic remains functionally identical - it skips filters when either the field name is invalid or the params entry is falsy.apps/dashboard/components/logs/overview-charts/overview-area-chart.tsx (3)
82-84: LGTM! Mouse event handling logic correctly refactored.The change
!(enableSelection && selection.start)is logically equivalent to!enableSelection || !selection.startand maintains the correct early return behavior for mouse move events.
98-100: LGTM! Selection validation logic correctly refactored.The condition
!(selection.startTimestamp && selection.endTimestamp)properly validates that both timestamps exist before proceeding with the selection callback, maintaining the same logic as the original!selection.startTimestamp || !selection.endTimestamp.
233-235: LGTM! Tooltip rendering guard correctly refactored.The change
!(active && payload?.length)maintains the correct early return behavior for the tooltip content, equivalent to!active || !payload?.length.apps/dashboard/components/logs/chart/index.tsx (2)
97-97: LGTM: Correct application of De Morgan's lawThe condition refactoring from
!selection.startTimestamp || !selection.endTimestampto!(selection.startTimestamp && selection.endTimestamp)is logically equivalent and improves readability by expressing "not both are present" more clearly.
157-157: LGTM: Consistent boolean expression styleThe refactoring maintains the same logic while following the consistent pattern established across the codebase. The condition correctly returns null when the tooltip should not be displayed.
apps/dashboard/lib/auth/workos.ts (5)
177-177: LGTM: Consistent parameter validation patternThe boolean expression refactoring maintains the same validation logic while following the established coding style throughout the codebase.
224-224: LGTM: Consistent validation logicParameter validation follows the same pattern as other methods in the class, maintaining logical correctness.
259-261: LGTM: Condition refactoring with updated error message logicThe boolean condition refactoring is correct. The error message assignment logic has also been updated to set an empty message when authenticated and the failure reason otherwise, which appears intentional and consistent with the control flow.
369-369: LGTM: Standard parameter validationThe refactoring maintains the required parameter validation while following the consistent boolean expression style.
418-418: LGTM: Consistent invitation validationThe parameter validation follows the established pattern and maintains the same logic for required fields.
apps/dashboard/components/ui/chart.tsx (2)
76-76: LGTM: Appropriate lint suppression with clear explanationThe biome-ignore comment properly documents the intentional use of
dangerouslySetInnerHTMLfor dynamic CSS generation, which is necessary for chart theming functionality.
158-158: LGTM: Consistent tooltip rendering conditionThe boolean expression refactoring follows the same pattern used throughout the codebase while maintaining the correct early return logic for tooltip rendering.
apps/api/src/routes/v1_identities_updateIdentity.ts (2)
143-143: LGTM: Correct identifier validation logicThe refactoring correctly applies De Morgan's law to maintain the validation requirement that at least one of
identityIdorexternalIdmust be provided.
237-245: LGTM: Logic structure improvementThe conditional reordering maintains the same functionality while potentially improving code readability by handling the positive case (update) before the negative case (delete).
apps/dashboard/components/logs/overview-charts/overview-bar-chart.tsx (2)
99-99: LGTM: Correct application of De Morgan's law.The transformation from
!selection.startTimestamp || !selection.endTimestampto!(selection.startTimestamp && selection.endTimestamp)is logically equivalent and improves readability.
191-191: LGTM: Consistent logical transformation.The boolean logic refactoring using negated conjunction is correct and aligns with the consistent style being applied across the codebase.
internal/clickhouse/src/verifications.ts (3)
72-85: LGTM: Improved conditional logic readability.The refactoring from negated condition checking to positive condition checking makes the code more readable while maintaining the same logical behavior. The fallback to
"TRUE"is handled correctly in both branches.
90-98: LGTM: Consistent cursor condition refactoring.The cursor condition logic has been appropriately inverted to check for the presence of
cursorTimefirst, improving code clarity while preserving the same conditional behavior.
493-516: LGTM: Logical branch restructuring.The conditional branches in the merge function have been restructured to handle the positive case (
existingPoint) first, which is more intuitive than checking for the negative case. The logic remains functionally equivalent.internal/clickhouse/src/ratelimits.ts (3)
304-317: LGTM: Consistent conditional refactoring pattern.The filter condition construction has been improved by checking for the presence of filters first rather than their absence. This aligns with the refactoring pattern applied across other ClickHouse modules and improves code readability.
Also applies to: 319-336
497-529: LGTM: Consistent application of the same pattern.The same logical improvements have been correctly applied to the overview logs function, maintaining consistency across the module.
588-603: LGTM: Improved cursor condition logic.The cursor condition has been refactored to handle the positive case first, which is more intuitive and consistent with the refactoring pattern applied throughout the codebase.
internal/clickhouse/src/keys/keys.ts (2)
133-146: LGTM: Consistent filter condition refactoring.The outcome and key ID filter conditions have been refactored to use positive conditional checks, which is more readable and consistent with the pattern applied across all ClickHouse query modules in this PR.
Also applies to: 148-165
217-232: LGTM: Improved cursor pagination logic.The cursor condition logic has been appropriately restructured to handle the presence of
cursorTimefirst, making the pagination logic more intuitive while maintaining the same functional behavior.
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.utils.ts
Outdated
Show resolved
Hide resolved
ogzhanolguncu
changed the title
1 participant
What does this PR do?
This PR mostly fixes yoda expressions and useless else clauses.
Fixes # (issue)
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