-
Couldn't load subscription status.
- Fork 5.5k
Share Launchdarkly OAuth components #15279
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
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a suite of new modules for the LaunchDarkly OAuth integration. New action modules for evaluating, toggling, and updating feature flags have been added, all of which extend existing components using a shared utility, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuthApp as LaunchDarkly OAuth App
participant ExternalAPI as External API
Client->>OAuthApp: Call feature flag action (e.g., update-feature-flag)
OAuthApp->>OAuthApp: Execute getHeaders() to construct appropriate headers
OAuthApp->>ExternalAPI: Send HTTP request with OAuth headers
ExternalAPI-->>OAuthApp: Return response
OAuthApp-->>Client: Relay response
sequenceDiagram
participant Webhook
participant OAuthApp as LaunchDarkly OAuth App
participant EventHandler
Webhook->>OAuthApp: Trigger event (e.g., new access token event)
OAuthApp->>EventHandler: Process event using merged common properties
EventHandler-->>OAuthApp: Confirm event processing
OAuthApp-->>Webhook: Acknowledge event receipt
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/gather/gather.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/changes_page/changes_page.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/milvus/milvus.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (12)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 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 (
|
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 (2)
components/launch_darkly_oauth/common/utils.mjs (1)
1-40: Add input validation and error handling.The utility function could be more robust with proper input validation and error handling.
Consider these improvements:
export function adjustPropDefinitions(props, app) { + if (!props || typeof props !== 'object') { + throw new Error('props must be a non-null object'); + } + if (!app) { + throw new Error('app identifier is required'); + } + return Object.fromEntries( Object.entries(props).map(([ key, prop, ]) => { if (typeof prop === "string") return [ key, prop, ]; const { - propDefinition, ...otherValues + propDefinition = null, ...otherValues } = prop || {}; if (propDefinition) { const [ - , ...otherDefs + , ...otherDefs ] = propDefinition; return [ key, { propDefinition: [ app, ...otherDefs, ], ...otherValues, }, ]; } return [ key, otherValues.type === "app" ? null : otherValues, ]; }) .filter(([ , value, ]) => value), ); }components/launch_darkly_oauth/actions/update-feature-flag/update-feature-flag.mjs (1)
1-21: Consider reducing code duplication across action components.While the implementation is correct, there's identical code structure across all three action components. Consider extracting this pattern into a higher-order function.
Example implementation:
// components/launch_darkly_oauth/common/utils.mjs export function createOAuthComponent(baseComponent, componentKey, app) { const { name, description, type, ...others } = baseComponent; const props = adjustPropDefinitions(others.props, app); return { ...baseComponent, key: `launch_darkly_oauth-${componentKey}`, version: "0.0.1", name, description, type, props: { app, ...props, }, }; } // components/launch_darkly_oauth/actions/update-feature-flag/update-feature-flag.mjs import { createOAuthComponent } from "../../common/utils.mjs"; import component from "../../../launchdarkly/actions/update-feature-flag/update-feature-flag.mjs"; import app from "../../launch_darkly_oauth.app.mjs"; export default createOAuthComponent(component, "update-feature-flag", app);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
components/launch_darkly_oauth/actions/evaluate-feature-flag/evaluate-feature-flag.mjs(1 hunks)components/launch_darkly_oauth/actions/toggle-feature-flag/toggle-feature-flag.mjs(1 hunks)components/launch_darkly_oauth/actions/update-feature-flag/update-feature-flag.mjs(1 hunks)components/launch_darkly_oauth/common/utils.mjs(1 hunks)components/launch_darkly_oauth/launch_darkly_oauth.app.mjs(1 hunks)components/launch_darkly_oauth/package.json(1 hunks)components/launch_darkly_oauth/sources/common/webhook.mjs(1 hunks)components/launch_darkly_oauth/sources/new-access-token-event/new-access-token-event.mjs(1 hunks)components/launch_darkly_oauth/sources/new-flag-event/new-flag-event.mjs(1 hunks)components/launch_darkly_oauth/sources/new-user-event/new-user-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/launch_darkly_oauth/package.json
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (11)
components/launch_darkly_oauth/sources/new-flag-event/new-flag-event.mjs (3)
1-2: Clean import structure!The imports effectively separate the OAuth-specific functionality (common/webhook) from the core LaunchDarkly component, aligning with the PR's objective of sharing components.
4-6: Clean property extraction!The destructuring pattern efficiently extracts only the necessary metadata properties from the original component.
8-16: Verify component structure and naming consistency.The component structure looks good, maintaining original functionality while properly integrating OAuth-specific changes.
Let's verify the naming pattern consistency across other LaunchDarkly OAuth components:
✅ Verification successful
Component structure and naming patterns are consistent
The component follows the established patterns across all LaunchDarkly OAuth components, maintaining consistent key prefixes and version numbers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent naming patterns in LaunchDarkly OAuth components # Check if all OAuth components follow the same key prefix pattern echo "Checking OAuth component key prefix consistency..." rg -l "key: \"launch_darkly_oauth-" components/launch_darkly_oauth/ # Check version consistency across OAuth components echo "Checking version consistency..." rg "version: \"[0-9]+\.[0-9]+\.[0-9]+\"" components/launch_darkly_oauth/Length of output: 1421
components/launch_darkly_oauth/sources/common/webhook.mjs (1)
1-14: LGTM! Well-structured common webhook module.The implementation correctly:
- Extends the base LaunchDarkly webhook functionality
- Configures necessary OAuth app integration
- Sets up proper database service reference
- Enables custom HTTP response handling for webhooks
components/launch_darkly_oauth/sources/new-user-event/new-user-event.mjs (1)
1-16: LGTM! Consider version management strategy.The implementation correctly extends the base component while maintaining proper separation. Starting at version 0.0.1 is appropriate for a new component.
Consider documenting the version management strategy. Run this to check version consistency across OAuth components:
✅ Verification successful
Version consistency verified across all components
All LaunchDarkly OAuth components are consistently versioned at 0.0.1, properly aligned with their base components. This is appropriate for new components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check version consistency across OAuth components echo "Checking version consistency across OAuth components..." rg '"version":\s*"([^"]*)"' components/launch_darkly_oauth/ -g '*.mjs'Length of output: 193
Script:
#!/bin/bash # First verify the file exists and check its structure echo "Checking file existence and structure..." fd "new-user-event.mjs" components/launch_darkly_oauth/ echo -e "\nExamining file content..." cat components/launch_darkly_oauth/sources/new-user-event/new-user-event.mjsLength of output: 705
Script:
#!/bin/bash # Check base component version and find other OAuth components echo "Base component version:" cat components/launchdarkly/sources/new-user-event/new-user-event.mjs echo -e "\nFinding other OAuth components:" fd -e mjs . components/launch_darkly_oauth/sources/ --exclude commonLength of output: 1337
Script:
#!/bin/bash # Check versions of all OAuth components echo "Checking versions of all OAuth components:" for file in components/launch_darkly_oauth/sources/new-access-token-event/new-access-token-event.mjs \ components/launch_darkly_oauth/sources/new-flag-event/new-flag-event.mjs; do echo -e "\n=== $file ===" cat "$file" | grep -A 1 'version:' doneLength of output: 1104
components/launch_darkly_oauth/launch_darkly_oauth.app.mjs (2)
1-14: LGTM! Clean OAuth integration.The implementation correctly:
- Extends base LaunchDarkly functionality
- Implements proper OAuth headers
- Maintains component consistency
8-13: Verify OAuth token security handling.While the implementation is correct, ensure proper token refresh handling and error cases are covered.
Run this to check for proper token error handling:
✅ Verification successful
OAuth implementation is correctly utilizing platform features
The implementation is secure as it:
- Uses platform's axios client which handles HTTP errors
- Relies on Pipedream's OAuth framework for token management
- Properly passes the OAuth token in Authorization header
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for token error handling echo "Checking for token error handling..." ast-grep --pattern 'catch ($_) { $$$' components/launch_darkly_oauth/ rg 'oauth_access_token' components/launch_darkly_oauth/ -C 5Length of output: 1031
Script:
#!/bin/bash # Check base implementation and framework handling echo "Checking base implementation..." cat components/launch_darkly_oauth/launch_darkly.app.mjs echo -e "\nChecking for framework OAuth handling..." rg -l "class.*extends.*\bApp\b" components/launch_darkly_oauth/ rg "extends" components/launch_darkly_oauth/ -A 5Length of output: 429
Script:
#!/bin/bash # Find LaunchDarkly related files echo "Searching for LaunchDarkly files..." fd -t f -e mjs launch_darkly echo -e "\nChecking imports in OAuth file..." rg "import|require" components/launch_darkly_oauth/launch_darkly_oauth.app.mjsLength of output: 399
Script:
#!/bin/bash # Check base implementation and OAuth error handling echo "Checking base implementation..." cat components/launchdarkly/launchdarkly.app.mjs echo -e "\nChecking for error handling patterns..." rg "throw|catch|error" components/launchdarkly/launchdarkly.app.mjs components/launch_darkly_oauth/launch_darkly_oauth.app.mjs -A 3Length of output: 5612
components/launch_darkly_oauth/sources/new-access-token-event/new-access-token-event.mjs (2)
1-24: LGTM! Well-structured event source component.The implementation correctly:
- Extends base component functionality
- Maintains consistent OAuth integration
- Properly references common props
15-23: Verify memberId prop definition availability.The implementation correctly references the memberId prop definition, but we should verify its existence in the app.
Run this to check the prop definition:
✅ Verification successful
✓ MemberId prop definition is correctly available
The memberId prop definition is properly implemented in the LaunchDarkly app and inherited by the OAuth component through the app extension chain. The implementation includes proper type definition, labeling, and async options for member selection.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check memberId prop definition echo "Checking memberId prop definition..." rg '"memberId".*propDefinition' components/launch_darkly_oauth/ -C 5 rg '"memberId".*propDefinition' components/launchdarkly/ -C 5Length of output: 217
Script:
#!/bin/bash # First, let's find the common file referenced echo "Searching for common file..." fd common.mjs components/launch_darkly_oauth/ -x cat {} echo -e "\nSearching for member-related definitions..." rg -i "member.*prop" components/launch_darkly_oauth/ -C 5 rg -i "member.*prop" components/launchdarkly/ -C 5 echo -e "\nSearching for app.js or app.mjs files..." fd "app\.(js|mjs)$" components/launch_darkly_oauth/ components/launchdarkly/ -x cat {}Length of output: 6161
components/launch_darkly_oauth/actions/evaluate-feature-flag/evaluate-feature-flag.mjs (1)
1-21: LGTM! Clean implementation following component sharing pattern.The implementation correctly extends the base LaunchDarkly component while maintaining its functionality and only replacing the app property.
components/launch_darkly_oauth/actions/toggle-feature-flag/toggle-feature-flag.mjs (1)
1-21: LGTM! Consistent implementation with evaluate-feature-flag.The implementation maintains consistency with the evaluate-feature-flag component, following the same clean pattern for component sharing.
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.
Hi @GTFalcao lgtm! Ready for QA!
Closes #15151
The components are being shared replacing only the app prop being imported, which changes the base request. There should be no behavior differences between the components of the two apps.
Summary by CodeRabbit
New Features
Chores