Skip to content

Fix trending articles shows current article #791

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

Merged
merged 1 commit into from
Dec 23, 2021

Conversation

faissaloux
Copy link
Contributor

Fix #790

@driesvints
Copy link
Member

Why did you remove caching here? This will re-introduce the performance issues we had. This doesn't seem like the correct fix to the issue? The same article will still be present, no?

@faissaloux
Copy link
Contributor Author

The caching was the issue in this case.

When you open a random article, trending articles saved in the cache. After that if you open a trending article you got the cached trending articles in the bottom (Including the current article which is saved in the cache previously).

@driesvints
Copy link
Member

I see. In that case we should add a cache per article but that's probably not feasible for now. Thanks!

@driesvints driesvints merged commit 3b4a925 into laravelio:main Dec 23, 2021
@faissaloux faissaloux deleted the fix-trending-articles branch December 23, 2021 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

"Other articles" also shows current articles
2 participants