Conversation
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant updates across several components in the dashboard application, primarily focusing on enhancing feature access control through a new utility function, Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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)
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! 🙏 |
![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
🧹 Outside diff range and nitpick comments (9)
apps/dashboard/lib/utils.ts (2)
34-56: Documentation is comprehensive but needs minor improvementsThe documentation is thorough but could be enhanced:
- Return type in documentation mentions "undefined" but code returns "null"
- Example code shows undefined check but should show null check
Apply these documentation improvements:
* Checks if a workspace has access to a specific feature or beta feature -* Returns the feature value if access is granted, undefined otherwise +* Returns the feature value if access is granted, null otherwise * Note: Always returns true in development environment * * @param flagName - The name of the feature to check * @param workspace - The workspace to check access for -* @returns The feature value (boolean | number | string) if access granted, undefined otherwise +* @returns The feature value (boolean | number | string) if access granted, null otherwise * * @example * ```typescript * // Check if workspace has access to logs page * if (!hasWorkspaceAccess("logsPage", workspace)) { * return notFound(); * } * * // Check if workspace has access to a feature with numeric value * const userLimit = hasWorkspaceAccess("userLimit", workspace); -* if (userLimit === undefined) { +* if (userLimit === null) { * return notFound(); * } * ```
57-82: Consider security implications of development bypassThe implementation looks solid but has some security considerations:
- Development bypass returns
truefor all features- No logging of bypassed checks in development
Consider these improvements:
- Add environment variable to explicitly enable bypass mode
- Add debug logging in development mode
- Consider allowing selective feature bypass instead of all-or-nothing
Example implementation:
const FEATURE_BYPASS_ENABLED = process.env.FEATURE_BYPASS_ENABLED === 'true'; const BYPASS_FEATURES = process.env.BYPASS_FEATURES?.split(',') || []; export function hasWorkspaceAccess<T extends keyof ConfigObject>( flagName: T, workspace: Partial<WorkspaceFeatures>, ): FlagValue<T> | null { if (process.env.NODE_ENV === "development" && FEATURE_BYPASS_ENABLED) { if (BYPASS_FEATURES.length === 0 || BYPASS_FEATURES.includes(flagName)) { console.debug(`[Feature Bypass] Enabling feature: ${flagName}`); return true as FlagValue<T>; } } // ... rest of the implementation }apps/dashboard/app/(app)/logs/page.tsx (2)
32-34: Consider enhancing error handling and user feedbackThe implementation correctly uses
hasWorkspaceAccessbut could benefit from better error handling:
- Consider showing a more user-friendly message instead of 404
- Add logging for debugging purposes in development
Consider this enhancement:
if (!hasWorkspaceAccess("logsPage", workspace)) { + if (process.env.NODE_ENV === "development") { + console.debug("[Logs Page] Access denied: logsPage feature not enabled"); + } + return ( + <div className="text-center p-4"> + <h2>Logs Page Access Restricted</h2> + <p>Please contact your administrator to enable this feature.</p> + </div> + ); - return notFound(); }
Line range hint
44-46: Enhance error handling for ClickHouse errorsThe error handling for ClickHouse could be more robust:
if (logs.err) { - throw new Error(`Something went wrong when fetching logs from ClickHouse: ${logs.err.message}`); + console.error("[ClickHouse] Error fetching logs:", logs.err); + return ( + <div className="text-center p-4"> + <h2>Unable to Load Logs</h2> + <p>There was an error loading the logs. Please try again later.</p> + {process.env.NODE_ENV === "development" && ( + <pre className="mt-4 p-2 bg-gray-100 rounded"> + {logs.err.message} + </pre> + )} + </div> + ); }apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (1)
59-61: Consider extracting the default ratelimit override limit as a constantThe hardcoded value of 5 should be extracted into a named constant to improve maintainability and document its purpose.
+const DEFAULT_RATELIMIT_OVERRIDE_LIMIT = 5; + {Intl.NumberFormat().format( - hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) ?? 5, + hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) ?? DEFAULT_RATELIMIT_OVERRIDE_LIMIT, )}apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx (1)
Line range hint
51-64: Consider moving database operations to a service layerThe keyAuth size update logic should be moved to a dedicated service layer for better separation of concerns and maintainability. Additionally, consider:
- Extracting the cache duration (60_000ms) as a named constant
- Adding a lock mechanism to prevent race conditions during concurrent updates
Example service layer implementation:
// services/keyAuth.ts const KEY_AUTH_SIZE_CACHE_DURATION = 60_000; // 1 minute export class KeyAuthService { static async updateSizeIfStale(keyAuth: KeyAuth): Promise<void> { if (keyAuth.sizeLastUpdatedAt >= Date.now() - KEY_AUTH_SIZE_CACHE_DURATION) { return; } // TODO: Implement distributed locking mechanism const res = await db .select({ count: sql<string>`count(*)` }) .from(schema.keys) .where(and( eq(schema.keys.keyAuthId, keyAuth.id), isNull(schema.keys.deletedAt) )); await db .update(schema.keyAuth) .set({ sizeApprox: Number.parseInt(res?.at(0)?.count ?? "0"), sizeLastUpdatedAt: Date.now(), }) .where(eq(schema.keyAuth.id, keyAuth.id)); } }apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts (1)
Line range hint
62-68: Extract support email into configurationThe support email address is hardcoded in error messages. Consider extracting it into a configuration value for easier maintenance.
+const SUPPORT_EMAIL = process.env.SUPPORT_EMAIL ?? "support@unkey.dev"; + if (!hasWorkspaceAccess("ipWhitelist", api.workspace)) { throw new TRPCError({ code: "FORBIDDEN", message: - "IP Whitelisting is only available for enterprise plans. Please contact support@unkey.dev.", + `IP Whitelisting is only available for enterprise plans. Please contact ${SUPPORT_EMAIL}.`, }); }apps/dashboard/app/(app)/workspace-navigations.tsx (1)
93-93: Consider memoizing feature access checksThe implementation looks good, but there are multiple calls to
hasWorkspaceAccessfor different features. Consider memoizing these checks to optimize performance.Consider creating a memoized helper:
const getFeatureAccess = (workspace: Workspace) => { const memo = new Map<string, boolean>(); return (feature: string) => { if (!memo.has(feature)) { memo.set(feature, hasWorkspaceAccess(feature, workspace)); } return memo.get(feature)!; }; };Then use it in createWorkspaceNavigation:
const checkAccess = getFeatureAccess(workspace); // ... hidden: !checkAccess("webhooks"), // ...Also applies to: 100-100, 108-108, 122-122
apps/dashboard/app/(app)/audit/[bucket]/page.tsx (1)
94-94: Consider refactoring the retention days logic for better readabilityThe current nested nullish coalescing with ternary operator makes the logic hard to follow. Consider breaking it down into more explicit conditions.
Here's a suggested refactor:
- hasWorkspaceAccess("auditLogRetentionDays", workspace) ?? workspace.plan === "free" ? 30 : 90; + const customRetentionDays = hasWorkspaceAccess("auditLogRetentionDays", workspace); + if (customRetentionDays !== null) { + return customRetentionDays; + } + return workspace.plan === "free" ? 30 : 90;This makes it clearer that we:
- First check for custom retention days via feature flag
- Fall back to plan-based retention days if no custom value is set
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx(3 hunks)apps/dashboard/app/(app)/audit/[bucket]/page.tsx(3 hunks)apps/dashboard/app/(app)/identities/layout.tsx(2 hunks)apps/dashboard/app/(app)/logs/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx(2 hunks)apps/dashboard/app/(app)/workspace-navigations.tsx(5 hunks)apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts(2 hunks)apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts(2 hunks)apps/dashboard/lib/utils.ts(2 hunks)
🔇 Additional comments (8)
apps/dashboard/lib/utils.ts (1)
28-32: LGTM: Well-structured type definitions
The type definitions are well thought out:
WorkspaceFeaturescorrectly picks only relevant fieldsConfigObjectunion type ensures type safety across featuresFlagValuegeneric type provides precise typing for feature values
apps/dashboard/app/(app)/identities/layout.tsx (1)
Line range hint 25-29: LGTM: Clean implementation of feature check
The implementation correctly uses hasWorkspaceAccess and gracefully handles the case when access is not granted by showing the OptIn component.
However, consider adding error handling for the case when workspace is undefined:
+ if (!workspace) {
+ return redirect("/auth/sign-in");
+ }
+
if (!hasWorkspaceAccess("identities", workspace)) {
children = (
<OptIn title="Identities" description="Identities are in beta" feature="identities" />
);
}apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts (1)
7-7: LGTM: hasWorkspaceAccess integration
The integration of hasWorkspaceAccess for feature flag checking aligns well with the PR objectives and provides a cleaner way to handle feature access control.
apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx (1)
32-32: Props and feature check implementation looks good!
The changes improve type safety and consistency:
- Using the full
Workspacetype instead of partial type - Simplified component signature without unnecessary
React.FC - Consistent feature checking with
hasWorkspaceAccess
Also applies to: 41-41, 43-43
apps/dashboard/app/(app)/workspace-navigations.tsx (1)
47-47: Type improvements look good!
The simplified type definitions and component signatures improve code clarity:
- Removed unnecessary React.FC
- Simplified Workspace type in createWorkspaceNavigation
Also applies to: 59-59
apps/dashboard/app/(app)/audit/[bucket]/page.tsx (3)
8-8: LGTM: Import statement aligns with feature flag implementation
The import of hasWorkspaceAccess is correctly placed and follows the project's import organization pattern.
37-39: LGTM: Type definition formatting improves readability
The reformatted type definition enhances code clarity while maintaining the same functionality.
94-94: Verify consistent feature flag access pattern
Let's verify that other instances of feature access have been updated to use the new hasWorkspaceAccess utility.
✅ Verification successful
Feature flag access pattern is consistently implemented
The codebase shows consistent usage of the hasWorkspaceAccess utility across all feature flag checks. No direct access to workspace.features was found, indicating proper implementation of the feature flag pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for direct feature access patterns that might need updating
rg -g '!{*.test.*,*.spec.*}' -A 2 'workspace\.features\.'
# Search for existing hasWorkspaceAccess usage for consistency
rg -g '!{*.test.*,*.spec.*}' -A 2 'hasWorkspaceAccess\('
Length of output: 3543
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
apps/dashboard/lib/utils.ts (3)
61-63: Consider using a type-safe development mode bypassThe current type assertion
true as FlagValue<T>might hide type mismatches when the actual feature value is expected to be a number or string.Consider this safer alternative:
- return true as FlagValue<T>; + return (typeof workspace?.features?.[flagName] === 'number' ? 1 : true) as FlagValue<T>;
34-56: Enhance documentation with return type examplesThe documentation is comprehensive but could be more explicit about return types for different feature types.
Add examples for numeric and string feature flags:
* @example * ```typescript * // Check if workspace has access to logs page * if (!flag("logsPage", workspace)) { * return notFound(); * } * * // Check if workspace has access to a feature with numeric value * const userLimit = flag("userLimit", workspace); * if (userLimit === undefined) { * return notFound(); * } +* +* // Example with string feature value +* const tier = flag("subscriptionTier", workspace); +* if (tier === "premium") { +* // Enable premium features +* } * ```
69-79: Improve type safety of feature value castingThe type assertions when returning feature values could potentially hide type mismatches.
Consider adding runtime type checking:
- return betaFeature as FlagValue<T>; + const expectedType = typeof workspace.features?.[flagName]; + if (expectedType !== typeof betaFeature) { + console.warn(`Type mismatch for feature ${String(flagName)}: expected ${expectedType}, got ${typeof betaFeature}`); + } + return betaFeature as FlagValue<T>;apps/dashboard/app/(app)/workspace-navigations.tsx (1)
59-59: Consider adding type safety for segments parameterWhile the workspace type has been improved, the segments parameter could benefit from stronger typing.
Consider adding a union type for valid segments:
- segments: string[], + segments: Array<'apis' | 'ratelimits' | 'authorization' | 'audit' | 'verifications' | 'logs' | 'success' | 'semantic-cache' | 'identities' | 'settings'>,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx(3 hunks)apps/dashboard/app/(app)/audit/[bucket]/page.tsx(3 hunks)apps/dashboard/app/(app)/identities/layout.tsx(2 hunks)apps/dashboard/app/(app)/logs/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx(3 hunks)apps/dashboard/app/(app)/workspace-navigations.tsx(5 hunks)apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts(2 hunks)apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts(2 hunks)apps/dashboard/lib/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/dashboard/app/(app)/identities/layout.tsx
- apps/dashboard/app/(app)/logs/page.tsx
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx
- apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts
- apps/dashboard/app/(app)/audit/[bucket]/page.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx
🧰 Additional context used
📓 Learnings (1)
apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts (1)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.
🔇 Additional comments (3)
apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts (1)
63-63: LGTM! Maintains existing fallback behavior
The change correctly uses the new flag function while preserving the intended fallback behavior where max defaults to 5 when the feature flag returns 0 or false.
apps/dashboard/app/(app)/workspace-navigations.tsx (2)
Line range hint 47-56: LGTM! Modern type definition
The Tag component's type definition has been improved to use a more modern and explicit approach.
Line range hint 93-122: LGTM! Consistent feature flag usage
The implementation consistently uses the new flag function for checking feature availability across all navigation items.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
apps/dashboard/lib/utils.ts (2)
28-32: Consider more domain-specific type namesWhile the types are well-structured, consider renaming them to be more domain-specific:
WorkspaceFeatures→WorkspaceFeatureConfigConfigObject→FeatureFlagConfigFlagValue→FeatureFlagValueThis would make their purpose more immediately clear to other developers.
34-56: Fix return type documentationThe documentation states "undefined otherwise" but the function returns
null. Update the documentation to match the implementation:- * Returns the feature value if access is granted, undefined otherwise + * Returns the feature value if access is granted, null otherwiseAlso in the example:
- * if (userLimit === undefined) { + * if (userLimit === null) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx(3 hunks)apps/dashboard/app/(app)/audit/[bucket]/page.tsx(3 hunks)apps/dashboard/app/(app)/identities/layout.tsx(2 hunks)apps/dashboard/app/(app)/logs/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx(3 hunks)apps/dashboard/app/(app)/workspace-navigations.tsx(5 hunks)apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts(2 hunks)apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts(2 hunks)apps/dashboard/lib/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/dashboard/app/(app)/identities/layout.tsx
- apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts
- apps/dashboard/app/(app)/logs/page.tsx
- apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts
- apps/dashboard/app/(app)/audit/[bucket]/page.tsx
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx
- apps/dashboard/app/(app)/workspace-navigations.tsx
🔇 Additional comments (2)
apps/dashboard/lib/utils.ts (2)
4-5: LGTM!
The import is necessary and correctly structured.
57-82: Rename function to match documentation
The function is named getFlag but the documentation examples use flag. Consider renaming for consistency:
-export function getFlag<T extends keyof ConfigObject>(
+export function flag<T extends keyof ConfigObject>(This matches the examples and provides a more concise API.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
apps/dashboard/lib/utils.ts (1)
34-68: Add @throws to JSDoc documentation.The documentation is comprehensive but should document the error case when no workspace is found.
Add the following to the JSDoc:
* @returns The feature value (boolean | number | string) based on environment and settings +* @throws {Error} When no workspace is found in the database * * @example
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx(3 hunks)apps/dashboard/app/(app)/audit/[bucket]/page.tsx(4 hunks)apps/dashboard/app/(app)/identities/layout.tsx(2 hunks)apps/dashboard/app/(app)/logs/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx(2 hunks)apps/dashboard/app/(app)/workspace-navigations.tsx(5 hunks)apps/dashboard/lib/constants.ts(1 hunks)apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts(2 hunks)apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts(2 hunks)apps/dashboard/lib/utils.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/dashboard/lib/constants.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx
- apps/dashboard/app/(app)/logs/page.tsx
- apps/dashboard/app/(app)/identities/layout.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx
- apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts
- apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts
🔇 Additional comments (5)
apps/dashboard/lib/utils.ts (2)
28-32: LGTM! Well-structured type definitions.
The type definitions provide strong type safety through:
- Proper use of TypeScript's utility types (Pick, NonNullable)
- Clear separation of concerns between workspace features and configuration
90-98:
Add runtime type validation for feature values.
The type assertions (as FlagValue<TFlagName>) could be unsafe if the feature values don't match the expected types.
Consider adding runtime type validation:
+function validateFeatureValue<T>(value: unknown, flagName: string): value is T {
+ // Add type-specific validation based on flagName
+ return true;
+}
const betaFeature = workspace.betaFeatures?.[flagName as keyof Workspace["betaFeatures"]];
-if (betaFeature !== undefined) {
+if (betaFeature !== undefined && validateFeatureValue<FlagValue<TFlagName>>(betaFeature, flagName)) {
return betaFeature as FlagValue<TFlagName>;
}
const feature = workspace.features?.[flagName as keyof Workspace["features"]];
-if (feature !== undefined) {
+if (feature !== undefined && validateFeatureValue<FlagValue<TFlagName>>(feature, flagName)) {
return feature as FlagValue<TFlagName>;
}Likely invalid or redundant comment.
apps/dashboard/app/(app)/workspace-navigations.tsx (3)
16-16: LGTM: Import of getFlag utility
The addition of the getFlag utility import aligns with the PR objectives to implement type-safe feature flag checks.
Line range hint 47-56: LGTM: Improved type safety in Tag component
The change from React.FC to inline type definition is a good practice as it:
- Makes the component props more explicit
- Removes the implicit
childrenprop that wasn't being used
59-59: Verify workspace type change impact
The simplified type Workspace & { llmGateways: { id: string }[] } might expose more workspace fields than necessary. Consider using a more specific type that only includes the required fields.
✅ Verification successful
Type change is safe to use
Based on the codebase analysis, the simplified type Workspace & { llmGateways: { id: string }[] } is appropriate for this context because:
- The type is used specifically for navigation creation where only feature flags and llmGateways are accessed
- The intersection type maintains type safety by explicitly declaring the additional llmGateways requirement
- The usage in the function shows controlled access to workspace properties through the
getFlagutility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Workspace type definition to understand exposed fields
ast-grep --pattern 'type Workspace = {
$$$
}'
Length of output: 49
Script:
#!/bin/bash
# Search for Workspace type/interface definition with different patterns
ast-grep --pattern 'interface Workspace {
$$$
}'
# Also search for potential type imports
rg -l "Workspace.*from.*@"
# Search for Workspace type usage patterns
rg "type.*Workspace" -A 2
Length of output: 4019
Script:
#!/bin/bash
# Let's check the actual Workspace type definition in internal/db
cat internal/db/src/types.ts
# Let's also check the schema definition
cat internal/db/src/schema.ts
# Check the actual usage context
cat apps/dashboard/app/\(app\)/workspace-navigations.tsx
Length of output: 6809
Script:
#!/bin/bash
# Let's check the schema definition in a different location since it wasn't found
fd schema.ts
# And then read the content of any relevant schema files
cat internal/db/schema.ts
Length of output: 298
| } | ||
|
|
||
| if (!workspace.betaFeatures.identities) { | ||
| if (getFlag(workspace, "identities", { prodFallback: true, devFallback: false })) { |
There was a problem hiding this comment.
you flipped it around, so when they had it set to true in our database, we would actually deny them access
| if (getFlag(workspace, "identities", { prodFallback: true, devFallback: false })) { | |
| if (!getFlag(workspace, "identities", { prodFallback: false, devFallback: true })) { |
| } | ||
|
|
||
| if (!workspace.betaFeatures.logsPage) { | ||
| if (getFlag(workspace, "logsPage", { devFallback: false, prodFallback: true })) { |
There was a problem hiding this comment.
| if (getFlag(workspace, "logsPage", { devFallback: false, prodFallback: true })) { | |
| if (!getFlag(workspace, "logsPage", { devFallback: true, prodFallback: false })) { |
| label: "Monitors", | ||
| active: segments.at(0) === "verifications", | ||
| hidden: !workspace.features.webhooks, | ||
| hidden: getFlag(workspace, "webhooks", { |
There was a problem hiding this comment.
that's reversed too
it's too confusing this way I think
in the db I'd set it to { "webhooks": true } if I want to give someone access, but now that would revoke access...
2 participants
What does this PR do?
This PR allows us to bypass features and betaFeatures checks when developing locally. It also allows us to pick any feature flag with type safety.
Fixes #2704 (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
Release Notes
New Features
getFlag, improving clarity and maintainability across various components, including navigation and feature visibility.Bug Fixes
Documentation