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

feat: initial plausible setup #1038

Merged
merged 9 commits into from
Jan 28, 2024
Merged

feat: initial plausible setup #1038

merged 9 commits into from
Jan 28, 2024

Conversation

limwa
Copy link
Member

@limwa limwa commented Nov 22, 2023

Closes #1031.

This pull requests adds analytics collection to uni.

As of now, pageviews are collected using flutter_plausible's PlausibleNavigatorObserver, which automatically tracks which routes are being viewed. Clicks and other events will be collected as well, but I am still figuring out what is the best way to go about it.

Review checklist

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes a new summary in whatsnew/whatsnew-pt-PT
  • Properly adds an entry in changelog.md with the change
  • If PR includes UI updates/additions, its description has screenshots
  • Behavior is as expected
  • Clean, well-structured code

@limwa limwa marked this pull request as ready for review November 23, 2023 18:51
@limwa
Copy link
Member Author

limwa commented Nov 23, 2023

I still need to add widget click detection, but it has proven harder than I initially though, especially since dart:mirrors is not available in Flutter. As such, I'll have to use code generation (something I am completely unfamiliar with) to mark uni's widgets, so that, later on, I know which ones should be sent in the analytics event. Therefore, I suggest we split this PR into two different PRs: one for pageviews (this one) and another for click detection.

Regarding this PR, the terms and conditions still need to be updated and battery level / connectivity detection still need to be added.

@limwa limwa marked this pull request as draft November 23, 2023 19:04
@limwa
Copy link
Member Author

limwa commented Nov 23, 2023

Also, any clues on how to fix the linting error? @bdmendes @LuisDuarte1

@limwa limwa mentioned this pull request Nov 23, 2023
7 tasks
@bdmendes
Copy link
Collaborator

bdmendes commented Nov 23, 2023

@limwa:

  • I'm not really sure what is causing the linting error, since the env file never existed on the repository and it never caused an error.
  • I agree with splitting the PRs, but we should tackle the T&C here since this will deployed immediately to the beta track
  • Also, about the code generation build steps, see i18n migration #475 for an example

@limwa
Copy link
Member Author

limwa commented Nov 23, 2023

  • I'm not really sure what is causing the linting error, since the env file never existed on the repository and it never caused an error.

The linting error is being caused because I added the env file to the pubspec.yml, since I need it to be shipped with the app. It never caused an error because it was never added to the application bundle since the linting configurations changed.

  • I agree with splitting the PRs, but we should tackle the T&C here since this will deployed immediately to the beta track

I agree. Who is going to handle this? If this is too sensitive to be discussed in public, you can DM me.

@bdmendes
Copy link
Collaborator

@limwa

  • Aren't we using a new package that moved the env out of the assets? Just recalling it now
  • Let's discuss that in-person

@limwa
Copy link
Member Author

limwa commented Nov 23, 2023

Huuu, I'm not sure, are we?

Update: I don't think we are, the GitHub Actions don't touch any .env file.

Update 2: We are using flutter_dotenv (https://pub.dev/packages/flutter_dotenv). You are still required to add the .env file to your assets.

@limwa limwa force-pushed the feature/add-analytics branch from 64d0a83 to 7cd75b3 Compare January 8, 2024 01:38
@limwa limwa marked this pull request as ready for review January 12, 2024 23:20
@bdmendes bdmendes requested a review from a team January 19, 2024 14:07
Copy link

codecov bot commented Jan 19, 2024

Codecov Report

Merging #1038 (51d41b9) into develop (66fcc86) will decrease coverage by 0%.
Report is 2 commits behind head on develop.
The diff coverage is 0%.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #1038   +/-   ##
=======================================
- Coverage       17%     16%   -0%     
=======================================
  Files          212     213    +1     
  Lines         6472    6535   +63     
=======================================
  Hits          1044    1044           
- Misses        5428    5491   +63     

@limwa limwa force-pushed the feature/add-analytics branch from 32f7114 to 5416985 Compare January 19, 2024 14:29
@limwa limwa force-pushed the feature/add-analytics branch from 7cd82cd to 9b9c29b Compare January 20, 2024 21:37
@limwa limwa requested review from bdmendes and a team January 20, 2024 22:48
@limwa limwa requested a review from a team January 22, 2024 16:09
Copy link
Member

@LuisDuarte1 LuisDuarte1 left a comment

Choose a reason for hiding this comment

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

🚀 Good work!

@bdmendes bdmendes merged commit 6494bf8 into develop Jan 28, 2024
6 checks passed
@bdmendes bdmendes deleted the feature/add-analytics branch January 28, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collect click/page duration analytics
3 participants