Improve Segment events #1704
Improve Segment events #1704
Conversation
🦋 Changeset detectedLatest commit: 6d7aad1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
Differences Found✅ No packages or licenses were added. SummaryExpand
|
vercel
bot
temporarily deployed
to
Preview – saleor-app-products-feed
|
|
||
| - Changed what we sent to Segment to be in sync with their [spec](https://segment.com/docs/connections/spec/ecommerce/v2/) | ||
| - Added new Saleor event - `OrderConfirmed` that will be mapped to Segment `OrderCompleted` | ||
| - Removed Saleor event - `OrderCreated` - it didn't have respective Segment event |
There was a problem hiding this comment.
hmm, I was wondering what is better approach - they have spec for ecommerce but segment is a generic tool and it can consume any events, including custom one
So maybe we should actually keep these? It doesn't have to have representation in Segment but users will still be able to build on top of them. Especially if we treat user actions - order is placed by user on "Created" event, while staff users or other systems can lead to next status which is confirmation.
But, it may also make sense to remove some of them if these are internal Saleor events that are not important from Saleor perspective.
But one thing I think makes sense - to keep timestamps related to user actions (if we use OrderConfirmed, we use timestamp from order.createdAt, not the event)
There was a problem hiding this comment.
I was thinking about something like having right now events that are up to Segment ecommerce spec. After we gather feedback from users of the app we could add additional events like order fully paid or order created.
| @@ -0,0 +1,12 @@ | |||
| fragment OrderConfirmedSubscriptionPayload on OrderConfirmed { | |||
| issuedAt | |||
There was a problem hiding this comment.
as written before I think we should map order creation date
There was a problem hiding this comment.
I'm wondering what will user expect in Segment (as this will be mapped to timestamp property indicating when event happened in time) and I think it will be the time of when order confirmed or other event has happened instead of when order was created 🤔
There was a problem hiding this comment.
As we discussed internally we will use issuedAt for the moment as it shouldn't be difference between it and order update date (for this case).
Scope of the PR
OrderConfirmedthat will be mapped to SegmentOrderCompletedOrderCreated- it didn't have respective Segment eventRelated issues
Checklist