-
Notifications
You must be signed in to change notification settings - Fork 1
feat(braze): remove feature flag for platform-specific API keys #2297
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: develop
Are you sure you want to change the base?
feat(braze): remove feature flag for platform-specific API keys #2297
Conversation
📝 WalkthroughWalkthroughThis change replaces feature-flag-based gating in the Braze UI configuration with explicit connectionMode-based conditions. The top-level expression operator switches from AND to OR, and platform-specific visibility/prerequisite logic for API key fields now depends on connectionMode values and usePlatformSpecificApiKeys. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2024-10-08T05:19:11.373ZApplied to files:
📚 Learning: 2024-10-08T15:52:59.819ZApplied to files:
📚 Learning: 2025-05-23T09:56:13.947ZApplied to files:
📚 Learning: 2024-12-09T04:17:28.675ZApplied to files:
📚 Learning: 2024-12-09T04:17:05.013ZApplied to files:
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2297 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 72 72
Branches 8 8
=========================================
Hits 72 72 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/configurations/destinations/braze/ui-config.json (6)
21-88: Critical: Inverted visibility logic for platform-specific API keys toggle.The visibility condition will hide the "Enable Platform-Specific App Identifier Keys" checkbox when device or hybrid connection modes are configured. According to the PR objectives, this toggle should be visible when these modes are configured.
Current behavior:
action: "hide"triggers when the OR expression evaluates totrue(i.e., when any device/hybrid mode is set), which hides the checkbox.Expected behavior: The checkbox should be shown when device/hybrid modes are configured.
The expression logic is inverted. Since the conditions framework uses
action: "hide"to hide fields when the expression is true, you need to either:
- Change the semantic interpretation if
action: "show"is supported, OR- Invert the expression logic to evaluate to
truewhen you want to hide the field (i.e., when device/hybrid modes are not configured)This affects the core functionality of the feature flag removal and will prevent users from accessing platform-specific API keys when they should be available.
98-175: Critical: Inverted visibility logic for Default App Identifier Key.The visibility condition hides the "Default App Identifier Key" when
usePlatformSpecificApiKeys = falseAND device/hybrid modes are configured. This is inverted logic.Current behavior: Field hidden when platform-specific keys are disabled (false).
Expected behavior: Field should be shown when platform-specific keys are disabled, as the default key serves as the fallback for all platforms when platform-specific keys are not in use.
The correct logic should hide this field when
usePlatformSpecificApiKeys = true(enabled), not when it's false.
186-233: Critical: Inverted visibility logic for Android App Identifier Key.The condition hides the "Android App Identifier Key" field when
usePlatformSpecificApiKeys = trueAND Android device/hybrid modes are configured—precisely when it should be shown.Current behavior: Field hidden when platform-specific keys are enabled and relevant Android connection modes are active.
Expected behavior: Field should be visible in this scenario to allow users to configure the Android-specific API key.
The expression logic must be inverted.
244-291: Critical: Inverted visibility logic for iOS App Identifier Key.The condition hides the "iOS App Identifier Key" field when
usePlatformSpecificApiKeys = trueAND iOS device/hybrid modes are configured—precisely when it should be shown.This is the same inversion issue as the Android key field. The field should be visible when platform-specific keys are enabled and relevant iOS connection modes are active.
302-329: Critical: Inverted visibility logic for Web App Identifier Key.The condition hides the "Web App Identifier Key" field when
usePlatformSpecificApiKeys = trueAND web device/hybrid modes are configured—precisely when it should be shown.This is the same inversion issue. The field should be visible when platform-specific keys are enabled and web connection modes are active.
180-185: Contradictory requirement and fallback note.The fields are marked
"required": true(lines 185, 243, 301), but the notes state that "The Default App Identifier Key will be used if left blank" (lines 180, 238, 296).If a field is required, it cannot be left blank by definition. This creates confusion about whether users must provide platform-specific keys or can rely on the default fallback.
Consider either:
- Removing
"required": trueif the fallback behavior is intentional and allows blank values- Updating the note to clarify that the field is required when visible, and the fallback only applies if platform-specific keys are not enabled
Also applies to: 238-243, 296-301
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/configurations/destinations/braze/ui-config.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: ItsSudip
Repo: rudderlabs/rudder-integrations-config PR: 2031
File: src/configurations/destinations/zoho/db-config.json:5-5
Timestamp: 2025-05-23T09:56:13.947Z
Learning: In the Rudder Integrations Config repository, flags like `isAudienceSupported` in destination configurations (`db-config.json` files) don't require schema updates when they're used as internal feature flags rather than user-configurable inputs.
📚 Learning: 2024-10-08T05:19:11.373Z
Learnt from: saikumarrs
Repo: rudderlabs/rudder-integrations-config PR: 1691
File: src/configurations/destinations/ga4/schema.json:345-345
Timestamp: 2024-10-08T05:19:11.373Z
Learning: When reviewing changes to `src/configurations/destinations/ga4/schema.json`, ensure that added enum values like "iubenda" are correctly identified across all supported platforms before flagging an issue.
Applied to files:
src/configurations/destinations/braze/ui-config.json
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: gs-nwolfe
Repo: rudderlabs/rudder-integrations-config PR: 1684
File: src/configurations/destinations/gainsight_px_browser/ui-config.json:109-220
Timestamp: 2024-10-08T15:52:59.819Z
Learning: In `ui-config.json` files, using multiple conditions for the same `configKey` in the `preRequisites`, such as for `AMP_enable-gcm`, is acceptable and commonly used across multiple destinations.
Applied to files:
src/configurations/destinations/braze/ui-config.json
📚 Learning: 2025-05-23T09:56:13.947Z
Learnt from: ItsSudip
Repo: rudderlabs/rudder-integrations-config PR: 2031
File: src/configurations/destinations/zoho/db-config.json:5-5
Timestamp: 2025-05-23T09:56:13.947Z
Learning: In the Rudder Integrations Config repository, flags like `isAudienceSupported` in destination configurations (`db-config.json` files) don't require schema updates when they're used as internal feature flags rather than user-configurable inputs.
Applied to files:
src/configurations/destinations/braze/ui-config.json
📚 Learning: 2024-12-09T04:17:28.675Z
Learnt from: accoilmj
Repo: rudderlabs/rudder-integrations-config PR: 1807
File: src/configurations/destinations/accoil_analytics/schema.json:148-284
Timestamp: 2024-12-09T04:17:28.675Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated. Avoid suggesting changes to autogenerated files.
Applied to files:
src/configurations/destinations/braze/ui-config.json
📚 Learning: 2024-12-09T04:17:05.013Z
Learnt from: accoilmj
Repo: rudderlabs/rudder-integrations-config PR: 1807
File: src/configurations/destinations/accoil_analytics/schema.json:11-147
Timestamp: 2024-12-09T04:17:05.013Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated and should not be manually modified.
Applied to files:
src/configurations/destinations/braze/ui-config.json
What are the changes introduced in this PR?
This PR removes the
AMP_enable-braze-dest-multi-app-key-supportfeature flag from the Braze destination configuration, making the platform-specific API keys feature generally available to all users.What is the related Linear task?
Resolves SDK-4257
Please explain the objectives of your changes below
The platform-specific API keys feature for Braze has been successfully validated and is ready for general availability. This change removes the feature flag gate that was previously controlling access to this functionality.
Previously, users needed the
AMP_enable-braze-dest-multi-app-key-supportfeature flag enabled to configure separate Android, iOS, and Web API keys for proper data attribution in Braze. With this change, the feature is now accessible to all users when they configure device or hybrid connection modes for their mobile and web sources.Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
The platform-specific API keys feature in Braze destination is now always available when device/hybrid connection modes are configured. This change:
AND(featureFlag, OR(connectionModes))to justOR(connectionModes)No breaking changes - this only makes an existing feature more accessible.
Any new dependencies introduced with this change?
N/A
Any new checks got introduced or modified in test suites. Please explain the changes.
N/A
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
I have executed schemaGenerator tests and updated schema if needed
Are sensitive fields marked as secret in definition config?
My test cases and placeholders use only masked/sample values for sensitive fields
Is the PR limited to 10 file changes & one task?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.