search

elgentos / magento2-serversideanalytics

Server side analytics for Magento 2
MIT License
64 stars 21 forks source link

Add support for GA4 #14

Closed indykoning closed 2 years ago

indykoning commented 2 years ago

This PR will add an option to either enable UA, enable GA4 or enable both at the same time (although they would use the same tracking id which seems to work from the testing i've done) Config paths have been kept as is so it should not break existing installations (so long as no plugins have been made on the GAClient)

The old Model/GAClient had been renamed to Model/UAClient since this is the more appropriate name, other than that it shouldn't have changed.

peterjaap commented 2 years ago

LGTM! Saw you also fixed some small bugs, nice :)

peterjaap commented 2 years ago

@WouterSteen you also want to take a look before I merge this in?

DavidLambauer commented 2 years ago

@indykoning I think you forget to include https://github.com/br33f/php-GA4-Measurement-Protocol as a dependency to the composer.json. The whole thing would break otherwise, right? Besides that, the changes look good to me.

The new dependency is quite new and does not support all possible events. For example, I currently work on the add_to_cart event, which is not supported. This leads to the question if it might be more clever to not rely on a third part lib, or maybe contribute to the lib.

DavidLambauer commented 2 years ago

I have to correct myself. I just saw that the add_to_cart event is supported by the lib.

peterjaap commented 2 years ago

@DavidLambauer he did include the GA4 protocol; https://github.com/elgentos/magento2-serversideanalytics/pull/14/files#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34R6

DavidLambauer commented 2 years ago

Wow didn't see that at all. Big yikes. Then, I'd say let's merge it. The lib seems to be fine. We'll see if it stays maintained.

peterjaap commented 2 years ago

:partying_face: