Handle Reactions Without Users When Sending Notifications

[abdellani]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update

Description

I tried to fix #5508 by checking the reaction owners' existence in the database when creating reaction_siblings. The problem is that the query becomes slower compared to the original code.
If this solution is not acceptable, could you please give me some guidance to improve it?

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Added to documentation?

• docs.dev.to
• readme
• no documentation needed

[optional] What gif best describes this PR or how it makes you feel?

[rhymes]

Thanks @abdellani for taking this on!

The first thing that pops in my mind is: we should write a test that makes sure this solves the problem for good, what do you think?

You also mention the query becomes slower, can you post some data about it?

[abdellani]

I used the following code before the after the new code :

u=User.first
rxn = Reaction.create(
user_id: User.last.id,
category: "like",
reactable: u.articles.first
)

Notification.send_reaction_notification_without_delay(rxn, u)

Form the last instruction, I got the following results using the original code:

(byebug) reaction_siblings
  Reaction Load (3.4ms)  SELECT  "reactions".* FROM "reactions" WHERE "reactions"."reactable_id" = $1 AND "reactions"."reactable_type" = $2 AND "reactions"."user_id" != $3 ORDER BY created_at DESC LIMIT $4  [["reactable_id", 3], ["reactable_type", "Article"], ["user_id", 1], ["LIMIT", 11]]
  Article Load (1.2ms)  SELECT "articles".* FROM "articles" WHERE "articles"."id" = $1  [["id", 3]]
  User Load (0.9ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1  [["id", 10]]

After the update, I had :

(byebug) reaction_siblings
SQL (6.1ms) ...
  Article Load (0.8ms)  SELECT "articles".* FROM "articles" WHERE "articles"."id" = $1  [["id", 3]]
#<ActiveRecord::Relation [#<Reaction id: 1, category: "like", created_at: "2020-01-31 12:52:53", points: 1.0, reactable_id: 3, reactable_type: "Article", status: "valid", updated_at: "2020-01-31 12:52:53", user_id: 10>]>

User Load (0.9ms) and Reaction Load (3.4ms) are replaced with one instruction SQL (6.1ms). The database contains 25 articles, 10 users and 1 reaction.
I noticed that the SQL execution time variates. I tried to called reaction_siblings several times without changing anything in the database, and every time I get a different duration which can take up to 14ms under exactly the same context.
I wonder how it will perform under a bigger database.

[mstruve]

I second @rhymes, let's write a test for this otherwise I think the solution LGTM!

[mstruve]

LGTM! Thanks for adding the spec! 🚀

[benhalpern]

This looks good to me. We'll see in prod if it's meaningfully slower.