feat(push): bridge open web URL actions to Dart#116
Conversation
open_url taps are dispatched externally by the native SDK and never reach the host app, so only deep_link/open_app action buttons are bridged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirror the native SDK's klaviyoDeepLinkURL semantics: a url value that is absent, not a string, or fails URL parsing does not take precedence over web_url, so the bridge should still emit push_open_web_url. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback: guard that the tapped button carries an action type (Dart's fromMap requires it) instead of silently emitting an event the Dart layer drops, log unmatched button identifiers, apply Swift if-let shorthand, and remove a redundant comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat(push): bridge open_url action and push action button events to Dart
📝 WalkthroughWalkthroughAdds typed push-action events for Klaviyo push notification taps and action buttons. Native iOS and Android plugins forward events through Flutter, Dart decodes them into sealed models, and the example, documentation, tests, and native SDK versions are updated. ChangesPush action events
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PushNotification
participant NativePlugin
participant NativeWrapper
participant KlaviyoSDK
participant AppListener
PushNotification->>NativePlugin: user taps notification or action button
NativePlugin->>NativeWrapper: push action event payload
NativeWrapper->>KlaviyoSDK: onPushActionEvent
KlaviyoSDK->>KlaviyoSDK: filter and decode event
KlaviyoSDK-->>AppListener: KlaviyoPushAction
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. 🔧 Checkov (3.3.8)android/build.gradleTraceback (most recent call last): 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e58931. Configure here.
| Logger.klaviyoFlutterSDK.notice("Flutter not ready. Caching push action event.") | ||
| } | ||
| cachedPushAction = eventPayload | ||
| } |
There was a problem hiding this comment.
Stale iOS push action cache
Medium Severity
When Flutter is not ready, forwardPushAction stores a push action in cachedPushAction but returns early when the latest tap has no typed action without clearing that cache. After a second notification open overwrites cachedOpenedNotification, onListen can deliver a mismatched push_open_web_url or button event from an earlier tap.
Reviewed by Cursor Bugbot for commit 0e58931. Configure here.
|
This PR will need the updated Android and iOS SDK's |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/klaviyo_sdk.dart`:
- Around line 440-447: Add the required trailing comma after the multiline
callback argument in the expand call handling KlaviyoPushAction.fromMap within
the event transformation chain, preserving the existing error handling and
behavior.
In `@lib/src/services/klaviyo_native_wrapper.dart`:
- Line 356: Update the native push action logging around the `_logger.info` call
so it no longer emits the raw `eventData` payload at INFO. Log only the event
type or button identifier, or redact URL fields; retain the full payload only
behind explicitly enabled debug logging.
- Around line 354-358: Update the native push-event flow around handleIntent()
and onListen() to queue push_notification_opened and push_action_button_tapped
payloads when eventSink is unavailable. Store them in a native pending queue,
then flush and clear that queue when onListen() initializes the stream listener,
preserving normal immediate delivery after subscription.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 33364b69-56b0-4545-b4fc-138b5104b813
📒 Files selected for processing (15)
.gitignoreCHANGELOG.mdREADME.mdandroid/build.gradleandroid/src/main/kotlin/com/klaviyo/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.ktexample/lib/main.dartexample/lib/tabs/profile_tab.dartios/klaviyo_flutter_sdk.podspecios/klaviyo_flutter_sdk/Sources/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.swiftlib/klaviyo_flutter_sdk.dartlib/src/klaviyo_sdk.dartlib/src/models/klaviyo_push_action.dartlib/src/services/klaviyo_native_wrapper.darttest/klaviyo_push_action_test.darttest/push_action_stream_test.dart
| .expand((event) { | ||
| try { | ||
| return [KlaviyoPushAction.fromMap(event)]; | ||
| } catch (e) { | ||
| _logger.warning('Dropping malformed push action event: $e'); | ||
| return []; | ||
| } | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate klaviyo_sdk.dart =="
fd -a 'klaviyo_sdk\.dart$' .
echo "== Extract relevant lines =="
if [ -f lib/src/klaviyo_sdk.dart ]; then
sed -n '425,455p' lib/src/klaviyo_sdk.dart | cat -n
fi
echo "== Check require_trailing_commas config references =="
rg -n "require_trailing_commas|trailing_commas|avoid_trailing_commas|dart format" . -g '!build/**' -g '!dist/**' -g '!node_modules/**' || true
echo "== Check local analyzer/dart configuration files =="
for f in analysis_options.yaml pubspec.yaml pubspec.yaml.lock .dart_tool/package_config.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,220p' "$f"
fi
done
echo "== Extract surrounding stream chain with line numbers =="
nl -ba lib/src/klaviyo_sdk.dart | sed -n '420,455p'Repository: klaviyo/klaviyo-flutter-sdk
Length of output: 4125
🌐 Web query:
Dart analyzer require_trailing_commas trailing comma lint Dart style
💡 Result:
The require_trailing_commas lint rule in Dart is designed to enforce the use of trailing commas in multi-line parameter and argument lists [1]. Key details regarding its usage and behavior: How to Enable: To enable this lint, add it to your analysis_options.yaml file under the linter rules section [1]: linter: rules: - require_trailing_commas Behavior and Limitations: - The rule encourages using trailing commas for all multi-line parameter and argument lists. If a list fits on a single line, a trailing comma is not required [1]. - It is intended to complement the Dart formatter (dart format), which also automatically manages trailing commas [1][2]. - NOTE: The rule assumes code has already been processed by dart format; it may produce false positives on unformatted code [1]. - For projects targeting Dart language version 3.7 or later (using the new "tall" style formatter), the require_trailing_commas lint rule is automatically disabled, as the formatter's automated behavior takes precedence [3]. Interaction with Formatter Configuration: In Dart 3.7 and later, the default formatter behavior is to automatically add or remove trailing commas based on whether a construct spans multiple lines [2][4]. Users who prefer manual control over line breaks—where the presence of a trailing comma forces a construct to split across lines—can opt into this behavior by configuring the formatter in analysis_options.yaml [2][5][6]: formatter: trailing_commas: preserve When this preserve setting is used, it restores the ability to force line breaks via trailing commas, which was a standard feature of the older Dart formatter [5][6]. Using the lint rule alongside this setting helps maintain consistent application of that style across the codebase [7].
Citations:
- 1: https://dart.dev/tools/linter-rules/require_trailing_commas
- 2: https://dart.dev/tools/dart-format
- 3: https://github.com/dart-lang/sdk/blob/234127f73aa592ade9ab00a3cd07e0d2a946ee78/pkg/linter/lib/src/rules/require_trailing_commas.dart
- 4: https://pub.dev/documentation/dart_style/latest/dart_style/TrailingCommas.html
- 5: https://github.com/dart-lang/dart_style/wiki/Configuration
- 6: Add an optional configuration to preserve trailing commas. dart-lang/dart_style#1672
- 7:
require_trailing_commasand the newpreserve-trailing-commasconfiguration dart-lang/sdk#61338
🌐 Web query:
Flutter 3.24 Flutter SDK Dart SDK version version
💡 Result:
The Flutter 3.24 SDK release is paired with Dart 3.5 [1][2]. This release was officially announced on August 6, 2024 [1][2].
Citations:
- 1: https://blog.flutter.dev/flutter-3-24-dart-3-5-204b7d20c45d
- 2: https://blog.flutter.dev/whats-new-in-flutter-3-24-6c040f87d1e4
Add the required trailing comma to the multiline expand call.
The repository enforces require_trailing_commas, so keep the callable comma even though the argument happens to be a block expression:
Proposed formatting fix
- .expand((event) {
+ .expand(
+ (event) {
try {
return [KlaviyoPushAction.fromMap(event)];
} catch (e) {
_logger.warning('Dropping malformed push action event: $e');
return [];
}
- });
+ },
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .expand((event) { | |
| try { | |
| return [KlaviyoPushAction.fromMap(event)]; | |
| } catch (e) { | |
| _logger.warning('Dropping malformed push action event: $e'); | |
| return []; | |
| } | |
| }); | |
| .expand( | |
| (event) { | |
| try { | |
| return [KlaviyoPushAction.fromMap(event)]; | |
| } catch (e) { | |
| _logger.warning('Dropping malformed push action event: $e'); | |
| return []; | |
| } | |
| }, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/src/klaviyo_sdk.dart` around lines 440 - 447, Add the required trailing
comma after the multiline callback argument in the expand call handling
KlaviyoPushAction.fromMap within the event transformation chain, preserving the
existing error handling and behavior.
Source: Coding guidelines
| case 'push_open_web_url': | ||
| case 'push_action_button_tapped': | ||
| _logger.info('Native push action event: $eventData'); | ||
| _pushActionController.add(eventData); | ||
| break; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A20 -B8 \
'forwardActionButtonTap|push_action_button_tapped|onListen|eventSink\?\.success' \
android/src/main/kotlin/com/klaviyo/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.ktRepository: klaviyo/klaviyo-flutter-sdk
Length of output: 7789
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Candidate files =="
git ls-files | rg '(^android/|^ios/|^lib/src/services/klaviyo_native_wrapper.dart|README|CHANGELOG|pubspec.yaml)' || true
echo
echo "== Android push click/action methods around source =="
if [ -f android/src/main/kotlin/com/klaviyo/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.kt ]; then
rg -n -C12 'onNotificationOpened|onNotificationClick|Button ID|Button Action|forwardActionButtonTap|push_notification_opened|push_action_button_tapped' android/src/main/kotlin/com/klaviyo/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.kt
fi
echo
echo "== iOS candidate search for action-button cache behavior =="
for f in $(git ls-files | rg 'ios/.*\.(swift|m|m\.mm)$|ios/Pods/.*\.h$' 2>/dev/null || true); do
if rg -q 'push_action_button_tapped|action.*button|Button Action|Button Link|deep_link|open_app|cache|pending'| "$f" 2>/dev/null; then
echo "--- $f ---"
rg -n -C8 'push_action_button_tapped|action.*button|Button Action|Button Link|deep_link|open_app|cache|pending|eventSink' "$f" 2>/dev/null || true
fi
done
echo
echo "== Dart stream controller and event mapping =="
sed -n '1,460p' lib/src/services/klaviyo_native_wrapper.dartRepository: klaviyo/klaviyo-flutter-sdk
Length of output: 3766
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Android listener registration and full onNotificationOpened =="
rg -n -C8 'Klaviyo\.|onNotificationOpened|KlaviyoNotificationOpened|KlaviyoNotificationAction|deepLink|Button|onActivityResult|ActivityCompat|notification' android/src/main/kotlin/com/klaviyo/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.kt
echo
echo "== iOS implementation relevant to notification taps/actions =="
if [ -f ios/klaviyo_flutter_sdk/Sources/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.swift ]; then
sed -n '1,260p' ios/klaviyo_flutter_sdk/Sources/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.swift
fi
echo
echo "== lib/dart stream setup =="
sed -n '1,220p' lib/src/services/klaviyo_native_wrapper.dartRepository: klaviyo/klaviyo-flutter-sdk
Length of output: 36139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Android ActivityAware full implementation =="
sed -n '530,576p' android/src/main/kotlin/com/klaviyo/klaviyo_flutter_sdk/KlaviyoFlutterSdkPlugin.kt
echo
echo "== BufferedBroadcastStreamController behavior =="
if [ -f lib/src/utils/buffered_broadcast_stream_controller.dart ]; then
sed -n '1,220p' lib/src/utils/buffered_broadcast_stream_controller.dart
fiRepository: klaviyo/klaviyo-flutter-sdk
Length of output: 2246
Cache Android push action events before the EventChannel listener subscribes.
handleIntent() runs during Android cold-start, but eventSink is only initialized from onListen(). Both the push_notification_opened and push_action_button_tapped payloads are discarded when eventSink is null, while the Android cold-start flow can precede the Dart EventChannel subscription. Retain these native events in a native pending queue and flush them when the stream listener is added instead of relying only on the Dart buffer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/src/services/klaviyo_native_wrapper.dart` around lines 354 - 358, Update
the native push-event flow around handleIntent() and onListen() to queue
push_notification_opened and push_action_button_tapped payloads when eventSink
is unavailable. Store them in a native pending queue, then flush and clear that
queue when onListen() initializes the stream listener, preserving normal
immediate delivery after subscription.
| break; | ||
| case 'push_open_web_url': | ||
| case 'push_action_button_tapped': | ||
| _logger.info('Native push action event: $eventData'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid logging raw push-action URLs at INFO.
eventData contains URLs verbatim, including schemes and query strings that may carry PII or tokens. Log only the event type/button identifier, or redact URL fields and keep the full payload behind opt-in debug logging.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/src/services/klaviyo_native_wrapper.dart` at line 356, Update the native
push action logging around the `_logger.info` call so it no longer emits the raw
`eventData` payload at INFO. Log only the event type or button identifier, or
redact URL fields; retain the full payload only behind explicitly enabled debug
logging.
1 participant
Description
Ships the "open web URL" SDK bridge work for the 0.3.0 release: forwards
open_urland push action button taps from the native SDKs to Dart via a typedonPushActionEventChannel stream.Due Diligence
Release/Versioning Considerations
PatchContains internal changes or backwards-compatible bug fixes.MinorContains changes to the public API.MajorContains breaking changes.feature/open-web-urlfeature branch per repo convention; this PR promotes that branch torel/0.3.0.Changelog / Code Overview
Consolidates PR #110 (PUSH-639) from the
feature/open-web-urlbranch:KlaviyoPushActionmodel,onPushActionstream, and iOS/Android EventChannel forwarding foropen_url+ action button taps.Note: this branch still pins native SDKs at KlaviyoSwift 5.3.1 / klaviyo-android-sdk 4.4.0. Bumping to the versions landing in the sibling iOS (rel/5.4.0) and Android (rel/4.5.0) open-web-url PRs — required for Android
web_urlsupport — is a tracked follow-up, not included here.Test Plan
Verified end-to-end on Android emulator and physical iPhone against local native SDK builds (see PUSH-639 notes). This PR merges the feature branch as-is into
rel/0.3.0.Related Issues/Tickets
PUSH-639 — part of the "open web URL" project.
Summary by CodeRabbit
New Features
Documentation
Chores