-
Notifications
You must be signed in to change notification settings - Fork 5
feat(types): complete PurchaselyEventProperties with missing public event fields #242
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 all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -223,6 +223,57 @@ export type PurchaselyEventProperties = { | |
| selected_product?: string; | ||
| plan_change_type?: string; | ||
| running_subscriptions?: PurchaselyEventPropertySubscription[]; | ||
| event_created_at_ms_original?: number; | ||
| event_created_at_original?: string; | ||
| is_fallback_presentation?: boolean; | ||
| presentation_type?: string; | ||
| audience_id?: string; | ||
| ab_test_id?: string; | ||
| ab_test_variant_id?: string; | ||
| content_id?: string; | ||
| campaign_id?: string; | ||
| flow_id?: string; | ||
| step_id?: string; | ||
| flow_version?: string; | ||
| flow_session_id?: string; | ||
| from_action_id?: string; | ||
| from_step_id?: string; | ||
| display_mode?: string; | ||
| display_method?: string; | ||
| method_to_display?: string; | ||
| orientation?: string; | ||
| paywall_request_duration_in_ms?: number; | ||
| paywall_display_time_in_ms?: number; | ||
| paywall_rendering_time_in_ms?: number; | ||
| promo_offer?: string; | ||
| eligible_to_intro_offer?: boolean; | ||
| eligible_to_intro_offer_sk2?: boolean; | ||
| eligible_to_promo_offer?: boolean; | ||
| eligible_to_promo_offer_sk2?: boolean; | ||
| billing_plan_type?: string; | ||
| commitment?: string; | ||
| commitment_progress?: string; | ||
| storekit_version?: string; | ||
| selected_option_id?: string; | ||
| selected_options?: string[]; | ||
| displayed_options?: string[]; | ||
| session_id?: string; | ||
| session_duration?: number; | ||
| session_count?: number; | ||
| app_installed_at?: string; | ||
| app_installed_at_ms?: number; | ||
| screen_duration?: number; | ||
| screen_displayed_at?: string; | ||
| screen_displayed_at_ms?: number; | ||
| is_sdk_started?: boolean; | ||
| sdk_start_error?: string; | ||
| sdk_start_duration_in_ms?: number; | ||
| error_code?: string; | ||
| web_checkout_provider?: string; | ||
| web_checkout_url?: string; | ||
| client_reference_id?: string; | ||
| stripe_checkout_session_id?: string; | ||
| stripe_purchase_id?: string; | ||
|
Comment on lines
+226
to
+276
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.
The Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/purchasely/src/types.ts
Line: 226-276
Comment:
**No test coverage for the 51 new fields**
The `AGENTS.md` guideline states "Add tests under `packages/purchasely/src/__tests__/` when adding public APIs or types." The existing `PurchaselyEvent` test at line 326 only exercises a handful of pre-existing properties; none of the fields added here are exercised. A consumer who assigns, say, `flow_id`, `stripe_purchase_id`, or `eligible_to_intro_offer` would get no compile-time evidence that the type accepts them correctly. Adding a test block that constructs a `PurchaselyEventProperties` object with at least a representative selection of the new fields would cover this.
How can I resolve this? If you propose a fix, please make it concise. |
||
| }; | ||
|
|
||
| export type PLYPresentationPlan = { | ||
|
|
||
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.
web_checkout_providerfield is typed asstring, butPLYWebCheckoutProvideralready exists in the codebase with exact string enum values ('stripe'|'other') that match what the native SDK emits. Using the enum here would give consumers compile-time type safety and IDE autocomplete — the same pattern already used forevent_name: PurchaselyEventsNames. If the native SDK can emit values outside of those two strings,stringis fine, but that would be worth a comment.Prompt To Fix With AI