-
Notifications
You must be signed in to change notification settings - Fork 6
Feat/sdk setup for migrations #68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 15 commits
a235620
b504733
36100dd
cfdd3a6
571cbc2
3853764
f5df99d
0f6ca95
8f07d35
eed2545
edb2d37
ed492c7
b7e4db0
36bae96
5aaf3e3
43aaf58
951c779
b563962
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
SikoraKam marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| import { setAdvertisingId, setTrackingAuthorizationStatus } from "../links/utils/deviceIdentifiers"; | ||
| import { DetourEventNames } from "./types"; | ||
| import type { Conversion } from "./types"; | ||
| import { analyticsEmitter } from "./utils/analyticsEmitter"; | ||
| import { setConsent } from "./utils/consent"; | ||
| import { setUserId } from "./utils/userIdentity"; | ||
|
|
||
| export const logEvent = (eventName: DetourEventNames | `${DetourEventNames}`, data?: any) => { | ||
| analyticsEmitter.emit({ eventName, data }); | ||
|
|
@@ -9,7 +13,27 @@ export const logRetention = (retentionEventName: string) => { | |
| analyticsEmitter.emit({ eventName: retentionEventName, isRetention: true }); | ||
| }; | ||
|
|
||
| export type ConversionParams = Conversion & { | ||
| eventName?: DetourEventNames | `${DetourEventNames}`; | ||
| }; | ||
|
|
||
| // Revenue reporting inherently needs the host to call in — the SDK has no | ||
| // signal for transaction amount. Defaults to Purchase since that's the | ||
| // overwhelming majority case; still rides the existing event endpoint (see | ||
| // events.ts), just with revenue/currency guaranteed as top-level fields. | ||
| export const logConversion = ({ | ||
| eventName = DetourEventNames.Purchase, | ||
| ...conversion | ||
| }: ConversionParams) => { | ||
| analyticsEmitter.emit({ eventName, conversion }); | ||
| }; | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't assume such things when you create tool for community and you don't know the use case
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. We should consider which option is better. but revenue fields stay as nullable columns on the generic events table — gets wider/sparser over time as more signal types get added. Option 2: but needs new backend work (endpoint + table), blocks this PR; have to duplicate the enrichment logic for the new endpoint; breaks the single event timeline, so funnels need to join across tables |
||
| export const DetourAnalytics = { | ||
| logEvent, | ||
| logRetention, | ||
| logConversion, | ||
| setUserId, | ||
| setConsent, | ||
| setAdvertisingId, | ||
| setTrackingAuthorizationStatus, | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was needed to keep test app working, but after cleanup of testing app i will also get rid of this dependency