Skip to content

Don't overwrite custom user defined event id - #884

Closed
Seltsamsel wants to merge 1 commit into
snowplow:masterfrom
Seltsamsel:master
Closed

Don't overwrite custom user defined event id#884
Seltsamsel wants to merge 1 commit into
snowplow:masterfrom
Seltsamsel:master

Conversation

@Seltsamsel

Copy link
Copy Markdown

In some cases, we already set an event id provided via payloadPairs and it's important for us to keep that event id. However, currently in the core tracker the event id is overwritten no matter what. One very simple solution to this issue is to switch the order of adding the event id and the payloadPairs. In the normal way of using the tracker without providing an own event id, nothing changes.

@snowplowcla Snowplow CLA bot (snowplowcla) added the cla:yes [Auto generated] Snowplow Contributor License Agreement has been signed. label Jan 8, 2021
@paulboocock

Copy link
Copy Markdown
Contributor

Thanks for the PR @mcepok

We're well on the way to v3 now which sees a significant reshuffle of the codebase. I'm happy to include this change in v3.

If you want to build your own version of sp.js whilst you wait for v3, then you should be able to run: npm run build:all and it'll build core and the tracker then output a new sp.js to /dist.

@Seltsamsel

Copy link
Copy Markdown
Author

We actually want this behavior for the snowplow-nodejs-tracker which, as far as I understood, is based since version 0.4.0 on the core-tracker of this repository. If the next release of the core-tracker is coupled with the javascript-tracker release, that's also fine.

Thanks a lot!

@paulboocock Paul Boocock (paulboocock) mentioned this pull request Mar 5, 2021
20 tasks
@paulboocock

Copy link
Copy Markdown
Contributor

I've cherry-picked this into the release/3.0.0 branch and opened corresponding issue #911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla:yes [Auto generated] Snowplow Contributor License Agreement has been signed.

Development

Successfully merging this pull request may close these issues.

3 participants