User traits - #33
Conversation
…ity management - Added support for posting user traits via `POST /v1/sessions` with `eventType: traits_updated`. - Implemented caching of `traitsId` for subsequent event batches. - Introduced `setUserId` to update the runtime user ID and notify the collector with `eventType: user_changed`. - Updated documentation to reflect changes in session lifecycle and identity management. - Enhanced tests to cover new functionality for traits and user ID management.
- Updated package version in `pubspec.yaml`, `README.md`, and `CHANGELOG.md` to reflect the new version 0.5.1. - Adjusted compatibility information in documentation to align with the new version.
There was a problem hiding this comment.
Pull request overview
This PR adds first-class support for “user traits” and a runtime user id in the HTTP collector integration, including new session lifecycle event types (traits_updated, user_changed) and caching/stamping of a collector-issued traitsId across subsequent event batches. It also bumps the package version to 0.5.1 and documents the new identity behavior.
Changes:
- Introduces collector session lifecycle event typing (
TugboatCollectorSessionEventType) and extends session/event payload mapping to supporttraitsandtraitsId. - Adds identity state handling end-to-end:
TugboatReplay.setTraits/setUserId→ controller →CollectorHttpSink, including cachingtraitsIdfrom accepted session responses and stamping it on event batches. - Updates tests and docs, and bumps version numbers across package metadata and documentation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/tugboat/lib/src/tugboat.dart | Adds process-local pending identity state and public setTraits / setUserId APIs; wires initial identity into controller creation. |
| packages/tugboat/lib/src/controller.dart | Plumbs initial traits/traitsId/userId into CollectorHttpSink; exposes controller pass-through methods and identity getters. |
| packages/tugboat/lib/src/collector_http_sink.dart | Implements traits/user-id lifecycle posting, caches traitsId from responses, and stamps traitsId onto event batches. |
| packages/tugboat/lib/src/collector_mapper.dart | Adds session event type enum and optional traitsId/traits mapping behavior. |
| packages/tugboat/test/collector_mapper_test.dart | Extends mapper tests for wire values and traits/traitsId precedence rules. |
| packages/tugboat/test/collector_http_sink_test.dart | Adds sink integration tests for traits posting, caching, and event stamping. |
| packages/tugboat/README.md | Documents traits/user id behavior and updates version string. |
| docs/integration/collector.md | Updates collector route contract docs to include new session identity events and traitsId behavior. |
| packages/tugboat/CHANGELOG.md | Adds 0.5.1 release notes describing traits/user id support. |
| packages/tugboat/pubspec.yaml | Bumps package version to 0.5.1. |
| packages/tugboat/lib/src/sdk_version.dart | Updates SDK version constant to 0.5.1. |
| packages/tugboat/example/pubspec.yaml | Updates example dependency constraint to ^0.5.1. |
| docs/README.md | Updates documented current package version to 0.5.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (_pendingUserIdSet || controller.collectorUserId != null) { | ||
| _pendingUserId = controller.collectorUserId; | ||
| _pendingUserIdSet = true; | ||
| } |
There was a problem hiding this comment.
this is a good catch, is this fixed?
- Updated `setUserId` method to no-op when the provided user ID matches the current runtime ID, preventing unnecessary `user_changed` posts. - Enhanced documentation in `README.md` and `CHANGELOG.md` to reflect this change. - Added tests to verify the no-op behavior when the user ID remains unchanged.
|
@deepakg202 Let's add the ability to clear out a user id, right now if we pass null it will still retain the old user id, this would help for logged out users |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
packages/tugboat/lib/src/collector_mapper.dart:85
- mapTugboatSessionLifecycleToCollectorSession cannot represent an explicit "userId override to null" because it uses
userId ?? config.userId. When the collector config has a non-null userId and the runtime userId is intentionally cleared (setUserId(null)), this will still emit the config userId.
Map<String, Object?> mapTugboatSessionLifecycleToCollectorSession({
required String eventType,
required String sessionId,
required DateTime triggeredAt,
required TugboatCollectorConfig config,
String? userId,
Map<String, dynamic>? traits,
String? traitsId,
}) {
return {
'sessionId': sessionId,
'userId': userId ?? config.userId,
'eventType': eventType,
'triggeredAt': triggeredAt.toUtc().toIso8601String(),
packages/tugboat/lib/src/tugboat.dart:145
- _syncIdentityFromController() sets _pendingUserIdSet = true whenever controller.collectorUserId is non-null. Since CollectorHttpSink.userId is initialized from config.userId, this can mark a user-id override as active even when the host never called TugboatReplay.setUserId(), causing subsequent mounts to ignore updated TugboatReplayConfig.userId values.
static void _syncIdentityFromController() {
final controller = _controller;
if (controller == null) return;
final traits = controller.collectorTraits;
if (traits != null) {
_pendingTraits = Map<String, dynamic>.from(traits);
}
final traitsId = controller.collectorTraitsId;
if (traitsId != null && traitsId.isNotEmpty) {
_pendingTraitsId = traitsId;
}
if (_pendingUserIdSet || controller.collectorUserId != null) {
_pendingUserId = controller.collectorUserId;
_pendingUserIdSet = true;
}
packages/tugboat/lib/src/controller.dart:1213
- initialUserIdOverride intends to allow pre-activate setUserId(null) to override/clear a configured collector userId, but this code paths through TugboatCollectorConfig.withUserId(userId) and CollectorHttpSink(initialUserId: userId). Both currently treat null as “keep/fallback to existing config.userId”, so an explicit null override cannot take effect when collectorConfig.userId is set.
final userId = _initialUserIdOverride
? _initialUserId
: collectorConfig.withUserId(config.userId).userId;
_collectorHttpSink = CollectorHttpSink(
config: collectorConfig.withUserId(userId),
initialTraits: _initialTraits,
initialTraitsId: _initialTraitsId,
initialUserId: userId,
| final result = _classifyResponse(response.statusCode); | ||
| if (result == _SendResult.accepted && eventType == 'session_start') { | ||
| if (result == _SendResult.accepted) { | ||
| final decoded = jsonDecode(response.body) as Map<String, dynamic>; | ||
| final serverId = decoded['sessionId'] as String?; | ||
| if (_isCurrentEpoch(epoch)) { |
- Updated `setTraits` and `setUserId` methods to retain identity information when called after the controller mounts but before the HTTP sink is created. - Enhanced `CHANGELOG.md` to document the new behavior for identity retention during session starts.
- Refactored response acceptance logic in `CollectorHttpSink` to allow empty or non-JSON bodies without triggering retries. - Enhanced session ID management to ensure fallback to local session ID when server response is empty. - Updated tests to validate behavior for empty and non-JSON response bodies during session lifecycle events. - Adjusted `TugboatReplay` to ensure user ID updates are only applied when explicitly set, preventing unintended overrides.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/tugboat/lib/src/tugboat.dart:96
- The docstring says traits updates no-op when the map "deep-equals" the pending bag, but the implementation uses
mapEquals, which is not a deep equality for nested collections (nestedMap/Listvalues are compared via==). This can lead to unexpectedtraits_updatedposts when callers rebuild equivalent nested maps.
/// bag (no server-side merge). No-ops when [traits] deep-equals the pending
/// bag. Does not call `/v1/identify`.
packages/tugboat/lib/src/tugboat.dart:118
TugboatReplay.setUserIdmarkshasPendingUserIdOverridetrue even when the provideduserIdalready matches the current runtime/config user id. This can unintentionally “lock in” an override and cause future remounts to ignore updatedTugboatReplayConfig.userIdvalues, despite the doc/CHANGELOG stating unchanged ids are skipped.
/// No-ops when [userId] equals the current runtime id. When a capture
/// session is active and the id changes, sends `POST /v1/sessions` with
/// `eventType: user_changed` and includes the cached traits bag when set.
static Future<void> setUserId(String? userId) async {
_pendingUserId = userId;
packages/tugboat/lib/src/collector_http_sink.dart:184
- The docstring says
setTraitsno-ops when the provided traits map "deep-equals" the cached bag, but the code usesmapEquals(not deep for nested collections). This can cause redundanttraits_updatedlifecycle posts when callers provide equivalent-but-new nested maps/lists.
/// No-ops when [traits] deep-equals the cached bag (no `traits_updated`
/// post). When a capture session is active and the bag changes, posts
- Updated package version in `pubspec.yaml`, `README.md`, and `CHANGELOG.md` to reflect the new version 0.5.2. - Enhanced `setUserId` functionality to skip `user_changed` posts while a `session_start` is pending, ensuring the latest user ID is included in the start payload. - Updated documentation to clarify the behavior of `setUserId` and `setTraits` during session management. - Added tests to validate the new behavior of `setUserId` when called before a session starts.
…ents - Introduced a debouncing mechanism for `setUserId` and `setTraits` to coalesce updates into a single `session_identify` POST when both user ID and traits change within 3 seconds after a session starts. - Updated the `CollectorHttpSink` to manage identity state and ensure pending updates are flushed before `session_end`. - Enhanced documentation to clarify the new behavior of identity management during session lifecycles. - Added tests to validate the debouncing functionality and ensure correct event posting behavior.
- Added `_userTriggeredAt` and `_traitsTriggeredAt` properties to track the timestamps of user ID and traits updates. - Implemented logic in `_coalescedIdentityTriggeredAt` to determine the latest triggered time when both user ID and traits are dirty. - Updated the `setUserId` and `setTraits` methods to set the respective triggered timestamps. - Enhanced tests to validate the correct behavior of triggered timestamps during identity updates and debouncing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (6)
packages/tugboat/README.md:8
- The README claims the current package version is
0.5.1, butpackages/tugboat/pubspec.yamlandsrc/sdk_version.darthave been bumped to0.5.3. This should be kept in sync to avoid confusing consumers.
The current package version is `0.5.1`. Session JSON writers emit schema
docs/README.md:31
- Docs list the package version as
0.5.1, but the package itself is versioned0.5.3inpackages/tugboat/pubspec.yaml. Please keep the documentation version in sync with the published package version.
- package version: `0.5.1`;
packages/tugboat/example/pubspec.yaml:35
- The example app depends on
tugboat: ^0.5.1, but this PR bumps the package to0.5.3. If the example is intended to track the latest package in this repo, bump the constraint to match.
tugboat: ^0.5.1
packages/tugboat/lib/src/collector_http_sink.dart:205
- This doc comment says the traits bag comparison is "deep-equals", but the implementation uses
mapEquals, which is shallow (uses==for values). The comment should reflect the actual semantics to avoid surprising callers with nested maps/lists.
/// No-ops when [traits] deep-equals the cached bag. While `session_start` is
/// still pending, updates memory only (folded into start at send time).
/// Otherwise debounces `traits_updated` or `session_identify` when combined
/// with a pending user change within [_identityDebounceDuration].
packages/tugboat/lib/src/tugboat.dart:110
setUserIdsets_pendingUserIdSet = trueunconditionally, so it isn't a true no-op even when the same id is provided (it affects remount behavior viahasPendingUserIdOverride). The doc comment should clarify that the no-op refers to collector posting, not internal override state.
/// No-ops when [userId] equals the current runtime id. When a capture session
/// is active and the id changes, debounces lifecycle posts (3s). Combined
/// with a pending traits change, posts `session_identify`; otherwise
/// `user_changed`. While `session_start` is still pending, updates memory only.
packages/tugboat/test/collector_http_sink_test.dart:210
- These tests rely on a real 3.1s wall-clock delay to wait out the identity debounce, and the helper is used 9 times in this file (≈28s added to the suite). Consider making the debounce duration injectable/test-configurable or adding a test-only API to flush the debounced identity immediately so tests can advance without sleeping.
Future<void> awaitIdentityDebounce() async {
await Future<void>.delayed(const Duration(milliseconds: 3100));
}
- Updated package version in `pubspec.yaml`, `README.md`, and `CHANGELOG.md` to reflect the new version 0.5.3. - Adjusted compatibility information in documentation to align with the new version. - Enhanced documentation to clarify the behavior of identity management during session lifecycles.
No description provided.