-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: dataroom index #1936
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: dataroom index #1936
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughUpdates feature flag gating logic for dataroom index rebuild functionality across the UI component and API endpoint. Changes permission checks from plan-based-only to a combined condition using feature flags and plan presence, allowing both feature flag enabled users and datarooms-plus plan subscribers to proceed. Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/datarooms/actions/rebuild-index-button.tsx (1)
39-47: Clear permission logic with intentional upsell funnel.The boolean flags are well-defined with helpful comments. The logic creates an intentional UX funnel where:
shouldShowButton: Shows button to feature flag users OR any datarooms plan holderscanUseFeature: Allows usage only for feature flag users OR datarooms-plus plan holdersThis means users with the base datarooms plan will see the button but receive an upgrade prompt when clicking. While this drives upgrades, consider if this creates confusion or if hiding the button entirely for non-plus users would provide better UX.
If you prefer to hide the button for users who cannot use the feature, simplify to:
- // Show button if: feature flag is enabled OR user has datarooms plan or higher - const shouldShowButton = isDataroomIndexEnabled || hasDataroomsPlan; - - // Allow usage if: feature flag is enabled OR user has datarooms-plus plan - const canUseFeature = isDataroomIndexEnabled || hasDataroomsPlusPlan; + // Show button and allow usage if: feature flag is enabled OR user has datarooms-plus plan + const shouldShowButton = isDataroomIndexEnabled || hasDataroomsPlusPlan; + const canUseFeature = shouldShowButton; // Same condition
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/datarooms/actions/rebuild-index-button.tsx(3 hunks)pages/api/teams/[teamId]/datarooms/[id]/calculate-indexes.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/datarooms/actions/rebuild-index-button.tsx (2)
lib/hooks/use-feature-flags.ts (1)
useFeatureFlags(10-31)lib/swr/use-billing.ts (1)
usePlan(86-136)
pages/api/teams/[teamId]/datarooms/[id]/calculate-indexes.ts (1)
lib/featureFlags/index.ts (1)
getFeatureFlags(20-63)
🔇 Additional comments (5)
pages/api/teams/[teamId]/datarooms/[id]/calculate-indexes.ts (2)
43-65: LGTM! Efficient eager loading of team plan.The query modification correctly adds team plan selection while maintaining proper user membership validation. The minimal select fields keep the query efficient.
71-81: Consistent permission logic with frontend; error handling already in place.The combined feature flag and plan-based gating correctly implements OR logic that matches the frontend's
canUseFeaturecheck. ThegetFeatureFlagsfunction already includes proper error handling with a try-catch block that gracefully degrades to default features on failure, preventing endpoint errors from propagating.components/datarooms/actions/rebuild-index-button.tsx (3)
7-7: LGTM! Correct feature flag integration.The
useFeatureFlagshook is properly imported and used to check feature flag status.Also applies to: 34-34
50-58: LGTM! Proper guard conditions.The button rendering guard (line 50) and permission check (line 55) correctly use the new boolean flags, with a clear error message when the feature is not available.
142-161: LGTM! Appropriate upgrade flow.The DialogFooter correctly uses
canUseFeatureto show either action buttons or an upgrade modal, providing a clear path for users to gain access to the feature.
Summary by CodeRabbit