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

Improve NotifyFollowersOfNewPostJob #1542

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
14 changes: 14 additions & 0 deletions app/concerns/concealable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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
6 changes: 6 additions & 0 deletions app/concerns/owable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions app/controllers/notifications_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# frozen_string_literal: true
class NotificationsController < ApplicationController
before_action :login_required

Expand Down
13 changes: 9 additions & 4 deletions app/helpers/notification_helper.rb
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
14 changes: 14 additions & 0 deletions app/jobs/notify_coauthors_job.rb
Original file line number Diff line number Diff line change
@@ -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
102 changes: 70 additions & 32 deletions app/jobs/notify_followers_of_new_post_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions app/models/notification.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
10 changes: 0 additions & 10 deletions app/models/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
26 changes: 22 additions & 4 deletions app/models/post/author.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,33 @@ 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)
blocked_ids.each { |blocked| Rails.cache.delete(Block.cache_string_for(blocked, 'blocked')) }
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
9 changes: 9 additions & 0 deletions app/models/post/status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions app/models/post_viewer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
10 changes: 7 additions & 3 deletions app/models/reply.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions script/migrate_notifications.rb
Original file line number Diff line number Diff line change
@@ -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')
Expand Down
Loading