search

zevarito / mixpanel

Simple lib to track events in Mixpanel service. It can be used in any rack based framework.
MIT License
273 stars 84 forks source link

Make "persist" work with both 301 and 302 redirects #108

Closed levity closed 11 years ago

levity commented 11 years ago

This is important for Rails because when you setup a redirect in routes.rb, it uses a 301.

zevarito commented 11 years ago

Hi @levity,

Could you fix the test suite and also write a test for this contribution?

Thanks.

levity commented 11 years ago

It looks like the test suite is failing not due to my changes, but to some gem upgrades. Master fails as well when I run it locally. It's hard to tell exactly what, since Gemfile.lock is gitignored. I can dig into this but someone more familiar with the gem would probably be faster.

reconbot commented 11 years ago

The patch is good, however, I would argue that we should skip all 3xx responses as they usually don't include a body and if they do they're not always evaluated by browsers. I've opened a pull to do that.

https://en.wikipedia.org/wiki/HTTP_300#3xx_Redirection

zevarito commented 11 years ago

I agree that redirects should be ignored, the GET followed by the browser should generate the appropriate response.