diff --git a/app/jobs/notifications/milestone_job.rb b/app/jobs/notifications/milestone_job.rb new file mode 100644 index 000000000..e4e93ecd2 --- /dev/null +++ b/app/jobs/notifications/milestone_job.rb @@ -0,0 +1,12 @@ +module Notifications + class MilestoneJob < ApplicationJob + queue_as :send_milestone_notification + + def perform(milestone_hash, service = Notifications::Milestone::Send) + article = Article.find_by(id: milestone_hash[:article_id]) + return unless article + + service.call(milestone_hash[:type], article) + end + end +end diff --git a/app/labor/article_analytics_fetcher.rb b/app/labor/article_analytics_fetcher.rb index bf08770da..c945d8b7d 100644 --- a/app/labor/article_analytics_fetcher.rb +++ b/app/labor/article_analytics_fetcher.rb @@ -19,11 +19,11 @@ class ArticleAnalyticsFetcher page_views_obj = pageviews.to_h chunk.each do |article| article.update_columns(previous_positive_reactions_count: article.positive_reactions_count) - Notification.send_milestone_notification(type: "Reaction", article: article) + Notification.send_milestone_notification(type: "Reaction", article_id: article.id) next if article.page_views_count > page_views_obj[article.id].to_i article.update_columns(page_views_count: page_views_obj[article.id].to_i) - Notification.send_milestone_notification(type: "View", article: article) + Notification.send_milestone_notification(type: "View", article_id: article.id) end end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 051c3ae36..bc5724c25 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -29,7 +29,7 @@ class Notification < ApplicationRecord # for now, arguments are always: notifiable = article, action = "Published" json_data = { user: user_data(notifiable.user), - article: article_data(notifiable) + article: Notifications.article_data(notifiable) } followers = if notifiable.organization_id json_data[:organization] = organization_data(notifiable.organization) @@ -130,29 +130,12 @@ class Notification < ApplicationRecord end def send_milestone_notification(milestone_hash) - milestone_hash[:next_milestone] = next_milestone(milestone_hash) - return unless should_send_milestone?(milestone_hash) - - json_data = { article: article_data(milestone_hash[:article]), gif_id: RandomGif.new.random_id } - - Notification.create!( - user_id: milestone_hash[:article].user_id, - notifiable_id: milestone_hash[:article].id, - notifiable_type: "Article", - json_data: json_data, - action: "Milestone::#{milestone_hash[:type]}::#{milestone_hash[:next_milestone]}", - ) - return unless milestone_hash[:article].organization_id - - Notification.create!( - organization_id: milestone_hash[:article].organization_id, - notifiable_id: milestone_hash[:article].id, - notifiable_type: "Article", - json_data: json_data, - action: "Milestone::#{milestone_hash[:type]}::#{milestone_hash[:next_milestone]}", - ) + Notifications::MilestoneJob.perform_later(milestone_hash) + end + + def send_milestone_notification_without_delay(milestone_hash) + Notifications::MilestoneJob.perform_now(milestone_hash) end - handle_asynchronously :send_milestone_notification def remove_all(notifiable_hash) Notification.where( @@ -193,6 +176,10 @@ class Notification < ApplicationRecord Notifications.comment_data(comment) end + def article_data(article) + Notifications.article_data(article) + end + def reaction_notification_attributes(reaction, receiver) reactable_data = { reactable_id: reaction.reactable_id, @@ -206,45 +193,6 @@ class Notification < ApplicationRecord def organization_data(organization) Notifications.organization_data(organization) end - - def article_data(article) - Notifications.article_data(article) - end - - def should_send_milestone?(milestone_hash) - return if milestone_hash[:article].published_at < Time.zone.local(2019, 2, 25) - - last_milestone_notification = Notification.find_by( - user_id: milestone_hash[:article].user_id, - notifiable_type: "Article", - notifiable_id: milestone_hash[:article].id, - action: "Milestone::#{milestone_hash[:type]}::#{milestone_hash[:next_milestone]}", - ) - - if milestone_hash[:type] == "View" - last_milestone_notification.blank? && milestone_hash[:article].page_views_count > milestone_hash[:next_milestone] - elsif milestone_hash[:type] == "Reaction" - last_milestone_notification.blank? && milestone_hash[:article].positive_reactions_count > milestone_hash[:next_milestone] - end - end - - def next_milestone(milestone_hash) - case milestone_hash[:type] - when "View" - milestones = [1024, 2048, 4096, 8192, 16_384, 32_768, 65_536, 131_072, 262_144, 524_288, 1_048_576] - milestone_count = milestone_hash[:article].page_views_count - when "Reaction" - milestones = [64, 128, 256, 512, 1024, 2048, 4096, 8192] - milestone_count = milestone_hash[:article].positive_reactions_count - end - - closest_number = milestones.min_by { |num| (milestone_count - num).abs } - if milestone_count > closest_number - closest_number - else - milestones[milestones.index(closest_number) - 1] - end - end end # instance methods diff --git a/app/services/notifications/milestone/send.rb b/app/services/notifications/milestone/send.rb new file mode 100644 index 000000000..467c1fd8a --- /dev/null +++ b/app/services/notifications/milestone/send.rb @@ -0,0 +1,84 @@ +module Notifications + module Milestone + ARTICLE_FINAL_PUBLICATION_TIME_FOR_MILESTONE = Time.zone.local(2019, 2, 25) + + class Send + # @param type [String] - "View" or "Reaction" + # @param article [Object] - ActiveRecord Article object + def initialize(type, article) + @type = type + @article = article + @next_milestone = next_milestone + end + + def self.call(*args) + new(*args).call + end + + def call + return unless should_send_milestone? + + create_notification(user_id: article.user_id) + return unless article.organization_id + + create_notification(organization_id: article.organization_id) + end + + private + + attr_reader :type, :article + + def create_notification(hash_id) + Notification.create!({ + notifiable_id: article.id, + notifiable_type: "Article", + json_data: json_data, + action: "Milestone::#{type}::#{@next_milestone}" + }.merge(hash_id)) + end + + def json_data + { article: Notifications.article_data(article), gif_id: RandomGif.new.random_id } + end + + def article_published_behind_time? + article.published_at < ARTICLE_FINAL_PUBLICATION_TIME_FOR_MILESTONE + end + + def should_send_milestone? + return if article_published_behind_time? + + last_milestone_notification = Notification.find_by( + user_id: article.user_id, + notifiable_type: "Article", + notifiable_id: article.id, + action: "Milestone::#{type}::#{@next_milestone}", + ) + + if type == "View" + last_milestone_notification.blank? && article.page_views_count > @next_milestone + elsif type == "Reaction" + last_milestone_notification.blank? && article.positive_reactions_count > @next_milestone + end + end + + def next_milestone + case type + when "View" + milestones = [1024, 2048, 4096, 8192, 16_384, 32_768, 65_536, 131_072, 262_144, 524_288, 1_048_576] + milestone_count = article.page_views_count + when "Reaction" + milestones = [64, 128, 256, 512, 1024, 2048, 4096, 8192] + milestone_count = article.positive_reactions_count + end + + closest_number = milestones.min_by { |num| (milestone_count - num).abs } + if milestone_count > closest_number + closest_number + else + milestones[milestones.index(closest_number) - 1] + end + end + end + end +end diff --git a/spec/jobs/notifications/milestone_job_spec.rb b/spec/jobs/notifications/milestone_job_spec.rb new file mode 100644 index 000000000..032e7398d --- /dev/null +++ b/spec/jobs/notifications/milestone_job_spec.rb @@ -0,0 +1,39 @@ +require "rails_helper" + +RSpec.describe Notifications::MilestoneJob do + include_examples "#enqueues_job", "send_milestone_notification", {} + + describe "#perform_now" do + let(:milestone_hash) do + { type: "Reaction", article_id: 456 } + end + let(:milestone_service) { double } + let(:article) { double } + + before do + allow(milestone_service).to receive(:call) + end + + specify "fetch article id only if :article_id key in hash " do + allow(Article).to receive(:find_by) + described_class.perform_now(milestone_hash, milestone_service) + expect(Article).to have_received(:find_by).with(id: 456) + end + + describe "When it does not find article from id" do + it "does not call the service" do + allow(Article).to receive(:find_by) + described_class.perform_now(milestone_hash, milestone_service) + expect(milestone_service).not_to have_received(:call) + end + end + + describe "When finds article from id" do + it "calls the service" do + allow(Article).to receive(:find_by).and_return(article) + described_class.perform_now(milestone_hash, milestone_service) + expect(milestone_service).to have_received(:call).with("Reaction", article) + end + end + end +end diff --git a/spec/labor/article_analytics_fetcher_spec.rb b/spec/labor/article_analytics_fetcher_spec.rb index 2e2907856..361cccf91 100644 --- a/spec/labor/article_analytics_fetcher_spec.rb +++ b/spec/labor/article_analytics_fetcher_spec.rb @@ -34,7 +34,7 @@ RSpec.describe ArticleAnalyticsFetcher do it "sends send_milestone_notification for Reaction and View" do %w[Reaction View].each do |type| - expect(Notification).to have_received(:send_milestone_notification).with(type: type, article: article) + expect(Notification).to have_received(:send_milestone_notification).with(type: type, article_id: article.id) end end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index a465b9da9..e9bbfe5ee 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -301,66 +301,6 @@ RSpec.describe Notification, type: :model do end end - describe "#send_milestone_notification" do - # milestones = [64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072, 262144] - let(:view_milestone_hash) { { type: "View", article: article } } - let(:reaction_milestone_hash) { { type: "Reaction", article: article } } - - context "when a user has never received a milestone notification" do - it "sends the appropriate level view milestone notification" do - Notification.send_milestone_notification_without_delay(view_milestone_hash) - expect(user.notifications.first.action).to include "2048" - end - - it "sends the appropriate level reaction milestone notification" do - Notification.send_milestone_notification_without_delay(reaction_milestone_hash) - expect(user.notifications.first.action).to include "64" - end - end - - context "when a user has received a milestone notification before" do - def mock_previous_view_milestone_notification - Notification.send_milestone_notification_without_delay(view_milestone_hash) - article.update_column(:page_views_count, 9001) - Notification.send_milestone_notification_without_delay(view_milestone_hash) - end - - def mock_previous_reaction_milestone_notification - Notification.send_milestone_notification_without_delay(reaction_milestone_hash) - article.update_column(:positive_reactions_count, 150) - Notification.send_milestone_notification_without_delay(reaction_milestone_hash) - end - - it "sends the appropriate level view milestone notification" do - mock_previous_view_milestone_notification - expect(user.notifications.second.action).to include "8192" - end - - it "sends the appropriate level reaction milestone notification" do - mock_previous_reaction_milestone_notification - expect(user.notifications.last.action).to include "128" - end - - it "adds an additional view milestone notification" do - mock_previous_view_milestone_notification - expect(user.notifications.count).to eq 2 - end - - it "does not the same view milestone notification if called again" do - mock_previous_view_milestone_notification - Notification.send_milestone_notification_without_delay(view_milestone_hash) - expect(user.notifications.count).to eq 2 - end - - it "does not send a view milestone notification again if the latest number of views is not past the next milestone" do - mock_previous_view_milestone_notification - article.update_column(:page_views_count, rand(9002..16_383)) - Notification.send_milestone_notification_without_delay(view_milestone_hash) - expect(user.notifications.count).to eq 2 - end - end - end - describe "#update_notifications" do context "when there are notifications to update" do before do diff --git a/spec/services/notifications/milestone/send_spec.rb b/spec/services/notifications/milestone/send_spec.rb new file mode 100644 index 000000000..20a71284f --- /dev/null +++ b/spec/services/notifications/milestone/send_spec.rb @@ -0,0 +1,87 @@ +require "rails_helper" + +RSpec.describe Notifications::Milestone::Send do + let(:user) { create(:user) } + let(:article) { create(:article, user_id: user.id, page_views_count: 4000, positive_reactions_count: 70) } + + context "when a user has never received a milestone notification" do + it "sends the appropriate level view milestone notification" do + send_milestone_notification_view + expect(user.notifications.first.action).to include "2048" + end + + it "sends the appropriate level reaction milestone notification" do + send_milestone_notification_reaction + expect(user.notifications.first.action).to include "64" + end + end + + context "when a user has received a milestone notification before" do + def mock_previous_view_milestone_notification + send_milestone_notification_view + article.update_column(:page_views_count, 9001) + send_milestone_notification_view + end + + def mock_previous_reaction_milestone_notification + send_milestone_notification_reaction + article.update_column(:positive_reactions_count, 150) + send_milestone_notification_reaction + end + + describe "When send view type milestone notification" do + before do + mock_previous_view_milestone_notification + end + + it "sends the appropriate level view milestone notification" do + expect(user.notifications.second.action).to include "8192" + end + + it "adds an additional view milestone notification" do + expect(user.notifications.count).to eq 2 + end + + it "does not the same view milestone notification if called again" do + send_milestone_notification_view + expect(user.notifications.count).to eq 2 + end + + it "does not send a view milestone notification again if the latest number of views is not past the next milestone" do + article.update_column(:page_views_count, rand(9002..16_383)) + send_milestone_notification_view + expect(user.notifications.count).to eq 2 + end + + it "checks notification json data", :aggregate_failures do + expect(user.notifications.where(notifiable_type: "Article").first.json_data["article"]["class"]["name"]).to eq "Article" + expect(user.notifications.where(notifiable_id: article.id).first.json_data["article"]["id"]).to eq article.id + expect(user.notifications.where(notifiable_id: article.id).first.json_data["article"]["title"]).to eq article.title + end + end + + it "sends the appropriate level reaction milestone notification" do + mock_previous_reaction_milestone_notification + expect(user.notifications.last.action).to include "128" + end + + context "when an article is related to an organization" do + let(:organization) { create(:organization) } + + before { article.organization = organization } + + it "creates another notification related to organization" do + coll = Notification.where(organization_id: article.organization_id) + expect { send_milestone_notification_reaction }.to change { coll.count }.by(1) + end + end + end +end + +def send_milestone_notification_view + described_class.call("View", article) +end + +def send_milestone_notification_reaction + described_class.call("Reaction", article) +end