fix: update Android logEvent to support parameters of type List<Map<String, dynamic>> to match iOS#1458
fix: update Android logEvent to support parameters of type List<Map<String, dynamic>> to match iOS#1458tp wants to merge 7 commits into
logEvent to support parameters of type List<Map<String, dynamic>> to match iOS#1458Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
Still have to add tests for this PR. Just wanted to post this early, so I could get some feedback if anyone sees any issue with this approach. Works fine for us in production. |
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Sadly there wasn't a test of the Java implementation so far, and I don't know yet how to best add one. Any pointers would be appreciated. |
|
Not sure what happened to |
|
@collinjackson Can you have a look here please if what we're doing is OK? We've been running in production with this and it's working fine for us, not the lists are available via Google Tag Manager just like on iOS. |
|
I ran into this exact need, was about to create a PR but glad to find there's already one open! Any ideas what else might be holding this up @tp ? Any pointers @kroikie ? Happy to help where I can Btw, I've cloned this fork and rebased onto today's |
|
Would this throw any error if Value is too long? I tried using the custom event with string and it throws an error for lists too long. |
|
Updated to the latest release version. Now there is "only" an issue with iOS builds, but that also affects other PRs here and seems unrelated to my changes. @collinjackson @kroikie Could you help our here moving this forward? |
logEvent to support parameters of type List<Map<String, dynamic>> to match iOS
|
Closing this in favour of #4394 which is slightly further along in the review process and has tests. Thanks for sending this up though. |
Description
The
logEventparams on Android so far only support primitive values (numbers, Strings, boolean).This change adapts the Java implementation to also support sending
Lists withMap<String, dynamic>from the Flutter side.This worked for us already on iOS, so we just needed this adaptation for Android to reach feature parity.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?