feat: Adds support for PostHog analytics. #100
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughRefactors analytics into a multi-provider system: adds CompoundAnalytics delegating to MixPanelAnalytics and PostHogAnalytics, extends Analytics.track to accept properties and adds identify, introduces command-properties extraction/masking, and updates BetterCommandRunner and tests to emit and handle enriched analytics properties. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Command Execution
participant BCR as BetterCommandRunner
participant CPA as buildCommandPropertiesForAnalytics
participant CA as CompoundAnalytics
participant MP as MixPanelAnalytics
participant PH as PostHogAnalytics
participant HTTP as HTTP Endpoint
CLI->>BCR: Execute command with ArgResults
BCR->>CPA: Extract command properties (mask sensitive values)
CPA-->>BCR: Return properties map with masked options/flags and full_command
BCR->>CA: sendAnalyticsEvent(event, properties)
CA->>MP: track(event, properties)
CA->>PH: track(event, properties)
MP->>HTTP: POST to Mixpanel endpoint
PH->>HTTP: POST to PostHog endpoint
HTTP-->>MP: Response (ignored)
HTTP-->>PH: Response (ignored)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
🤖 Fix all issues with AI agents
In `@packages/cli_tools/lib/src/analytics/mixpanel.dart`:
- Around line 58-75: The track method currently ignores the caller-supplied
properties parameter; update the payload construction in track to merge the
incoming properties into the payload's 'properties' map so caller metadata is
sent to Mixpanel. Inside track, create a merged Map (e.g., start with
...properties and then add the internal keys 'distinct_id', 'token', 'platform',
'dart_version', 'is_ci', 'version' so internal metadata remains present) and use
that merged map as the value for 'properties' in the jsonEncode payload before
calling _quietPost.
In `@packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart`:
- Around line 220-229: The function sendAnalyticsEvent currently prints the
event and properties unconditionally, leaking analytics payloads even when
analyticsEnabled() is false; update sendAnalyticsEvent so the print/log line is
executed only after confirming analyticsEnabled() returns true (or replace the
print with a debug/logger call that is gated by analyticsEnabled()), and keep
the try/catch invoking onAnalyticsEvent unchanged; reference sendAnalyticsEvent,
analyticsEnabled(), and onAnalyticsEvent to locate and change the unconditional
print to be conditional or routed to a non-stdout debug logger.
In `@packages/cli_tools/test/analytics/command_properties_test.dart`:
- Around line 1-241: The file containing tests for
buildCommandPropertiesForAnalytics (the test suite with CreateCommand and
TemplateCommand and the main() group) is not formatted; run the Dart formatter
to fix styling. Fix by running `dart format` on the affected file (or `dart
format .` for the package), review the changes to the tests referencing
CreateCommand, TemplateCommand and buildCommandPropertiesForAnalytics, and
commit the formatted file so CI passes.
🧹 Nitpick comments (2)
packages/cli_tools/lib/src/analytics/analytics.dart (1)
24-55: Consider isolating provider failures to keep analytics best‑effort.If a provider throws, the exception can bubble and prevent other providers from running. Wrapping each call avoids that.
♻️ Suggested defensive wrapping
void cleanUp() { for (final provider in providers) { - provider.cleanUp(); + try { + provider.cleanUp(); + } catch (_) {} } } @@ void track({ required final String event, final Map<String, dynamic> properties = const {}, }) { for (final provider in providers) { - provider.track( - event: event, - properties: properties, - ); + try { + provider.track( + event: event, + properties: properties, + ); + } catch (_) {} } } @@ void identify({ final String? email, final Map<String, dynamic>? properties, }) { for (final provider in providers) { - provider.identify( - email: email, - properties: properties, - ); + try { + provider.identify( + email: email, + properties: properties, + ); + } catch (_) {} } }packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart (1)
337-343: Skip building analytics properties when analytics are disabled.
buildCommandPropertiesForAnalyticsis computed even when analytics are disabled. Guarding avoids unnecessary work and reduces accidental exposure.♻️ Proposed tweak
- sendAnalyticsEvent( - command.name!, - buildCommandPropertiesForAnalytics( - topLevelResults: topLevelResults, - argParser: argParser, - commands: commands, - ), - ); + if (analyticsEnabled()) { + sendAnalyticsEvent( + command.name!, + buildCommandPropertiesForAnalytics( + topLevelResults: topLevelResults, + argParser: argParser, + commands: commands, + ), + ); + }
packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart
Show resolved
Hide resolved
Collaborator
christerswahn
left a comment
christerswahn
left a comment
There was a problem hiding this comment.
Looks good to me! Just had a code structure suggestion.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds:
CompoundAnalytics- the ability to combine multiple providers into a single one.Breaking change:
OnAnalyticsEventnow requires an additionalpropertiesparameter.Summary by CodeRabbit
New Features
Breaking Changes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.