Skip to content

small updates to the default mappings in the Reddit CAPI integration #2893

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akhsueh
Copy link
Contributor

@akhsueh akhsueh commented Apr 29, 2025

Hi, this is updating some of the default mappings for the Reddit Conversions API destination.

  • Updated the "conversion_id" to be mapped to the Segment "messageID". This Conversion ID is used for deduplication so that if we receive the same ID in both the Browser Destination as well as this destination, then our system will dedupe those events.
  • Minor updates to the transaction value mappings. Updated the default to be "revenue" and fall back to "total". Also updated it to be "price" for the Segment event "Product Added" and "Product Added to Wishlist" which does not have "revenue" or "total" available as a default.

Testing

  • [ X ] Updated the unit tests to account for hashing of the messageID for the conversion_id field. Also added that it will default to revenue first if revenue and total are both present. The test was failing when I tried to use "price" in the test unit for the "Lead" event for example, but the request looks OK in the testing environment itself. The Unit Testing seems to be only looking at the default mappings.
  • [ X ] Tested end-to-end using the [local server] and the request and JSON response looks correct.
  • [ X ] [If destination is already live] Tested for backward compatibility of destination. Note: New required fields are a breaking change.
  • [Segmenters] Tested in the staging environment
  • [Segmenters] [If applicable for this change] Tested for regression with Hadron.

Copy link
Contributor

@joe-ayoub-segment joe-ayoub-segment left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @akhsueh thanks for this PR and apologies for the delay with the review.

Looks good to me!

Just a heads up that this change will only take effect for new instances of the Destination. If a customer is already using the Integration it won't change anything.

@akhsueh
Copy link
Contributor Author

akhsueh commented Apr 30, 2025

Hi @joe-ayoub-segment , welcome back! And thanks for the review! Noted about it only being updated for new instances, thanks for the heads up there as well.

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

Successfully merging this pull request may close these issues.

2 participants