search

CultivateLabs / storytime

Storytime is a Rails 4+ CMS and blogging engine, with a core focus on content. It is built and maintained by @cultivatelabs
MIT License
752 stars 81 forks source link

fix Storytime::Site.current_id double assign problem #149

Closed rusikf closed 9 years ago

rusikf commented 9 years ago

See discussion on #143

dvanderbeek commented 9 years ago

Can you add a test that fails without this change? I don't think we have seen the bug you're experiencing, so it would be good to have a test that reproduces the issue.

Also, we should make sure that there's no way for Storytime::Site.current_id does not persist across requests without this ensure block.

rusikf commented 9 years ago

Does Storytime::Site.current_id should not be persisted ? I think, it should be persisted, otherwise, this whole logic will be invalid https://github.com/FlyoverWorks/storytime/blob/master/app/models/storytime/site.rb#L25-L35

dvanderbeek commented 9 years ago

We only want Storytime::Site.current_id to be set during the current request. If you watch Railscasts episode #388, there's a bit more explanation, but basically we want to clear out the current_id after the request in case the user switches to a different site or an exception is raised.

One issue might be that our around filter is not getting triggered in time before the storytime_snippet method is called. Can you test out changing scope_current_site back to the way it was, but use prepend_around_filter so that it gets triggered earlier?

rusikf commented 9 years ago

I can't remember how to reproduce this problem Now I created project with domain_name localhost:3000 and go to localhost:3000 main_app and snippet shows fine when sign in, it looks really strange behaviour.