Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add android app id and ios bundle id in GA metrics #8177

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hangyujin
Copy link
Contributor

@hangyujin hangyujin commented Aug 9, 2024

issue: (#8068)

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or there is a reason for not adding tests.

build.yaml badge

If you need help, consider asking for help on Discord.

@hangyujin hangyujin requested a review from a team as a code owner August 9, 2024 00:03
@hangyujin hangyujin requested review from elliette and removed request for a team August 9, 2024 00:03
@hangyujin hangyujin requested review from kenzieschmoll and chunhtai and removed request for elliette August 9, 2024 00:15
@@ -326,6 +334,13 @@ class DeepLinksController extends DisposableController
target: target,
);
_iosLinks[selectedAndroidVariantIndex.value] = result;
ga.impression(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's similar to ga.select but nothing is selected here.

Comment on lines +141 to +142
external String? get android_app_id;
external String? get ios_bundle_id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These will need to be added to the custom dimensions and metrics in the GA console for the legacy DevTools analytics if we want to track there. Otherwise they should automatically be included in the unified_analytics data for DevTools.

@hangyujin
Copy link
Contributor Author

looks like I need to update the event in pkgs/unified_analytics first, I opened PR dart-lang/tools#295

Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hangyujin hangyujin closed this Aug 19, 2024
@hangyujin hangyujin reopened this Sep 23, 2024
this.iosBundleId,
});

/// The anroid app id of the flutter project.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sp: android

r/flutter/Flutter (same below)

Copy link
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also bump the min dep of unified_analytics to ^6.1.4 to include the changes here: dart-lang/tools#295

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants