From 62f25b11be6968ef02882b68889b12d0ab217cec Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 2 Apr 2024 10:51:45 +0200 Subject: [PATCH 1/7] fix: allow tracking on PageRoute screens using the PosthogObserver as well --- CHANGELOG.md | 2 ++ lib/src/posthog_observer.dart | 31 ++++++++++++++++++------------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3db56f..1eb2637 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Next +- Allow tracking non `PageRoute` screens using the `PosthogObserver` ([#108]( + ## 4.3.0 - add PrivacyInfo ([#94](https://github.com/PostHog/posthog-flutter/pull/94)) diff --git a/lib/src/posthog_observer.dart b/lib/src/posthog_observer.dart index e973924..135f935 100644 --- a/lib/src/posthog_observer.dart +++ b/lib/src/posthog_observer.dart @@ -12,39 +12,44 @@ class PosthogObserver extends RouteObserver> { final ScreenNameExtractor _nameExtractor; - void _sendScreenView(PageRoute route) { - String? screenName = _nameExtractor(route.settings); - if (screenName != null) { + bool _isTrackeableRoute(String? name) { + return name != null && name.trim().isNotEmpty; + } + + void _sendScreenView(Route? route) { + if (route == null) { + return; + } + + var screenName = _nameExtractor(route.settings); + if (_isTrackeableRoute(screenName)) { // if the screen name is the root route, we send it as root ("/") instead of only "/" if (screenName == '/') { screenName = 'root (\'/\')'; } - Posthog().screen(screenName: screenName); + Posthog().screen(screenName: screenName!); } } @override void didPush(Route route, Route? previousRoute) { super.didPush(route, previousRoute); - if (route is PageRoute) { - _sendScreenView(route); - } + + _sendScreenView(route); } @override void didReplace({Route? newRoute, Route? oldRoute}) { super.didReplace(newRoute: newRoute, oldRoute: oldRoute); - if (newRoute is PageRoute) { - _sendScreenView(newRoute); - } + + _sendScreenView(newRoute); } @override void didPop(Route route, Route? previousRoute) { super.didPop(route, previousRoute); - if (previousRoute is PageRoute && route is PageRoute) { - _sendScreenView(previousRoute); - } + + _sendScreenView(previousRoute); } } From b2173f590ad8bd87ad41612f2af2ddba48c55bc8 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 2 Apr 2024 11:47:52 +0200 Subject: [PATCH 2/7] tests --- lib/src/posthog.dart | 7 +- pubspec.yaml | 4 +- ...sthog_flutter_platform_interface_fake.dart | 13 +++ test/posthog_observer_test.dart | 82 +++++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 test/posthog_flutter_platform_interface_fake.dart create mode 100644 test/posthog_observer_test.dart diff --git a/lib/src/posthog.dart b/lib/src/posthog.dart index 65c5ffe..3a3121a 100644 --- a/lib/src/posthog.dart +++ b/lib/src/posthog.dart @@ -4,7 +4,7 @@ class Posthog { static PosthogFlutterPlatformInterface get _posthog => PosthogFlutterPlatformInterface.instance; - static final Posthog _instance = Posthog._internal(); + static final _instance = Posthog._internal(); factory Posthog() { return _instance; @@ -122,4 +122,9 @@ class Posthog { } Posthog._internal(); + + // @internal + // void overridePostHogForTesting(PosthogFlutterPlatformInterface posthog) { + // PosthogFlutterPlatformInterface.instance = posthog; + // } } diff --git a/pubspec.yaml b/pubspec.yaml index 3721730..dbb0b52 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -18,7 +18,9 @@ dependencies: plugin_platform_interface: ^2.0.2 dev_dependencies: - flutter_lints: ^2.0.0 + flutter_lints: ^3.0.0 + flutter_test: + sdk: flutter # For information on the generic Dart part of this file, see the # following page: https://dart.dev/tools/pub/pubspec diff --git a/test/posthog_flutter_platform_interface_fake.dart b/test/posthog_flutter_platform_interface_fake.dart new file mode 100644 index 0000000..fd67908 --- /dev/null +++ b/test/posthog_flutter_platform_interface_fake.dart @@ -0,0 +1,13 @@ +import 'package:posthog_flutter/src/posthog_flutter_platform_interface.dart'; + +class PosthogFlutterPlatformFake extends PosthogFlutterPlatformInterface { + String? screenName; + + @override + Future screen({ + required String screenName, + Map? properties, + }) async { + this.screenName = screenName; + } +} diff --git a/test/posthog_observer_test.dart b/test/posthog_observer_test.dart new file mode 100644 index 0000000..46d8d23 --- /dev/null +++ b/test/posthog_observer_test.dart @@ -0,0 +1,82 @@ +import 'dart:math'; + +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:posthog_flutter/src/posthog.dart'; +import 'package:posthog_flutter/src/posthog_flutter_io.dart'; +import 'package:posthog_flutter/src/posthog_flutter_platform_interface.dart'; +import 'package:posthog_flutter/src/posthog_observer.dart'; + +import 'posthog_flutter_platform_interface_fake.dart'; + +void main() { + PageRoute route(RouteSettings? settings) => PageRouteBuilder( + pageBuilder: (_, __, ___) => Container(), + settings: settings, + ); + + final fake = PosthogFlutterPlatformFake(); + + setUp(() { + TestWidgetsFlutterBinding.ensureInitialized(); + PosthogFlutterPlatformInterface.instance = fake; + }); + + tearDown(() { + fake.screenName = null; + PosthogFlutterPlatformInterface.instance = PosthogFlutterIO(); + }); + + PosthogObserver getSut( + {ScreenNameExtractor nameExtractor = defaultNameExtractor}) { + return PosthogObserver(nameExtractor: nameExtractor); + } + + test('returns current route name', () { + final currentRoute = route(const RouteSettings(name: 'Current Route')); + + final sut = getSut(); + sut.didPush(currentRoute, null); + + // expectAsync0(() => expect(fake.screenName, 'Current Route')); + expect(fake.screenName, 'Current Route'); + }); + + test('returns overriden route name', () { + final currentRoute = route(const RouteSettings(name: 'Current Route')); + + String? nameExtractor(RouteSettings settings) => 'overriden'; + + final sut = getSut(nameExtractor: nameExtractor); + sut.didPush(currentRoute, null); + + expect(fake.screenName, 'overriden'); + }); + + test('returns overriden root route name', () { + final currentRoute = route(const RouteSettings(name: '/')); + + final sut = getSut(); + sut.didPush(currentRoute, null); + + expect(fake.screenName, 'root (\'/\')'); + }); + + test('does not capture not named routes', () { + final currentRoute = route(const RouteSettings(name: null)); + + final sut = getSut(); + sut.didPush(currentRoute, null); + + expect(fake.screenName, null); + }); + + test('does not capture blank routes', () { + final currentRoute = route(const RouteSettings(name: ' ')); + + final sut = getSut(); + sut.didPush(currentRoute, null); + + expect(fake.screenName, null); + }); +} From dab2470585e2af0d22fd023c8d2662c98e48cdc9 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 2 Apr 2024 11:50:13 +0200 Subject: [PATCH 3/7] tests --- .github/workflows/ci.yml | 3 +++ lib/src/posthog.dart | 5 ----- test/posthog_observer_test.dart | 1 - 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29197a1..f22de49 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,9 @@ jobs: working-directory: ./example run: flutter pub get + - name: Test + run: flutter test + - name: Build iOS working-directory: ./example run: flutter build ios --simulator --no-codesign diff --git a/lib/src/posthog.dart b/lib/src/posthog.dart index 3a3121a..d955fc3 100644 --- a/lib/src/posthog.dart +++ b/lib/src/posthog.dart @@ -122,9 +122,4 @@ class Posthog { } Posthog._internal(); - - // @internal - // void overridePostHogForTesting(PosthogFlutterPlatformInterface posthog) { - // PosthogFlutterPlatformInterface.instance = posthog; - // } } diff --git a/test/posthog_observer_test.dart b/test/posthog_observer_test.dart index 46d8d23..1725be0 100644 --- a/test/posthog_observer_test.dart +++ b/test/posthog_observer_test.dart @@ -38,7 +38,6 @@ void main() { final sut = getSut(); sut.didPush(currentRoute, null); - // expectAsync0(() => expect(fake.screenName, 'Current Route')); expect(fake.screenName, 'Current Route'); }); From 092c8cdfb5759ce5481127909aecb75459e1e937 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 2 Apr 2024 11:50:54 +0200 Subject: [PATCH 4/7] fix pr id --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eb2637..35d827e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Next -- Allow tracking non `PageRoute` screens using the `PosthogObserver` ([#108]( +- Allow tracking non `PageRoute` screens using the `PosthogObserver` ([#95](https://github.com/PostHog/posthog-flutter/pull/95)) ## 4.3.0 From 91ceae78075ff9bf08f0c9d74f9b4f37fa7321a4 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 2 Apr 2024 14:37:17 +0200 Subject: [PATCH 5/7] fix test --- CHANGELOG.md | 2 +- lib/src/posthog_observer.dart | 28 +++++++++++++++++++-- test/posthog_observer_test.dart | 44 +++++++++++++++++++++++++++++---- 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35d827e..4a11ef4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Next -- Allow tracking non `PageRoute` screens using the `PosthogObserver` ([#95](https://github.com/PostHog/posthog-flutter/pull/95)) +- Allow overriding the route filtering usint a ctor param `routeFilter` ([#95](https://github.com/PostHog/posthog-flutter/pull/95)) ## 4.3.0 diff --git a/lib/src/posthog_observer.dart b/lib/src/posthog_observer.dart index 135f935..00b85de 100644 --- a/lib/src/posthog_observer.dart +++ b/lib/src/posthog_observer.dart @@ -4,14 +4,26 @@ import 'posthog.dart'; typedef ScreenNameExtractor = String? Function(RouteSettings settings); +/// [PostHogRouteFilter] allows to filter out routes that should not be tracked. +/// +/// By default, only [PageRoute]s are tracked. +typedef PostHogRouteFilter = bool Function(Route? route); + String? defaultNameExtractor(RouteSettings settings) => settings.name; +bool defaultPostHogRouteFilter(Route? route) => route is PageRoute; + class PosthogObserver extends RouteObserver> { - PosthogObserver({ScreenNameExtractor nameExtractor = defaultNameExtractor}) - : _nameExtractor = nameExtractor; + PosthogObserver( + {ScreenNameExtractor nameExtractor = defaultNameExtractor, + PostHogRouteFilter routeFilter = defaultPostHogRouteFilter}) + : _nameExtractor = nameExtractor, + _routeFilter = routeFilter; final ScreenNameExtractor _nameExtractor; + final PostHogRouteFilter _routeFilter; + bool _isTrackeableRoute(String? name) { return name != null && name.trim().isNotEmpty; } @@ -36,6 +48,10 @@ class PosthogObserver extends RouteObserver> { void didPush(Route route, Route? previousRoute) { super.didPush(route, previousRoute); + if (!_routeFilter(route)) { + return; + } + _sendScreenView(route); } @@ -43,6 +59,10 @@ class PosthogObserver extends RouteObserver> { void didReplace({Route? newRoute, Route? oldRoute}) { super.didReplace(newRoute: newRoute, oldRoute: oldRoute); + if (!_routeFilter(newRoute)) { + return; + } + _sendScreenView(newRoute); } @@ -50,6 +70,10 @@ class PosthogObserver extends RouteObserver> { void didPop(Route route, Route? previousRoute) { super.didPop(route, previousRoute); + if (!_routeFilter(previousRoute)) { + return; + } + _sendScreenView(previousRoute); } } diff --git a/test/posthog_observer_test.dart b/test/posthog_observer_test.dart index 1725be0..c51dd03 100644 --- a/test/posthog_observer_test.dart +++ b/test/posthog_observer_test.dart @@ -1,8 +1,5 @@ -import 'dart:math'; - import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:posthog_flutter/src/posthog.dart'; import 'package:posthog_flutter/src/posthog_flutter_io.dart'; import 'package:posthog_flutter/src/posthog_flutter_platform_interface.dart'; import 'package:posthog_flutter/src/posthog_observer.dart'; @@ -28,8 +25,10 @@ void main() { }); PosthogObserver getSut( - {ScreenNameExtractor nameExtractor = defaultNameExtractor}) { - return PosthogObserver(nameExtractor: nameExtractor); + {ScreenNameExtractor nameExtractor = defaultNameExtractor, + PostHogRouteFilter routeFilter = defaultPostHogRouteFilter}) { + return PosthogObserver( + nameExtractor: nameExtractor, routeFilter: routeFilter); } test('returns current route name', () { @@ -78,4 +77,39 @@ void main() { expect(fake.screenName, null); }); + + test('does not capture filtered routes', () { + // CustomOverlawRoute isn't a PageRoute + final overlayRoute = CustomOverlawRoute( + settings: const RouteSettings(name: 'Overlay Route'), + ); + + final sut = getSut(); + sut.didPush(overlayRoute, null); + + expect(fake.screenName, null); + }); + + test('allows overriding the route filter', () { + final overlayRoute = CustomOverlawRoute( + settings: const RouteSettings(name: 'Overlay Route'), + ); + + bool defaultPostHogRouteFilter(Route? route) => + route is PageRoute || route is OverlayRoute; + + final sut = getSut(routeFilter: defaultPostHogRouteFilter); + sut.didPush(overlayRoute, null); + + expect(fake.screenName, 'Overlay Route'); + }); +} + +class CustomOverlawRoute extends OverlayRoute { + CustomOverlawRoute({super.settings}); + + @override + Iterable createOverlayEntries() { + return []; + } } From f1add3815bb40661269aa7ab90d619d4a7c9eb15 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 2 Apr 2024 14:39:56 +0200 Subject: [PATCH 6/7] fix readme --- CHANGELOG.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a11ef4..6266087 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ ## Next -- Allow overriding the route filtering usint a ctor param `routeFilter` ([#95](https://github.com/PostHog/posthog-flutter/pull/95)) +- chore: Allow overriding the route filtering using a ctor param `routeFilter` ([#95](https://github.com/PostHog/posthog-flutter/pull/95)) + +```dart +bool myRouteFilter(Route? route) => + route is PageRoute || route is OverlayRoute; +final observer = PosthogObserver(routeFilter: myRouteFilter); +``` ## 4.3.0 From 42a4d3255df0babfa7808adb911551fc0764b1e5 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 2 Apr 2024 14:53:02 +0200 Subject: [PATCH 7/7] fix --- lib/src/posthog_observer.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/posthog_observer.dart b/lib/src/posthog_observer.dart index 00b85de..3482e67 100644 --- a/lib/src/posthog_observer.dart +++ b/lib/src/posthog_observer.dart @@ -13,7 +13,7 @@ String? defaultNameExtractor(RouteSettings settings) => settings.name; bool defaultPostHogRouteFilter(Route? route) => route is PageRoute; -class PosthogObserver extends RouteObserver> { +class PosthogObserver extends RouteObserver> { PosthogObserver( {ScreenNameExtractor nameExtractor = defaultNameExtractor, PostHogRouteFilter routeFilter = defaultPostHogRouteFilter})