Won't fix xf_post: Upgrade sets reactions and reaction_users to NULL, Thread\Replier sets them to "[]"

Steffen

Well-known member
Affected version
2.1 RC2
This is not strictly a bug but I think it's odd that (for posts without any likes/reactions) the upgrader sets the xf_post columns "reactions" and "reaction_score" to NULL but creating a new reply (using the Thread\Replier, for example) sets both columns to []. I guess the code treats both values the same but... as I said this feels a little odd. :)
 
Ok, so I've looked at this in a bit more detail.

There's a bit of an inconsistency in the thread title / post content here but to be clear I think we're talking about the reactions and reaction_users columns here (not the reaction_score column).

There's kind of different causes for this.

The change to reaction_users I actually think is a bug, but not directly related to the migration to reactions. It actually happens when it is still the like_users column.

It actually happens in other places too, in all actuality. It's related to the serialized to JSON approach:
PHP:
$db->update($tableName, [$column => ''], "{$column} = ?", ['a:0:{}']);
IMO, this should be:
PHP:
$db->update($tableName, [$column => '[]'], "{$column} = ?", ['a:0:{}']);
Which should solve that inconsistency. I've made that change now.

With regards to the reactions column, this is a new column entirely. Columns of this type cannot have a default value. The decision to make it nullable was partially for backwards compatibility. If we hadn't made it null, then any existing add-ons or integrations (especially maybe existing importers, though clearly we needed changes there anyway) would have all died when inserting records because it "doesn't have a default value". Making it nullable gets around this. We were kind of aware of the inconsistency this left in the data, but ultimately it's probably the lesser evil, and saves a blanket update query on potentially huge tables in the upgrader. The updates like that would have likely been quick, but on the whole a slight inconsistency when ultimately the values mean the same thing isn't a huge deal.
 
Thank you for the explanation and the fix/change! :)

There's a bit of an inconsistency in the thread title / post content here but to be clear I think we're talking about the reactions and reaction_users columns here (not the reaction_score column).
Oops. 😇
 
Top Bottom