Skip to content

feat: analytics interface in :common#20736

Open
criticalAY wants to merge 1 commit intoankidroid:mainfrom
criticalAY:feat/analytics-common
Open

feat: analytics interface in :common#20736
criticalAY wants to merge 1 commit intoankidroid:mainfrom
criticalAY:feat/analytics-common

Conversation

@criticalAY
Copy link
Copy Markdown
Contributor

Purpose / Description

Create interface in :common for analytics, I want to use this i.e. actual implementation after GA4 PR is merged
after everything is in modules this will look something like this

// In :widgets — just use it
AnalyticsProvider.instance.sendAnalyticsEvent("CardAnalysisWidget", "enabled")

Fixes

NA

Approach

see commit

How Has This Been Tested?

NA

Learning (optional, can help others)

Things are very coupled to move in one go

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@criticalAY criticalAY changed the title feat: analytics interface in :common feat: analytics interface in :common Apr 13, 2026
Copy link
Copy Markdown
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

You should implement and switch to the interface in this PR, rather than leaving it as 'dead' code.

You may want to make UsageAnalytics private, to be sure you're not accessing the object directly.

Add a public accessor for which setsAnalytics
And a public accessor (test only)


calling the methods is more verbose:

- sendAnalyticsEvent("", "") 
+ AnalyticsProvider.instance.sendAnalyticsEvent("", "") 

Analytics would be a better name for the object, not the interface

@criticalAY criticalAY force-pushed the feat/analytics-common branch from eabadbc to c82ee0e Compare April 13, 2026 11:34
@criticalAY
Copy link
Copy Markdown
Contributor Author

Can we get this in and then I can update the usage in GA4 PR?

@david-allison
Copy link
Copy Markdown
Member

Hmm... let's check with @ankidroid/reviewers

@david-allison david-allison added the Needs reviewer reply Waiting for a reply from another reviewer label Apr 13, 2026
@criticalAY
Copy link
Copy Markdown
Contributor Author

Sharing my reply to David from Discord to support my plans in GA4

Analytics: as long as you can explain how you would block consumers of the implementation, I'm happy
I'm pretty sure the private class + accessor will work

Implementation lives in :AnkiDroid (AnkiDroidUsageAnalytics)

Interface + provider lives in :common (UsageAnalytics interface + Analytics object)

Clean calling Analytics.sendAnalyticsEvent("Widget", "enabled")
Block direct coupling: make AnkiDroidUsageAnalytics internal in :AnkiDroid, only exposed via Analytics.setAnalytics(). Other modules can't import it because internal is module-scoped.

So:

// In :AnkiDroid internal, invisible to other modules
internal object AnkiDroidUsageAnalytics : UsageAnalytics { ... }

// In onCreate()
Analytics.setAnalytics(AnkiDroidUsageAnalytics)

@david-allison
Copy link
Copy Markdown
Member

I think it's best if we work on this pattern initially in CrashReportService, drafting this PR, then bringing what we've learned over to Analytics

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

Labels

Needs Review Needs reviewer reply Waiting for a reply from another reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants