Previous post(s) attributes not updated after replying to it

[davwheat]

Bug Report

Title could be made clearer by a core dev, most likely. Not sure how to word it.

Current Behavior If two people (A, B) view a discussion, then person A edits a post, and person B replies to the edited post, person B sees the updated content in A's post, but no 'Edited' label shows next to the post. (Confusing, but see video for example.)

The same also occurs with custom post attributes used by extensions (e.g. fof/best-answer: previous post doesn't show as best answer even if it has been set).

Steps to Reproduce

1. Person A creates discussion
2. Person B views discussion
3. Person A edits their OP
4. Person B replies to A's OP
5. Person B views edited post content, but no Edited label displays

Expected Behavior The updated post should display the Edited label next to it (like it does after a full refresh).

Screenshots

https://user-images.githubusercontent.com/7406822/102798918-ab746c80-43a9-11eb-8c35-0088f619f4c1.mp4

Environment

• Flarum version: 0.1.0-beta.15
• Website URL: n/a
• Webserver: Apache
• Hosting environment: Docker in WSL2
• PHP version: 7.4.12
• Browser: Chrome 87 (and all others)

php flarum info

Flarum core 0.1.0-beta.15
PHP version: 7.4.12
Loaded extensions: Core, date, libxml, openssl, pcre, sqlite3, zlib, ctype, curl, dom, fileinfo, filter, ftp, hash, iconv, json, mbstring, SPL, PDO, session, posix, readline, Reflection, standard, SimpleXML, pdo_sqlite, Phar, tokenizer, xml, xmlreader, xmlwriter, mysqlnd, apcu, bcmath, calendar, exif, gd, geoip, gettext, gmp, intl, ldap, memcached, mysqli, pcntl, pdo_mysql, pdo_sqlsrv, redis, sodium, sqlsrv, sysvmsg, tidy, timezonedb, xsl, zip, Zend OPcache
+------------------------------+---------------------+------------------------------------------+
| Flarum Extensions            |                     |                                          |
+------------------------------+---------------------+------------------------------------------+
| ID                           | Version             | Commit                                   |
+------------------------------+---------------------+------------------------------------------+
| flarum-approval              | v0.1.0-beta.15      |                                          |
| flarum-bbcode                | v0.1.0-beta.15      |                                          |
| flarum-emoji                 | v0.1.0-beta.15      |                                          |
| flarum-lang-english          | v0.1.0-beta.15      |                                          |
| flarum-flags                 | v0.1.0-beta.15      |                                          |
| flarum-lock                  | v0.1.0-beta.15      |                                          |
| flarum-markdown              | v0.1.0-beta.15      |                                          |
| flarum-mentions              | v0.1.0-beta.15      |                                          |
| flarum-statistics            | v0.1.0-beta.15      |                                          |
| flarum-sticky                | v0.1.0-beta.15      |                                          |
| flarum-subscriptions         | v0.1.0-beta.15      |                                          |
| flarum-suspend               | v0.1.0-beta.15      |                                          |
| flarum-tags                  | v0.1.0-beta.15      |                                          |
| fof-formatting               | 0.3.1               |                                          |
| fof-user-bio                 | 0.4.0               |                                          |
| fof-ignore-users             | 0.3.0               |                                          |
| bokt-cache-assets            | 0.2                 |                                          |
| fof-impersonate              | 0.7.0               |                                          |
| fof-links                    | 0.5.0               |                                          |
| fof-merge-discussions        | 0.5.1               |                                          |
| fof-moderator-notes          | 0.4.0               |                                          |
| fof-profile-image-crop       | 0.2.1               |                                          |
| fof-best-answer              | dev-im/settingspage | aa8027a2d8c66056b0e3b78190234457f59bb983 |
| fof-reactions                | 0.5.0               |                                          |
| fof-sitemap                  | 0.6.0               |                                          |
| fof-byobu                    | 0.6.0               |                                          |
| fof-default-user-preferences | 0.3.1               |                                          |
| fof-follow-tags              | 0.6.1               |                                          |
| fof-split                    | 0.6.0               |                                          |
| fof-upload                   | 0.12.0              |                                          |
| ianm-follow-users            | dev-master          | 8df42c5775ac61f70e4aaa08212669ed65e4b189 |
+------------------------------+---------------------+------------------------------------------+
Base URL: http://localhost
Installation path: /var/www
Debug mode: ON
Don't forget to turn off debug mode! It should never be turned on in a production system.

Possible Solution

Additional Context The backend seems to not pass changed attributes, only contentHtml and some other info of the mentioned post. This means that the client gets the new content, but doesn't know that it was edited.

Either:

• we could compare the content and add the edited label in front-end JS, or
• we could pass all attributes, which will also fix these sorts of issues in extensions too

[askvortsov1]

This is (to an extent) on purpose. We don't see the editedAt attribute being passed when the post is loaded as a relation, because the BasicPostSerializer is used, which does not include this attribute. In contrast, when the post is loaded as the primary target, PostSerializer, which includes editedAt and other attributes, it used. I suppose editedAt could be moved to BasicPostSerializer, but it doesn't seem like an absolutely essential detail to include (for instance, whether a post has been hidden/soft deleted isn't transmitted either with BasicPostSerializer, and that could also be important to UX). We could also switch to using PostSerializer for the mentions relationships, but that could mean excessive overhead and slight performance decreases.

[davwheat]

I do think that switching to use PostSerializer might not be the way to go, for the reasons you mentioned, especially as Flarum and forums which use Flarum grow.

How realistic would it be to generate a hash of a post's details (attributes and content) and transmit this as part of the relationship. This could be compared to a client-side generated (or given when the post is initially loaded) hash, and, if they don't match, the post could be reloaded. This would solve the issue of transmitting all the post details multiple times, at the expense of a longer delay in updating posts (but as the chance of the post being updated is so slim, it won't matter much).

[askvortsov1]

How realistic would it be to generate a hash of a post's details (attributes and content) and transmit this as part of the relationship.

Well, to do that, we'd need to pull in the same data that would be needed if we were using PostSerializer, so no backend speed benefit there (just data transmission). Plus there'd have to be a mechanism watching for changed hashes and reloading posts if that's the case. It's possible, but the implementations that immediately come to mind would all be quite messy.

[davwheat]

Yeah, that's a good point about loading all the data. What about simply including edited_at? Or maybe a new column, like attributes_changed_at which should be updated if anything about the post changes, which can also be tapped in to by other extensions?

If this differs between the relationship provided by the server and the client's local copy, then the post is reloaded?

[askvortsov1]

That's possible, but we'd need to standardize it across all models somehow, or communicate to the frontend which models do/don't support it. Not inherently opposed as an eventual change though.

[stale[bot]]

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]]

We are closing this issue as it seems to have grown stale. If you still encounter this problem with the latest version, feel free to re-open it.