From 0a7a4eae99a76b988f63f708f694e936dd4e0e91 Mon Sep 17 00:00:00 2001 From: riking Date: Tue, 26 Aug 2014 17:58:43 -0700 Subject: [PATCH 1/3] Publish lightboxing on the message bus With this change, images appear to lightbox instantly on my development machine. --- app/jobs/regular/process_post.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/jobs/regular/process_post.rb b/app/jobs/regular/process_post.rb index e8753e7e167..49d9684a092 100644 --- a/app/jobs/regular/process_post.rb +++ b/app/jobs/regular/process_post.rb @@ -16,7 +16,16 @@ module Jobs cp.post_process(args[:bypass_bump]) # If we changed the document, save it - post.update_column(:cooked, cp.html) if cp.dirty? + 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 ) + end end end From 3396e6fea3b410e850873bcf65442c1f239d78b1 Mon Sep 17 00:00:00 2001 From: riking Date: Thu, 28 Aug 2014 20:34:32 -0700 Subject: [PATCH 2/3] 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 From 085b18577c1d2ce4453b472d248d10cab434da15 Mon Sep 17 00:00:00 2001 From: riking Date: Thu, 28 Aug 2014 22:07:40 -0700 Subject: [PATCH 3/3] Remove unnecessary user/topic load in PostCreator --- lib/post_creator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/post_creator.rb b/lib/post_creator.rb index dfe34c3926e..6ae33ebf9cb 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -281,8 +281,8 @@ class PostCreator def track_topic return if @opts[:auto_track] == false - TopicUser.change(@post.user.id, - @post.topic.id, + TopicUser.change(@post.user_id, + @topic.id, posted: true, last_read_post_number: @post.post_number, seen_post_count: @post.post_number)