Skip to content

feat: support survey-interaction segment filters (ENG-1275) - #73

Merged
pandeymangg merged 5 commits into
mainfrom
feat/interaction-based-segments
Aug 5, 2026
Merged

feat: support survey-interaction segment filters (ENG-1275)#73
pandeymangg merged 5 commits into
mainfrom
feat/interaction-based-segments

Conversation

@pandeymangg

Copy link
Copy Markdown
Contributor

What & why

The web app now supports survey-interaction segment filters — targeting contacts by whether they have seen / have not seen / have started responding to / have completed / have not completed a survey within a time window (formbricks#8588).

Membership for those filters is computed server-side, and it can flip the moment a contact interacts with a survey. js-core reacts by refetching user state right away; this SDK had no equivalent, so it kept using the segment list it received at app launch. A rule like "completed survey A → show survey B" would not fire in the same session.

Of the four SDKs this had to be ported to, this one was the closest fit — UpdateQueue already exposes updateUserId and processUpdates exactly like js-core's, so the js-core recipe transfers as-is.

What changed

The gateinteractionRefresh on TSurvey

interactionRefresh?: {
  onDisplay: boolean;
  onResponse: boolean;
  onFinished: boolean;
};

Absent for workspaces that don't use interaction targeting, and present-but-all-false for surveys no interaction filter references — both handled by ?? false.

The refreshlib/user/interaction-refresh.ts

Mirrors js-core's refreshSegmentsAfterInteraction. Gated twice, because a /user sync is not cheap:

  • no-op for anonymous users, who never receive segments in the first place
  • no-op unless the server set the bit for that survey and that event

Routed through UpdateQueue so a display → response → finish burst debounces into a single request.

onFinished bridged from the WebView

Two thirds of this were already in place and unused: onFinished is a long-standing prop of the surveys library, and ZJsRNWebViewOnMessageData already declared the flag. What was missing was the harness shim that posts it and the handler that reads it — so haveCompleted / haveNotCompleted had no client-side trigger.

Because the harness passes getSetIsResponseSendingFinished, isResponseSendingFinished starts false, so onFinished genuinely means the finished response was accepted by the backend.

Wired into onMessage

All three lifecycle branches now call the refresh after the local optimistic config update, matching js-core's ordering.

Verification

168 tests, 0 failures across 19 files. tsc --noEmit and biome check both clean.

13 new tests: 9 covering the gate (anonymous, absent, all-false, mismatched source, each matching source, partial object, and that it routes through the queue rather than sending directly) and 4 covering the harness.

Confirmed with mutation testing — each of these makes a test go red:

Mutation Result
remove onFinished from the surveyProps object 1 test fails
remove the anonymous-user guard 1 test fails
ignore the gate and always refresh 4 tests fail

Notes for reviewers

  1. renderHtml is now exported. Only so the harness can be asserted in tests — it is not re-exported from src/index.ts, so it stays package-internal. Without it there is no way to check that onFinished is actually handed to renderSurvey, which is the mutation the harness test catches.
  2. __DEV__ is stubbed in vitest.setup.ts. React Native injects it at build time, so any test importing a module that branches on it fails at import. false is the right value for tests and it is a shared setup file, so this benefits future component tests too.
  3. No TTL bug here, unlike the native SDKs. addUserStateExpiryCheckListener extends expiresAt every 60s rather than refetching — same as js-core. That is by design, and it is exactly why the interaction refresh matters: nothing else pulls fresh segments mid-session.
  4. Known gap, deliberately left alone. onResponseCreated fires optimistically from the surveys library, before the response-create POST completes, so an interaction-driven /user refresh can land before the row is committed and the server's array wins. js-core has the same characteristic, so this matches it rather than diverging. The real fix is upstream — moving onResponseCreated onto ResponseQueue's confirmed hook — which fixes every platform at once.

pandeymangg and others added 4 commits July 21, 2026 12:50
Move the "<" escaping into a `const optionsJson` before the template and
use `replaceAll` + `String.raw` instead of `replace(/</g, "<")`.

Clears three SonarQube smells introduced by the fix:
- S7781 (prefer replaceAll over replace with a global regex)
- S7780 (prefer String.raw over a string literal with an escaped backslash)
- the same S7780 on the outer template literal, whose raw text previously
  contained "<" via the inline explanatory comment (now moved out).

Behaviour is unchanged: String.raw`<` yields the same 6-char escape,
so survey content still can't break out of the inline <script>.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ports the client half of the web SDK change for interaction-based segment
filters ("have seen X", "have completed X", ...). Membership for those filters
is computed server-side and can flip the moment a contact interacts with a
survey, so the SDK now refetches user state instead of waiting for it to expire.

- Add the per-survey `interactionRefresh` gate to `TSurvey`.
- Add `refreshSegmentsAfterInteraction`, mirroring js-core: no-op for anonymous
  users, no-op unless the server flagged that survey and event, otherwise nudge
  the UpdateQueue so a display -> response -> finish burst debounces into one
  request.
- Bridge `onFinished` from the WebView. The prop and the zod flag already
  existed but nothing ever fired or read them, so "have completed X" had no
  client-side trigger.
- Wire the refresh into all three lifecycle branches of the WebView's onMessage
  handler, after the local optimistic config update.

`renderHtml` is exported so the harness can be asserted in tests; it is not
re-exported from the package entry point. `__DEV__` is stubbed in the vitest
setup because React Native injects it at build time and modules that branch on
it fail to import otherwise.
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The change adds optional survey interaction refresh configuration for display, response, and completion events. Enabled refreshes queue identified-user updates through UpdateQueue. The WebView now emits an onFinished message and forwards lifecycle events to the refresh handler. Serialized options escape < characters before inline JavaScript embedding. Tests cover queue gating, lifecycle messaging, callback propagation, serialization escaping, and the Vitest __DEV__ global.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: support for survey-interaction segment filters.
Description check ✅ Passed The description explains the survey-interaction segment filter support and the related SDK changes.
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.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@packages/react-native/src/lib/user/interaction-refresh.ts`:
- Around line 39-41: Handle the promise returned by UpdateQueue.processUpdates()
in the interaction refresh flow by attaching rejection handling, while
preserving the queue’s existing failure logging and avoiding duplicate error
reporting.
🪄 Autofix

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 Plus

Run ID: 322d414d-2120-470e-95f4-8acba59b32a1

📥 Commits

Reviewing files that changed from the base of the PR and between 779d353 and 7d93929.

📒 Files selected for processing (6)
  • packages/react-native/src/components/survey-web-view.tsx
  • packages/react-native/src/components/tests/survey-web-view-harness.test.ts
  • packages/react-native/src/lib/user/interaction-refresh.ts
  • packages/react-native/src/lib/user/tests/interaction-refresh.test.ts
  • packages/react-native/src/types/survey.ts
  • packages/react-native/vitest.setup.ts

Comment thread packages/react-native/src/lib/user/interaction-refresh.ts Outdated
`processUpdates()` rejects when the flush throws, and the refresh calls it
fire-and-forget — a bare `void` left that as an unhandled promise rejection,
which React Native surfaces loudly. Attach a `.catch`; the queue already logs the
real cause, so swallowing here avoids reporting it twice.

The test asserts `.catch` is actually attached rather than watching
`process.on("unhandledRejection")`. Node only reports an unhandled rejection on a
later tick, so the listener approach passed with and without the fix — it looked
like coverage without pinning anything.

Also re-establish the mock's resolved value in the local `beforeEach`: the shared
vitest setup calls `resetAllMocks`, which strips implementations, so
`processUpdates()` was returning undefined.
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@pandeymangg
pandeymangg requested a review from Dhruwang August 5, 2026 15:14
@pandeymangg
pandeymangg added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit b61e175 Aug 5, 2026
10 checks passed
@pandeymangg
pandeymangg deleted the feat/interaction-based-segments branch August 5, 2026 15:17
@pandeymangg pandeymangg mentioned this pull request Aug 5, 2026
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