Fix empty string for CLIENT_ID from being sent when user decides to opt in

[eliasyishak]

Fixes:

• Incorrectly sending empty CLIENT_ID when enabling telemetry #143

This update will now ensure that an opted out user who decides to opt back in will have their newly generated CLIENT_ID string being sent to the event that is emitted on opt in/out

---

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[andrewkolos]

LGTM with mild concern

[andrewkolos]

I wonder if _clientId should be behind an abstraction that prevents it becoming out-of-sync with the file contents, but I am afraid I don't have time to read the entire code base.

[eliasyishak]

This wouldn't be a bad idea actually. Creating a getter that just reads the contents of the file should suffice on the AnalyticsImpl instance.

Does that help with the mild concern?

[andrewkolos]

I would expect some construct that exposes a getter and setter. The getter would read the file (or return a cached value if we are confident or don't care that it has not changed). The setter would write the file and update the cached file. These two methods would be the only things that 1) interact with the file and 2) store the current/cached value.

If AnalyticsImpl were the only context we are interacting with the client ID file, then you could use a class property with a getter/setter. However, Initializer also interacts with the file, so you would need another construct (or rework Initializer into something else that's more general purpose (e.g. User or UserConfig). To reiterate, I have not read the whole codebase.

[eliasyishak]

Ah I see what you mean. I'll keep that in mind if we are altering the client id anywhere else within the instance but at the moment it is only being read in the AnalyticsImpl constructor and only gets reassigned within the setTelemetry method