fix: targeted surveys shouldn't be shown to anon users #14
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (2)
packages/react-native/src/lib/common/utils.ts (1)
116-117: Consider adding a descriptive comment for the segmentFiltersLength logic.While the logic is correct, adding a brief comment explaining why we check for filter length would improve code readability.
+ // Check if survey has targeting filters - anonymous users can't be segmented const segmentFiltersLength = survey.segment?.filters?.length ?? 0; return segmentFiltersLength === 0;packages/react-native/src/lib/common/tests/utils.test.ts (1)
161-175: Consider adding a complementary test case for surveys without segment filters.While the current test verifies that surveys with filters are excluded for anonymous users, consider adding a test that confirms surveys without segment filters are still included for anonymous users to ensure complete coverage.
+ test("includes surveys without segment filters for anonymous users", () => { + environment.data.surveys = [ + { + ...baseSurvey, + id: mockSurveyId1, + segment: undefined, // No segment at all + } as TSurvey, + { + ...baseSurvey, + id: mockSurveyId2, + segment: { id: mockSegmentId1 }, // Segment but no filters + } as TSurvey, + ]; + + const result = filterSurveys(environment, user); + expect(result).toHaveLength(2); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/react-native/src/lib/common/tests/utils.test.ts(1 hunks)packages/react-native/src/lib/common/utils.ts(1 hunks)
🔇 Additional comments (2)
packages/react-native/src/lib/common/utils.ts (1)
113-119: LGTM! Clean implementation of anonymous user filtering.The implementation correctly filters out surveys with segment filters when no
userIdis present. The use of optional chaining and nullish coalescing properly handles potential undefined/null values in the segment structure.packages/react-native/src/lib/common/tests/utils.test.ts (1)
161-175: Excellent test coverage for the new anonymous user filtering behavior.The test case properly validates that surveys with segment filters are excluded when no
userIdis present. The test setup is realistic and follows the existing testing patterns in the file.
|
2 participants
Fixes #12
Adds a filter in the
filterSurveysfunction that filters out surveys having segment filters ifuserIdis not set (Anonymous usage). Also adds a test for thisHow to test?
userIdisn't setSummary by CodeRabbit
Bug Fixes
Tests