search

railslove / rack-tracker

Tracking made easy: Don’t fool around with adding tracking and analytics partials to your app and concentrate on the things that matter.
https://www.railslove.com/open-source
MIT License
648 stars 121 forks source link

Allow for dynamic Facebook Pixel options #101

Closed smadeja closed 6 years ago

smadeja commented 6 years ago

This allows the Facebook Pixel id (as well as other tracker options) to be dynamic, and change on a per-request basis. It's based on some other trackers that offer this functionality (:user_id on the Google Analytics one, for example).

smadeja commented 6 years ago

@bumi Would you like me to make some more changes?

DonSchado commented 6 years ago

I personally don't like the use of a constant in the facebook handler. Since constants are hard to mock etc... I would prefer something like a hook method on the top handler class. If we do it on the class level it would also become more macro/dsl like.

But maybe this is something we can refactor later... and go with this solution for now...

bumi commented 6 years ago

nice! looks great! thanks again for the contribution.

@DonSchado I actually agree. also it would then be similar to the self.position= call. But that's also used in the other PR for the google stuff (https://github.com/railslove/rack-tracker/pull/97/files#diff-8c96bad81146eddcac1965e5645e68f3R2) @smadeja what do you think? we could do that the same way as: https://github.com/railslove/rack-tracker/blob/master/lib/rack/tracker/handler.rb#L13-L14

bumi commented 6 years ago

should actually be simple to replace, and I think it is more consistent then.

smadeja commented 6 years ago

The constant is dead. ;-)

bumi commented 6 years ago

awesome! let's merge! @DonSchado you're last chance to comment.

DonSchado commented 6 years ago

💚

DonSchado commented 6 years ago

Finally this is included in the last release 1.6.0, sorry that it took so long :/

smadeja commented 6 years ago

Sweet, thanks!