From 97678456d66464495966e5f5c8cd01c44bc7b9a2 Mon Sep 17 00:00:00 2001 From: Vaidehi Joshi Date: Wed, 7 Apr 2021 06:46:32 -0700 Subject: [PATCH] [15 Min Fix]: Extract a post's followers out of service and onto Article model (#13229) * Move authors followers out of Notifications::NotifiableAction::Send and into Article model * Rename authors_followers to followers * Update followers method in service, oops * Update app/services/notifications/notifiable_action/send.rb Co-authored-by: Michael Kohl Co-authored-by: Michael Kohl --- app/models/article.rb | 13 +++++++++++++ .../notifications/notifiable_action/send.rb | 17 +++-------------- .../notifications/notifiable_action_worker.rb | 2 +- spec/models/article_spec.rb | 10 ++++++++++ 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/app/models/article.rb b/app/models/article.rb index df7a3bb6b..2f41a947a 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -435,6 +435,19 @@ class Article < ApplicationRecord doc.to_html end + def followers + # This will return an array, but the items will NOT be ActiveRecord objects. + # The followers may also occasionally be nil because orphaned follows can possibly exist in the database. + followers = user.followers_scoped.where(subscription_status: "all_articles").map(&:follower) + + if organization_id + org_followers = organization.followers_scoped.where(subscription_status: "all_articles") + followers += org_followers.map(&:follower) + end + + followers.uniq.compact + end + private def search_score diff --git a/app/services/notifications/notifiable_action/send.rb b/app/services/notifications/notifiable_action/send.rb index 107cc85ce..1068c7fb4 100644 --- a/app/services/notifications/notifiable_action/send.rb +++ b/app/services/notifications/notifiable_action/send.rb @@ -16,6 +16,8 @@ module Notifications end def call + return unless notifiable.is_a?(Article) + json_data = { user: user_data(notifiable.user), article: article_data(notifiable) @@ -23,9 +25,7 @@ module Notifications json_data[:organization] = organization_data(notifiable.organization) if notifiable.organization_id notifications_attributes = [] - # followers is an array and not an activerecord object - # followers can occasionally be nil because orphaned follows can possibly exist in the db (for now) - followers.sort_by(&:updated_at).reverse[0..10_000].each do |follower| + notifiable.followers.sort_by(&:updated_at).last(10_000).reverse_each do |follower| now = Time.current notifications_attributes.push( user_id: follower.id, @@ -53,17 +53,6 @@ module Notifications attr_reader :notifiable, :action - def followers - followers = notifiable.user.followers_scoped.where(subscription_status: "all_articles").map(&:follower) - - if notifiable.organization_id - org_followers = notifiable.organization.followers_scoped.where(subscription_status: "all_articles") - followers += org_followers.map(&:follower) - end - - followers.uniq.compact - end - def choose_upsert_index(action) return :index_notifications_on_user_notifiable_and_action_not_null if action.present? diff --git a/app/workers/notifications/notifiable_action_worker.rb b/app/workers/notifications/notifiable_action_worker.rb index d9d206c0c..3a4a914f0 100644 --- a/app/workers/notifications/notifiable_action_worker.rb +++ b/app/workers/notifications/notifiable_action_worker.rb @@ -4,7 +4,7 @@ module Notifications sidekiq_options queue: :low_priority, retry: 10 def perform(notifiable_id, notifiable_type, action) - # checking type, but leaving space for notifyable types + # checking type, but leaving space for notifiable types return unless notifiable_type == "Article" notifiable = notifiable_type.constantize.find_by(id: notifiable_id) diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 6835377c9..81602a8a1 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -1118,6 +1118,16 @@ RSpec.describe Article, type: :model do end end + describe "#followers" do + it "returns an array of users who follow the article's author" do + following_user = create(:user) + following_user.follow(user) + + expect(article.followers.length).to eq(1) + expect(article.followers.first.username).to eq(following_user.username) + end + end + describe "#update_score" do it "stably sets the correct blackbox values" do create(:reaction, reactable: article, points: 1)