feat: setEmbeddedData() + clearEmbeddedData() for app surveys [ENG-2472] - #44
Conversation
A host app can attach context to future responses without tying it to a
trigger. `track()` takes a name and nothing else, so today the only way to
get context onto a response is to declare it in the survey and have the
respondent type it.
Formbricks.setEmbeddedData({'screen': 'checkout', 'plan': 'pro'});
Formbricks.setEmbeddedData({'screen': null}); // remove one key
Formbricks.clearEmbeddedData('plan'); // same, explicitly
Formbricks.clearEmbeddedData(); // everything
Merge, never replace, so refreshing a volatile field cannot wipe a stable
one. `null` removes a key; a key left out is untouched, which is how a host
skips a field it has no value for this screen.
`clearEmbeddedData` takes `Object?` around a private sentinel rather than a
plain `String?`, so "called with no argument" and "called with a key that
evaluated to null" stay different things — the distinction the JS SDK draws
by argument count. `clearEmbeddedData(prefs['fieldToClear'])` with that
state empty must not wipe the bag; it is a logged no-op.
In-memory and never persisted: persisting would blur the Embedded Data ↔
contact-attribute boundary and create a PII-at-rest surface. Cleared on an
identity switch and on logout so one user's context cannot ride onto the
next user's responses on a shared device; kept on first identification,
because a host legitimately pushes context before it knows who the user is.
Synchronous and outside the command queue, unlike every other public method:
a host that pushes context at launch must not have the value dropped because
setup had not finished.
Snapshotted in `_present()`, which runs when the survey is actually shown
after any configured delay, and frozen for its lifetime. The bag rides the
render options that already exist, under `hiddenFieldsRecord` — no new
bridge message, and deliberately so: a `setEmbeddedData` after display must
not reach the survey on screen. It is passed raw and unfiltered, because the
ingest contract lives in the renderer and the server re-runs all of it.
A value of an unsupported type, and a non-finite number, are logged and
skipped: `jsonEncode` throws on NaN, and the payload it would refuse is the
whole survey's render options, so one bad value would cost the survey rather
than the field.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018L6PrECN38Jbe1FEdFccTz
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 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. Comment |
|
Four things from manual testing + review (tested locally — the pipe works end-to-end 👍):
|
…ENG-2472] Four review findings, all real: 1. The sentinel was forgeable. Dart canonicalizes const instances, so the `const Object()` marker was `identical` to every `const Object()` in the program and `Formbricks.clearEmbeddedData(const Object())` from host code wiped the whole bag instead of being refused — verified before the fix. It is now a const instance of a library-private `_ClearWholeBag`: still a valid default parameter value (which must be a compile-time constant), but host code cannot name the type, so it cannot produce one. 2. The sentinel's dartdoc had stolen the `Formbricks` class doc — it sat between the class's `///` block and the declaration with no blank line, and Dart attaches a contiguous run of doc comments to the next declaration. The sentinel now lives above the class doc entirely. 3. The pipe's load-bearing line had no test. `_present()` passing `hiddenFieldsRecord: EmbeddedDataStore.instance.snapshot()` is the single line joining the store, the render options and the renderer, and deleting it left every other test green while the survey still rendered — a silent failure. Two widget tests now drive `_present()`: one immediate, one that sets the value during the delay so "read at display, not at mount" is pinned through the real widget. 4. setEmbeddedData succeeded in silence. Mirrors the js-core debug trace from formbricks/formbricks#9091: keys set and removed, what the bag now holds, and the sentence that pre-empts the next question. Keys only, never values — the documented use of this bag includes hashed identity fields. flutter test 339 passed (was 330) · analyze --fatal-infos --fatal-warnings clean · format clean. Mutation-checked: deleting the pipe line turns exactly the two new widget tests red; restoring `const Object()` turns exactly the forgery test red.
|
|
All four done in 2. The sentinel was forgeable. Confirmed before fixing — and the doc comment on it claimed the opposite ("no caller can produce a value The fix is not quite what you proposed, because a default parameter value must be a compile-time constant — class _ClearWholeBag {
const _ClearWholeBag();
}
const Object _clearWholeBag = _ClearWholeBag();Const-ness is fine once the type is unforgeable: canonicalization only makes two 3. The dartdoc theft. Confirmed — the 1. The pipe had no test. You're right, and the mutation check confirms it: deleting
4. Debug trace. Mirrors formbricks/formbricks#9091, at
Generated by Claude Code |
2 participants
What & why
Was:
track()takes a name and nothing else, so the only way to get context onto an app-survey response was to declare a field and have the respondent type it. Now: the host app can attach context to future responses without tying it to a trigger.Mirrors js-core's
setEmbeddedDatakey for key (formbricks/formbricks#8989), so web and mobile behave identically.nullremoves a key; a key left out is untouched — that is how a host skips a field it has no value for this screen.clearEmbeddedDatatakesObject?around a private sentinel, not a plainString?. Dart has neither overloads nor arity, and "no argument" must stay different from "a key that evaluated to null" — the distinction the JS SDK draws by argument count.clearEmbeddedData(prefs['fieldToClear'])with that state empty is a logged no-op, not a wipe. The sentinel is a const instance of a library-private class, not aconst Object(): const instances are canonicalized, so the latter isidenticalto everyconst Object()in the program and would let host code wipe the bag by accident.SharedPreferences— persisting would blur the Embedded Data ↔ contact-attribute boundary and create a PII-at-rest surface. Cleared on an identity switch and onlogout; kept on first identification, because a host legitimately pushes context before it knows who the user is.setuphad not finished._present(), which runs after any configured delay.hiddenFieldsRecord— no new bridge message. It goes out raw: the ingest contract (allow-list, coercion,locked, size caps) lives in the renderer, and the server re-runs all of it.Where to look:
lib/src/survey/embedded_data.dart(the store, every lifetime rule, and the trace) · the sentinel inlib/src/widgets/formbricks_widget.dart·lib/src/user/user.dart(identity-switch clearing) · the two new pipe tests intest/widgets/survey_webview_test.dart.Requires the renderer change in formbricks/formbricks#9067 for the auto-capture half; this PR is independent of it and needs no server change.
Linear ticket
https://linear.app/formbricks/issue/ENG-2472/mobile-sdk-parity-for-embedded-data-one-batched-release-per-sdk
How this was tested
Run against Flutter 3.44.9 / Dart 3.12.2 (the repo needs Dart
^3.12.0).flutter test✅ 339 passed, up from 313 onmain·make analyze-ci(--fatal-infos --fatal-warnings) ✅ No issues found ·make format-check✅ clean (dart format+ prettier on markdown)test/survey/embedded_data_test.dart(26) covers merge,nullremoval, omitted-key no-op, single-key clear, clear-all,clearEmbeddedData(null)as a no-op rather than a wipe, the two sentinel-forgery cases, a non-String key, every scalar withDateTime→ISO 8601, localDateTimenormalized to UTC, JSON-encodability, the non-finite-number guard, unsupported value types, the detached snapshot, works-before-setup, and the five debug-trace cases.test/widgets/survey_webview_test.dartgains two widget tests driving_present()— see the fold.test/user/user_test.dartgains four: switch clears · first identification keeps · same id keeps · logout clears. Mutation-checked — commenting out bothEmbeddedDataStore.instance.clear()calls inlib/src/user/user.dartturns exactly "switching to a different userId clears the bag" and "logout clears the bag" red, leaving the two "keeps" cases green.test/widgets/survey_html_test.dartgains two pinning thathiddenFieldsRecordreaches the render options — including a key the survey does not declare (the SDK must not filter) — and that it is{}when the host set nothing.Why a non-finite number is refused at the door
jsonEncodethrows onNaN/Infinity, and the payload it would refuse is the whole survey's render options —buildSurveyHtmlwould produce nothing and no survey would appear. A singlesetEmbeddedData({'x': double.nan})from host code would therefore cost the survey rather than the field. The store drops it with a log instead, and a test asserts the snapshot staysjsonEncode-able. Same reasoning refusesList/Mapvalues, which the ingest contract cannot store either.Review follow-ups (commit
6565b2b)Four findings, three of them defects in this diff:
1. The clear-everything sentinel was forgeable. Dart canonicalizes const instances, so the
const Object()marker wasidenticalto everyconst Object()in the program — and its own doc comment claimed the opposite. Verified before fixing:A
static final _Sentinel()cannot be used, because a default parameter value must be a compile-time constant. It is now a const instance of a library-private_ClearWholeBag: canonicalization only unifies twoconst _ClearWholeBag()expressions, and host code cannot name the type to write one. Reverting turns exactly the forgery test red.2. The sentinel's dartdoc had stolen the class doc. It sat between the
Formbricksclass's///block and the declaration with no blank line, and Dart attaches a contiguous run of doc comments to the next declaration — so the class shipped undocumented. The sentinel now lives above the class doc entirely.3. The pipe's load-bearing line had no test.
_present()passinghiddenFieldsRecord: EmbeddedDataStore.instance.snapshot()is the single line joining the store, the render options and the renderer; the store, the options and the identity-clear were each tested in isolation, so deleting it left every test green while the survey still rendered — a silent failure. Two widget tests now drive_present(): one immediate, and one on a delayed survey where the bag is empty at mount and filled while the timer runs, which pins "read at display, not at mount" through the real widget. Deleting the line now turns exactly those two red.4. The debug success trace, mirroring formbricks/formbricks#9091.
Loggerroutes throughdebugPrint, whichflutter_testlets a test swap, so no new seam was needed — the tests assert the keys, the omitted-removedcase, that no value ever appears, and that the trace is silent at the default error level.Breaking changes
None. Two new static methods and one new optional field on
SurveyHtmlOptions(defaulted, so existing construction is unaffected).setup,track,setUserId,setAttribute(s),setLanguageandlogoutkeep their signatures;logout()and an identity-switchingsetUserId()additionally clear the new in-memory bag, which did not exist before.QA / Test Plan
How to test
planon an app survey. CallFormbricks.setEmbeddedData({'plan': 'pro'}), thenFormbricks.track()the survey's action → the response showsplan = pro.setEmbeddedData({'plan': 'pro'})thensetEmbeddedData({'screen': 'checkout'})→ the response carries both. Merge, not replace.setEmbeddedData({'plan': null})→planis absent from the next response,screenis still there.clearEmbeddedData()with no argument → the next response carries none of the fields.clearEmbeddedData(null)→ nothing is cleared, and a line is logged. This is the one that is easy to get backwards.clearEmbeddedData(const Object())→ nothing is cleared. Before this PR's follow-up commit that call wiped the bag.logLevel: LogLevel.debug, watch the console while callingsetEmbeddedData/clearEmbeddedData→ each call logs the keys it set or removed and what the bag now holds. No values appear in any line. At the default level, nothing is logged.setEmbeddedData({'plan': 'enterprise'})while it is on screen, finish it → the response records the value from when the survey appeared.setUserId('a')→setEmbeddedData({'plan': 'pro'})→setUserId('b')→ survey → the response carries noplan.logout()after setting values → the next response carries none of them.setEmbeddedData({'items': ['a','b']})or{'x': double.nan}from host code → logged and skipped, the survey still renders.Preconditions / test data
surveys.umd.cjs, andapps/playgroundor a host app. Response card is the readout.Risks & regressions
logout()and identity-switchingsetUserId()now also clear the bag. Nothing else observes it, so no other behaviour changes.scriptSafeJsonstill neutralizes<,>and the JS line separators over the whole blob, so a host-supplied value cannot break out of the inline script — worth re-checking with a value containing</script>.Migrations / env / cutover
Generated by Claude Code