-
Notifications
You must be signed in to change notification settings - Fork 279
STRATCONN-6051 - [Clay] - New Destination #3092
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
Conversation
Thanks for the PR @ayush-goyal . |
Thanks for the review @joe-ayoub-segment . Clay wants to integrate with Segment to allow our customers to ingest page visit data from their site. From this, we want to allow them to track visitor data across different pages and identify high-intent prospects for sales. Could you please further explain what other analytics data might be helpful for our use case? I thought that Page events was the only one necessary as people navigate a site and these events are emitted. I've updated the code to collect the page |
hi @ayush-goyal , page() events just track some basic details of each page the customer visits. It doesn't capture the interractions on pages though. track() events capture interactions on web pages, mobile apps and other events from back end services. Usually, Partners will collect track() and identify() at a minimum, and optionally page() and screen(). Happy to discuss in person if you'd like. Here's a Calendly link: https://calendly.com/joe_ayoub/30-minute-workshop Best regards, |
Thanks for the helpful information on the call today @joe-ayoub-segment , I've made some changes and also included the |
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.
Looks good to me. Couple of minor comments. I also need to run CI to see if all the tests pass.
packages/destination-actions/src/destinations/clay/pageVisit/index.ts
Outdated
Show resolved
Hide resolved
packages/destination-actions/src/destinations/clay/track/index.ts
Outdated
Show resolved
Hide resolved
Thanks @joe-ayoub-segment , I updated your comments and removed these types instead |
Hi @ayush-goyal the code is good. However the snapshot test is failing.
Then commit the change. |
Thanks @joe-ayoub-segment , I just fixed the failing snapshot test |
Thanks @joe-ayoub-segment are you able to merge this PR in? I'm not sure how to fix the failing linter checks |
hi @ayush-goyal the PR is good to go. You can ignore the 2 failing checks. I'll merge if just before I deploy it tomorrow. I'll be in touch once it is live! |
@joe-ayoub-segment great, sounds good thanks! |
* [Clay] Create page action * [Clay] Collect page properties * Add in track event * Remove type from fields and hardcode * Fix snapshot test
Created a new partner action for clay. This includes a page event type that sends data to Clay.
Testing
We've tested this locally and with unit tests.