flarum / issue-archive

0 stars 0 forks source link

Sticky posts remain on top after a "mark all as read" is performed #138

Open brigcam opened 3 years ago

brigcam commented 3 years ago

Bug Report

Current Behavior Sticky topics should remain on top of the "All Discussions" view as long as they're unread. However, clicking on the "Mark All as Read" icon marks them as read, but they remain on top of the view. To un-stick them, one has to open each topic and scroll to its very bottom, only then the topic won't appear anymore on top (after a refresh).

Steps to Reproduce

  1. Go to the "All Discussions" View
  2. With some sticky topics on top, click on the "Mark All as Read" button then refresh the page

Expected Behavior The sticky topics shouldn't be on top anymore, but they're still there. The workaround is to open them one by one and scroll them to their bottom.

Screenshots I made a video available here: https://cdn.discordapp.com/attachments/268575717103501312/815763248963780689/2021-03-01_02-48-17.mkv

Environment

Flarum core 0.1.0-beta.15 (f8edc2d827fee47dc78fbb75ee1d3b28580ed11d)
PHP version: 7.4.15
Loaded extensions: Core, date, libxml, openssl, pcre, zlib, filter, hash, pcntl, Reflection, SPL, session, standard, sodium, mysqlnd, PDO, xml, calendar, ctype, curl, dom, mbstring, FFI, fileinfo, ftp, gd, gettext, iconv, igbinary, imagick, json, exif, mysqli, pdo_mysql, Phar, posix, readline, redis, shmop, SimpleXML, sockets, sysvmsg, sysvsem, sysvshm, tokenizer, xmlreader, xmlwriter, xsl, zip, Zend OPcache
+------------------------------+------------------+------------------------------------------+
| Flarum Extensions            |                  |                                          |
+------------------------------+------------------+------------------------------------------+
| ID                           | Version          | Commit                                   |
+------------------------------+------------------+------------------------------------------+
| fof-links                    | 0.5.4            |                                          |
| flarum-bbcode                | v0.1.0-beta.15   |                                          |
| fof-formatting               | 0.2.0            |                                          |
| flarum-lang-english          | v0.1.0-beta.15   | db94e7e178be298f266558953555cb6f5373a94e |
| flarum-emoji                 | v0.1.0-beta.15   | e51c59f0128e9c877ff61e2ce9efd63e861209b8 |
| flarum-likes                 | v0.1.0-beta.15   | 985ca7a2022148f64ac291c2e18c3300a0b1423c |
| flarum-lock                  | v0.1.0-beta.15   | 3dcbfc67af206e34095e32af5f96a603145db752 |
| flarum-markdown              | v0.1.0-beta.15   | d27e1087b37ac9ae5375252d6b9bff31ff6ca71d |
| flarum-mentions              | v0.1.0-beta.15   | e785d89b43e483d9b6f5d070b9828c87b7bcb4b6 |
| v17development-seo           | v1.6.2           |                                          |
| flarum-statistics            | v0.1.0-beta.15   | 8764dc652d29ba68bc5dd69b7892b7e5c88c23ef |
| flarum-sticky                | v0.1.0-beta.15.1 | 0b7a172e0eed21aef05f7220542390b2526a04ce |
| flarum-subscriptions         | v0.1.0-beta.15   | f1ab545679f4fd14633ebdf66742d804314749f4 |
| flarum-suspend               | v0.1.0-beta.15   | c00094d89baf32d9fae2e56fd82b54877eae4a06 |
| flarum-tags                  | v0.1.0-beta.15   | 9f3a12376998b3d43bb996dd5ab1b4d822e677c2 |
| matteocontrini-imgur-upload  | v3.6.0           |                                          |
| fof-sitemap                  | 0.6.0            |                                          |
| nearata-embed-video          | v1.1.1           |                                          |
| glowingblue-composer-preview | 0.2.0            |                                          |
+------------------------------+------------------+------------------------------------------+
Base URL: https://forum.gameloop.it
Installation path: /var/www/html/forum
Debug mode: off

Possible Solution I looked into the sticky plugin code, and here https://github.com/flarum/sticky/blob/master/src/Listener/PinStickiedDiscussionsToTop.php

I ran into this line (number 78), which I guess could be the culprit:

->whereColumn('sticky.last_read_post_number', '>=', 'last_post_number');

I guess this should be exchanged for a check of the general "read" status of the topic.

Additional Context None.

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!

askvortsov1 commented 3 years ago

The relevant source is now https://github.com/flarum/sticky/blob/02d8e6822bd02df85eda2252857b0974cfe1393c/src/PinStickiedDiscussionsToTop.php#L50-L50. We should filter out posts made before the global "read all" time.

https://github.com/flarum/core/blob/d82c093c0ff0984f70cfa52a2ea19cbdb5baa6ea/src/User/User.php#L273-L273 is how "all read" is set, so we'd use the marked_all_as_read_at field.