RudderStack Cloud Mode destination in Survicate#4907
RudderStack Cloud Mode destination in Survicate#4907anna-belka wants to merge 12 commits intorudderlabs:developfrom
Conversation
|
Thank you @anna-belka for contributing this PR. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Configuration src/v0/destinations/survicate/config.ts |
New module exporting BASE_URL, ENDPOINT_CONFIG (identify/group/track endpoints + method/content-type), DEST_TYPE, and HEADERS_CONFIG. |
Types & Validation src/v0/destinations/survicate/types.ts |
Adds Zod schemas and TypeScript types: SurvicateDestinationConfigSchema, SurvicateMessageSchema, runtime types (SurvicateDestinationConfig, SurvicateMessage, SurvicateDestination, SurvicateRouterRequest), and payload interfaces IdentifyPayload, GroupPayload, TrackPayload. |
Transformation & Routing src/v0/destinations/survicate/transform.ts |
New router and per-event handlers (identify, group, track), process and processRouterDest exports; input validation, payload construction, header assembly, and routing to Survicate endpoints. |
Sequence Diagram(s)
sequenceDiagram
participant Event as RudderStack Event
participant Router as Survicate Router
participant Handler as Event Handler
participant API as Survicate API
Event->>Router: process(event)
Router->>Router: validate destinationKey & message type
alt identify
Router->>Handler: route to identify handler
Handler->>Handler: validate user_id, flatten traits, build payload
Handler->>API: POST /endpoint/rudder-stack/identify (headers + body)
API-->>Handler: response
else group
Router->>Handler: route to group handler
Handler->>Handler: validate user_id & group_id, build payload
Handler->>API: POST /endpoint/rudder-stack/group
API-->>Handler: response
else track
Router->>Handler: route to track handler
Handler->>Handler: validate user_id & event, build payload
Handler->>API: POST /endpoint/rudder-stack/track
API-->>Handler: response
end
Router-->>Event: return formatted request(s)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly summarizes the main change: introducing a Survicate Cloud Mode destination integration for RudderStack, which aligns with the changeset. |
| Description check | ✅ Passed | The PR description follows the template structure with all required sections completed, including objectives, explanations of changes, dependency status, and developer/reviewer checklists. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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 @coderabbitai help to get the list of available commands and usage tips.
|
This PR is considered to be stale. It has been open for 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
@devops-github-rudderstack we still working on it. Please do not close this PR. |
|
This PR is considered to be stale. It has been open for 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/v0/destinations/survicate/transform.ts`:
- Around line 169-176: processTrackEvent is only forwarding message.properties,
message_id and timestamp, so track payloads miss the filtered context and
flattened context.traits that identify/group include; update processTrackEvent
to attach the same filtered context block (e.g., context: filteredContext) and
merge filteredContext.traits into the payload.properties (preserving existing
properties) so locale, campaign, userAgent and user traits are sent with
message, referencing the same filtering/flattening helpers used by
identify/group to keep behavior consistent.
- Around line 48-50: The merge of message.context.traits into payload (in
transform.ts) can overwrite canonical fields like user_id, group_id, timestamp,
and message_id; before calling Object.assign(payload, message.context.traits)
(and the analogous merge at the other occurrence around lines 113-115) filter
out reserved keys ["user_id","group_id","timestamp","message_id"] from
message.context.traits or re-apply
payload.user_id/payload.group_id/payload.timestamp/payload.message_id after the
merge so trait values cannot replace canonical IDs; update the merge in the
transform function(s) to use the filtered traits object or reassign reserved
fields afterward.
In `@src/v0/destinations/survicate/types.ts`:
- Around line 25-31: The SurvicateMessage Zod schema currently uses snake_case
field names (user_id, group_id, message_id) which mismatch RudderStack's
camelCase fields; update the schema inside the SurvicateMessage definition to
use camelCase names (userId, groupId, messageId) and any other camelCase fields
(e.g., anonymousId, originalTimestamp) to match incoming messages, or
alternatively add a normalization step at the message entry point (before
validation) that maps snake_case keys to camelCase; adjust any code referencing
the old snake_case keys to use the new camelCase symbols so validation and
downstream transforms (the SurvicateMessage schema consumer) work correctly.
- Around line 32-38: Update the Zod records to the Zod 4 two-argument form:
replace occurrences of z.record(z.any()) for the fields properties, traits,
context.traits, and context.campaign with z.record(z.string(), z.unknown()) so
the schemas in the Survicate type definitions (the properties and traits
top-level fields and the context object’s traits and campaign fields) use a key
schema and a value schema compatible with Zod v4.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c273f488-edcc-4930-afae-010ef25954c0
⛔ Files ignored due to path filters (7)
src/v0/destinations/survicate/README.mdis excluded by!**/*.mdtest/integrations/component.test.tsis excluded by!**/test/**test/integrations/destinations/survicate/processor/data.tsis excluded by!**/test/**test/integrations/destinations/survicate/processor/group.tsis excluded by!**/test/**test/integrations/destinations/survicate/processor/identify.tsis excluded by!**/test/**test/integrations/destinations/survicate/processor/track.tsis excluded by!**/test/**test/integrations/destinations/survicate/router/data.tsis excluded by!**/test/**
📒 Files selected for processing (3)
src/v0/destinations/survicate/config.tssrc/v0/destinations/survicate/transform.tssrc/v0/destinations/survicate/types.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/v0/destinations/survicate/types.ts (1)
25-30:⚠️ Potential issue | 🟠 MajorSwitch this message schema to the standard camelCase RudderStack fields.
user_id,group_id, andmessage_iddo not match the normal incoming message contract (userId,groupId,messageId). With the current schema plus the matching snake_case reads insrc/v0/destinations/survicate/transform.ts, valid identified/group events will be treated as anonymous or losemessageIdunless some unseen normalization runs first. Normalize before validation, or update this schema and the downstream accessors together.Patch sketch for this file
export const SurvicateMessageSchema = z .object({ type: z.enum(['identify', 'group', 'track']), - user_id: z.string().optional(), + userId: z.string().optional(), anonymousId: z.string().optional(), - group_id: z.string().optional(), + groupId: z.string().optional(), event: z.string().optional(), - message_id: z.string().optional(), + messageId: z.string().optional(), originalTimestamp: z.string().optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/types.ts` around lines 25 - 30, Change the Survicate message Zod schema to use standard RudderStack camelCase fields (userId, groupId, messageId) instead of snake_case (user_id, group_id, message_id) so validation matches incoming messages; update any downstream accessors in src/v0/destinations/survicate/transform.ts that read message_id/user_id/group_id to read messageId/userId/groupId (or alternatively add a normalization step that converts snake_case to camelCase before calling the existing schema/transform functions), ensuring the symbols type (the Zod schema object in types.ts) and the transform functions refer to the same field names.
🧹 Nitpick comments (1)
src/v0/destinations/survicate/types.ts (1)
58-67: Align the exported identify/group payload types with the payloads actually built.
processIdentifyEventandprocessGroupEventaddtimestamp,message_id, and optionalcontext, butIdentifyPayload/GroupPayloaddo not model those fields and the implementation currently falls back toRecord<string, any>. Either make these interfaces match the real request shape and use them intransform.ts, or drop them to avoid a stale public type surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/types.ts` around lines 58 - 67, The exported IdentifyPayload and GroupPayload interfaces are out-of-date: update IdentifyPayload and GroupPayload to include the fields added by processIdentifyEvent/processGroupEvent (timestamp: string | number, message_id: string, and optional context: Record<string, any>) so the types model the actual request shape used in transform.ts, and then use these interfaces from transform.ts instead of falling back to Record<string, any>; alternatively, if you prefer not to expose them, remove these stale exports so there is no mismatch between the public type surface and the implementations in processIdentifyEvent/processGroupEvent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/v0/destinations/survicate/types.ts`:
- Around line 30-31: Update the Zod schema that defines message_id and
originalTimestamp so they are required at validation time: change the
definitions for message_id and originalTimestamp from z.string().optional() to
z.string() in the Survicate event schema (the object where message_id and
originalTimestamp are currently defined) so events missing these stamped values
are rejected during validation rather than later.
---
Duplicate comments:
In `@src/v0/destinations/survicate/types.ts`:
- Around line 25-30: Change the Survicate message Zod schema to use standard
RudderStack camelCase fields (userId, groupId, messageId) instead of snake_case
(user_id, group_id, message_id) so validation matches incoming messages; update
any downstream accessors in src/v0/destinations/survicate/transform.ts that read
message_id/user_id/group_id to read messageId/userId/groupId (or alternatively
add a normalization step that converts snake_case to camelCase before calling
the existing schema/transform functions), ensuring the symbols type (the Zod
schema object in types.ts) and the transform functions refer to the same field
names.
---
Nitpick comments:
In `@src/v0/destinations/survicate/types.ts`:
- Around line 58-67: The exported IdentifyPayload and GroupPayload interfaces
are out-of-date: update IdentifyPayload and GroupPayload to include the fields
added by processIdentifyEvent/processGroupEvent (timestamp: string | number,
message_id: string, and optional context: Record<string, any>) so the types
model the actual request shape used in transform.ts, and then use these
interfaces from transform.ts instead of falling back to Record<string, any>;
alternatively, if you prefer not to expose them, remove these stale exports so
there is no mismatch between the public type surface and the implementations in
processIdentifyEvent/processGroupEvent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c3a1d26-f9ce-4207-ba4b-436d1d15553c
📒 Files selected for processing (1)
src/v0/destinations/survicate/types.ts
…udder-transformer into survicate-cloud-mode
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/v0/destinations/survicate/types.ts (1)
30-31:⚠️ Potential issue | 🟠 MajorRequire
message_idandoriginalTimestampat the schema boundary.All three handlers stamp these into the outbound payload, but the schema still allows them to be absent. That means invalid events can pass validation and reach Survicate with
undefinedaudit fields instead of failing fast here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/types.ts` around lines 30 - 31, The schema currently marks message_id and originalTimestamp as optional, allowing invalid events through; update the Zod schema in src/v0/destinations/survicate/types.ts so that message_id and originalTimestamp are required (change their definitions from z.string().optional() to z.string()), ensuring the outbound payload validation (the schema used by the Survicate handlers) fails fast when those audit fields are missing.src/v0/destinations/survicate/transform.ts (1)
35-45:⚠️ Potential issue | 🔴 CriticalUse a single message-field convention across all handlers.
These branches currently mix
user_id/group_id/message_idwithuserId/groupId/messageId. As written, a normal RudderStack message can either fail the guard checks or produce an outbound payload with missing IDs. Normalize once at the boundary and read the same keys everywhere.Suggested fix
- if (!message.user_id) { + if (!message.userId) { throw new InstrumentationError( 'Anonymous identify calls are not supported. userId is required.', ); } const payload: Record<string, any> = { user_id: message.userId, timestamp: message.originalTimestamp, - message_id: message.message_id, + message_id: message.messageId, };const payload: Record<string, any> = { - user_id: message.user_id, - group_id: message.group_id, + user_id: message.userId, + group_id: message.groupId, traits: message.traits || {}, timestamp: message.originalTimestamp, - message_id: message.message_id, + message_id: message.messageId, };Update
src/v0/destinations/survicate/types.tsto the same convention in the same change so validation and runtime access stay aligned.Also applies to: 102-119, 174-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/transform.ts` around lines 35 - 45, Normalize incoming Rudder message ID fields to a single convention at the transformer boundary: map message.user_id → message.userId, message.group_id → message.groupId, message.message_id → message.messageId (or vice‑versa if you prefer snake_case) immediately when the transform starts, then use only that canonical key (message.userId/message.groupId/message.messageId) throughout the handler (guards, payload building, and any branches that currently read mixed keys); also update the destination type definitions used for validation to the same convention so runtime access and typing align.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/v0/destinations/survicate/transform.ts`:
- Around line 35-45: Normalize incoming Rudder message ID fields to a single
convention at the transformer boundary: map message.user_id → message.userId,
message.group_id → message.groupId, message.message_id → message.messageId (or
vice‑versa if you prefer snake_case) immediately when the transform starts, then
use only that canonical key (message.userId/message.groupId/message.messageId)
throughout the handler (guards, payload building, and any branches that
currently read mixed keys); also update the destination type definitions used
for validation to the same convention so runtime access and typing align.
In `@src/v0/destinations/survicate/types.ts`:
- Around line 30-31: The schema currently marks message_id and originalTimestamp
as optional, allowing invalid events through; update the Zod schema in
src/v0/destinations/survicate/types.ts so that message_id and originalTimestamp
are required (change their definitions from z.string().optional() to
z.string()), ensuring the outbound payload validation (the schema used by the
Survicate handlers) fails fast when those audit fields are missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e2bc78b3-26cb-480e-8934-088b0d560c53
⛔ Files ignored due to path filters (1)
test/integrations/destinations/survicate/router/data.tsis excluded by!**/test/**
📒 Files selected for processing (2)
src/v0/destinations/survicate/transform.tssrc/v0/destinations/survicate/types.ts
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/v0/destinations/survicate/transform.ts (3)
105-114: Consider extracting shared context enrichment logic.The context property extraction (
locale,campaign,userAgent) is duplicated across all three event processors (lines 105-114, 189-198, 278-285). Similarly, the reserved-key trait filtering appears three times.Extracting these into helper functions would reduce duplication and ensure consistency if the logic needs to change:
Example helper functions
function extractContextData(context: SurvicateMessage['context']): Record<string, any> | undefined { if (!context) return undefined; const data: Record<string, any> = {}; if (context.locale) data.locale = context.locale; if (context.campaign) data.campaign = context.campaign; if (context.userAgent) data.userAgent = context.userAgent; return Object.keys(data).length > 0 ? data : undefined; } function filterReservedTraits( traits: Record<string, unknown> | undefined, reserved: string[], ): Record<string, any> { if (!traits) return {}; const filtered: Record<string, any> = {}; for (const [k, v] of Object.entries(traits)) { if (!reserved.includes(k)) filtered[k] = v; } return filtered; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/transform.ts` around lines 105 - 114, Extract the duplicated context enrichment and reserved-trait filtering into helpers (e.g. create extractContextData(context: SurvicateMessage['context']): Record<string, any> | undefined and filterReservedTraits(traits, reservedKeys): Record<string, any>), then replace the repeated blocks that build payload.context from msg.context and the three trait-filtering loops with calls to these helpers (references: payload.context assignment, msg.context, the trait filtering logic around traits/reservedKeys in each event processor). Ensure helpers return undefined/empty when no data so existing checks (Object.keys(...).length) can be simplified or removed.
76-82: Manual field checks are redundant after schema validation.Lines 77-82 check for
messageIdandoriginalTimestamp, but these fields are already required inSurvicateMessageSchema(lines 34-35 in types.ts). TheSurvicateMessageSchema.parse(msg)call at line 67 would throw aZodErrorbefore reaching these checks if the fields were missing.If the intent is to provide user-friendly
InstrumentationErrormessages rather than Zod's default errors, consider catching theZodErrorand re-throwing with a custom message. Otherwise, these checks can be removed to reduce redundancy.The same observation applies to identical checks in
processGroupEvent(lines 160-166) andprocessTrackEvent(lines 244-250).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/transform.ts` around lines 76 - 82, The explicit checks for messageId and originalTimestamp after calling SurvicateMessageSchema.parse(msg) are redundant because SurvicateMessageSchema.parse(msg) will already throw a ZodError if those required fields are missing; remove the manual checks that throw new InstrumentationError(ERR_MESSAGE_ID_REQUIRED) and new InstrumentationError(ERR_ORIG_TS_REQUIRED) from the transform function (the block after SurvicateMessageSchema.parse) and the equivalent blocks in processGroupEvent and processTrackEvent, or alternatively wrap SurvicateMessageSchema.parse(msg) in a try/catch and convert the ZodError into user-friendly InstrumentationError messages if you prefer custom errors.
91-103: Consider filtering camelCase variants of reserved keys.The reserved key list only includes snake_case identifiers (
user_id,group_id, etc.). Ifcontext.traitscontains camelCase equivalents likeuserIdormessageId, they won't be filtered and could appear in the outgoing payload alongside the snake_case canonical fields.While this won't overwrite the primary identifiers (since Object.assign happens after they're set), it may send redundant data to Survicate.
Suggested improvement
- const reserved = ['user_id', 'group_id', 'timestamp', 'message_id']; + const reserved = [ + 'user_id', 'group_id', 'timestamp', 'message_id', + 'userId', 'groupId', 'originalTimestamp', 'messageId', + ];This pattern applies to the same filtering logic in
processGroupEvent(line 179) andprocessTrackEvent(line 263) as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/transform.ts` around lines 91 - 103, The current filtering only checks snake_case reserved keys (the reserved array and loop over msg.context.traits before Object.assign(payload, filtered)), so camelCase variants like userId/messageId slip through; update the filter logic used in this transform block and the same patterns in processGroupEvent and processTrackEvent to detect and exclude camelCase equivalents (e.g., userId, groupId, messageId, timestamp) by normalizing trait keys (for example map keys to a canonical form such as lowercased and underscores removed or explicitly check both snake_case and camelCase variants) before adding them to filtered, ensuring Object.assign(payload, filtered) never includes those reserved variants.src/v0/destinations/survicate/types.ts (1)
56-59: Consider explicit type parameters for router request type.The
SurvicateRouterRequestrelies on default values for theConnectionandMetadatatype parameters. For consistency with other destinations like PostHog (which usesRouterTransformationRequestData<PostHogMessage, PostHogDestination, undefined, Metadata>), you may want to be explicit:export type SurvicateRouterRequest = RouterTransformationRequestData< SurvicateMessage, - SurvicateDestination + SurvicateDestination, + undefined, + Metadata >;This would also require importing
Metadatafrom../../../types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/types.ts` around lines 56 - 59, The SurvicateRouterRequest currently uses default generics; update its declaration to explicitly pass the Connection/Metadata type parameters by changing the type alias for SurvicateRouterRequest to use RouterTransformationRequestData<SurvicateMessage, SurvicateDestination, undefined, Metadata> and add an import for Metadata from ../../../types so the Metadata symbol is available for the type argument.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/v0/destinations/survicate/transform.ts`:
- Around line 105-114: Extract the duplicated context enrichment and
reserved-trait filtering into helpers (e.g. create extractContextData(context:
SurvicateMessage['context']): Record<string, any> | undefined and
filterReservedTraits(traits, reservedKeys): Record<string, any>), then replace
the repeated blocks that build payload.context from msg.context and the three
trait-filtering loops with calls to these helpers (references: payload.context
assignment, msg.context, the trait filtering logic around traits/reservedKeys in
each event processor). Ensure helpers return undefined/empty when no data so
existing checks (Object.keys(...).length) can be simplified or removed.
- Around line 76-82: The explicit checks for messageId and originalTimestamp
after calling SurvicateMessageSchema.parse(msg) are redundant because
SurvicateMessageSchema.parse(msg) will already throw a ZodError if those
required fields are missing; remove the manual checks that throw new
InstrumentationError(ERR_MESSAGE_ID_REQUIRED) and new
InstrumentationError(ERR_ORIG_TS_REQUIRED) from the transform function (the
block after SurvicateMessageSchema.parse) and the equivalent blocks in
processGroupEvent and processTrackEvent, or alternatively wrap
SurvicateMessageSchema.parse(msg) in a try/catch and convert the ZodError into
user-friendly InstrumentationError messages if you prefer custom errors.
- Around line 91-103: The current filtering only checks snake_case reserved keys
(the reserved array and loop over msg.context.traits before
Object.assign(payload, filtered)), so camelCase variants like userId/messageId
slip through; update the filter logic used in this transform block and the same
patterns in processGroupEvent and processTrackEvent to detect and exclude
camelCase equivalents (e.g., userId, groupId, messageId, timestamp) by
normalizing trait keys (for example map keys to a canonical form such as
lowercased and underscores removed or explicitly check both snake_case and
camelCase variants) before adding them to filtered, ensuring
Object.assign(payload, filtered) never includes those reserved variants.
In `@src/v0/destinations/survicate/types.ts`:
- Around line 56-59: The SurvicateRouterRequest currently uses default generics;
update its declaration to explicitly pass the Connection/Metadata type
parameters by changing the type alias for SurvicateRouterRequest to use
RouterTransformationRequestData<SurvicateMessage, SurvicateDestination,
undefined, Metadata> and add an import for Metadata from ../../../types so the
Metadata symbol is available for the type argument.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 29558b72-f05d-4e0c-9572-24df3d015448
⛔ Files ignored due to path filters (5)
src/v0/destinations/survicate/README.mdis excluded by!**/*.mdtest/integrations/destinations/survicate/processor/group.tsis excluded by!**/test/**test/integrations/destinations/survicate/processor/identify.tsis excluded by!**/test/**test/integrations/destinations/survicate/processor/track.tsis excluded by!**/test/**test/integrations/destinations/survicate/router/data.tsis excluded by!**/test/**
📒 Files selected for processing (2)
src/v0/destinations/survicate/transform.tssrc/v0/destinations/survicate/types.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/v0/destinations/survicate/types.ts (2)
26-48: Previous feedback addressed; consider adding user identifier validation.The schema correctly addresses the prior review comments:
- Field names are now camelCase
- Uses two-argument
z.record()formmessageIdandoriginalTimestampare requiredOne potential improvement: the schema allows both
userIdandanonymousIdto be absent. If the Survicate API requires at least one user identifier, consider adding a refinement:💡 Optional refinement to enforce user identifier
export const SurvicateMessageSchema = z .object({ type: z.enum(['identify', 'group', 'track']), userId: z.string().optional(), anonymousId: z.string().optional(), // ... rest of schema }) - .passthrough(); + .passthrough() + .refine( + (msg) => msg.userId || msg.anonymousId, + { message: 'Either userId or anonymousId must be provided' } + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/types.ts` around lines 26 - 48, SurvicateMessageSchema currently allows both userId and anonymousId to be missing; update SurvicateMessageSchema to enforce that at least one identifier is present by adding a refinement (e.g., .refine or .superRefine) on SurvicateMessageSchema that checks either data.userId or data.anonymousId is truthy and returns a clear validation message if both are absent; ensure the refinement references the existing schema object so other passthrough behavior remains unchanged.
63-81: Consider usingunknowninstead ofanyfor stricter type safety.The snake_case field names are appropriate here since these interfaces represent outbound payloads to the Survicate API. However, the
anytype in index signatures andRecordtypes bypasses TypeScript's type checking.💡 Suggested improvement for type safety
export interface IdentifyPayload { user_id?: string; - [key: string]: any; + [key: string]: unknown; } export interface GroupPayload { user_id?: string; group_id?: string; - [key: string]: any; + [key: string]: unknown; } export interface TrackPayload { user_id?: string; event?: string; - properties?: Record<string, any>; + properties?: Record<string, unknown>; timestamp?: string; message_id?: string; }This change would require explicit type assertions or narrowing in the transformation code when accessing dynamic properties, but it provides better compile-time safety against unintended type coercions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v0/destinations/survicate/types.ts` around lines 63 - 81, Replace the permissive any types in the outbound Survicate payload interfaces (IdentifyPayload, GroupPayload, TrackPayload) with unknown for index signatures and property records to tighten type safety; update IdentifyPayload and GroupPayload index signatures from [key: string]: any to [key: string]: unknown and change TrackPayload properties?: Record<string, any> to properties?: Record<string, unknown>, then adjust the transformation code where these payloads are read or written to perform explicit type narrowing/casts or runtime checks so the compiler is satisfied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/v0/destinations/survicate/types.ts`:
- Around line 26-48: SurvicateMessageSchema currently allows both userId and
anonymousId to be missing; update SurvicateMessageSchema to enforce that at
least one identifier is present by adding a refinement (e.g., .refine or
.superRefine) on SurvicateMessageSchema that checks either data.userId or
data.anonymousId is truthy and returns a clear validation message if both are
absent; ensure the refinement references the existing schema object so other
passthrough behavior remains unchanged.
- Around line 63-81: Replace the permissive any types in the outbound Survicate
payload interfaces (IdentifyPayload, GroupPayload, TrackPayload) with unknown
for index signatures and property records to tighten type safety; update
IdentifyPayload and GroupPayload index signatures from [key: string]: any to
[key: string]: unknown and change TrackPayload properties?: Record<string, any>
to properties?: Record<string, unknown>, then adjust the transformation code
where these payloads are read or written to perform explicit type
narrowing/casts or runtime checks so the compiler is satisfied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58738f56-adeb-4efc-bc3d-043e4ca61a43
⛔ Files ignored due to path filters (1)
test/integrations/destinations/survicate/router/data.tsis excluded by!**/test/**
📒 Files selected for processing (2)
src/v0/destinations/survicate/transform.tssrc/v0/destinations/survicate/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/v0/destinations/survicate/transform.ts
2 participants
What are the changes introduced in this PR?
This PR introduces the Survicate Cloud Mode Destination integration for RudderStack. The implementation transforms RudderStack events (identify, group, and track) into Survicate API format for integration with the Survicate Contacts service.
What is the related Linear task?
Resolves INT-XXX
Please explain the objectives of your changes below
The objective is to enable RudderStack users to seamlessly send user identification, group association, and event tracking data to Survicate's Contacts service. The integration:
Processes three event types:
identify: Sends user identification data with traits and selected context
group: Associates users with groups including group-specific traits
track: Tracks user events with event names and properties
Ensures data consistency by always including: userId, timestamp, messageId and filtered context (only locale, campaign, userAgent)
Validates required fields to ensure data integrity at the Contacts service
Flattens nested structures by extracting context traits and merging them into the payload
Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
N
Any new dependencies introduced with this change?
N
Any new generic utility introduced or modified. Please explain the changes.
N
Any technical or performance related pointers to consider with the change?
N
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
Is the PR limited to 10 file changes?
Is the PR limited to one linear task?
Are relevant unit and component test-cases added in new readability format?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.