diff --git a/app/concerns/concealable.rb b/app/concerns/concealable.rb index 6b5b2c7294..ec910778c4 100644 --- a/app/concerns/concealable.rb +++ b/app/concerns/concealable.rb @@ -13,6 +13,8 @@ module Concealable registered: 4, # all registered accounts, both full and read-only }, _prefix: true + after_commit :notify_followers_privacy, on: :update + def visible_to?(user) # does not support access lists at this time return true if privacy_public? @@ -23,5 +25,17 @@ def visible_to?(user) return true if user.admin? user.id == user_id end + + def notify_followers_privacy + return unless saved_change_to_privacy? + change = saved_change_to_privacy + if ['access_list', 'private'].include?(change[0]) && ['public', 'registered'].include?(change[1]) + NotifyFollowersOfNewPostJob.perform_later(self.id, [], 'public') + elsif change[0] == 'private' && change[1] == 'access_list' + viewers.each do |viewer| + NotifyFollowersOfNewPostJob.perform_later(self.id, viewer.id, 'access') + end + end + end end end diff --git a/app/concerns/owable.rb b/app/concerns/owable.rb index 0a6a6d2960..1befe38ed4 100644 --- a/app/concerns/owable.rb +++ b/app/concerns/owable.rb @@ -20,6 +20,7 @@ module Owable after_create :add_creator_to_authors after_save :update_board_cameos + after_commit :notify_followers, on: :create attr_accessor :private_note @@ -59,5 +60,10 @@ def update_board_cameos return if new_cameos.empty? new_cameos.each { |author| board.board_authors.create!(user_id: author, cameo: true) } end + + def notify_followers + return if is_import + NotifyFollowersOfNewPostJob.perform_later(self.id, [], 'new') + end end end diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 93b0dbe764..8680fce764 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class NotificationsController < ApplicationController before_action :login_required diff --git a/app/helpers/notification_helper.rb b/app/helpers/notification_helper.rb index f1252f9eaa..363c376264 100644 --- a/app/helpers/notification_helper.rb +++ b/app/helpers/notification_helper.rb @@ -1,9 +1,14 @@ +# frozen_string_literal: true module NotificationHelper NOTIFICATION_MESSAGES = { - 'import_success' => 'Post import succeeded', - 'import_fail' => 'Post import failed', - 'new_favorite_post' => 'An author you favorited has written a new post', - 'joined_favorite_post' => 'An author you favorited has joined a post', + 'import_success' => 'Post import succeeded', + 'import_fail' => 'Post import failed', + 'new_favorite_post' => 'An author you favorited has written a new post', + 'joined_favorite_post' => 'An author you favorited has joined a post', + 'accessible_favorite_post' => 'An author you favorited has given you access to a post', + 'published_favorite_post' => 'An author you favorited has made a post public', + 'resumed_favorite_post' => 'A favorite post has resumed', + 'coauthor_invitation' => 'You have been invited as an author for a post', } def subject_for_type(notification_type) diff --git a/app/jobs/notify_coauthors_job.rb b/app/jobs/notify_coauthors_job.rb new file mode 100644 index 0000000000..0cd9280835 --- /dev/null +++ b/app/jobs/notify_coauthors_job.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +class NotifyCoauthorsJob < ApplicationJob + queue_as :notifier + + def perform(post_id, user_id) + post = Post.find_by(id: post_id) + user = User.find_by(id: user_id) + return unless post && user + return if post.privacy_private? + return if post.privacy_access_list? && post.viewers.exclude?(user) + + Notification.notify_user(user, :coauthor_invitation, post: post) + end +end diff --git a/app/jobs/notify_followers_of_new_post_job.rb b/app/jobs/notify_followers_of_new_post_job.rb index 9721b37755..1211bdc38e 100644 --- a/app/jobs/notify_followers_of_new_post_job.rb +++ b/app/jobs/notify_followers_of_new_post_job.rb @@ -2,24 +2,54 @@ class NotifyFollowersOfNewPostJob < ApplicationJob queue_as :notifier - def perform(post_id, user_id) + ACTIONS = ['new', 'join', 'access', 'public', 'active'] + + def perform(post_id, user_id, action) post = Post.find_by(id: post_id) - user = User.find_by(id: user_id) - return unless post && user + return unless post && ACTIONS.include?(action) return if post.privacy_private? - if post.user_id == user_id - notify_of_post_creation(post, user) + if ['join', 'access'].include?(action) + user = User.find_by(id: user_id) + return unless user + end + + case action + when 'new' + notify_of_post_creation(post) + when 'join' + notify_of_post_joining(post, user) + when 'access' + notify_of_post_access(post, user) + when 'public' + notify_of_post_publication(post) + when 'active' + notify_of_post_activity(post) + end + end + + def self.notification_about(post, user, unread_only: false, exclude_resumed: false) + previous_types = [:new_favorite_post, :joined_favorite_post, :accessible_favorite_post, :published_favorite_post] + previous_types << :resumed_favorite_post unless exclude_resumed + notif = Notification.find_by(post: post, notification_type: previous_types) + if notif + return notif if !unread_only || notif.unread else - notify_of_post_joining(post, user) + messages = Message.where(recipient: user, sender_id: 0).where('created_at >= ?', post.created_at) + messages = messages.unread if unread_only + messages.find_each do |notification| + return notification if notification.message.include?(ScrapePostJob.view_post(post.id)) + end end + nil end - def notify_of_post_creation(post, post_user) - favorites = Favorite.where(favorite: post_user).or(Favorite.where(favorite: post.board)) - user_ids = favorites.select(:user_id).distinct.pluck(:user_id) - users = filter_users(post, user_ids) + private + def notify_of_post_creation(post) + favorites = favorites_for(post) + user_ids = favorites.select(:user_id).distinct.pluck(:user_id) + users = filter_users(post, user_ids, skip_previous: true) return if users.empty? users.each { |user| Notification.notify_user(user, :new_favorite_post, post: post) } @@ -28,39 +58,47 @@ def notify_of_post_creation(post, post_user) def notify_of_post_joining(post, new_user) users = filter_users(post, Favorite.where(favorite: new_user).pluck(:user_id)) return if users.empty? + users.each { |user| Notification.notify_user(user, :joined_favorite_post, post: post) } + end - users.each do |user| - next if already_notified_about?(post, user) - Notification.notify_user(user, :joined_favorite_post, post: post) - end + def notify_of_post_access(post, viewer) + return if filter_users(post, [viewer.id]).empty? + return unless favorites_for(post).where(user: viewer).exists? + Notification.notify_user(viewer, :accessible_favorite_post, post: post) + end + + def notify_of_post_publication(post) + favorites = favorites_for(post) + users = filter_users(post, favorites.select(:user_id).distinct.pluck(:user_id)) + return if users.empty? + users.each { |user| Notification.notify_user(user, :published_favorite_post, post: post) } + end + + def notify_of_post_activity(post) + favorites = favorites_for(post).or(Favorite.where(favorite: post)) + users = filter_users(post, favorites.select(:user_id).distinct.pluck(:user_id), skip_resumed: true) + users = users.reject { |user| self.class.notification_about(post, user, unread_only: true).present? } + return if users.empty? + users.each { |user| Notification.notify_user(user, :resumed_favorite_post, post: post) } end - def filter_users(post, user_ids) + def favorites_for(post) + Favorite.where(favorite: post.authors).or(Favorite.where(favorite: post.board)) + end + + def filter_users(post, user_ids, skip_previous: false, skip_resumed: false) user_ids &= PostViewer.where(post: post).pluck(:user_id) if post.privacy_access_list? user_ids -= post.author_ids user_ids -= blocked_user_ids(post) return [] unless user_ids.present? users = User.where(id: user_ids, favorite_notifications: true) users = users.full if post.privacy_full_accounts? - users + return users if skip_previous + users.reject { |user| already_notified_about?(post, user, exclude_resumed: skip_resumed) } end - def already_notified_about?(post, user) - self.class.notification_about(post, user).present? - end - - def self.notification_about(post, user, unread_only: false) - notif = Notification.find_by(post: post, notification_type: [:new_favorite_post, :joined_favorite_post]) - if notif - return notif if !unread_only || notif.unread - else - messages = Message.where(recipient: user, sender_id: 0).where('created_at >= ?', post.created_at) - messages = messages.unread if unread_only - messages.find_each do |notification| - return notification if notification.message.include?(ScrapePostJob.view_post(post.id)) - end - end - nil + def already_notified_about?(post, user, exclude_resumed: false) + self.class.notification_about(post, user, exclude_resumed: exclude_resumed).present? end def blocked_user_ids(post) diff --git a/app/models/notification.rb b/app/models/notification.rb index a9a45f2418..64894371ba 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class Notification < ApplicationRecord belongs_to :user, inverse_of: :notifications, optional: false belongs_to :post, inverse_of: :notifications, optional: true @@ -20,6 +21,10 @@ class Notification < ApplicationRecord import_fail: 1, new_favorite_post: 2, joined_favorite_post: 3, + accessible_favorite_post: 4, + published_favorite_post: 5, + resumed_favorite_post: 6, + coauthor_invitation: 7, } attr_accessor :skip_email diff --git a/app/models/post.rb b/app/models/post.rb index eaecba708a..0161556e34 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -44,7 +44,6 @@ class Post < ApplicationRecord before_validation :set_last_user, on: :create before_create :build_initial_flat_post, :set_timestamps before_update :set_timestamps - after_commit :notify_followers, on: :create after_commit :invalidate_caches, on: :update NON_EDITED_ATTRS = %w(id created_at updated_at edited_at tagged_at last_user_id last_reply_id section_order) @@ -268,10 +267,6 @@ def last_user_deleted? last_user.deleted? end - def user_joined(user) - NotifyFollowersOfNewPostJob.perform_later(self.id, user.id) - end - def prev_post(user) adjacent_posts_for(user) { |relation| relation.reverse_order.find_by('section_order < ?', self.section_order) } end @@ -335,11 +330,6 @@ def reset_warnings(_warning) Post::View.where(post_id: id).update_all(warnings_hidden: false) # rubocop:disable Rails/SkipsModelValidations end - def notify_followers - return if is_import - NotifyFollowersOfNewPostJob.perform_later(self.id, user_id) - end - def invalidate_caches return unless saved_change_to_authors_locked? Post::Author.clear_cache_for(authors) diff --git a/app/models/post/author.rb b/app/models/post/author.rb index 1af4e460ff..adbec861f4 100644 --- a/app/models/post/author.rb +++ b/app/models/post/author.rb @@ -6,10 +6,8 @@ class Post::Author < ApplicationRecord validates :user, uniqueness: { scope: :post } after_commit :invalidate_caches, on: [:create, :destroy] - - def invalidate_caches - self.class.clear_cache_for(user) - end + after_commit :notify_followers, on: :create + after_commit :notify_coauthor, on: :create def self.clear_cache_for(authors) blocked_ids = Block.where(blocking_user: authors, hide_me: [:posts, :all]).pluck(:blocked_user_id) @@ -17,4 +15,24 @@ def self.clear_cache_for(authors) hiding_ids = Block.where(blocked_user: authors, hide_them: [:posts, :all]).pluck(:blocking_user_id) hiding_ids.each { |blocker| Rails.cache.delete(Block.cache_string_for(blocker, 'hidden')) } end + + private + + def invalidate_caches + self.class.clear_cache_for(user) + end + + def notify_coauthor + return if post.is_import || post.last_reply&.is_import + return if user_id == post.user_id + return if joined? + NotifyCoauthorsJob.perform_later(post_id, user_id) + end + + def notify_followers + return if post.is_import || post.last_reply&.is_import + return if user_id == post.user_id + return unless joined? + NotifyFollowersOfNewPostJob.perform_later(post_id, user_id, 'join') + end end diff --git a/app/models/post/status.rb b/app/models/post/status.rb index 2a274d70b7..f59dc6ebb5 100644 --- a/app/models/post/status.rb +++ b/app/models/post/status.rb @@ -10,8 +10,17 @@ module Post::Status abandoned: 3, } + after_commit :notify_followers_activity, on: :update + def on_hiatus? hiatus? || (active? && tagged_at < 1.month.ago) end + + private + + def notify_followers_activity + return unless status_before_last_save == 'abandoned' + NotifyFollowersOfNewPostJob.perform_later(self.id, [], 'active') + end end end diff --git a/app/models/post_viewer.rb b/app/models/post_viewer.rb index 14ff4beac7..b38b1c86a4 100644 --- a/app/models/post_viewer.rb +++ b/app/models/post_viewer.rb @@ -6,6 +6,7 @@ class PostViewer < ApplicationRecord validates :post, uniqueness: { scope: :user } after_commit :invalidate_cache + after_commit :notify_followers, on: :create CACHE_VERSION = 2 @@ -18,4 +19,9 @@ def self.cache_string_for(user_id) def invalidate_cache Rails.cache.delete(PostViewer.cache_string_for(self.user.id)) end + + def notify_followers + return unless post.privacy_access_list? + NotifyFollowersOfNewPostJob.perform_later(post_id, user_id, 'access') + end end diff --git a/app/models/reply.rb b/app/models/reply.rb index 931e58c674..26df584efa 100644 --- a/app/models/reply.rb +++ b/app/models/reply.rb @@ -17,6 +17,7 @@ class Reply < ApplicationRecord after_update :update_post after_destroy :set_previous_reply_to_last, :remove_post_author, :update_flat_post after_save :update_flat_post + after_create_commit :notify_followers_if_hiatused attr_accessor :skip_notify, :skip_post_update, :is_import, :skip_regenerate @@ -121,9 +122,6 @@ def update_post_authors else post.post_authors.create!(user_id: user_id, joined: true, joined_at: created_at) end - - return if is_import - post.user_joined(user) end def remove_post_author @@ -142,6 +140,12 @@ def remove_post_author end end + def notify_followers_if_hiatused + last_tagged = post.replies.ordered.second_to_last.present? ? post.replies.ordered.second_to_last.updated_at : post.edited_at + return unless (post.saved_change_to_status? && post.status_before_last_save == 'hiatus') || last_tagged < 1.month.ago + NotifyFollowersOfNewPostJob.perform_later(post.id, [], 'active') + end + def ordered_attributes [:post_id] end diff --git a/script/migrate_notifications.rb b/script/migrate_notifications.rb index 644b55a383..59b7a008fd 100644 --- a/script/migrate_notifications.rb +++ b/script/migrate_notifications.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true def migrate_notifications site_messages = Message.where(sender_id: 0, notification_id: nil).ordered_by_id import_success_messages = site_messages.where(subject: 'Post import succeeded') diff --git a/spec/jobs/notify_coauthors_job_spec.rb b/spec/jobs/notify_coauthors_job_spec.rb new file mode 100644 index 0000000000..42011bedd6 --- /dev/null +++ b/spec/jobs/notify_coauthors_job_spec.rb @@ -0,0 +1,141 @@ +RSpec.describe NotifyCoauthorsJob do + include ActiveJob::TestHelper + + let(:author) { create(:user) } + let(:coauthor) { create(:user) } + let(:unjoined) { create(:user) } + let(:post) { create(:post, user: author, unjoined_authors: [coauthor, unjoined]) } + + before(:each) { clear_enqueued_jobs } + + context "validations" do + let(:user) { create(:user) } + let(:post) { create(:post) } + + it "does nothing with invalid post id" do + expect { + NotifyCoauthorsJob.perform_now(-1, user.id) + }.not_to change { Notification.count } + end + + it "does nothing with invalid coauthor id" do + expect { + NotifyCoauthorsJob.perform_now(post.id, -1) + }.not_to change { Notification.count } + end + + it "does nothing if post is private" do + post.update!(privacy: :private) + expect { + NotifyCoauthorsJob.perform_now(post.id, user.id) + }.not_to change { Notification.count } + end + + it "does nothing if potential author cannot view post" do + post.update!(privacy: :access_list) + expect { + NotifyCoauthorsJob.perform_now(post.id, user.id) + }.not_to change { Notification.count } + end + + it "works" do + expect { + NotifyCoauthorsJob.perform_now(post.id, user.id) + }.to change { Notification.count }.by(1) + notif = Notification.last + expect(notif.notification_type).to eq('coauthor_invitation') + expect(notif.user).to eq(user) + expect(notif.post).to eq(post) + end + end + + context "on post creation" do + it "does not notify post author" do + expect { perform_enqueued_jobs { post } }.not_to change { Notification.where(user: author, notification_type: :coauthor_invitation).count } + end + + it "notifies all listed coauthors" do + expect { perform_enqueued_jobs { post } }.to change { Notification.where(notification_type: :coauthor_invitation).count }.by(2) + notifs = Notification.where(notification_type: :coauthor_invitation) + expect(notifs.pluck(:user_id)).to match_array([coauthor.id, unjoined.id]) + expect(notifs.pluck(:post_id)).to eq([post.id, post.id]) + end + + it "does not notify if post is private" do + expect { + perform_enqueued_jobs do + create(:post, user: author, privacy: :private, unjoined_authors: [coauthor, unjoined]) + end + }.not_to change { Notification.where(notification_type: :coauthor_invitation).count } + end + + it "only notifies for authors with access" do + expect { + perform_enqueued_jobs do + create(:post, user: author, privacy: :access_list, viewers: [coauthor], unjoined_authors: [coauthor, unjoined]) + end + }.to change { Notification.where(notification_type: :coauthor_invitation).count }.by(1) + notif = Notification.find_by(notification_type: :coauthor_invitation) + expect(notif.user).to eq(coauthor) + end + + it "does not queue for imported posts" do + create(:post, user: author, unjoined_authors: [coauthor, unjoined], is_import: true) + expect(NotifyCoauthorsJob).not_to have_been_enqueued + end + end + + context "on author add" do + let(:new) { create(:user) } + + before(:each) { create(:reply, post: post, user: coauthor) } + + it "works" do + expect { + perform_enqueued_jobs do + post.authors << new + end + }.to change { Notification.where(notification_type: :coauthor_invitation).count }.by(1) + notif = Notification.find_by(notification_type: :coauthor_invitation) + expect(notif.user).to eq(new) + end + + it "does not notify if post is private" do + post.update!(privacy: :private) + expect { + perform_enqueued_jobs do + post.authors << new + end + }.not_to change { Notification.where(notification_type: :coauthor_invitation).count } + end + + it "does not notify if author does not have access" do + post.update!(privacy: :access_list, viewers: [coauthor, unjoined]) + expect { + perform_enqueued_jobs do + post.authors << new + end + }.not_to change { Notification.where(notification_type: :coauthor_invitation).count } + end + end + + context "on reply" do + let(:replier) { create(:user) } + + before(:each) { create(:reply, post: post, user: coauthor) } + + it "does not notify" do + expect { + perform_enqueued_jobs do + create(:reply, user: replier, post: post) + end + }.not_to change { Notification.where(notification_type: :coauthor_invitation).count } + end + + it "does not queue on imported posts" do + clear_enqueued_jobs + create(:reply, user: replier, post: post, is_import: true) + expect(NotifyCoauthorsJob).not_to have_been_enqueued + end + end +end diff --git a/spec/jobs/notify_followers_of_new_post_job_spec.rb b/spec/jobs/notify_followers_of_new_post_job_spec.rb index 67eaf69290..a003a9da92 100644 --- a/spec/jobs/notify_followers_of_new_post_job_spec.rb +++ b/spec/jobs/notify_followers_of_new_post_job_spec.rb @@ -1,84 +1,129 @@ RSpec.describe NotifyFollowersOfNewPostJob do include ActiveJob::TestHelper + + let(:author) { create(:user) } + let(:coauthor) { create(:user) } + let(:unjoined) { create(:user) } + let(:notified) { create(:user) } + let(:board) { create(:board) } + before(:each) { clear_enqueued_jobs } - it "does nothing with invalid post id" do - expect(Favorite).not_to receive(:where) - user = create(:user) - NotifyFollowersOfNewPostJob.perform_now(-1, user.id) + context "validations" do + it "does nothing with invalid post id" do + expect(Favorite).not_to receive(:where) + user = create(:user) + NotifyFollowersOfNewPostJob.perform_now(-1, user.id, 'new') + end + + it "does nothing with invalid user id on join" do + expect(Favorite).not_to receive(:where) + post = create(:post) + NotifyFollowersOfNewPostJob.perform_now(post.id, -1, 'join') + end + + it "does nothing with invalid user id on access" do + expect(Favorite).not_to receive(:where) + post = create(:post) + NotifyFollowersOfNewPostJob.perform_now(post.id, -1, 'access') + end + + it "does nothing with invalid action" do + expect(Favorite).not_to receive(:where) + post = create(:post) + NotifyFollowersOfNewPostJob.perform_now(post.id, post.user_id, '') + end end - it "does nothing with invalid user id" do - expect(Favorite).not_to receive(:where) - post = create(:post) - NotifyFollowersOfNewPostJob.perform_now(post.id, -1) + shared_examples "general" do + it "works" do + expect { perform_enqueued_jobs { do_action } }.to change { Notification.where(user: notified).count }.by(1) + notif = Notification.where(user: notified).last + expect(notif.notification_type).to eq(type) + expect(notif.post).to eq(Post.last) + end + + it "does not send if reader has config disabled" do + notified.update!(favorite_notifications: false) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.where(user: notified).count } + end end - context "on new posts" do - let(:author) { create(:user) } - let(:coauthor) { create(:user) } - let(:notified) { create(:user) } - let(:board) { create(:board) } - let(:title) { 'test subject' } + shared_examples 'authors' do + it "does not send to authors" do + Favorite.delete_all + authors = [author, coauthor, unjoined].reject { |u| u == favorite } + authors.each { |u| create(:favorite, user: u, favorite: favorite) } + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.where.not(notification_type: :coauthor_invitation).count } + end + end - shared_examples "new" do - it "works" do - expect { - perform_enqueued_jobs do - create(:post, user: author, unjoined_authors: [coauthor], board: board, subject: title) - end - }.to change { Notification.count }.by(1) - author_msg = Notification.where(user: notified).last - expect(author_msg.notification_type).to eq('new_favorite_post') - expect(author_msg.post).to eq(Post.last) - end + shared_examples 'privacy' do + it "does not send for private posts" do + expect { perform_enqueued_jobs { do_action(privacy: :private) } }.not_to change { Notification.count } + end - it "does not send for private posts" do - expect { - perform_enqueued_jobs do - create(:post, user: author, board: board, privacy: :private) - end - }.not_to change { Notification.count } + it "does not send to readers for full accounts privacy posts" do + unnotified = create(:reader_user) + create(:favorite, user: unnotified, favorite: author) + perform_enqueued_jobs do + create(:post, user: author, board: board, privacy: :full_accounts) end - it "does not send to readers for full accounts privacy posts" do - unnotified = create(:reader_user) - create(:favorite, user: unnotified, favorite: author) - perform_enqueued_jobs do - create(:post, user: author, board: board, privacy: :full_accounts) - end + expect { + perform_enqueued_jobs { do_action(privacy: :full_accounts) } + }.not_to change { Notification.where(user: unnotified).count } + end + + it "does not send to non-viewers for access-locked posts" do + unnotified = create(:user) + create(:favorite, user: unnotified, favorite: favorite) + expect { + perform_enqueued_jobs { do_action(privacy: :access_list, viewers: [coauthor, notified]) } + }.not_to change { Notification.where(user: unnotified).count } + end + end + + shared_examples 'blocking' do + context "with blocking" do + before(:each) { create(:favorite, user: notified, favorite: board) } - # don't use change format because other non-reader users may be notified - expect(Message.where(recipient_id: unnotified.id).count).to eq(0) + it "does not send to users the poster has blocked" do + create(:block, blocking_user: author, blocked_user: notified, hide_me: :posts) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.where(user: notified).count } end - it "does not send to non-viewers for access-locked posts" do - unnotified = create(:user) - create(:favorite, user: unnotified, favorite: favorite) - expect { - perform_enqueued_jobs do - create(:post, user: author, board: board, unjoined_authors: [coauthor], privacy: :access_list, viewers: [coauthor, notified]) - end - }.to change { Notification.count }.by(1) - expect(Notification.where(user: unnotified)).not_to be_present + it "does not send to users a coauthor has blocked" do + create(:block, blocking_user: coauthor, blocked_user: notified, hide_me: :posts) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.where(user: notified).count } end - it "does not send if reader has config disabled" do - notified.update!(favorite_notifications: false) - expect { - perform_enqueued_jobs do - create(:post, user: author, board: board) - end - }.not_to change { Notification.count } + it "does not send to users who are blocking the poster" do + create(:block, blocked_user: author, blocking_user: notified, hide_them: :posts) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.where(user: notified).count } end - it "does not send to coauthors" do - expect { - perform_enqueued_jobs do - create(:post, user: author, board: board, unjoined_authors: [notified]) - end - }.not_to change { Notification.count } + it "does not send to users who are blocking a coauthor" do + create(:block, blocked_user: coauthor, blocking_user: notified, hide_them: :posts) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.where(user: notified).count } end + end + end + + context "on new posts" do + let(:title) { 'test subject' } + let(:post) { build(:post, user: author, unjoined_authors: [coauthor, unjoined], board: board, subject: title) } + let(:type) { 'new_favorite_post' } + + def do_action(privacy: :public, viewers: []) + post.assign_attributes(privacy: privacy, viewers: viewers) + post.save! + end + + shared_examples "new" do + include_examples 'general' + include_examples 'authors' + include_examples 'privacy' it "does not queue on imported posts" do create(:post, user: author, board: board, is_import: true) @@ -98,7 +143,7 @@ perform_enqueued_jobs do create(:post, user: author, unjoined_authors: [], board: board, subject: title) end - }.to change { Notification.count }.by(1) + }.to change { Notification.where(user: notified).count }.by(1) author_msg = Notification.where(user: notified).last expect(author_msg.notification_type).to eq('new_favorite_post') @@ -106,6 +151,14 @@ end end + context "with favorited coauthor" do + let(:favorite) { coauthor } + + before(:each) { create(:favorite, user: notified, favorite: coauthor) } + + include_examples "new" + end + context "with favorited board" do let(:favorite) { board } @@ -115,53 +168,22 @@ it "does not send twice if the user has favorited both the poster and the continuity" do create(:favorite, user: notified, favorite: author) - expect { - perform_enqueued_jobs do - create(:post, user: author, board: board) - end - }.to change { Notification.count }.by(1) - end - - it "does not send to the poster" do - expect { - perform_enqueued_jobs do - create(:post, user: notified, board: board) - end - }.not_to change { Notification.count } + expect { perform_enqueued_jobs { do_action } }.to change { Notification.where(user: notified).count }.by(1) end end - describe "with blocking" do - let(:post) { create(:post, user: author, board: board, authors: [coauthor]) } - - before(:each) { create(:favorite, user: notified, favorite: board) } - - it "does not send to users the poster has blocked" do - create(:block, blocking_user: author, blocked_user: notified, hide_me: :posts) - expect { perform_enqueued_jobs { post } }.not_to change { Notification.count } - end - - it "does not send to users a coauthor has blocked" do - create(:block, blocking_user: coauthor, blocked_user: notified, hide_me: :posts) - expect { perform_enqueued_jobs { post } }.not_to change { Notification.count } - end - - it "does not send to users who are blocking the poster" do - create(:block, blocked_user: author, blocking_user: notified, hide_them: :posts) - expect { perform_enqueued_jobs { post } }.not_to change { Notification.count } - end - - it "does not send to users who are blocking a coauthor" do - create(:block, blocked_user: coauthor, blocking_user: notified, hide_them: :posts) - expect { perform_enqueued_jobs { post } }.not_to change { Notification.count } - end - end + include_examples 'blocking' end context "on joined posts" do - let(:author) { create(:user) } let(:replier) { create(:user) } - let(:notified) { create(:user) } + let(:post) { create(:post, user: author, board: board, unjoined_authors: [coauthor, unjoined]) } + let(:type) { 'joined_favorite_post' } + + def do_action(privacy: nil, viewers: []) + post.update!(privacy: privacy, viewers: viewers) if privacy + create(:reply, post: post, user: replier) + end context "with both authors favorited" do before(:each) do @@ -172,30 +194,29 @@ it "does not send twice if the user has favorited both the poster and the replier" do expect { perform_enqueued_jobs do - post = create(:post, user: author) + post create(:reply, post: post, user: replier) end - }.to change { Notification.count }.by(1) + }.to change { Notification.where(user: notified).count }.by(1) end it "does not send twice if the poster changes their username" do expect { perform_enqueued_jobs do - post = create(:post, user: author) + post author.update!(username: author.username + 'new') create(:reply, post: post, user: replier) end - }.to change { Notification.count }.by(1) + }.to change { Notification.where(user: notified).count }.by(1) end it "does not send twice if the post subject changes" do expect { perform_enqueued_jobs do - post = create(:post, user: author) post.update!(subject: post.subject + 'new') create(:reply, post: post, user: replier) end - }.to change { Notification.count }.by(1) + }.to change { Notification.where(user: notified).count }.by(1) end it "does not send twice if notified by message" do @@ -215,8 +236,8 @@ it "sends twice for different posts" do expect { - perform_enqueued_jobs { create(:post, user: author) } - }.to change { Notification.count }.by(1) + perform_enqueued_jobs { post } + }.to change { Notification.where(user: notified).count }.by(1) not_favorited_post = nil expect { @@ -234,102 +255,442 @@ end context "with favorited replier" do - before(:each) { create(:favorite, user: notified, favorite: replier) } + let(:favorite) { replier } - it "sends the right message" do - post = create(:post, user: author) + before(:each) do + create(:reply, user: coauthor, post: post) + create(:favorite, user: notified, favorite: replier) + end + + include_examples 'general' + include_examples 'privacy' + include_examples 'authors' + it "does not queue on imported replies" do + clear_enqueued_jobs + create(:reply, user: replier, post: post, is_import: true) + expect(NotifyFollowersOfNewPostJob).not_to have_been_enqueued + end + + it "does not queue if replier is already an unjoined author" do + post.update!(unjoined_authors: [coauthor, unjoined, replier]) + clear_enqueued_jobs + create(:reply, user: replier, post: post) + expect(NotifyFollowersOfNewPostJob).not_to have_been_enqueued + end + end + + describe "with blocking" do + before(:each) do + create(:reply, user: coauthor, post: post) + create(:favorite, user: notified, favorite: replier) + end + + it "does not send to users the joining user has blocked" do + create(:block, blocking_user: replier, blocked_user: notified, hide_me: :posts) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.count } + end + + it "does not send to users who are blocking the joining user" do + create(:block, blocked_user: replier, blocking_user: notified, hide_them: :posts) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.count } + end + end + + include_examples 'blocking' + end + + context "on newly accessible posts" do + let(:type) { 'accessible_favorite_post' } + let(:post) do + create(:post, user: author, unjoined_authors: [coauthor, unjoined], board: board, privacy: :access_list, viewers: [coauthor, unjoined]) + end + let(:do_action) { PostViewer.create!(user: notified, post: post) } + + before(:each) { create(:reply, user: coauthor, post: post) } + + shared_examples "access" do + include_examples 'general' + include_examples 'authors' + + it "does not send" do + clear_enqueued_jobs expect { perform_enqueued_jobs do - create(:reply, post: post, user: replier) + create(:post, + user: author, + privacy: :access_list, + unjoined_authors: [coauthor, unjoined], + viewers: [coauthor, unjoined, notified], + board: board, + ) end - }.to change { Notification.count }.by(1) + }.not_to change { Notification.where(notification_type: :accessible_favorite_post).count } + end + + it "does not send for public threads" do + post.update!(privacy: :public) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.count } + end + + it "does not send for private threads" do + post.update!(privacy: :private) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.count } + end + + it "does not send to authors" do + Favorite.delete_all + PostViewer.delete_all + authors = [author, coauthor, unjoined].reject { |u| u == favorite } - message = Notification.last - expect(message.user).to eq(notified) - expect(message.notification_type).to eq('joined_favorite_post') - expect(message.post).to eq(post) + authors.each do |user| + create(:favorite, user: user, favorite: favorite) + expect { + perform_enqueued_jobs { PostViewer.create!(user: user, post: post) } + }.not_to change { Notification.count } + end + end + end + + context "with favorited author" do + let(:favorite) { author } + + before(:each) { create(:favorite, user: notified, favorite: author) } + + include_examples "access" + + it "works for self-threads" do + post = create(:post, user: author, board: board, privacy: :access_list) + create(:reply, user: author, post: post) + + expect { perform_enqueued_jobs { PostViewer.create!(user: notified, post: post) } }.to change { Notification.count }.by(1) + + notif = Notification.last + expect(notif.user).to eq(notified) + expect(notif.notification_type).to eq('accessible_favorite_post') + expect(notif.post).to eq(post) + end + end + + context "with favorited coauthor" do + let(:favorite) { coauthor } + + before(:each) { create(:favorite, user: notified, favorite: coauthor) } + + include_examples "access" + end + + context "with favorited unjoined coauthor" do + let(:favorite) { unjoined } + + before(:each) { create(:favorite, user: notified, favorite: unjoined) } + + include_examples "access" + end + + context "with favorited board" do + let(:favorite) { board } + + before(:each) { create(:favorite, user: notified, favorite: board) } + + include_examples "access" + + it "does not send twice if the user has favorited both the poster and the continuity" do + create(:favorite, user: notified, favorite: author) + expect { perform_enqueued_jobs { do_action } }.to change { Notification.count }.by(1) + end + end + + context "with privacy change" do + let(:do_action) { post.update!(privacy: :access_list) } + + before(:each) do + create(:favorite, user: notified, favorite: author) + post.update!(privacy: :private, viewers: [coauthor, unjoined, notified]) + clear_enqueued_jobs end - it "does not send unless visible" do + include_examples 'general' + + it "does not send twice for new viewer" do + PostViewer.find_by(user: notified, post: post).destroy! + post.reload expect { perform_enqueued_jobs do - post = create(:post, privacy: :access_list, viewers: [replier]) - create(:reply, post: post, user: replier) + post.update!(privacy: :access_list, viewers: [coauthor, unjoined, notified]) end + }.to change { Notification.count }.by(1) + end + + it "does not send if already notified" do + post.update!(privacy: :access_list, viewers: [coauthor, unjoined]) + expect { + perform_enqueued_jobs { PostViewer.create!(user: notified, post: post) } + }.to change { Notification.count }.by(1) + + post.update!(privacy: :private) + + expect { + perform_enqueued_jobs { post.update!(privacy: :access_list) } }.not_to change { Notification.count } end - it "does not send if reader has config disabled" do - notified.update!(favorite_notifications: false) - expect { perform_enqueued_jobs { create(:reply, user: replier) } }.not_to change { Notification.count } + it "does not send for public threads" do + expect { + perform_enqueued_jobs { post.update!(privacy: :public) } + }.not_to change { Notification.where(notification_type: :accessible_favorite_post).count } end - it "does not queue on imported replies" do - post = create(:post) + it "does not send for registered threads" do + expect { + perform_enqueued_jobs { post.update!(privacy: :registered) } + }.not_to change { Notification.where(notification_type: :accessible_favorite_post).count } + end + + it "does not send for previously public threads" do + post.update!(privacy: :public) + post.reload clear_enqueued_jobs - create(:reply, user: replier, post: post, is_import: true) - expect(NotifyFollowersOfNewPostJob).not_to have_been_enqueued + expect { perform_enqueued_jobs { post.update!(privacy: :access_list) } }.not_to change { Notification.count } end end - it "does not send to the poster" do - create(:favorite, user: author, favorite: replier) - expect { - perform_enqueued_jobs do - post = create(:post, user: author) - create(:reply, post: post, user: replier) + include_examples 'blocking' + end + + context "on newly published posts" do + let!(:post) { create(:post, user: author, board: board, authors: [coauthor, unjoined], privacy: :access_list) } + let(:type) { 'published_favorite_post' } + + before(:each) { create(:reply, user: coauthor, post: post) } + + [:registered, :public].each do |privacy| + context "to #{privacy}" do + let(:do_action) { post.update!(privacy: privacy) } + + shared_examples "publication" do + include_examples 'general' + include_examples 'authors' + + it "works for previously private posts" do + post.update!(privacy: :private) + clear_enqueued_jobs + + expect { perform_enqueued_jobs { do_action } }.to change { Notification.count }.by(1) + notif = Notification.where(user: notified).last + expect(notif.notification_type).to eq('published_favorite_post') + expect(notif.post_id).to eq(post.id) + end + + it "does not send on post creation" do + clear_enqueued_jobs + expect { + perform_enqueued_jobs do + create(:post, user: author, board: board, authors: [coauthor, unjoined], privacy: :access_list) + end + }.not_to change { Notification.where(notification_type: :published_favorite_post).count } + end + end + + context "with favorited author" do + let(:favorite) { author } + + before(:each) { create(:favorite, user: notified, favorite: author) } + + include_examples "publication" + + it "works for self-threads" do + post = create(:post, user: author, board: board, privacy: :access_list) + create(:reply, post: post, user: author) + + expect { perform_enqueued_jobs { post.update!(privacy: privacy) } }.to change { Notification.count }.by(1) + + notif = Notification.where(user: notified).last + expect(notif.notification_type).to eq('published_favorite_post') + expect(notif.post_id).to eq(post.id) + end + end + + context "with favorited coauthor" do + let(:favorite) { coauthor } + + before(:each) { create(:favorite, user: notified, favorite: coauthor) } + + include_examples "publication" + end + + context "with favorited board" do + let(:favorite) { board } + + before(:each) { create(:favorite, user: notified, favorite: board) } + + include_examples "publication" + + it "does not send twice if the user has favorited both the poster and the continuity" do + create(:favorite, user: notified, favorite: author) + expect { perform_enqueued_jobs { do_action } }.to change { Notification.count }.by(1) + end + end + + context "with favorited unjoined coauthor" do + let(:favorite) { unjoined } + + before(:each) { create(:favorite, user: notified, favorite: unjoined) } + + include_examples "publication" end - }.not_to change { Notification.count } + + include_examples 'blocking' + end end + end - it "does not send to coauthors" do - unjoined = create(:user) - create(:favorite, user: unjoined, favorite: replier) - expect { - perform_enqueued_jobs do - post = create(:post, user: author, unjoined_authors: [replier, unjoined]) - create(:reply, post: post, user: replier) + context "on revived posts" do + let(:type) { 'resumed_favorite_post' } + + shared_examples "reactivation" do + shared_examples "active" do + include_examples 'general' + include_examples 'authors' + include_examples 'privacy' + + it "only notifies once" do + create(:notification, notification_type: :resumed_favorite_post, post: post, user: notified, unread: true) + expect { perform_enqueued_jobs { do_action } }.not_to change { Notification.count } + end + + it "renotifies if previous notification is read" do + create(:notification, notification_type: :resumed_favorite_post, post: post, user: notified, unread: false) + expect { perform_enqueued_jobs { do_action } }.to change { Notification.count }.by(1) end - }.not_to change { Notification.count } + end + + context "with favorited author" do + let(:favorite) { author } + + before(:each) { create(:favorite, user: notified, favorite: author) } + + include_examples 'active' + + it "works for self-threads" do + post.last_reply.update_columns(user_id: author.id) # rubocop:disable Rails/SkipsModelValidations + post.post_authors.where.not(user_id: author.id).delete_all + + expect { perform_enqueued_jobs { do_action } }.to change { Notification.count }.by(1) + + notif = Notification.where(user: notified).last + expect(notif.notification_type).to eq('resumed_favorite_post') + expect(notif.post_id).to eq(post.id) + end + + it "works with only top post" do + post.last_reply.destroy! + expect { perform_enqueued_jobs { do_action } }.to change { Notification.count }.by(1) + end + end + + context "with favorited coauthor" do + let(:favorite) { coauthor } + + before(:each) { create(:favorite, user: notified, favorite: coauthor) } + + include_examples 'active' + end + + context "with favorited unjoined coauthor" do + let(:favorite) { unjoined } + + before(:each) { create(:favorite, user: notified, favorite: unjoined) } + + include_examples 'active' + end + + context "with favorited board" do + let(:favorite) { board } + + before(:each) { create(:favorite, user: notified, favorite: board) } + + include_examples 'active' + + it "does not send twice if the user has favorited both the poster and the continuity" do + create(:favorite, user: notified, favorite: author) + expect { perform_enqueued_jobs { do_action } }.to change { Notification.count }.by(1) + end + end + + context "with favorited post" do + let(:favorite) { post } + + before(:each) { create(:favorite, user: notified, favorite: post) } + + include_examples 'active' + end + + include_examples 'blocking' end - describe "with blocking" do - let(:coauthor) { create(:user) } - let!(:post) { create(:post, user: author, unjoined_authors: [coauthor]) } - let(:reply) { create(:reply, post: post, user: replier) } + context "with abandoned posts" do + let(:post) { create(:post, user: author, board: board, authors: [coauthor, unjoined]) } - before(:each) { create(:favorite, user: notified, favorite: replier) } + before(:each) do + create(:reply, user: coauthor, post: post) + post.update!(status: :abandoned) + end - it "does not send to users the joining user has blocked" do - create(:block, blocking_user: replier, blocked_user: notified, hide_me: :posts) - expect { perform_enqueued_jobs { reply } }.not_to change { Notification.count } + def do_action(privacy: nil, viewers: []) + if privacy + post.update!(privacy: privacy, viewers: viewers, status: :active) + else + post.update!(status: :active) + end end - it "does not send to users who are blocking the joining user" do - create(:block, blocked_user: replier, blocking_user: notified, hide_them: :posts) - expect { perform_enqueued_jobs { reply } }.not_to change { Notification.count } + include_examples "reactivation" + end + + context "with manually hiatused posts" do + let(:post) { create(:post, user: author, board: board, authors: [coauthor, unjoined]) } + + before(:each) do + create(:reply, user: coauthor, post: post) + post.update!(status: :hiatus) end - it "does not send to users the original poster has blocked" do - create(:block, blocking_user: author, blocked_user: notified, hide_me: :posts) - expect { perform_enqueued_jobs { reply } }.not_to change { Notification.count } + def do_action(privacy: nil, viewers: []) + post.update!(privacy: privacy, viewers: viewers, is_import: true) if privacy + create(:reply, user: author, post: post) end - it "does not send to users who are blocking the original poster" do - create(:block, blocked_user: author, blocking_user: notified, hide_them: :posts) - expect { perform_enqueued_jobs { reply } }.not_to change { Notification.count } + include_examples "reactivation" + end + + context "with auto-hiatused posts" do + let(:now) { Time.zone.now } + let!(:post) do + Timecop.freeze(now - 2.months) do + create(:post, user: author, board: board, authors: [coauthor, unjoined]) + end end - it "does not send to users who a coauthor has blocked" do - create(:block, blocking_user: coauthor, blocked_user: notified, hide_me: :posts) - expect { perform_enqueued_jobs { reply } }.not_to change { Notification.count } + before(:each) do + Timecop.freeze(now - 2.months + 1.day) do + create(:reply, user: coauthor, post: post) + end end - it "does not send to users who are blocking a coauthor" do - create(:block, blocked_user: coauthor, blocking_user: notified, hide_them: :posts) - expect { perform_enqueued_jobs { reply } }.not_to change { Notification.count } + def do_action(privacy: nil, viewers: []) + if privacy + Timecop.freeze(now - 2.months) do + post.update!(privacy: privacy, viewers: viewers, is_import: true) + end + end + + Timecop.freeze(now) do + create(:reply, user: author, post: post) + end end + + include_examples "reactivation" end end end diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index ec34da622a..79f2707058 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -1339,7 +1339,7 @@ notified = create(:user) create(:favorite, user: notified, favorite: author) post = create(:post, user: author) - expect(NotifyFollowersOfNewPostJob).to have_been_enqueued.with(post.id, post.user_id).on_queue('notifier') + expect(NotifyFollowersOfNewPostJob).to have_been_enqueued.with(post.id, [], 'new').on_queue('notifier') end it "should only enqueue a message on authors' first join" do @@ -1348,7 +1348,7 @@ # first post triggers job post = create(:post, user: author) - expect(NotifyFollowersOfNewPostJob).to have_been_enqueued.with(post.id, post.user_id).on_queue('notifier') + expect(NotifyFollowersOfNewPostJob).to have_been_enqueued.with(post.id, [], 'new').on_queue('notifier') # original author posting again does not trigger job expect {