Remove feature flags #2742
Remove feature flags #2742
Conversation
Signed-off-by: Efren Lim <[email protected]>
Signed-off-by: Efren Lim <[email protected]>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request involves a comprehensive removal of feature flag and integration-related code across multiple frontend files. The changes systematically eliminate dependencies, environment variables, configuration settings, and utility functions associated with feature flag management, particularly for services like Unleash, HubSpot, and GitHub. The modifications simplify the codebase by removing conditional logic related to feature toggles and integration checks, suggesting a shift towards a more streamlined approach to feature management. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/modules/automation/components/automation-toggle.vue (1)
Line range hint
3-9: Consider adding error handling for failed state changes.While the component has guards (
canEnableand permission checks) to prevent invalid interactions, there's no visible error handling if thechangePublishStateaction fails.Consider wrapping the
handleChangemethod with try-catch and using element-plus's notification system:const handleChange = (value) => { - changePublishState(props.automation.id, value); + try { + await changePublishState(props.automation.id, value); + } catch (error) { + ElMessage.error('Failed to update automation state. Please try again.'); + console.error('Failed to update automation state:', error); + } };frontend/src/modules/eagle-eye/pages/eagle-eye-onboard-page.vue (1)
79-84: Consider extracting string literals as constants.While the function logic is clear, the strings 'Enterprise' and 'Scale' should be extracted as named constants. This would improve maintainability and ensure consistency if these values are used elsewhere in the application.
+const PREMIUM_FEATURE_TYPES = { + ENTERPRISE: 'Enterprise', + SCALE: 'Scale', +} as const; + const isPremiumFeatureCopy = () => { if (config.isCommunityVersion) { - return 'Enterprise'; + return PREMIUM_FEATURE_TYPES.ENTERPRISE; } - return 'Scale'; + return PREMIUM_FEATURE_TYPES.SCALE; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
frontend/package.json(0 hunks)frontend/scripts/docker-entrypoint.sh(0 hunks)frontend/src/app.vue(0 hunks)frontend/src/config.js(0 hunks)frontend/src/integrations/hubspot/components/hubspot-connect.vue(1 hunks)frontend/src/modules/automation/components/automation-list.vue(0 hunks)frontend/src/modules/automation/components/automation-toggle.vue(1 hunks)frontend/src/modules/eagle-eye/pages/eagle-eye-onboard-page.vue(2 hunks)frontend/src/modules/member/components/member-dropdown-content.vue(0 hunks)frontend/src/modules/member/member-enrichment.js(0 hunks)frontend/src/shared/modules/filters/components/Filter.vue(1 hunks)frontend/src/shared/modules/filters/components/FilterDropdown.vue(1 hunks)frontend/src/utils/featureFlag/index.js(0 hunks)frontend/src/utils/logRocket/index.ts(1 hunks)
💤 Files with no reviewable changes (8)
- frontend/src/modules/member/member-enrichment.js
- frontend/package.json
- frontend/src/modules/automation/components/automation-list.vue
- frontend/scripts/docker-entrypoint.sh
- frontend/src/config.js
- frontend/src/app.vue
- frontend/src/modules/member/components/member-dropdown-content.vue
- frontend/src/utils/featureFlag/index.js
🔇 Additional comments (10)
frontend/src/modules/automation/components/automation-toggle.vue (1)
7-7: Verify the removal of validation logic.The change from a function to
truefor:before-changeremoves a validation step. While this aligns with the PR objective of removing feature flags, we should verify that no critical validation was lost.Let's check for any validation logic that might have been in the removed
beforeChangefunction:✅ Verification successful
:before-change="true"modification is safe and maintains component integrityThe component retains robust validation through:
- Computed
canEnableproperty for type-specific validation- Permission checks via
hasPermission(LfPermission.automationEdit)- Type-specific guards through
automationTypes[type]?.enableGuard🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the previous implementation of beforeChange git log -p frontend/src/modules/automation/components/automation-toggle.vue | rg -A 10 "beforeChange.*=.*\{"Length of output: 0
Script:
#!/bin/bash # Get the full component code cat frontend/src/modules/automation/components/automation-toggle.vue # Find similar switch components using el-switch rg -l "el-switch" frontend/src/ # Look for validation patterns in automation components rg "beforeChange|validate|canEnable" frontend/src/modules/automation/Length of output: 2583
frontend/src/shared/modules/filters/components/Filter.vue (1)
13-13: LGTM! Simplified condition aligns with feature flag removal.The simplified condition now relies solely on the Pinia store state
isDeveloperModeActive, which is consistent with the PR's objective of removing feature flags.Let's verify that all feature flag checks for developer mode have been consistently removed:
✅ Verification successful
Verified: Feature flag removal is complete and consistent
The simplified condition in Filter.vue aligns with the codebase state, where all developer mode feature flag checks have been successfully removed. No remaining references to
developerModeEnabledor Unleash feature flags were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining feature flag checks for developer mode # across the frontend codebase # Search for any remaining references to developerMode feature flag rg -i "developerMode.*flag" frontend/ # Search for any remaining Unleash feature flag imports rg -i "import.*unleash" frontend/ # Search for any remaining feature flag configuration rg -i "featureflag.*developer" frontend/Length of output: 118
Script:
#!/bin/bash # Search for any remaining developerModeEnabled references rg "developerModeEnabled" frontend/ # Search for any Unleash-related code rg -i "unleash" frontend/ # Search for feature flag related code in store files fd store.ts -e ts -e js frontend/ --exec rg -l "feature|flag|developer" # Look for any remaining feature flag configurations fd config -e ts -e js frontend/ --exec rg -l "feature|flag|developer"Length of output: 375
Script:
#!/bin/bash # Check content of matched files echo "=== FilterConfig.ts ===" cat frontend/src/shared/modules/filters/types/FilterConfig.ts echo -e "\n=== founded/config.ts ===" cat frontend/src/modules/organization/config/filters/founded/config.ts echo -e "\n=== github/config.js ===" cat frontend/src/integrations/github/config.jsLength of output: 4953
frontend/src/modules/eagle-eye/pages/eagle-eye-onboard-page.vue (3)
57-57: LGTM! Clean import addition.The config import is properly placed and follows Vue.js conventions.
90-90: LGTM! Clean feature flag removal.The update successfully replaces the feature flag with a direct configuration check, aligning with the PR's objective of removing feature flags.
Line range hint
1-150: Verify complete removal of feature flag dependencies.Let's ensure all feature flag related code has been properly removed.
✅ Verification successful
Feature flag code has been properly removed
The only feature-related code found is the
isPremiumFeatureCopyfunction, which is legitimate business logic for UI text and not related to feature flag functionality. No remaining feature flag or unleash-related code was found in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining feature flag related code # Check for any remaining feature flag imports or usage echo "Checking for remaining feature flag imports or usage..." rg -i "featureflag|unleash" frontend/src/modules/eagle-eye # Check for related configuration echo "Checking for related configuration..." rg -i "featureflag|unleash" frontend/src/configLength of output: 316
Script:
#!/bin/bash # Broader search for feature flag related code # Check for feature-related terms echo "Checking for feature-related terms..." rg -i "feature.*enabled|is.*feature|feature.*flag" frontend/src/modules/eagle-eye # Check configuration and constant files echo "Checking configuration files..." fd -e json -e js -e ts "config|constant" frontend/src/modules/eagle-eye --exec cat {} # Check for any remaining feature toggles echo "Checking for feature toggles..." rg -i "toggle|feature" frontend/src/modules/eagle-eye/constantsLength of output: 721
frontend/src/integrations/hubspot/components/hubspot-connect.vue (2)
13-14: LGTM! Clean import statements.The removal of feature flag related imports aligns with the PR objective of removing feature flags.
3-3: Verify the impact of removing conditional integration checks.The HubSpot integration is now always enabled by hardcoding
:has-integration="true". Please ensure this aligns with your business requirements and doesn't bypass any necessary access controls.Also applies to: 6-6
✅ Verification successful
Removal of conditional checks is safe due to backend protection
The hardcoding of
:has-integration="true"is acceptable because:
- Backend routes are protected by feature flag middleware
- Parent components still handle conditional rendering based on
enabledForsettings🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining feature flag checks or conditional integration logic # Search for any remaining feature flag checks rg -i "hubspot.*enabled|feature.*flag.*hubspot" # Search for any conditional integration checks rg -i "has-integration.*hubspot|hubspot.*integration.*check"Length of output: 4437
frontend/src/shared/modules/filters/components/FilterDropdown.vue (1)
92-92: Verify the impact of showing all filter options.The removal of feature flag filtering means all configurations will now be displayed. Please ensure this doesn't expose any features that should be restricted.
✅ Verification successful
No security concerns with showing all filter options.
The codebase scan shows that filter configurations only contain standard data properties and common functionality. No sensitive or restricted features are controlled through feature flags in the filtering system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any sensitive or restricted configurations that might be exposed # Search for configuration definitions rg -A 5 "FilterConfig.*=|config.*=.*FilterConfig" # Search for any remaining feature flag checks in filter configurations rg -i "feature.*flag.*filter|filter.*config.*enabled"Length of output: 47716
frontend/src/utils/logRocket/index.ts (2)
Line range hint
9-24: LGTM! LogRocket integration remains properly gated.The dependent functions (
init,captureException, andidentify) continue to properly gate LogRocket functionality based on the environment check, maintaining secure and appropriate usage of the service.
7-7: Verify environment configuration across deployment targets.The simplified implementation aligns with the PR objective of removing feature flags. However, we should verify that the environment configuration is correctly set across all deployment targets to ensure LogRocket is enabled only in production.
Let's verify the environment configuration:
✅ Verification successful
Environment configuration for LogRocket is correctly implemented.
The environment check
config.env === 'production'is consistent with the application's environment configuration system and aligns with how other production-only services are handled (Hotjar, Datadog RUM). The build system properly supports different environments through dedicated build targets (localhost, staging, production).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check environment configuration usage and deployment settings # Check for environment configuration usage rg -A 3 "config.env === 'production'" frontend/ # Check deployment configuration files fd -e env -e yaml -e json . frontend/ --exec cat {} \;Length of output: 74899
Script:
#!/bin/bash # Search for environment configuration files fd -e env . frontend/ --exec cat {} \; # Search for environment configuration imports and usage rg -A 3 "import.*config" frontend/Length of output: 78774
Script:
#!/bin/bash # Search for environment configuration files fd -e env.dist -e env.local -e env.prod -e env.staging . frontend/ --exec cat {} \; # Search for environment configuration imports and initialization rg -A 3 "import.*env" frontend/Length of output: 4484
gaspergrom
left a comment
gaspergrom
left a comment
There was a problem hiding this comment.
There are few places feature flags are present
FilterConfig,
tenant store
automation-types/index.ts
app.vue
Please remove it also from there
| "VUE_APP_STRIPE_CUSTOMER_PORTAL_LINK" | ||
| "VUE_APP_UNLEASH_URL" | ||
| "VUE_APP_UNLEASH_API_KEY" | ||
| "VUE_APP_SAMPLE_TENANT_ID" |
There was a problem hiding this comment.
Sample tenant envs can be removed
Signed-off-by: Efren Lim <[email protected]>
Signed-off-by: Efren Lim <[email protected]>
Changes proposed ✍️
What
Why
Code cleanup
Ticket: LFX-1913
Checklist ✅
Feature,Improvement, orBug.Summary by CodeRabbit
Release Notes
Dependency Changes
unleash-proxy-clientpackageFeature Flag Management
Component Updates
Integrations
Monitoring