flarum / framework

Simple forum software for building great communities.
http://flarum.org/
6.37k stars 835 forks source link

[Approval] User comment count not updated when approving reply #3714

Closed clarkwinkelmann closed 1 year ago

clarkwinkelmann commented 1 year ago

Current Behavior

Approving a reply held for approval does not update the user comment_count property.

If a user is subject to approval in every tag they have access to, this means their comment_count value stays at zero until one of the post is deleted or if a discussion is approved, at which point it'll be recalculated.

Steps to Reproduce

  1. Configure "Reply to discussions without approval" permission to exclude the test user.
  2. Take note of test user comment_count value.
  3. Write reply on existing discussion with test user.
  4. Approve new post using an admin account.
  5. Check comment_count value on the test user. The value is still the same as before.

Expected Behavior

comment_count should be updated when a reply is approved.

Screenshots

No response

Environment

Output of php flarum info

Flarum core 1.6.2
PHP version: 8.1.13
MySQL version: 8.0.31-0ubuntu0.20.04.1
Loaded extensions: Core, date, libxml, openssl, pcre, zlib, filter, hash, json, pcntl, Reflection, SPL, session, standard, sodium, mysqlnd, PDO, xml, bcmath, bz2, calendar, ctype, curl, dom, mbstring, FFI, fileinfo, ftp, gd, gettext, iconv, imagick, exif, mysqli, pdo_mysql, Phar, posix, readline, shmop, SimpleXML, sockets, sysvmsg, sysvsem, sysvshm, tokenizer, xmlreader, xmlwriter, xsl, zip, Zend OPcache
+---------------------------+------------+------------------------------------------+
| Flarum Extensions         |            |                                          |
+---------------------------+------------+------------------------------------------+
| ID                        | Version    | Commit                                   |
+---------------------------+------------+------------------------------------------+
| flarum-tags               | v1.6.1     |                                          |
| flarum-lock               | v1.6.1     |                                          |
| flarum-flags              | v1.6.1     |                                          |
| v17development-blog       | v0.6.4     |                                          |
| flarum-sticky             | v1.6.1     |                                          |
| flarum-approval           | v1.6.1     |                                          |
| migratetoflarum-fake-data | dev-master | 45a06cb693e0413fbf7c748bc0b32c6411be9a49 |
| kilowhat-audit-free       | dev-master | 01f23babb7249bd586e04c7b80b2c1dfb12129bc |
| fof-byobu                 | 1.1.8      |                                          |
| flarum-subscriptions      | v1.6.1     |                                          |
| flarum-mentions           | v1.6.1     |                                          |
| flarum-markdown           | v1.6.1     |                                          |
| flarum-lang-french        | v4.3.0     |                                          |
| flarum-lang-english       | v1.6.0     |                                          |
| flarum-bbcode             | v1.6.0     |                                          |
+---------------------------+------------+------------------------------------------+
Base URL: http://1.6.flarum.localhost
Installation path: /home/clark/Projects/flarum-1.6
Queue driver: sync
Session driver: file
Mail driver: smtp
Debug mode: ON

Possible Solution

The code does call User::refreshCommentCount(), but on the discussion author rather than the post author. It looks it might have been an error when the code was written since it's not really needed to call that method when approving the first post of a discussion which wouldn't count towards the comment count value.

https://github.com/flarum/framework/blob/d7b9a03f31847c39631ba495df8f515509774610/extensions/approval/src/Listener/UpdateDiscussionAfterPostApproval.php#L35-L38

Additional Context

Reported here https://discuss.flarum.org/d/25055-first-post-approval/80

jslirola commented 1 year ago

Will this issue be included in milestone 1.7?

SychO9 commented 1 year ago

probably not

SychO9 commented 1 year ago

The code does call User::refreshCommentCount(), but on the discussion author rather than the post author. It looks it might have been an error when the code was written since it's not really needed to call that method when approving the first post of a discussion which wouldn't count towards the comment count value.

Actually, looking into the code, even the first post counts toward the comment count value, so it's not an error, it seems that's just the wanted behavior when introduced.