Product analtyics improvements #4794
Conversation
WalkthroughThe changes introduce enhancements to the Changes
Possibly related PRs
Poem
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review due to trivial changes (1)
Warning Review ran into problemsProblemsGit: Failed to clone repository. Please contact CodeRabbit support. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
- add ability to redact custom properties - add ability to disable url tracking
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
app/packages/analytics/src/analytics.test.ts (1)
159-176: LGTM, but remove the console.log statement.The test case for obfuscating URI properties of events looks good. It correctly sets up the Analytics instance with redact set to ["uri"] and verifies that the track method is called with the uri property set to "".
However, there is a console.log statement at line 170 that should be removed:
- console.log(mockSegment.track.mock.calls[0]);
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/packages/analytics/src/analytics.test.ts (4 hunks)
- app/packages/analytics/src/usingAnalytics.ts (6 hunks)
Additional context used
Path-based instructions (2)
app/packages/analytics/src/usingAnalytics.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/analytics/src/analytics.test.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
Additional comments not posted (8)
app/packages/analytics/src/usingAnalytics.ts (5)
9-10: LGTM!The additions to the
AnalyticsInfotype are correctly typed and serve to enhance the functionality of theAnalyticsclass by allowing for more granular control over data tracking and privacy.
34-35: LGTM!The additions of the
_disableUrlTrackingand_redactedPropertiesprivate properties to theAnalyticsclass are correctly typed and initialized, and serve to manage the new functionalities introduced in the class.
43-51: LGTM!The addition of the
redactmethod to theAnalyticsclass is correctly implemented and serves to handle the redaction of specified properties from the analytics data.
62-65: LGTM!The updates to the
loadmethod are correctly implemented and serve to initialize the new_redactedPropertiesand_disableUrlTrackingproperties based on the provided configuration.
92-92: LGTM!The updates to the
page,track,identify, andgroupmethods are correctly implemented and serve to ensure that any properties passed to these methods are processed for redaction, and that URL tracking is disabled when specified.Also applies to: 99-99, 115-119, 128-128, 134-134
app/packages/analytics/src/analytics.test.ts (3)
80-80: LGTM!The test case for debouncing duplicate events within the debounce interval looks good. It correctly mocks the Date.now() function to simulate time passing and verifies that the track method is called the expected number of times based on the debounce interval.
141-157: LGTM!The test case for disabling URL tracking looks good. It correctly sets up the Analytics instance with disableUrlTracking set to true and verifies that the track method is called with context.page.url set to undefined.
178-197: LGTM!The test case for the redact method looks good. It correctly sets up the Analytics instance with redact set to ["uri"] and verifies that the redact method behaves as expected for various input scenarios, including:
- Objects with the specified properties (uri) are obfuscated
- Objects without the specified properties are left intact
- Empty objects are handled correctly
- Undefined values are handled correctly
The test case covers all the necessary scenarios and ensures the redact method works properly.
| }); | ||
| analytics.track("random_event", { uri: "@my_name/my_plugin/my_operator" }); | ||
| // segment should be called with properties.uri = "<redacted>" | ||
| console.log(mockSegment.track.mock.calls[0]); |
There was a problem hiding this comment.
| console.log(mockSegment.track.mock.calls[0]); |
Adds the ability to disable url tracking and redact given event properties.
Summary by CodeRabbit
New Features
Analyticsclass with new methods for improved data privacy and configurability.Bug Fixes
Analyticsclass.Documentation