Skip to content

Conversation

FelipeRiveraC
Copy link
Contributor

@FelipeRiveraC FelipeRiveraC commented Sep 30, 2025

  • Introduced a new optional setting skipZeroPricePurchases to filter out purchase events with items that have zero or missing unit price.
  • Updated the Settings interface and corresponding destination definition.
  • Implemented filtering logic in the purchase action.
  • Added unit tests to verify the new functionality, including scenarios for enabling and disabling the setting.

A summary of your pull request, including the what change you're making and why.

Testing

Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [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.

- Introduced a new optional setting `skipZeroPricePurchases` to filter out purchase events with items that have zero or missing unit price.
- Updated the `Settings` interface and corresponding destination definition.
- Implemented filtering logic in the purchase action.
- Added unit tests to verify the new functionality, including scenarios for enabling and disabling the setting.
@FelipeRiveraC FelipeRiveraC requested a review from a team as a code owner September 30, 2025 14:36
@FelipeRiveraC FelipeRiveraC changed the title Add skipZeroPricePurchases setting to Topsort destination [TOPSORT] Add skipZeroPricePurchases setting to Topsort destination Sep 30, 2025
const filteredItems = payload.items.filter((item) => item.unitPrice != null && item.unitPrice > 0)

if (filteredItems.length === 0) {
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @FelipeRiveraC ,

You could throw a PayloadValidationError here if you liked - it provides feedback to the customer about what happened.

It's OK not to also. Up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, we actually dont want to provide feedback, so its ok. Thanks!

@joe-ayoub-segment
Copy link
Contributor

Hi @FelipeRiveraC just one minor comment. Let me know if you want this deployed or if you want to tweak it as per my comment.

@joe-ayoub-segment
Copy link
Contributor

Ready to deploy

@FelipeRiveraC
Copy link
Contributor Author

When is this getting deployed? @joe-ayoub-segment

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