remove rails-observers

Rails yanked out observers many many years ago, instead the functionality
was yanked out to a gem that is very lightly maintained.

For example: if we want to upgrade to rails 5 there is no published gem

Internally the usage of observers had quite a few problem.

The series of refactors renamed a bunch of classes to give us more clarity
and removed some magic.
This commit is contained in:
Sam 2016-12-22 16:46:22 +11:00
parent 019f1a1d06
commit c531f4ded5
29 changed files with 73 additions and 89 deletions

View File

@ -9,7 +9,6 @@ end
if rails_master?
gem 'arel', git: 'https://github.com/rails/arel.git'
gem 'rails', git: 'https://github.com/rails/rails.git'
gem 'rails-observers', git: 'https://github.com/rails/rails-observers.git'
gem 'seed-fu', git: 'https://github.com/SamSaffron/seed-fu.git', branch: 'discourse'
else
# Rails 5 is going to ship with Action Cable, we have no use for it as
@ -29,8 +28,6 @@ else
# gem 'railties'
# gem 'sprockets-rails'
gem 'rails', '~> 4.2'
gem 'rails-observers'
gem 'seed-fu', '~> 2.3.5'
end

View File

@ -260,8 +260,6 @@ GEM
rails-deprecated_sanitizer (>= 1.0.1)
rails-html-sanitizer (1.0.3)
loofah (~> 2.0)
rails-observers (0.1.2)
activemodel (~> 4.0)
rails_multisite (1.0.6)
rails (> 4.2, < 5)
railties (4.2.7.1)
@ -453,7 +451,6 @@ DEPENDENCIES
rack-mini-profiler
rack-protection
rails (~> 4.2)
rails-observers
rails_multisite
rake
rb-fsevent

View File

@ -1,12 +0,0 @@
class AnonSiteJsonCacheObserver < ActiveRecord::Observer
observe :category, :post_action_type, :user_field, :group
def after_destroy(object)
Site.clear_anon_cache!
end
def after_save(object)
Site.clear_anon_cache!
end
end

View File

@ -6,6 +6,7 @@ class Category < ActiveRecord::Base
include Positionable
include HasCustomFields
include CategoryHashtag
include AnonCacheInvalidator
belongs_to :topic, dependent: :destroy
belongs_to :topic_only_relative_url,

View File

@ -0,0 +1,13 @@
module AnonCacheInvalidator
extend ActiveSupport::Concern
included do
after_destroy do
Site.clear_anon_cache!
end
after_save do
Site.clear_anon_cache!
end
end
end

View File

@ -1,5 +1,6 @@
class Group < ActiveRecord::Base
include HasCustomFields
include AnonCacheInvalidator
has_many :category_groups, dependent: :destroy
has_many :group_users, dependent: :destroy

View File

@ -23,6 +23,8 @@ class PostAction < ActiveRecord::Base
after_save :update_counters
after_save :enforce_rules
after_save :create_user_action
after_save :update_notifications
after_create :create_notifications
after_commit :notify_subscribers
def disposed_by_id
@ -281,7 +283,7 @@ SQL
post_action.recover!
action_attrs.each { |attr, val| post_action.send("#{attr}=", val) }
post_action.save
PostAlertObserver.after_create_post_action(post_action)
PostActionNotifier.post_action_created(post_action)
else
post_action = create(where_attrs.merge(action_attrs))
if post_action && post_action.errors.count == 0
@ -460,6 +462,16 @@ SQL
end
end
def update_notifications
if self.deleted_at.present?
PostActionNotifier.post_action_deleted(self)
end
end
def create_notifications
PostActionNotifier.post_action_created(self)
end
def notify_subscribers
if (is_like? || is_flag?) && post
post.publish_change_to_clients! :acted

View File

@ -5,6 +5,8 @@ class PostActionType < ActiveRecord::Base
after_save :expire_cache
after_destroy :expire_cache
include AnonCacheInvalidator
def expire_cache
ApplicationSerializer.expire_cache_fragment!("post_action_types")
ApplicationSerializer.expire_cache_fragment!("post_action_flag_types")

View File

@ -6,6 +6,8 @@ class PostRevision < ActiveRecord::Base
serialize :modifications, Hash
after_create :create_notification
def self.ensure_consistency!
# 1 - fix the numbers
PostRevision.exec_sql <<-SQL
@ -34,6 +36,10 @@ class PostRevision < ActiveRecord::Base
update_column(:hidden, false)
end
def create_notification
PostActionNotifier.after_create_post_revision(self)
end
end
# == Schema Information

View File

@ -1,4 +1,7 @@
class UserField < ActiveRecord::Base
include AnonCacheInvalidator
validates_presence_of :name, :description, :field_type
has_many :user_field_options, dependent: :destroy
accepts_nested_attributes_for :user_field_options

View File

@ -1,27 +1,18 @@
class PostAlertObserver < ActiveRecord::Observer
observe :post_action, :post_revision
class PostActionNotifier
def self.disable
@disabled = true
end
def self.enable
@disabled = false
end
def self.alerter
@alerter ||= PostAlerter.new
end
def alerter
self.class.alerter
end
# Dispatch to an after_save_#{class_name} method
def after_save(model)
method_name = callback_for('after_save', model)
send(method_name, model) if respond_to?(method_name)
end
# Dispatch to an after_create_#{class_name} method
def after_create(model)
method_name = callback_for('after_create', model)
send(method_name, model) if respond_to?(method_name)
end
def refresh_like_notification(post, read)
def self.refresh_like_notification(post, read)
return unless post && post.user_id
usernames = post.post_actions.where(post_action_type_id: PostActionType.types[:like])
@ -49,7 +40,10 @@ class PostAlertObserver < ActiveRecord::Observer
end
end
def after_save_post_action(post_action)
def self.post_action_deleted(post_action)
return if @disabled
# We only care about deleting post actions for now
return if post_action.deleted_at.blank?
@ -74,7 +68,10 @@ class PostAlertObserver < ActiveRecord::Observer
end
end
def self.after_create_post_action(post_action)
def self.post_action_created(post_action)
return if @disabled
# We only notify on likes for now
return unless post_action.is_like?
@ -91,11 +88,10 @@ class PostAlertObserver < ActiveRecord::Observer
)
end
def after_create_post_action(post_action)
self.class.after_create_post_action(post_action)
end
def self.after_create_post_revision(post_revision)
return if @disabled
def after_create_post_revision(post_revision)
post = post_revision.post
return unless post
@ -113,10 +109,4 @@ class PostAlertObserver < ActiveRecord::Observer
)
end
protected
def callback_for(action, model)
"#{action}_#{model.class.name.underscore.gsub(/.+\//, '')}"
end
end

View File

@ -80,11 +80,6 @@ module Discourse
config.assets.precompile << "locales/#{file.match(/([a-z_A-Z]+\.js)\.erb$/)[1]}"
end
# Activate observers that should always be running.
config.active_record.observers = [
:post_alert_observer,
]
# Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
# Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC.
config.time_zone = 'UTC'

View File

@ -5,7 +5,6 @@ require 'topic_subtype'
describe PostCreator do
before do
ActiveRecord::Base.observers.enable :all
end
let(:user) { Fabricate(:user) }

View File

@ -4,7 +4,6 @@ require 'post_destroyer'
describe PostDestroyer do
before do
ActiveRecord::Base.observers.enable :all
UserActionCreator.enable
end

View File

@ -9,7 +9,6 @@ describe UserActionsController do
end
it 'renders list correctly' do
ActiveRecord::Base.observers.enable :all
UserActionCreator.enable
post = Fabricate(:post)

View File

@ -68,7 +68,6 @@ describe CategoryUser do
context 'integration' do
before do
ActiveRecord::Base.observers.enable :all
NotificationEmailer.enable
end

View File

@ -20,7 +20,6 @@ describe DirectoryItem do
context 'refresh' do
before do
ActiveRecord::Base.observers.enable :all
UserActionCreator.enable
end

View File

@ -2,7 +2,6 @@ require 'rails_helper'
describe Notification do
before do
ActiveRecord::Base.observers.enable :all
NotificationEmailer.enable
end
@ -235,7 +234,6 @@ describe Notification do
describe 'ensure consistency' do
it 'deletes notifications if post is missing or deleted' do
ActiveRecord::Base.observers.disable :all
NotificationEmailer.disable
p = Fabricate(:post)

View File

@ -220,7 +220,9 @@ describe PostAction do
describe 'when a user likes something' do
it 'should generate notifications correctly' do
ActiveRecord::Base.observers.enable :all
PostActionNotifier.enable
PostAction.act(codinghorror, post, PostActionType.types[:like])
expect(Notification.count).to eq(1)

View File

@ -31,8 +31,6 @@ describe PostMover do
before do
p1.replies << p3
p2.replies << p4
# add a like to a post, enable observers so we get user actions
ActiveRecord::Base.observers.enable :all
UserActionCreator.enable
@like = PostAction.act(another_user, p4, PostActionType.types[:like])
end

View File

@ -81,8 +81,7 @@ describe PostTiming do
# integration test
it 'processes timings correctly' do
ActiveRecord::Base.observers.enable :all
PostActionNotifier.enable
post = Fabricate(:post)
user2 = Fabricate(:coding_horror, created_at: 1.day.ago)

View File

@ -3,9 +3,6 @@ require_dependency 'site'
describe Site do
it "omits categories users can not write to from the category list" do
ActiveRecord::Base.observers.enable :anon_site_json_cache_observer
category = Fabricate(:category)
user = Fabricate(:user)

View File

@ -68,12 +68,7 @@ describe TagUser do
end
context "integration" do
before do
ActiveRecord::Base.observers.enable :all
end
let(:user) { Fabricate(:user) }
let(:watched_tag) { Fabricate(:tag) }
let(:muted_tag) { Fabricate(:tag) }
let(:tracked_tag) { Fabricate(:tag) }

View File

@ -429,7 +429,6 @@ describe Topic do
let(:actions) { topic.user.user_actions }
it "should set up actions correctly" do
ActiveRecord::Base.observers.enable :all
UserActionCreator.enable
expect(actions.map{|a| a.action_type}).not_to include(UserAction::NEW_TOPIC)

View File

@ -208,7 +208,6 @@ describe TopicUser do
context 'private messages' do
it 'should ensure recepients and senders are watching' do
ActiveRecord::Base.observers.enable :all
target_user = Fabricate(:user)
post = create_post(archetype: Archetype.private_message, target_usernames: target_user.username);

View File

@ -3,7 +3,6 @@ require 'rails_helper'
describe UserAction do
before do
ActiveRecord::Base.observers.enable :all
UserActionCreator.enable
end
@ -51,6 +50,8 @@ describe UserAction do
end
it 'includes the events correctly' do
PostActionNotifier.enable
mystats = stats_for_user(user)
expecting = [UserAction::NEW_TOPIC, UserAction::NEW_PRIVATE_MESSAGE, UserAction::GOT_PRIVATE_MESSAGE, UserAction::BOOKMARK].sort
expect(mystats).to eq(expecting)

View File

@ -104,9 +104,7 @@ Spork.prefork do
#
# $redis = DiscourseMockRedis.new
#
# disable all observers, enable as needed during specs
#
ActiveRecord::Base.observers.disable :all
PostActionNotifier.disable
SearchIndexer.disable
UserActionCreator.disable
NotificationEmailer.disable

View File

@ -1,10 +1,10 @@
require 'rails_helper'
require_dependency 'post_destroyer'
describe PostAlertObserver do
describe PostActionNotifier do
before do
ActiveRecord::Base.observers.enable :post_alert_observer
PostActionNotifier.enable
end
let!(:evil_trout) { Fabricate(:evil_trout) }

View File

@ -63,8 +63,7 @@ describe PostAlerter do
context 'edits' do
it 'notifies correctly on edits' do
ActiveRecord::Base.observers.enable :all
PostActionNotifier.enable
post = Fabricate(:post, raw: 'I love waffles')
@ -89,7 +88,7 @@ describe PostAlerter do
context 'likes' do
it 'notifies on likes after an undo' do
ActiveRecord::Base.observers.enable :all
PostActionNotifier.enable
post = Fabricate(:post, raw: 'I love waffles')
@ -101,7 +100,7 @@ describe PostAlerter do
end
it 'notifies on does not notify when never is selected' do
ActiveRecord::Base.observers.enable :all
PostActionNotifier.enable
post = Fabricate(:post, raw: 'I love waffles')
@ -110,12 +109,11 @@ describe PostAlerter do
PostAction.act(evil_trout, post, PostActionType.types[:like])
expect(Notification.where(post_number: 1, topic_id: post.topic_id).count).to eq(0)
end
it 'notifies on likes correctly' do
ActiveRecord::Base.observers.enable :all
PostActionNotifier.enable
post = Fabricate(:post, raw: 'I love waffles')