braze-inc / braze-segment-kotlin

The Braze analytics-kotlin integration.
Other
1 stars 2 forks source link

kotlin-braze #1

Closed charleswilliamson7 closed 2 years ago

charleswilliamson7 commented 2 years ago

The Segment Analytics-Kotlin repo for Braze

charleswilliamson7 commented 2 years ago

campaignProps.getString("source")?: "", campaignProps.getString("name")?: "", campaignProps.getString("ad_group")?: "", campaignProps.getString("ad_creative")?: ""

On Mon, Jul 18, 2022 at 12:43 PM Wenxi Zeng @.***> wrote:

@.**** commented on this pull request.

In lib/src/main/java/com/segment/analytics/kotlin/destinations/braze/BrazeDestination.kt https://urldefense.com/v3/__https://github.com/segment-integrations/analytics-kotlin-braze/pull/1*discussion_r923792061__;Iw!!NCc8flgU!fQhhjjvJdil27wjLD3-SxMW9VUI6mSyu94eigyjZGIh_e5hyOD0Nx_lI2DlyGlZ79FFCMZKFmBjdOUY3Dg050aoszAtgkZK9$ :

+

  • val settings = settings ?: return payload
  • val properties: JsonObject = payload.properties
  • try {
  • if (eventName == "Install Attributed") {
  • val campaignProps: JsonObject? = properties["campaign"] as JsonObject?
  • val currentUser: BrazeUser? = getInternalInstance()?.getCurrentUser()
  • if (campaignProps != null && currentUser != null) {
  • currentUser.setAttributionData(
  • AttributionData(
  • campaignProps.get("source").toString(),
  • campaignProps.get("name").toString(),
  • campaignProps.get("ad_group").toString(),
  • campaignProps.get("ad_creative").toString()

there are two issues doing this way:

  1. campaignProps["YOUR KEY"] might not be exist and yields an illegal state or null exception.
  2. toString method is from JsonElement, it's not guaranteed the returning value is the content. use the campaignProps.getString("YOUR KEY") is recommended and it forces you to do the null check

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/segment-integrations/analytics-kotlin-braze/pull/1*discussion_r923792061__;Iw!!NCc8flgU!fQhhjjvJdil27wjLD3-SxMW9VUI6mSyu94eigyjZGIh_e5hyOD0Nx_lI2DlyGlZ79FFCMZKFmBjdOUY3Dg050aoszAtgkZK9$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AYFLNRPXES7XH34EAL7T7E3VUWXUXANCNFSM523HNC5A__;!!NCc8flgU!fQhhjjvJdil27wjLD3-SxMW9VUI6mSyu94eigyjZGIh_e5hyOD0Nx_lI2DlyGlZ79FFCMZKFmBjdOUY3Dg050aoszMiDihca$ . You are receiving this because you authored the thread.Message ID: <segment-integrations/analytics-kotlin-braze/pull/1/review/1042446067@ github.com>

-- charles williamson he/him True Fun Guy [image: %(logoAlt)s] https://www.twilio.com/?utm_source=email_signature MOBILE (480)862-9688 EMAIL @.*** LINKEDIN charleswilliamson1 https://linkedin.com/in/charleswilliamson1 View Our Privacy Policy https://www.twilio.com/legal/privacy | Unsubscribe https://pages.twilio.com/Email-Preference-Center

charleswilliamson7 commented 2 years ago

} else if (value is JsonArray) { currentUser.setCustomAttributeArray(key, (value as Array<String?>)) } else if (value is List<*>) {

On Wed, Aug 3, 2022 at 9:35 AM Wenxi Zeng @.***> wrote:

@.**** commented on this pull request.

In lib/src/main/java/com/segment/analytics/kotlin/destinations/braze/BrazeDestination.kt https://urldefense.com/v3/__https://github.com/segment-integrations/analytics-kotlin-braze/pull/1*discussion_r936907676__;Iw!!NCc8flgU!cuHqZwNk8GhZCBlmi-3rvUNqfulunCXnSTpOxGWo6tkAYHYaZAh6CJX_AkcRkPkqpnW3hg4A8-1xyeM2xm4Z62JEy2dt0L62$ :

  • currentUser.setCustomUserAttribute(keyString, value)
  • } else if (value is Double) {
  • currentUser.setCustomUserAttribute(keyString, value)
  • } else if (value is Float) {
  • currentUser.setCustomUserAttribute(keyString, value)
  • } else if (value is Long) {
  • currentUser.setCustomUserAttribute(keyString, value)
  • } else if (value is String) {
  • try {
  • val parseTime = Instant.parse(value)
  • currentUser.setCustomUserAttributeToSecondsFromEpoch(keyString, parseTime.epochSecond)
  • } catch (e: Exception) {
  • }
  • } else if (value is Array<*>) {
  • currentUser.setCustomAttributeArray(keyString, (value as Array<String?>?)!!)
  • } else if (value is List<*>) {

it doesn't matter whether if it looks good. it's the logic that is wrong. it's impossible for the conversion to happen, since JsonArray or JsonElement is completely different than Array and List<*>. the conversion will just failed, and thus losing data

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/segment-integrations/analytics-kotlin-braze/pull/1*discussion_r936907676__;Iw!!NCc8flgU!cuHqZwNk8GhZCBlmi-3rvUNqfulunCXnSTpOxGWo6tkAYHYaZAh6CJX_AkcRkPkqpnW3hg4A8-1xyeM2xm4Z62JEy2dt0L62$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AYFLNRJBSQTTIMOAXQIWUFLVXKNFHANCNFSM523HNC5A__;!!NCc8flgU!cuHqZwNk8GhZCBlmi-3rvUNqfulunCXnSTpOxGWo6tkAYHYaZAh6CJX_AkcRkPkqpnW3hg4A8-1xyeM2xm4Z62JEy9z_z3NK$ . You are receiving this because you authored the thread.Message ID: <segment-integrations/analytics-kotlin-braze/pull/1/review/1060681629@ github.com>

-- charles williamson he/him True Fun Guy [image: %(logoAlt)s] https://www.twilio.com/?utm_source=email_signature MOBILE (480)862-9688 EMAIL @.*** LINKEDIN charleswilliamson1 https://linkedin.com/in/charleswilliamson1 View Our Privacy Policy https://www.twilio.com/legal/privacy | Unsubscribe https://pages.twilio.com/Email-Preference-Center