Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: Count all reactions as likes with exceptions controlled by a site setting #267

Merged
merged 17 commits into from Feb 13, 2024

Conversation

martin-brennan
Copy link
Contributor

@martin-brennan martin-brennan commented Jan 29, 2024

This commit changes the reaction paradigm to instead count all reactions as
Likes (via a PostAction record) rather than only considering a single specific
reaction emoji as a Like equivalent.

A new site setting, discourse_reactions_excluded_from_like, is introduced defaulting
only to have 👎 . Any emojis on this list will not create a PostAction record that
represents a Like.

We now create a PostAction record along with the existing ReactionUser record
for reactions which are not excluded from Like, to help with history correction if
discourse_reactions_excluded_from_like changes.

For existing settings:

  • discourse_reactions_reaction_for_like is still used, since we need an easy way to map
    regular Likes to reactions if the plugin is enabled/disabled. This cannot be set to anything
    in discourse_reactions_excluded_from_like. If this is changed, history will not get changed.
    Any reaction using this setting will only make a PostAction record.
  • discourse_reactions_like_icon is still used, since that is purely a visual setting.

Also included a job that performs data consistency operations whenever
discourse_reactions_excluded_from_like is changed, to create or trash
relevant PostAction records, and recover already trashed ones as needed.
Also UserAction records for LIKE and WAS_LIKED are created and deleted
as needed.

For now discourse_reactions_like_sync_enabled controls whether we create/trash
PostAction and UserAction records when the discourse_reactions_excluded_from_like
is changed. We are not including a migration until this is running in the wild for a few weeks.

* Added specs
* Added new site setting and hid the old ones
* Some minor ReactionManager refactors
@martin-brennan martin-brennan changed the title DEV: Initial work FEATURE: Count all reactions as likes except those on a deny list Jan 29, 2024
@martin-brennan martin-brennan changed the title FEATURE: Count all reactions as likes except those on a deny list FEATURE: Count all reactions as likes with exceptions controlled by a site setting Jan 29, 2024
@martin-brennan martin-brennan force-pushed the feature/all-reactions-as-likes-with-deny-list branch from 2e0fbbd to 44f132b Compare January 31, 2024 03:41
Also add the missing model annotations
@martin-brennan martin-brennan force-pushed the feature/all-reactions-as-likes-with-deny-list branch from 44f132b to d144088 Compare January 31, 2024 03:47
@martin-brennan martin-brennan force-pushed the feature/all-reactions-as-likes-with-deny-list branch from dd966e6 to e8e3033 Compare January 31, 2024 05:57
@martin-brennan martin-brennan force-pushed the feature/all-reactions-as-likes-with-deny-list branch from ad097b5 to de249db Compare February 2, 2024 05:27
@martin-brennan martin-brennan force-pushed the feature/all-reactions-as-likes-with-deny-list branch from 6be42e1 to 8d772cc Compare February 7, 2024 05:01
@martin-brennan martin-brennan marked this pull request as ready for review February 7, 2024 05:07
plugin.rb Show resolved Hide resolved
plugin.rb Outdated Show resolved Hide resolved
plugin.rb Show resolved Hide resolved
@SamSaffron
Copy link
Member

Overall looking very good, 👍 approving

* Add a flag to prevent sync / history changes by default, it's a hidden site setting `discourse_reactions_like_sync_enabled`
* Instead of deleting `PostAction` records in the synchronizer, set them to trashed
* Create and delete `UserAction` `LIKED` and `WAS_LIKED` records in the synchronizer
* Restore trashed `PostAction` records when possible in the synchronizer
* Leave out the initial data migration backfill for now
@martin-brennan martin-brennan force-pushed the feature/all-reactions-as-likes-with-deny-list branch from 32dfd9c to 9712ce7 Compare February 12, 2024 06:58
Also unhide the sync site setting so people can opt-in
if they want
@martin-brennan martin-brennan merged commit 2b2a728 into main Feb 13, 2024
5 checks passed
@martin-brennan martin-brennan deleted the feature/all-reactions-as-likes-with-deny-list branch February 13, 2024 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants