UN-2781 [FIX] Fix admin route issue #1658
Conversation
This commit adds comprehensive support for admins to create custom Stripe subscription products for both Unstract and LLM Whisperer platforms. ## Frontend Changes ### Router Updates (Router.jsx) - Added dynamic import for LlmWhispererCustomCheckoutPage plugin component - Added public route `/llm-whisperer/custom-checkout` for LLM Whisperer custom plan checkout flow - Follows existing pattern with try/catch for Cloud-only features ### Main App Routes (useMainAppRoutes.js) - Added Unstract administration route at `/administration` - Protected with RequireAdmin wrapper (admin-only access) - Imported UnstractAdministrationPage component ### New Administration Page (UnstractAdministrationPage.jsx) - Plugin-loader pattern with try/catch for Cloud-only component - Falls back to "feature not available" message in OSS builds - Dynamically imports UnstractAdministration from Cloud plugins ## Backend Changes (Already Committed) The backend infrastructure was already added in previous commits: ### Subscription Routes (cloud_urls_v2.py) - Unstract admin endpoint: `/api/v1/unstract/admin/stripe/products/dynamic/` - LLM Whisperer admin endpoint: `/api/v1/llmwhisperer/admin/stripe/products/dynamic/portal/` ### Portal Proxy (subscription_v2/views.py) - `create_dynamic_stripe_product_portal_proxy()` method forwards LLM Whisperer admin requests to Portal service - Follows existing LLMWhispererService pattern for multi-backend architecture - Includes proper error handling and logging ### URL Patterns (subscription_v2/urls.py) - Added `admin_stripe_product_portal_proxy` view and URL pattern - Both endpoints protected with IsAuthenticated + IsAdmin permissions ## Architecture This implementation follows Unstract's plugin-based architecture: - OSS repo contains base routing and plugin-loader patterns - Cloud repo contains actual admin UI components (subscription-admin plugin) - Multi-backend support: Unstract uses local backend, LLM Whisperer proxies to Portal service - Graceful degradation for OSS builds (shows "Cloud-only feature" message) ## Related - Supersedes PR #1087 (monolithic admin UI) - Integrates checkout functionality from PR #1016 - Consolidates router changes from PR #1558 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Suppress SonarQube warnings for intentional empty catch blocks used in plugin loading architecture. These catch blocks are part of the graceful degradation pattern for Cloud-only features - when plugins are not available, the component remains undefined and React Router triggers NotFound page. This pattern is used throughout the codebase for optional plugin loading. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Move NOSONAR comments to separate lines to comply with Prettier formatting rules. Format multi-line Route component in useMainAppRoutes.js. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Place NOSONAR comment inline with catch statement for proper recognition. SonarQube requires the suppression comment on the same line as the issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add minimal NOSONAR comments to suppress SonarQube warnings for intentional empty catch blocks in plugin loading pattern. These catch blocks are part of the graceful degradation architecture for Cloud-only features. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…om-stripe-products-backend
…ation Add platform staff-only access control for custom Unstract subscription plans: - Add RequireStaff component to enforce platform staff access - Update custom plans route to use RequireStaff instead of RequireAdmin - Block access for open-source deployments (mock_org) Frontend Changes: - New RequireStaff.js component with isStaff and orgName validation - Update useMainAppRoutes.js to use RequireStaff for /admin/custom-plans route - Shows Unauthorized for non-staff users, NotFound for OSS deployments This complements the backend IsStaff permission class to ensure only platform staff can create custom Stripe products with dual pricing structure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove RequireStaff component as it's been replaced with inline staff checking pattern in route files. This avoids component dependency issues and follows the established Verticals pattern. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace RequireStaff component wrapper with inline staff check using useSessionStore in an IIFE. This matches the pattern used in Cloud plugin routes and fixes the build error where RequireStaff component was deleted but still referenced. The inline check ensures: - Staff-only access to admin custom plans route - Hidden in open-source deployments (orgName === "mock_org") - No component import dependencies 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Create UnstractAdministrationPage.css for fallback UI styling - Replace inline styles with className in UnstractAdministrationPage.jsx - Improves code maintainability and follows project's CSS patterns
- Move useSessionStore hook call to top level of function - Remove IIFE pattern that violated hooks rules - Simplify conditional rendering logic Fixes CodeRabbit review comment about hooks being called inside nested function.
- Always register admin/custom-plans route (no conditional) - Move staff permission check to UnstractAdministrationPage component - Check permissions at render time when session is loaded - Fixes route registration timing issue This ensures route exists and is accessible to staff users while maintaining security through component-level permission checks.
Summary by CodeRabbit
WalkthroughAdds session-based access control to UnstractAdministrationPage with staff and open-source status checks. Routes the guarded component to Changes
Sequence DiagramsequenceDiagram
participant User
participant Route as admin/custom-plans Route
participant Component as UnstractAdministrationPage
participant SessionStore as useSessionStore
User->>Route: Navigate to admin/custom-plans
Route->>Component: Render UnstractAdministrationPage
Component->>SessionStore: Fetch sessionDetails
alt sessionDetails loading
Component->>Component: Wait for load
else sessionDetails loaded
Component->>Component: Derive isStaff & isOpenSource
alt isStaff AND NOT isOpenSource
Component->>Component: Check cloud availability
alt UnstractAdministration available
Component-->>User: Render Admin Page
else unavailable
Component-->>User: Render Cloud-only Fallback
end
else Access Denied
Component-->>User: Render Access Denied
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
frontend/src/pages/UnstractAdministrationPage.css(1 hunks)frontend/src/pages/UnstractAdministrationPage.jsx(1 hunks)frontend/src/routes/Router.jsx(2 hunks)frontend/src/routes/useMainAppRoutes.js(2 hunks)
🔇 Additional comments (8)
frontend/src/routes/Router.jsx (2)
89-96: LGTM! Consistent plugin loading pattern.The dynamic import follows the established pattern for optional plugins, with appropriate error handling and NOSONAR annotation.
159-164: ****The route
/llm-whisperer/custom-checkoutdoes not actually render in the application. The referenced component file (LlmWhispererCustomCheckoutPage.jsx) does not exist in the codebase, and thepluginsdirectory does not exist. The dynamic require with try/catch block gracefully handles the missing component by leavingLlmWhispererCustomCheckoutPageundefined, which prevents the route from rendering at all due to the conditional check{LlmWhispererCustomCheckoutPage && (...)}. This is an intentional optional plugin architecture pattern. There is no authentication or security concern since the route will not be accessible when the plugin is not provided.Likely an incorrect or invalid review comment.
frontend/src/pages/UnstractAdministrationPage.jsx (3)
1-12: LGTM! Clean plugin loading setup.The dynamic import pattern and error handling are consistent with the codebase patterns, and the comment clearly explains the cloud-only nature of the feature.
20-28: Permission check logic is sound, but relies on fragile detection.The staff permission check correctly implements component-level authorization as intended by the PR. The logic appropriately restricts access for non-staff users and open-source deployments. However, this check's effectiveness depends on the reliable open-source detection logic flagged in the previous comment.
30-41: Insufficient backend evidence to verify the original concern—recommend manual verification.The search found no backend API endpoints for administration features or staff-only permission classes in the codebase. The
is_stafffield appears only as user data, not for access control. This suggests theUnstractAdministrationcomponent is either:
- A cloud-only plugin that doesn't call this backend at all, or
- Calls an external cloud service separately
The frontend permission check is correctly in place, but backend verification cannot be definitively confirmed due to absence of related backend APIs.
frontend/src/pages/UnstractAdministrationPage.css (1)
1-4: LGTM! Clean and minimal fallback styling.The CSS provides appropriate centering and spacing for the fallback messages.
frontend/src/routes/useMainAppRoutes.js (2)
25-25: LGTM! Clean import.
121-124: Excellent fix! Route now always registered with component-level authorization.This change directly addresses the PR objective by registering the route unconditionally during app initialization, eliminating the timing issue where session data (like
isStaff) loads asynchronously. The authorization check has been appropriately moved to theUnstractAdministrationPagecomponent itself.Note that unlike other admin routes (e.g., Lines 117-120, 178-182), this route is not wrapped in
<RequireAdmin />, which is intentional as the component now handles its own authorization.
Resolved conflicts by keeping fix version: - Route always registered (not conditional) - Staff permission check at component level - Removed unused imports and variables
- Add null check for sessionDetails before authorization - Prevents premature access denial during session load - Addresses CodeRabbit feedback on loading state - Maintains existing orgName === "mock_org" pattern consistent with RequireAdmin and usePostHogEvents 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/pages/UnstractAdministrationPage.jsx (1)
22-24: Fragile open-source detection remains unaddressed.The hardcoded
orgName === "mock_org"check (Line 24) was flagged in a previous review as a security and maintainability risk. This concern has not been addressed and remains valid.
🧹 Nitpick comments (1)
frontend/src/pages/UnstractAdministrationPage.jsx (1)
18-20: Consider adding a loading indicator for better UX.Returning
nullwhile waiting for session data is better than immediately showing "Access Denied," but users see a blank screen during load. A loading spinner or skeleton would improve the experience.Apply this diff to add a loading state:
// Wait for session to load before making authorization decisions if (!sessionDetails) { - return null; + return ( + <div className="administration-fallback"> + <p>Loading...</p> + </div> + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
frontend/src/pages/UnstractAdministrationPage.jsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
athul-rs
requested review from
jagadeeswaran-zipstack,
johnyrahul and
vishnuszipstack
athul-rs
changed the title
What
Why
Routes that depend on session data (like
isStaff) were not being registered correctly:How
1. Always register the route (
useMainAppRoutes.js):useSessionStoreimport2. Add component-level permission check (
UnstractAdministrationPage.jsx):useSessionStorehook to componentCan this PR break any existing features. If yes, please list possible items. If no, please explain why.
No, this will not break existing features:
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Test Cases:
Screenshots
N/A - Bug fix, no UI changes
Checklist
I have read and understood the Contribution Guidelines.