From 3396e6fea3b410e850873bcf65442c1f239d78b1 Mon Sep 17 00:00:00 2001 From: riking Date: Thu, 28 Aug 2014 20:34:32 -0700 Subject: [PATCH] Centralize MessageBus post updates After this change, only two files directly publish to MessageBus with a topic interpolated in the channel: Post and TopicUser. --- .../discourse/controllers/topic.js.es6 | 15 ++++++++---- app/jobs/regular/process_post.rb | 8 +------ app/models/post.rb | 9 ++++++++ app/models/post_action.rb | 8 +------ lib/post_creator.rb | 9 +------- lib/post_destroyer.rb | 23 +++++++------------ lib/post_revisor.rb | 14 ++--------- 7 files changed, 32 insertions(+), 54 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index 089374e7146..aedd0997049 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -496,25 +496,30 @@ export default ObjectController.extend(Discourse.SelectedPostsCount, { } var postStream = topicController.get('postStream'); - if (data.type === "revised" || data.type === "acted"){ + if (data.type === "revised" || data.type === "acted") { // TODO we could update less data for "acted" // (only post actions) postStream.triggerChangedPost(data.id, data.updated_at); return; } - if (data.type === "deleted"){ + if (data.type === "deleted") { postStream.triggerDeletedPost(data.id, data.post_number); return; } - if (data.type === "recovered"){ + if (data.type === "recovered") { postStream.triggerRecoveredPost(data.id, data.post_number); return; } - // Add the new post into the stream - postStream.triggerNewPostInStream(data.id); + if (data.type === "created") { + postStream.triggerNewPostInStream(data.id); + return; + } + + // log a warning + Em.Logger.warn("unknown topic bus message type", data); }); }, diff --git a/app/jobs/regular/process_post.rb b/app/jobs/regular/process_post.rb index 49d9684a092..394b2ae191c 100644 --- a/app/jobs/regular/process_post.rb +++ b/app/jobs/regular/process_post.rb @@ -18,13 +18,7 @@ module Jobs # If we changed the document, save it if cp.dirty? post.update_column(:cooked, cp.html) - - MessageBus.publish("/topic/#{post.topic_id}", { - type: "revised", - id: post.id, - updated_at: Time.now, - post_number: post.post_number - }, group_ids: post.topic.secure_group_ids ) + post.publish_change_to_clients! :revised end end diff --git a/app/models/post.rb b/app/models/post.rb index 7dd093ca103..b1b0e9672b3 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -87,6 +87,15 @@ class Post < ActiveRecord::Base end end + def publish_change_to_clients!(type) + MessageBus.publish("/topic/#{topic_id}", { + id: id, + post_number: post_number, + updated_at: Time.now, + type: type + }, group_ids: topic.secure_group_ids) + end + def trash!(trashed_by=nil) self.topic_links.each(&:destroy) super(trashed_by) diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 108b130fbca..f250acdbce8 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -375,13 +375,7 @@ class PostAction < ActiveRecord::Base def notify_subscribers if (is_like? || is_flag?) && post - MessageBus.publish("/topic/#{post.topic_id}",{ - id: post.id, - post_number: post.post_number, - type: "acted" - }, - group_ids: post.topic.secure_group_ids - ) + post.publish_change_to_clients! :acted end end diff --git a/lib/post_creator.rb b/lib/post_creator.rb index a0145bbc778..dfe34c3926e 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -270,14 +270,7 @@ class PostCreator return if @opts[:import_mode] return unless @post.post_number > 1 - MessageBus.publish("/topic/#{@post.topic_id}",{ - id: @post.id, - created_at: @post.created_at, - user: BasicUserSerializer.new(@post.user).as_json(root: false), - post_number: @post.post_number - }, - group_ids: @topic.secure_group_ids - ) + @post.publish_change_to_clients! :created end def extract_links diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 990bb114f1d..1719f926752 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -51,7 +51,7 @@ class PostDestroyer def staff_recovered @post.recover! - publish("recovered") + @post.publish_change_to_clients! :recovered end # When a post is properly deleted. Well, it's still soft deleted, but it will no longer @@ -75,21 +75,8 @@ class PostDestroyer update_associated_category_latest_topic update_user_counts end - publish("deleted") - end - def publish(message) - # edge case, topic is already destroyed - return unless @post.topic - - MessageBus.publish("/topic/#{@post.topic_id}",{ - id: @post.id, - post_number: @post.post_number, - updated_at: @post.updated_at, - type: message - }, - group_ids: @post.topic.secure_group_ids - ) + @post.publish_change_to_clients! :deleted if @post.topic end # When a user 'deletes' their own post. We just change the text. @@ -100,6 +87,9 @@ class PostDestroyer @post.update_flagged_posts_count @post.topic_links.each(&:destroy) end + + # covered by PostRevisor + # @post.publish_change_to_clients! :revised end def user_recovered @@ -109,6 +99,9 @@ class PostDestroyer @post.revise(@user, @post.revisions.last.modifications["raw"][0], force_new_version: true) @post.update_flagged_posts_count end + + # covered by PostRevisor + # @post.publish_change_to_clients! :revised end diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index dea3b55a4e8..b065b5a4f9a 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -21,6 +21,7 @@ class PostRevisor @opts = opts @new_raw = TextCleaner.normalize_whitespaces(new_raw).strip + # TODO this is not in a transaction - dangerous! return false unless should_revise? @post.acting_user = @editor revise_post @@ -30,7 +31,7 @@ class PostRevisor update_topic_word_counts @post.advance_draft_sequence PostAlerter.new.after_save_post(@post) - publish_revision + @post.publish_change_to_clients! :revised BadgeGranter.queue_badge_grant(Badge::Trigger::PostRevision, post: @post) true @@ -38,17 +39,6 @@ class PostRevisor private - def publish_revision - MessageBus.publish("/topic/#{@post.topic_id}",{ - id: @post.id, - post_number: @post.post_number, - updated_at: @post.updated_at, - type: "revised" - }, - group_ids: @post.topic.secure_group_ids - ) - end - def should_revise? @post.raw != @new_raw || @opts[:changed_owner] end