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(analytics): always track #870

Merged
merged 3 commits into from
Jul 21, 2022
Merged

feat(analytics): always track #870

merged 3 commits into from
Jul 21, 2022

Conversation

louis-bompart
Copy link
Collaborator

@louis-bompart louis-bompart commented Jul 21, 2022

Proposed changes

  • Removed the possibility of disabling analytics 1
  • Track API key user through a hash of the last subset of said API Key.

Testing

  • Unit Tests

Footnotes

  1. technically still possible by having CI=true in your env var

@github-actions
Copy link
Contributor

Thanks for your contribution @louis-bompart !
When your pull-request is ready to be merged, check the box below to merge it

  • Merge! :shipit:

@github-actions
Copy link
Contributor

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Copy link

@tcamire tcamire left a comment

Choose a reason for hiding this comment

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

not sure that I see a condition that blocks tracking if the user is not logged in, will that be in this PR? Correct me if I'm wrong, it's my first time roaming around in this code

Base automatically changed from feat/CDX-1081 to master July 21, 2022 19:11
@louis-bompart louis-bompart enabled auto-merge (squash) July 21, 2022 19:12
@y-lakhdar
Copy link
Contributor

not sure that I see a condition that blocks tracking if the user is not logged in, will that be in this PR? Correct me if I'm wrong, it's my first time roaming around in this code

Good catch. Most commands require user to be logged in both some few can be executed without being logged in.

Copy link
Contributor

@y-lakhdar y-lakhdar left a comment

Choose a reason for hiding this comment

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

Only track when user is logged in.

@louis-bompart
Copy link
Collaborator Author

if (!(await isLoggedIn())) {
// TODO: track event with anonymous user
return;
}
this does that

@louis-bompart louis-bompart merged commit da4bd10 into master Jul 21, 2022
@louis-bompart louis-bompart deleted the feat/CDX-1082 branch July 21, 2022 20:11
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.

3 participants