-
Notifications
You must be signed in to change notification settings - Fork 422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Topic show action touches topic (updates updated_at) #696
Comments
A lot of important sites use this gem. I think the main reason it isn't maintained as frequently as before is because there's just not much more left to do. |
@hakkiplaten indeed, we have a good size project that has been using the gem for years. I only mentioned that because of the message in the readme:
|
Btw, @hakkiplaten, what are your thoughts on topics getting touched on every show action? |
@benjaminwood same here. I don't want to switch to Thredded. Assuming you've found a solution, could you explain a bit to us younger folks why and how you're doing fragment caching in Forem, and how it has improved your app's performance? |
Hey @c2169000 basically just implementing what DHH describes here: https://signalvnoise.com/posts/3113-how-key-based-cache-expiration-works Caching has made all the difference for us. Forem was becoming quite slow before we did the caching work. Literally 10+ seconds to load some topics. Now pretty much everything loads in milliseconds to 1.5 seconds for larger topics/forums. |
@c2169000 Just want to point out that Thredded provides a migration that copies all the data over from Forem so switching might easier than you thought: https://github.com/thredded/thredded/wiki/Migrate-from-Forem |
This line touches the topic (changes it's
updated_at
) when the topics/show action is hit. If you are like me and you're implementing fragment caching in your views and you are using theupdated_at
column to determine your cache_key, the cache will be invalidated on every request. I don't see any reason that it should touch the record.I know the gem isn't really maintained at the moment, but I can create a PR to change this behavior if somebody with merge power can confirm the current behavior is not desired.
Thanks!! 😄
The text was updated successfully, but these errors were encountered: