diff --git a/app/models/post_alert_observer.rb b/app/models/post_alert_observer.rb index 579a8744e..fcefb7021 100644 --- a/app/models/post_alert_observer.rb +++ b/app/models/post_alert_observer.rb @@ -48,24 +48,6 @@ class PostAlertObserver < ActiveRecord::Observer alerter.create_notification(post.user, Notification.types[:edited], post, display_username: post_revision.user.username) end - def after_create_post(post) - if post.topic.private_message? - # If it's a private message, notify the topic_allowed_users - post.topic.all_allowed_users.reject{ |user| user.id == post.user_id }.each do |user| - next if user.blank? - - if TopicUser.get(post.topic, user).try(:notification_level) == TopicUser.notification_levels[:tracking] - next unless post.reply_to_post_number - next unless post.reply_to_post.user_id == user.id - end - - alerter.create_notification(user, Notification.types[:private_message], post) - end - elsif post.post_type != Post.types[:moderator_action] - # If it's not a private message and it's not an automatic post caused by a moderator action, notify the users - alerter.notify_post_users(post) - end - end protected diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index d0649dfc8..35dafa335 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -1,5 +1,31 @@ class PostAlerter + def self.post_created(post) + alerter = PostAlerter.new + alerter.after_create_post(post) + alerter.after_save_post(post) + post + end + + def after_create_post(post) + if post.topic.private_message? + # If it's a private message, notify the topic_allowed_users + post.topic.all_allowed_users.reject{ |user| user.id == post.user_id }.each do |user| + next if user.blank? + + if TopicUser.get(post.topic, user).try(:notification_level) == TopicUser.notification_levels[:tracking] + next unless post.reply_to_post_number + next unless post.reply_to_post.user_id == user.id + end + + create_notification(user, Notification.types[:private_message], post) + end + elsif post.post_type != Post.types[:moderator_action] + # If it's not a private message and it's not an automatic post caused by a moderator action, notify the users + notify_post_users(post) + end + end + def after_save_post(post) return if post.topic.private_message? diff --git a/lib/post_creator.rb b/lib/post_creator.rb index f945f286f..92973568a 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -69,12 +69,15 @@ class PostCreator ensure_in_allowed_users if guardian.is_staff? @post.advance_draft_sequence @post.save_reply_relationships - PostAlerter.new.after_save_post(@post) end - handle_spam - track_latest_on_category - enqueue_jobs + if @post + PostAlerter.post_created(@post) + + handle_spam + track_latest_on_category + enqueue_jobs + end @post end diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 50de3b5b3..bddab2e01 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -253,13 +253,12 @@ describe PostDestroyer do end context '@mentions' do - let!(:evil_trout) { Fabricate(:evil_trout) } - let!(:mention_post) { Fabricate(:post, raw: 'Hello @eviltrout')} - it 'removes notifications when deleted' do + user = Fabricate(:evil_trout) + post = create_post(raw: 'Hello @eviltrout') lambda { - PostDestroyer.new(Fabricate(:moderator), mention_post).destroy - }.should change(evil_trout.notifications, :count).by(-1) + PostDestroyer.new(Fabricate(:moderator), post).destroy + }.should change(user.notifications, :count).by(-1) end end diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index 8279c4802..d83d84a3f 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -210,8 +210,7 @@ describe PostRevisor do before do SiteSetting.stubs(:newuser_max_images).returns(0) url = "http://i.imgur.com/FGg7Vzu.gif" - # this test is problamatic, it leaves state in the onebox cache - Oneboxer.stubs(:onebox).with(url, anything).returns("") + Oneboxer.stubs(:cached_onebox).with(url, anything).returns("") subject.revise!(post.user, "So, post them here!\n#{url}") end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index c60b5f45b..57aa05957 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -20,29 +20,37 @@ describe Notification do let(:coding_horror) { Fabricate(:coding_horror) } describe 'replies' do + def process_alerts(post) + PostAlerter.post_created(post) + end + + let(:post) { + process_alerts(Fabricate(:post, post_args.merge(raw: "Hello @CodingHorror"))) + } - let(:post) { Fabricate(:post, post_args.merge(raw: "Hello @CodingHorror")) } it 'notifies the poster on reply' do lambda { - @reply = Fabricate(:basic_reply, user: coding_horror, topic: post.topic) + reply = Fabricate(:basic_reply, user: coding_horror, topic: post.topic) + process_alerts(reply) }.should change(post.user.notifications, :count).by(1) end it "doesn't notify the poster when they reply to their own post" do lambda { - @reply = Fabricate(:basic_reply, user: post.user, topic: post.topic) + reply = Fabricate(:basic_reply, user: post.user, topic: post.topic) + process_alerts(reply) }.should_not change(post.user.notifications, :count).by(1) end end describe 'watching' do it "does notify watching users of new posts" do - post = Fabricate(:post, post_args) + post = PostAlerter.post_created(Fabricate(:post, post_args)) user2 = Fabricate(:coding_horror) post_args[:topic].notify_watch!(user2) lambda { - Fabricate(:post, user: post.user, topic: post.topic) + PostAlerter.post_created(Fabricate(:post, user: post.user, topic: post.topic)) }.should change(user2.notifications, :count).by(1) end end @@ -126,6 +134,7 @@ describe Notification do before do @topic = Fabricate(:private_message_topic) @post = Fabricate(:post, topic: @topic, user: @topic.user) + PostAlerter.post_created(@post) @target = @post.topic.topic_allowed_users.reject{|a| a.user_id == @post.user_id}[0].user end @@ -256,8 +265,6 @@ describe Notification do fab(Notification.types[:liked], true) end - - it 'orders stuff correctly' do a = unread_pm regular diff --git a/spec/models/post_alert_observer_spec.rb b/spec/models/post_alert_observer_spec.rb index 93bddb0d7..c243f2cb1 100644 --- a/spec/models/post_alert_observer_spec.rb +++ b/spec/models/post_alert_observer_spec.rb @@ -40,59 +40,6 @@ describe PostAlertObserver do end end - context 'quotes' do - - it 'notifies a user by username' do - lambda { - Fabricate(:post, raw: '[quote="EvilTrout, post:1"]whatup[/quote]') - }.should change(evil_trout.notifications, :count).by(1) - end - - it "won't notify the user a second time on revision" do - p1 = Fabricate(:post, raw: '[quote="Evil Trout, post:1"]whatup[/quote]') - lambda { - p1.revise(p1.user, '[quote="Evil Trout, post:1"]whatup now?[/quote]') - }.should_not change(evil_trout.notifications, :count) - end - - it "doesn't notify the poster" do - topic = post.topic - lambda { - new_post = Fabricate(:post, topic: topic, user: topic.user, raw: '[quote="Bruce Wayne, post:1"]whatup[/quote]') - }.should_not change(topic.user.notifications, :count).by(1) - end - end - - context '@mentions' do - - let(:user) { Fabricate(:user) } - let(:mention_post) { Fabricate(:post, user: user, raw: 'Hello @eviltrout')} - let(:topic) { mention_post.topic } - - it 'notifies a user' do - lambda { - mention_post - }.should change(evil_trout.notifications, :count).by(1) - end - - it "won't notify the user a second time on revision" do - mention_post - lambda { - mention_post.revise(mention_post.user, "New raw content that still mentions @eviltrout") - }.should_not change(evil_trout.notifications, :count) - end - - it "doesn't notify the user who created the topic in regular mode" do - topic.notify_regular!(user) - mention_post - lambda { - Fabricate(:post, user: user, raw: 'second post', topic: topic) - }.should_not change(user.notifications, :count).by(1) - end - - end - - context 'private message' do let(:user) { Fabricate(:user) } let(:mention_post) { Fabricate(:post, user: user, raw: 'Hello @eviltrout')} @@ -106,6 +53,8 @@ describe PostAlertObserver do lambda { Guardian.any_instance.expects(:can_see?).with(instance_of(Post)).returns(false) mention_post + PostAlerter.new.after_create_post(mention_post) + PostAlerter.new.after_save_post(mention_post) }.should_not change(evil_trout.notifications, :count) end diff --git a/spec/models/post_analyzer_spec.rb b/spec/models/post_analyzer_spec.rb index d8f69462b..12f410bd3 100644 --- a/spec/models/post_analyzer_spec.rb +++ b/spec/models/post_analyzer_spec.rb @@ -17,8 +17,8 @@ describe PostAnalyzer do before { Oneboxer.stubs(:onebox) } - it 'fetches the onebox for any urls in the post' do - Oneboxer.expects(:onebox).with url + it 'fetches the cached onebox for any urls in the post' do + Oneboxer.expects(:cached_onebox).with url post_analyzer.cook(*args) end diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index 246417963..d3d13f1c5 100644 --- a/spec/models/user_action_spec.rb +++ b/spec/models/user_action_spec.rb @@ -149,8 +149,13 @@ describe UserAction do end describe 'when a user posts a new topic' do + def process_alerts(post) + PostAlerter.post_created(post) + end + before do @post = Fabricate(:old_post) + process_alerts(@post) end describe 'topic action' do @@ -173,6 +178,8 @@ describe UserAction do @other_user = Fabricate(:coding_horror) @mentioned = Fabricate(:admin) @response = Fabricate(:post, reply_to_post_number: 1, topic: @post.topic, user: @other_user, raw: "perhaps @#{@mentioned.username} knows how this works?") + + process_alerts(@response) end it 'should log user actions correctly' do diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb new file mode 100644 index 000000000..a02d70b0a --- /dev/null +++ b/spec/services/post_alerter_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe PostAlerter do + + let!(:evil_trout) { Fabricate(:evil_trout) } + + def create_post_with_alerts(args={}) + post = Fabricate(:post, args) + alerter = PostAlerter.new + alerter.after_create_post(post) + alerter.after_save_post(post) + post + end + + context 'quotes' do + + it 'notifies a user by username' do + lambda { + create_post_with_alerts(raw: '[quote="EvilTrout, post:1"]whatup[/quote]') + }.should change(evil_trout.notifications, :count).by(1) + end + + it "won't notify the user a second time on revision" do + p1 = create_post_with_alerts(raw: '[quote="Evil Trout, post:1"]whatup[/quote]') + lambda { + p1.revise(p1.user, '[quote="Evil Trout, post:1"]whatup now?[/quote]') + }.should_not change(evil_trout.notifications, :count) + end + + it "doesn't notify the poster" do + topic = create_post_with_alerts.topic + lambda { + Fabricate(:post, topic: topic, user: topic.user, raw: '[quote="Bruce Wayne, post:1"]whatup[/quote]') + }.should_not change(topic.user.notifications, :count).by(1) + end + end + + context '@mentions' do + + let(:user) { Fabricate(:user) } + let(:mention_post) { create_post_with_alerts(user: user, raw: 'Hello @eviltrout')} + let(:topic) { mention_post.topic } + + it 'notifies a user' do + lambda { + mention_post + }.should change(evil_trout.notifications, :count).by(1) + end + + it "won't notify the user a second time on revision" do + mention_post + lambda { + mention_post.revise(mention_post.user, "New raw content that still mentions @eviltrout") + }.should_not change(evil_trout.notifications, :count) + end + + it "doesn't notify the user who created the topic in regular mode" do + topic.notify_regular!(user) + mention_post + lambda { + create_post_with_alerts(user: user, raw: 'second post', topic: topic) + }.should_not change(user.notifications, :count).by(1) + end + + end +end