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
647 stars 121 forks source link

Remote form and AJAX request redirects never run events #152

Open jclusso opened 4 years ago

jclusso commented 4 years ago

I spent a lot of time reviewing the code and it's quite confusing to me why the Rack::Tracker#call method does this swap of moving the tracker hash in and out of the session. Because of this, if the request is not HTML, the tracker never gets stored in the session. Another question I have is why not just store the tracker hash in the session from the start? This swapping from env['tracker'] to the session['tracker'] just seems overly complex and I can't understand the purpose.

From what I can tell this seems to be the cause that remote forms and any AJAX request that redirects never get the events executed in the next HTML request. Since the Rack::Tracker#call method returns unless the request is HTML the env['tracker'] never moves into the session['tracker']. This means the events are just lost.

It appears #79 brings this issue up and #133 brings up a possible solution. #133 would be the most ideal solution but it's likely a pain and requires additional templates for every handler.

Right now I'm working on making this work using just the session and I'll submit a pull request. Just wanted to get the discussion started on this since I'm not sure why the session wasn't always just directly used.

DonSchado commented 4 years ago

Hi @jclusso! Thanks a lot for taking the effort. To be honest, I don't know exactly "why", but this library evolved from a former google analytics integration, trying to unify all the snippets into one place. I guess the architectural decisions just went over here without big reconsiderations.

But nonetheless thanks for challenging that. We will try to review the PR as soon as possible.

Could you already run this change in a production environment?

jclusso commented 4 years ago

Hey @DonSchado, it’s being used in production already. I think I fully figured out how everything works and I got the tests to pass. I wouldn’t say I’m 100% confident until someone that has some more experience with this library does a review.