segment-integrations / analytics-android-integration-amplitude

The Amplitude analytics-android integration.
https://segment.com/docs/connections/destinations/catalog/amplitude/
2 stars 6 forks source link

support string array traits when either traitsToIncrement or traitsToSetOnce are set #54

Closed aultimus closed 4 years ago

aultimus commented 4 years ago

What does this PR do? This PR adds support for string arrays in traits when either traitsToIncrement or traitsToSetOnce are set

Are there breaking changes in this PR? no

Has this been tested end-to-end? Please provide screenshots on how the fix now populates in the end tool. If not, what was done to test? Yes

Running the current version of code I sent a request via the amplitude sdk with the userId 'dog', setting a 'dog' string array property

Screen Shot 2020-02-28 at 11 22 29 AM Screen Shot 2020-02-28 at 11 19 24 AM

Above you can observe the absence of the property

Then I made the code change in this PR and sent another request 'zebra' with a string array property 'zebra'

Screen Shot 2020-02-28 at 11 22 20 AM Screen Shot 2020-02-28 at 11 19 33 AM

In this screenshot below we can observe that the property 'dog' is absent whereas the property 'dog' is not

Screen Shot 2020-02-28 at 11 21 54 AM

Any background context you want to provide? This PR was prompted by the change made here by zenly in their fork https://github.com/znly/analytics-android-integration-amplitude/commit/45d22e851dfa0c8915fa4dd8e813e262884a7470

I tested this change using the sample app in the analytics-android repo and made some changes including importing this repo as a module, the code i tested with is represented here https://github.com/segmentio/analytics-android/commit/76b60b86dbf661baa5be34b67e1219253689ec06

Is there parity with the server-side/analytics.js integration (if applicable)?

Does this require a metadata change? If so, please link the PR from https://github.com/segmentio/destination-scripts. no

What are the relevant tickets? https://segment.atlassian.net/browse/LIB-1605

Link to CC ticket CC in this PR

List all the tests accounts you have used to make sure this change works My own amplitude test account

Helpful Docs

Version for this change

brennan commented 4 years ago

@aultimus Would you please bump the version per semver here and update the changelog? https://github.com/segment-integrations/analytics-android-integration-amplitude/blob/master/gradle.properties#L3