forem icon indicating copy to clipboard operation
forem copied to clipboard

Comments area not paying attention to comments_order

Open benhalpern opened this issue 3 years ago • 2 comments

Describe the bug

In addition to the edge cache bug here: https://github.com/forem/forem/issues/17080, there appears to be a fragment cache issue where we are not passing down.

The bug is that comments are not refreshing, so I don't want to continue to be overly presumptive about the problem, but that appears to be the case.

This is somewhat of a duplicate of the other bug, but circumstantially different, so I feel like this can be a new issue with the other one closed.

To Reproduce

Same as https://github.com/forem/forem/issues/17080

Additional context

This fragment wraps the whole area:

<% cache("whole-comment-area-#{@article.id}-#{@article.last_comment_at}-#{@article.show_comments}-#{@discussion_lock&.updated_at}", expires_in: 2.hours) do %>

But without passing @comments_order into that cache key, it is effectively ignored within... Except if it is passed, then it is trapped as the result, meaning we could "cache" latest as "top".

Extremely relevant post on the subject: https://dev.to/devteam/the-three-caches-of-forem-492p

benhalpern avatar Apr 01 '22 23:04 benhalpern

Thanks for the issue, we will take it into consideration! Our team of engineers is busy working on many types of features, please give us time to get back to you.

Feature requests that require more discussion may be closed. Read more about our feature request process on forem.dev.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem-team. The OSS Community Manager or the engineers on OSS rotation will follow up.

For full info on how to contribute, please check out our contributors guide.

github-actions[bot] avatar Apr 01 '22 23:04 github-actions[bot]

@benhalpern I am trying to resolve this , I don't understand this statement

Except if it is passed, then it is trapped as the result, meaning we could "cache" latest as "top".

Is it not possible to just cache it as follows by adding comments order. I tried in my local dev and seems to be working as expected.

<% cache("whole-comment-area-#{@article.id}-#{@article.last_comment_at}-#{@article.show_comments}-#{@discussion_lock&.updated_at}-#{@comments_order}", expires_in: 2.hours) do %>

Followed by this as well

<%= render partial: "articles/comment_tree", collection: Comment.tree_for(@article, @comments_to_show_count, @comments_order), as: :comment_node, cached: proc { |comment, _sub_comments| [comment, @comments_order] } %>

vishaldeepak avatar Aug 13 '22 08:08 vishaldeepak

This has been fixed

benhalpern avatar Oct 20 '22 13:10 benhalpern