Skip to content

fix: Add enum value to hash ecommerce event attributes #169

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.

Sign up for GitHub

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
Mansi-mParticle wants to merge 6 commits into
base: development
Choose a base branch
from
Open

fix: Add enum value to hash ecommerce event attributes #169

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
44965b6
fix: Add enum value to hash ecommerce event attributes
Mansi-mParticle Jul 10, 2024
edf1408
Update the logic based on feedback
Mansi-mParticle Jul 18, 2024
f932c03
Address review comment
Mansi-mParticle Jul 19, 2024
50902e2
change variable name
Mansi-mParticle Jul 19, 2024
fd439ba
fix unit test case
Mansi-mParticle Jul 19, 2024
c162935
add custom attributes
Mansi-mParticle Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 104 additions & 29 deletions src/main/kotlin/com/mparticle/kits/AppboyKit.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import com.braze.Braze
import com.braze.BrazeActivityLifecycleCallbackListener
import com.braze.BrazeUser
import com.braze.configuration.BrazeConfig
import com.braze.enums.*
import com.braze.enums.BrazeSdkMetadata
import com.braze.enums.Gender
import com.braze.enums.Month
import com.braze.enums.NotificationSubscriptionType
import com.braze.enums.SdkFlavor
import com.braze.events.IValueCallback
import com.braze.models.outgoing.BrazeProperties
import com.braze.push.BrazeFirebaseMessagingService
Expand All @@ -26,10 +30,16 @@ import com.mparticle.commerce.Promotion
import com.mparticle.identity.MParticleUser
import com.mparticle.internal.Logger
import com.mparticle.kits.CommerceEventUtils.OnAttributeExtracted
import com.mparticle.kits.KitIntegration.*
import com.mparticle.kits.KitIntegration.AttributeListener
import com.mparticle.kits.KitIntegration.CommerceListener
import com.mparticle.kits.KitIntegration.IdentityListener
import com.mparticle.kits.KitIntegration.PushListener
import java.math.BigDecimal
import java.text.SimpleDateFormat
import java.util.*
import java.util.Calendar
import java.util.Date
import java.util.EnumSet
import java.util.LinkedList

/**
* mParticle client-side Appboy integration
Expand Down Expand Up @@ -134,6 +144,19 @@ open class AppboyKit : KitIntegration(), AttributeListener, CommerceListener,
): List<ReportingMessage> = emptyList()

override fun logEvent(event: MPEvent): List<ReportingMessage> {
if (event.customAttributes != null) {
event.customAttributeStrings?.let {
changeUserArray(
it,
event.eventType.value,
event.eventName, false
)
}
}
return logBrazeEvent(event)
}

private fun logBrazeEvent(event: MPEvent): List<ReportingMessage> {
val newAttributes: MutableMap<String, Any?> = HashMap()
if (event.customAttributes == null) {
Braze.getInstance(context).logCustomEvent(event.eventName)
Expand All @@ -142,40 +165,55 @@ open class AppboyKit : KitIntegration(), AttributeListener, CommerceListener,
val brazePropertiesSetter = BrazePropertiesSetter(properties, enableTypeDetection)
event.customAttributeStrings?.let { it ->
for ((key, value) in it) {
newAttributes[key] = brazePropertiesSetter.parseValue(key, value)
try {
newAttributes[key] = brazePropertiesSetter.parseValue(key, value)
} catch (e: Exception) {
Logger.warning("Exception while parsing custom attributes $e")
}
}
}
Braze.getInstance(context).logCustomEvent(event.eventName, properties)
Braze.getInstance(context).getCurrentUser(object : IValueCallback<BrazeUser> {
override fun onSuccess(value: BrazeUser) {
val userAttributeSetter = UserAttributeSetter(value, enableTypeDetection)
event.customAttributeStrings?.let { it ->
for ((key, attributeValue) in it) {
val hashedKey =
KitUtils.hashForFiltering(event.eventType.value.toString() + event.eventName + key)

configuration.eventAttributesAddToUser?.get(hashedKey)?.let {
value.addToCustomAttributeArray(it, attributeValue)
}
configuration.eventAttributesRemoveFromUser?.get(hashedKey)?.let {
value.removeFromCustomAttributeArray(it, attributeValue)
}
configuration.eventAttributesSingleItemUser?.get(hashedKey)?.let {
userAttributeSetter.parseValue(it, attributeValue)
}
queueDataFlush()
return listOf(ReportingMessage.fromEvent(this, event).setAttributes(newAttributes))
}

private fun changeUserArray(
customAttributes: Map<String, String>,
eventType: Int,
eventName: String?,
isCommerceEvent: Boolean
) {
Braze.getInstance(context).getCurrentUser(object : IValueCallback<BrazeUser> {
override fun onSuccess(brazeUser: BrazeUser) {
val userAttributeSetter = UserAttributeSetter(brazeUser, enableTypeDetection)
customAttributes?.let { it ->
for ((key, attributeValue) in it) {
//for commerce event, event name is not required for generate hash
val hashedKey =
if (isCommerceEvent) {
KitUtils.hashForFiltering(eventType.toString() + key)
} else {
KitUtils.hashForFiltering(eventType.toString() + eventName + key)
}
configuration.eventAttributesAddToUser?.get(hashedKey)?.let {
brazeUser.addToCustomAttributeArray(it, attributeValue)
}
configuration.eventAttributesRemoveFromUser?.get(hashedKey)?.let {
brazeUser.removeFromCustomAttributeArray(it, attributeValue)
}
configuration.eventAttributesSingleItemUser?.get(hashedKey)?.let {
userAttributeSetter.parseValue(it, attributeValue)
}
}
}
}

override fun onError() {
Logger.warning("unable to acquire user to add or remove custom user attributes from events")
}
})
}
queueDataFlush()
return listOf(ReportingMessage.fromEvent(this, event).setAttributes(newAttributes))
override fun onError() {
Logger.warning("unable to acquire user to add or remove custom user attributes from events")
}
})
}

override fun logScreen(
screenName: String,
screenAttributes: Map<String, String>?
Expand Down Expand Up @@ -216,6 +254,42 @@ open class AppboyKit : KitIntegration(), AttributeListener, CommerceListener,

override fun logEvent(event: CommerceEvent): List<ReportingMessage> {
val messages: MutableList<ReportingMessage> = LinkedList()

//For CommerceEvent, Event Name is not required to generate hash. So, it will be always null.
event.products?.forEach {
it.customAttributes?.let {
changeUserArray(it, CommerceEventUtils.getEventType(event), null, true)
Copy link

@samdozor samdozor Jul 19, 2024

Choose a reason for hiding this comment

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

i don't think we're suppose to map custom attributes of products like this - does the ui even support mapping them? I think you mean to just have event.customAttributes here

}
}
event.transactionAttributes?.let {
val map = mapOf(
"Affiliation" to it.affiliation,
Copy link

@samdozor samdozor Jul 19, 2024

Choose a reason for hiding this comment

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

you should be using the constants that are in here: https://github.com/mParticle/mparticle-android-sdk/blob/main/android-kit-base/src/main/java/com/mparticle/kits/CommerceEventUtils.java#L324

and you are missing quite a few constants/mappings - look at every single call to "hashForFiltering()" in these methods - that is a potential constant that you should likely support:

  • filterCommerceEvent
  • filterCommerceEntities
  • filterCommerceEntityAttributes
  • filterCommerceEventAttributes.

I'd encourage you how to think about how to move this logic into that class so that we have complicated business logic close together vs separated. maybe a function in KitConfiguration or in CommerceEventUtils which accepts in a CommerceEvent and returns a map of <hash, value> - and then you can just use that map in the kit.

"Revenue" to it.revenue?.toString(),
"Shipping" to it.shipping?.toString(),
"Tax" to it.tax?.toString(),
"CouponCode" to it.couponCode?.toString(),
"Id" to it.id.toString()
).mapValues { it.value.toString() }
changeUserArray(map, CommerceEventUtils.getEventType(event), null, true)
}
event.customAttributeStrings?.let {
changeUserArray(it, CommerceEventUtils.getEventType(event), null, true)
}
event.impressions?.forEach { impression ->
impression.products.forEach { product ->
product.customAttributes?.let {
Copy link

@samdozor samdozor Jul 19, 2024

Choose a reason for hiding this comment

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

same comment as above - have you tested if the UI even populates a custom attributes that's sent into the product within a impression? if so, I doubt the hash here will be right...?

changeUserArray(it, CommerceEventUtils.getEventType(event), null, true)
}
}
}
event.promotions?.forEach {
val attributes: MutableMap<String, String> = HashMap()
if (event.customAttributeStrings != null) {
attributes.putAll(event.customAttributeStrings!!)
}
CommerceEventUtils.extractPromotionAttributes(it, attributes)
changeUserArray(attributes, CommerceEventUtils.getEventType(event), null, true)
}
if (!KitUtils.isEmpty(event.productAction) &&
event.productAction.equals(
Product.PURCHASE,
Expand Down Expand Up @@ -249,7 +323,7 @@ open class AppboyKit : KitIntegration(), AttributeListener, CommerceListener,
for (pair in map) {
e.customAttributes?.put(pair.key, pair.value)
}
logEvent(e)
logBrazeEvent(e)
messages.add(ReportingMessage.fromEvent(this, event))
} catch (e: Exception) {
Logger.warning("Failed to call logCustomEvent to Appboy kit: $e")
Expand Down Expand Up @@ -824,6 +898,7 @@ open class AppboyKit : KitIntegration(), AttributeListener, CommerceListener,
return impressionArray
}


internal abstract class StringTypeParser(var enableTypeDetection: Boolean) {
fun parseValue(key: String, value: String): Any {
if (!enableTypeDetection) {
Expand Down
4 changes: 4 additions & 0 deletions src/test/kotlin/com/braze/Braze.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ class Braze {
events.clear()
}

fun clearBrazeUser(){
currentUser.customUserAttributes.clear()
currentUser.customAttributeArray.clear()
}
val currentUser = BrazeUser()

@JvmStatic
Expand Down
Loading