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

Botonic Plugin Google Analytics #823

Merged
merged 16 commits into from
Jul 1, 2020
Merged

Conversation

asastre
Copy link
Contributor

@asastre asastre commented Jun 17, 2020

Description

New plugin to track user interaction or bot's behaviour to Google Analytics.

Context

Tracking information to Google Analytics is very usual nowadays and for some companies there is the need to track the user interaction with the bot to Google Analytics.

Approach taken / Explain the design

This plugin provides a basic tracking with its method track and it has to be done manually, there is no default tracking through the pre or post methods.

To documentate / Usage example

See Readme.md file for documentation/examples.

Testing

The pull request...

  • has unit tests
  • has integration tests
  • doesn't need tests because... no tests needed

Copy link
Contributor

@dpinol dpinol left a comment

Choose a reason for hiding this comment

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

🚀 a new plugin for botonic!

@asastre asastre added the WIP label Jun 17, 2020
Copy link
Contributor

@vanbasten17 vanbasten17 left a comment

Choose a reason for hiding this comment

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

Cool!! Publish it freely once you have considered the feedback! 🚀

@asastre
Copy link
Contributor Author

asastre commented Jun 18, 2020

@dpinol @vanbasten17 after doing some tests with the Google Analytics tracking I did a refactor of the track method to match correctly the fields of eventFields with the GA fields.

I've removed the async/await of track method because, as I said in a comment before, it doesn't make sense to stop the execution of the bot while doing the tracking.

I've also added more and better documentation of the track method.

@ericmarcos
Copy link
Contributor

@dpinol @vanbasten17 after doing some tests with the Google Analytics tracking I did a refactor of the track method to match correctly the fields of eventFields with the GA fields.

I've removed the async/await of track method because, as I said in a comment before, it doesn't make sense to stop the execution of the bot while doing the tracking.

I've also added more and better documentation of the track method.

@asastre please leave the async and state in the docs that you must await this call. As we're running this in Lambda, the main "thread" can finish and the process terminated leaving async calls that are not awaited terminated before they end the execution.

@asastre
Copy link
Contributor Author

asastre commented Jun 19, 2020

@dpinol @vanbasten17 @ericmarcos changes done. I restored the async/await in track method, as you said, in production the plugin will be executed in lambda and it needs to be sure it will execute.

@asastre asastre removed the WIP label Jun 19, 2020
@asastre asastre requested a review from vanbasten17 June 19, 2020 08:17
@dpinol
Copy link
Contributor

dpinol commented Jun 19, 2020

@asastre review the prettier errors in CI checks. More important, your pre-commit should not have let you comit this file. Please verify that you did "pre-commit install" on your botonic checkout
/home/runner/work/botonic/botonic/packages/botonic-plugin-google-analytics/src/index.js
##[error] 62:35 error Replace this.userId({·session·}),·this.userTraits({·session·}) with ⏎······this.userId({·session·}),⏎······this.userTraits({·session·})⏎···· prettier/prettier

@asastre
Copy link
Contributor Author

asastre commented Jun 19, 2020

@dpinol @ericmarcos I've made some changes/enhancements:

  • Refactor name methods defaultUserId and defaultUserTraits to getUserId and getUserTraits.
  • Refactor the option trackManually to automaticTracking to clarify its usage. If this option is not set, its default value will be true, enabling the automatic tracking on each user interaction.
  • Added a new option: eventFields to enable to send custom event fields to the automatic tracking done by the plugin (only used if automaticTracking is not set or is set to true)

…serTraits, options.eventFields to options.getUserId, options.getUserTraits, options.getEventFields for clarity
Copy link
Contributor

@dpinol dpinol left a comment

Choose a reason for hiding this comment

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

lgtm 🚀

…on't send userId/userTraits if getUserId is not set
@asastre asastre requested a review from dpinol June 20, 2020 14:40
Copy link
Contributor

@dpinol dpinol left a comment

Choose a reason for hiding this comment

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

can you comment on why you removed the default getUserId?

@asastre
Copy link
Contributor Author

asastre commented Jun 22, 2020

can you comment on why you removed the default getUserId?

I removed it because at first we were assuming that clientId and userId were the same for Google Analytics, but they are different concepts and userId is meant to represent a logged user, so the plugin can't add one by default, the user must provide one if they want to track it.

…rary with tracking through direct requests usin axios
@asastre
Copy link
Contributor Author

asastre commented Jun 25, 2020

@dpinol @ericmarcos The plugin wasn't working properly when tracking on the server side loosing events randomly and never reaching Google Analytics.
After trying various tracking packages (@analytics/google-analytics, universal-ga) and even proposing changes to one of them so the clientId parameter could be sent in the server side tracking (feature needed to keep the tracking flow for both client and server side), the tracking on the server side didn't work as expected, probably because it's executed in Lambda.
The new aproach (which is working fine) is to make direct requests to Google Analytics (using axios) sending the parameters needed each time.

@asastre asastre requested a review from dpinol June 25, 2020 09:42
@asastre asastre requested a review from dpinol July 1, 2020 13:02
@asastre asastre merged commit 3385f84 into master Jul 1, 2020
@asastre asastre deleted the feat/plugin-google-analytics branch July 29, 2021 14:12
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.

5 participants