fix: Allow all user identity types in Identity.search#1256
fix: Allow all user identity types in Identity.search#1256rmi22186 merged 5 commits intodevelopmentfrom
Conversation
PR SummaryMedium Risk Overview The guard rails were updated to skip the network call unless any identifier contains a non-empty string, and types/exports were adjusted by removing Reviewed by Cursor Bugbot for commit a0fb93c. Bugbot is set up for automated code reviews on this repo. Configure here. |
| const hasIdentifier = | ||
| knownIdentities && | ||
| typeof knownIdentities === 'object' && | ||
| Object.values(knownIdentities).some( | ||
| (v) => typeof v === 'string' && v.length > 0, | ||
| ); | ||
| if (!hasIdentifier) { |
There was a problem hiding this comment.
Making this a convenience method right before invocation doesn't really help with readability, IMO.
We probably have some utility methods that could help clean this up a bit instead, or we can roll this into a new utility method.
| const hasIdentifier = | |
| knownIdentities && | |
| typeof knownIdentities === 'object' && | |
| Object.values(knownIdentities).some( | |
| (v) => typeof v === 'string' && v.length > 0, | |
| ); | |
| if (!hasIdentifier) { | |
| if (!isEmpty(knownIdentities) && knownIdentities.some(v => !isEmpty(v)) { |
There was a problem hiding this comment.
fixed. I had the same idea and the robot was telling me isEmpty would let things like {email: 1234} through. However, we do validation prior to this so it ended up being a non issue.
- IUserIdentities: drop redundant `| undefined` from each property — the `?` modifier already implies undefined (per @alexs-mparticle review). - search.ts: route knownIdentities through `Validators.removeFalsyIdentityValues` before the guard and the wire spread. Strips undefined/0/false/'' (with a caller-side warning), preserves null (server accepts it as a "no value" sentinel). Resolves the Cursor bot concern about unvalidated values reaching the wire while matching `Identity.identify/login/modify` behavior. - search.ts: simplify the no-usable-identifier guard with `isEmpty` plus a string-value check (per @alexs-mparticle review). - tests-search.ts: add coverage for the falsy-stripping + null-preservation behavior — `{ email: 'valid@example.com', customerid: null, other: '' }` hits the wire as `{ email: 'valid@example.com', customerid: null }`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- IUserIdentities: drop redundant `| undefined` from each property — the `?` modifier already implies undefined (per @alexs-mparticle review). - search.ts: route knownIdentities through `Validators.removeFalsyIdentityValues` before the guard and the wire spread. Strips undefined/0/false/'' (with a caller-side warning), preserves null (server accepts it as a "no value" sentinel). Resolves the Cursor bot concern about unvalidated values reaching the wire while matching `Identity.identify/login/modify` behavior. - search.ts: simplify the no-usable-identifier guard with `isEmpty` plus a string-value check (per @alexs-mparticle review). - identity.js: trim the `Identity.search` JSDoc to a one-liner — the long identifier-list inventory drifts from the type and adds noise without helping readers. - tests-search.ts: add coverage for the falsy-stripping + null-preservation behavior — `{ email: 'valid@example.com', customerid: null, other: '' }` hits the wire as `{ email: 'valid@example.com', customerid: null }`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
24274c0 to
4dcf5a7
Compare
Co-authored-by: Alex S <49695018+alexs-mparticle@users.noreply.github.com>
Co-authored-by: Alex S <49695018+alexs-mparticle@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a6672c9. Configure here.
|
|
🎉 This PR is included in version 2.67.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |




Summary
Expands
Identity.searchso it forwards every identifier the partner has, not justemail. Builds on #1255 (the initial IDSync Search client API), which restrictedknown_identitiesto{ email }.In production, partners sometimes pass hashed email through
other,other2-10, orcustomeridrather than the plainemailfield. The previous shape silently dropped those, so search returned 404 even when a match existed under a non-email key.What changed
src/identity/search.tssendSearchRequestnow acceptsUserIdentities(the SDK's full identity surface) instead of the bespoke{ email: string }type.emailspecifically).{ ...knownIdentities }to the wire instead of{ email: knownIdentities.email }.IIdentitySearchKnownIdentitiesinterface in favor ofUserIdentities.Public types (
src/public-types.ts,src/identity.interfaces.ts)IIdentitySearchKnownIdentitiesfrom the public surface (it was never load-bearing for kits — only one prior consumer, this PR removes it).IUserIdentitiesfor consumption by KitSDKIdentityApi.searchandexecuteSearchRequestupdated to useUserIdentities.Tests (
test/src/tests-search.ts){ email: 12345, other: null }(mixed wrong-typed + null).forwards non-email identifiers (e.g. hashed email in 'other') to the wire— asserts that{ other: 'sha256:abc123', customerid: 'cust-1' }produces a 200 round-trip with the full identifier set in the request body.Breaking changes
IIdentitySearchKnownIdentitiesis removed from the public exports. It was added in #1255 and (per `git grep`) had no external consumers, so the practical blast radius is zero — but worth flagging for changelog purposes.Test plan
🤖 Generated with Claude Code
pair with mparticle-integrations/mparticle-javascript-integration-rokt#93