flarum / issue-archive

0 stars 0 forks source link

Flags not displayed when scrolling a discussion with posts already in the store #177

Open clarkwinkelmann opened 3 years ago

clarkwinkelmann commented 3 years ago

Bug Report

Current Behavior Some posts are not shown as flagged when scrolling in the discussion.

Steps to Reproduce

  1. Create long discussion (more than 20 posts), so that Flarum will have to load paginated posts when scrolling to the top/bottom.
  2. Mark discussion as read, so that clicking on the discussion loads the bottom.
  3. Flag the first post with any reason
  4. Refresh the browser on the homepage of the forum
  5. Open flags dropdown, but don't click
  6. Click on the discussion in the discussion list of the homepage
  7. Scroll to the top where the flagged post is
  8. Observe post does not has the "flagged" state visible

Expected Behavior The post should always be shown as flagged

Screenshots Left: when clicking the notification Right: when following the steps described above image

Environment

Cause When opening the flags list, the post is loaded into the store, but doesn't have the flags relationship loaded. When loading up the discussion from the notification, api/posts?filter[discussion]=<discussion>&page[near]=<post> is used, which re-loads the post and the flags relationship. When a discussion is opened from the discussion list, api/discussions/<discussion>?page[near]=<post> is used, which also re-loads the post and the flags relationship. However scrolling in a discussion uses api/posts?filter[id]=<posts> with only posts that are considered "not loaded" based on the check in PostStreamState:

https://github.com/flarum/core/blob/7055f6d9412bfdfbb5c5499cbecd45d9e6615d6d/js/src/forum/states/PostStreamState.js#L278-L282

Because a post without the flags relationship doesn't meet those criteria, it's not reloaded.

The error can also likely happen with other extensions, even if they don't interact with flags at all. As long as something loads a post together with its discussion relationship, this will also cause a skip during pagination.

Possible Solution We could manually set the reverse relationship when the flags list is loaded by the dropdown, but I don't think that would be easy since there can be multiple flags per post. It would also not fix the issue in case another extension loaded the post into the store. I thought we had an existing issue about this kind of solution but I can't find it anymore.

My preferred solution would be to force re-loading those posts without flags relationship when scrolling in a discussion. To do that however, we need to expose the logic that decides post re-loading to extensions. It's essentially what was proposed in https://github.com/flarum/core/issues/1844#issuecomment-691685594 , so implementing this would allow those other fixes as well.

askvortsov1 commented 3 years ago

My preferred solution would be to force re-loading those posts without flags relationship when scrolling in a discussion. To do that however, we need to expose the logic that decides post re-loading to extensions. It's essentially what was proposed in #1844 (comment) , so implementing this would allow those other fixes as well.

This makes sense to me, I'm in favor.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

Hari-Bonda commented 2 years ago

šŸ™‹šŸ¼ā€ā™‚ļø