Skip to content

feat: filter surveys by eligibility before triggering (ENG-1128)#11

Merged
pandeymangg merged 4 commits into
mainfrom
feat/survey-filtering
Jun 12, 2026
Merged

feat: filter surveys by eligibility before triggering (ENG-1128)#11
pandeymangg merged 4 commits into
mainfrom
feat/survey-filtering

Conversation

@itsjavi

@itsjavi itsjavi commented Jun 11, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Implements survey eligibility filtering for the Flutter SDK — a 1:1 port of the React Native SDK's filterSurveys pipeline.

Previously trackAction showed every cached survey whose trigger name matched. Now:

  • filterSurveys (new filter_surveys.dart) computes the eligible set in three stages: displayOption (respondMultiple / displayOnce / displayMultiple / displaySome + displayLimit), recontactDays (survey-level value with workspace settings.recontactDays fallback, compared against lastDisplayAt), and segment targeting (anonymous sessions drop segment-filtered surveys; identified users only see surveys whose segment.id is in user.segments).
  • TSurvey gains the typed eligibility fields (displayOption, displayLimit, recontactDays, displayPercentage, segment — tolerant of the legacy {filters: [...]} shape). Raw JSON round-trip stays lossless.
  • trackAction reads config.filteredSurveys instead of the raw workspace survey list, and triggerSurvey applies the displayPercentage dice-roll (plain Random(), injectable for tests; skips are logged).
  • filteredSurveys is recomputed and persisted at every state-change point: setup (both paths), workspace refresh (expiry ticker), successful createOrUpdateUser, survey display/response/close (WebView events, matching RN survey-web-view.tsx), and tearDown/logout. Both ENG-1126 TODO(filtering ticket) markers are gone.

Intentional divergences from RN (documented inline): an unknown/missing displayOption excludes only that survey instead of throwing; non-num workspace recontactDays is treated as unset instead of crashing the filter; the expiry ticker re-reads config after the network round-trip so concurrent writes aren't clobbered.

Each touched source file ships with its test file.

Also adds apps/playground/integration_test/survey_filtering_qa_test.dart — a manual E2E QA harness (not executed by CI; flutter test only runs test/).

How should this be tested?

Unit/widget suite (CI runs this):

  • make test — 272 SDK tests + playground tests green.
  • make analyze and make format-check — clean.
  • Coverage on touched files is 86–100% (make test-sdk-coverage).

E2E on a simulator against a real backend (manual, validated on iOS):

  1. Start a local Formbricks instance with an app survey triggered by a code action (key code). Segment scenarios need an enterprise license.
  2. Boot a simulator and run a scenario:
    cd apps/playground
    fvm flutter test integration_test/survey_filtering_qa_test.dart -d <udid> \
      --dart-define=APP_URL=http://localhost:3000 \
      --dart-define=WORKSPACE_ID=<workspaceId> \
      --dart-define=SURVEY_ID=<surveyId> \
      --dart-define=SCENARIO=baseline
  3. Repeat with SCENARIO=displayOnce | displayLimit2 | recontactDays | displayPercentage | segmentAnonymous | segmentIdentified, flipping the survey's dashboard settings to match (Recontact options, Cooldown Period, "Show survey to % of users", Target Audience filters).
  4. All seven scenarios passed against a local instance: displayOnce drops immediately after its display; displaySome stops at the limit; survey-level and workspace-level recontact windows suppress re-shows; ~50% of triggers display at displayPercentage: 50; segment-filtered surveys are hidden from anonymous sessions, shown to a matching identified user, and dropped again on logout.

Manual smoke via the playground app:

  1. make run ios RUN_ARGS="--dart-define=APP_URL=... --dart-define=WORKSPACE_ID=..."
  2. Tap "Trigger Code Action" and use "Log Local Storage" to inspect filteredSurveys after each display/close/identity change.

Fixes ENG-1128

itsjavi added 3 commits June 11, 2026 14:53
Port RN's filterSurveys pipeline: displayOption, recontactDays,
and segment targeting stages, persisted as TConfig.filteredSurveys
and recomputed at every state-change point (setup, sync, user
update, display, response, close, logout, expiry).

trackAction now reads the filtered set instead of raw workspace
surveys, and triggerSurvey applies the per-display displayPercentage
roll. Add displayOption, displayLimit, recontactDays,
displayPercentage, and segment fields to TSurvey, plus a minimal
TSurveySegment type. Extract tryParseSurvey into common/utils.
Add unit and widget tests for the survey filtering pipeline introduced
with eligibility-based triggering:

- filter_surveys: diffInDays, segment-filter detection, display/recontact
  limits, and full filtering scenarios
- expiry_ticker: refetch recomputes filteredSurveys for anonymous and
  identified users, and survives concurrent config writes (Ok and Err)
- setup, action, update_queue, user, survey_webview, survey type, and
  utils: extend fixtures with segments/displays and assert filtered
  results drive triggering
Add integration_test driver and scenario-based E2E coverage
(baseline, displayOnce, displayLimit2, recontactDays,
displayPercentage, segmentAnonymous, segmentIdentified) for
ENG-1128, running against a real backend and survey runtime.
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR implements survey eligibility filtering for the Formbricks Flutter SDK (ENG-1128). It introduces a three-stage filtering algorithm (displayOption rules, recontactDays constraints, and segment-based targeting) that determines which surveys should be shown to each user. The filtering logic is applied throughout the SDK lifecycle: at initial setup, during workspace expiry sync, when triggering code actions, on user state changes, and when recording display/response events. Survey types are extended with new targeting fields, and display percentage gates are applied probabilistically at trigger time. The implementation includes comprehensive unit tests for the filtering algorithm and integration tests for each SDK flow, plus an end-to-end integration test validating real-world scenarios.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: filtering surveys by eligibility before triggering as part of ENG-1128.
Description check ✅ Passed The description comprehensively explains what the PR does, how it implements survey eligibility filtering (displayOption, recontactDays, segment targeting), which files are modified, testing approach, and relates directly to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/playground/integration_test/survey_filtering_qa_test.dart`:
- Around line 120-353: The switch over _scenario has many case blocks
('baseline', 'displayOnce', 'displayLimit2', 'recontactDays',
'displayPercentage', 'segmentAnonymous', 'segmentIdentified') that are missing
terminating statements and cause a compile error; fix by adding an explicit
terminator (e.g., break; or return;) at the end of each case body (after the
final await/_expect/_closeSurvey calls) so control does not fall through to the
next case—apply this to the blocks handling _scenario values named above in the
switch(_scenario) block.

In `@packages/formbricks_flutter/lib/src/survey/action.dart`:
- Around line 31-41: The gating currently only runs when displayPercentage > 0
so a value of 0 bypasses the gate; update the logic around the displayPercentage
variable (used with shouldDisplayBasedOnPercentage and Logger.debug for
survey.id) to run whenever displayPercentage != null and treat 0 as a
forced-suppress: i.e., if displayPercentage == 0 OR
shouldDisplayBasedOnPercentage(displayPercentage, random: random) returns false
then log the skip and return. This ensures a configured 0% actually suppresses
the survey.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 71afccdb-b855-4db9-ab14-4186d701f9d5

📥 Commits

Reviewing files that changed from the base of the PR and between e03ec92 and 8127aed.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • apps/playground/integration_test/survey_filtering_qa_test.dart
  • apps/playground/pubspec.yaml
  • packages/formbricks_flutter/lib/src/common/expiry_ticker.dart
  • packages/formbricks_flutter/lib/src/common/filter_surveys.dart
  • packages/formbricks_flutter/lib/src/common/setup.dart
  • packages/formbricks_flutter/lib/src/common/utils.dart
  • packages/formbricks_flutter/lib/src/survey/action.dart
  • packages/formbricks_flutter/lib/src/types/survey.dart
  • packages/formbricks_flutter/lib/src/user/update_queue.dart
  • packages/formbricks_flutter/lib/src/widgets/survey_webview.dart
  • packages/formbricks_flutter/test/common/expiry_ticker_test.dart
  • packages/formbricks_flutter/test/common/filter_surveys_test.dart
  • packages/formbricks_flutter/test/common/setup_test.dart
  • packages/formbricks_flutter/test/common/utils_test.dart
  • packages/formbricks_flutter/test/survey/action_test.dart
  • packages/formbricks_flutter/test/types/survey_test.dart
  • packages/formbricks_flutter/test/user/update_queue_test.dart
  • packages/formbricks_flutter/test/user/user_test.dart
  • packages/formbricks_flutter/test/widgets/survey_webview_test.dart

Comment thread apps/playground/integration_test/survey_filtering_qa_test.dart
Comment thread packages/formbricks_flutter/lib/src/survey/action.dart
@itsjavi itsjavi requested a review from pandeymangg June 11, 2026 14:14
@sonarqubecloud

Copy link
Copy Markdown

@pandeymangg pandeymangg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@pandeymangg pandeymangg merged commit 6144afc into main Jun 12, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants