feat(firebase_analytics): update logEvent() & setDefaultParameters() to assert input types.

[russellwheatley]

Description

This PR is designed to avoid confusion about what values are accepted when logging a custom event via logEvent(). I've implemented an assert on the values passed to logEvent which throws when the parameter values are not String or num. The same for setDefaultEventParameters() except it also allows parameter values to be null which resets that value as a default parameter.

The Firebase documentation states that the only values accepted are strings and numbers:

From android documentation (see bold):

The map of event parameters. Passing null indicates that the event has no parameters. Parameter names can be up to 40 characters long and must start with an alphabetic character and contain only alphanumeric characters and underscores. String, long and double param types are supported. String parameter values can be up to 100 characters long. The "firebase_", "google_" and "ga_" prefixes are reserved and should not be used for parameter names.

I've also tested on web, iOS and android:

android:
Successful custom event without items list:

Unsuccessful custom event including items list (Note error code 21):

iOS:
Successful custom event without items list:

Unsuccessful custom event including items list (Note error code 21):

Web:
Web is slightly different as it uses a Chrome extension to debug. It does pass the items list in the request for a custom event, but it appears to stringify the value. See:

logEvent(
  analytics, 
  'test_event',
  { some_values: [{some:'thing'}, 3, 'hello'] }
);

The result in the debugger console for some_values looks like this:

ep.some_values=%5Bobject%20Object%5D%2C3%2Chello&_et=3

Once decoded, it shows the value as one string (including the object within the array, hence [object Object])

[object Object],3,hello&_et=3

setDefaultEventParameters

I've also changed Int to Long for the event parameters bundle in android. The documentation for android says Long & Double only. If you try to add Int when running setDefaultEventParameters() you will get the following warning in the console logs:

W/FA: Invalid default event parameter type. Name, value: someInt, 400
W/FA: Invalid default event parameter type. Name, value: longNumber, 230485
W/FA: Invalid default event parameter type. Name, value: inter, 9

It also doesn't show up in the debugger console.

When I changed putInt() to putLong(), the warnings disappear and the default parameters appear in the console:

TODO

• Use EventParameters for setDefaultEventParameters() as this only allows strings , null and numbers. See: 🐛 [firebase_analytics] cannot set default parameter due to invalid type #9698

Related Issues

closes #8861, #9698

Other related issues which highlight confusion:
#4353
#2835

Replace this paragraph with a list of issues related to this PR from the issue database. Indicate, which of these issues are resolved or fixed by this PR. Note that you'll have to prefix the issue numbers with flutter/flutter#.

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. Updating the pubspec.yaml and changelogs is not required.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (melos run analyze) does not report any problems on my PR.
• I read and followed the Flutter Style Guide.
• I signed the CLA.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change.
• No, this is not a breaking change.

[Lyokone]

We don't have items anymore?

[russellwheatley]

Not for custom events (logEvent()). See the description for why 😄

[mateusfccp]

IMO, this is not much better. We are allowing the consumer to pass both of them, and he's going to have a runtime exception if he does it. We could make a safe API that wouldn't allow the user to pass any kind of invalid data, so that, if it compiles, it works.

[russellwheatley]

Fair point. I think I will change addParameter to addString & addNumber to improve it.

I will be keeping the fromMap() function, as its main purpose is to help transition to the new EventParameters API, and it will still throw assert on incorrect property values before it ever makes it to the native side.

[victor-tinoco]

WDYT about to introduce an addBool method, which behind the scenes saves it as value.toString()?

This way we might support most of these base types.

[russellwheatley]

I would prefer not as it isn't stored as a boolean and hence misleading, true.toString() is painless enough.

[mockturtl]

From android documentation (see bold):

The map of event parameters. Passing null indicates that the event has no parameters. Parameter names can be up to 40 characters long and must start with an alphabetic character and contain only alphanumeric characters and underscores. String, long and double param types are supported. String parameter values can be up to 100 characters long. The "firebase_", "google_" and "ga_" prefixes are reserved and should not be used for parameter names.

Source: https://developers.google.com/android/reference/com/google/firebase/analytics/FirebaseAnalytics#logEvent(java.lang.String,%20android.os.Bundle)