Skip to content
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

fix: Add enum types for commerce event #498

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

Mansi-mParticle
Copy link
Contributor

Instructions

  1. PR target branch should be against development
  2. PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
  3. PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

  • Added enums for e-commerce events.
  • 'expandProductAction' to create an event instead of a transaction. Separate it into different e-commerce event types.

Testing Plan

  • Was this tested locally? If not, explain why.
  • Tested it with a sample application and ran unit tests

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

Copy link
Member

@samdozor samdozor left a comment

Choose a reason for hiding this comment

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

let's chat about another way to approach this, i'd like to understand the problem better.


Unknown(0), Navigation(1), Location(2), Search(3), Transaction(4), UserContent(5), UserPreference(6),
Copy link
Member

Choose a reason for hiding this comment

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

this would be an incorrect public api change that'd likely cause a lot of pain and bugs :-) please revert

this enum is the one for custom events. it isn't meant to be used for all event types. ios does it this way and it's been a bug and complaint for years - customers for example log regular custom events with incorrect event types.

create some other private enum if we need it internally? these enum values shouldn't be exposed to customers.

MPEvent.Builder itemEvent;
switch (productAction) {
case Product.ADD_TO_CART:
itemEvent = new MPEvent.Builder(String.format(ITEM_NAME, productAction), MParticle.EventType.AddToCart);
Copy link
Member

Choose a reason for hiding this comment

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

and this is exactly what customers do and is not correct. then we end up with weird data, broken filtering, integrations, etc.

MPEvent is custom event and cannot have AddToCart (or any of the others below) as a custom event type in our schema.

@Mansi-mParticle Mansi-mParticle requested a review from samdozor July 19, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants