diff --git a/app/controllers/bulk_update_requests_controller.rb b/app/controllers/bulk_update_requests_controller.rb index 49b6be349..14e133574 100644 --- a/app/controllers/bulk_update_requests_controller.rb +++ b/app/controllers/bulk_update_requests_controller.rb @@ -38,7 +38,7 @@ class BulkUpdateRequestsController < ApplicationController def destroy if @bulk_update_request.editable?(CurrentUser.user) - @bulk_update_request.reject! + @bulk_update_request.reject!(CurrentUser.user) flash[:notice] = "Bulk update request rejected" respond_with(@bulk_update_request, :location => bulk_update_requests_path) else diff --git a/app/logical/forum_updater.rb b/app/logical/forum_updater.rb index 0951eec38..036a7f0b9 100644 --- a/app/logical/forum_updater.rb +++ b/app/logical/forum_updater.rb @@ -21,9 +21,10 @@ class ForumUpdater end def create_response(body) - forum_topic.posts.create( - :body => body - ) + forum_topic.posts.create({ + :body => body, + :skip_mention_notifications => true + }, :without_protection => true) end def update_title(title_tag) @@ -33,6 +34,6 @@ class ForumUpdater end def update_post(body) - forum_post.update(:body => "#{forum_post.body}\n\n#{body}") + forum_post.update({:body => "#{forum_post.body}\n\nEDIT: #{body}", :skip_mention_notifications => true }, :without_protection => true) end end diff --git a/app/logical/mentionable.rb b/app/logical/mentionable.rb index 290c42baf..355fa6362 100644 --- a/app/logical/mentionable.rb +++ b/app/logical/mentionable.rb @@ -9,7 +9,7 @@ module Mentionable @mentionable_options = options message_field = mentionable_option(:message_field) - after_save :queue_mention_messages, if: :"#{message_field}_changed?" + after_save :queue_mention_messages end def mentionable_option(key) @@ -19,6 +19,9 @@ module Mentionable def queue_mention_messages message_field = self.class.mentionable_option(:message_field) + return if !send("#{message_field}_changed?") + return if self.skip_mention_notifications + text = send(message_field) text_was = send("#{message_field}_was") diff --git a/app/logical/tag_alias_request.rb b/app/logical/tag_alias_request.rb index 6a238b634..6c37f7063 100644 --- a/app/logical/tag_alias_request.rb +++ b/app/logical/tag_alias_request.rb @@ -31,7 +31,9 @@ class TagAliasRequest @forum_topic = build_forum_topic(@tag_alias.id) @forum_topic.save - @tag_alias.update_attributes(forum_topic_id: @forum_topic.id, forum_post_id: @forum_topic.posts.first.id) + @tag_alias.forum_topic_id = @forum_topic.id + @tag_alias.forum_post_id = @forum_topic.posts.first.id + @tag_alias.save end end diff --git a/app/logical/tag_implication_request.rb b/app/logical/tag_implication_request.rb index 0c29c3944..1d2b44de7 100644 --- a/app/logical/tag_implication_request.rb +++ b/app/logical/tag_implication_request.rb @@ -31,7 +31,9 @@ class TagImplicationRequest @forum_topic = build_forum_topic(@tag_implication.id) @forum_topic.save - @tag_implication.update_attributes(:forum_topic_id => @forum_topic.id, :forum_post_id => @forum_topic.posts.first.id) + @tag_implication.forum_topic_id = @forum_topic.id + @tag_implication.forum_post_id = @forum_topic.posts.first.id + @tag_implication.save end end diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 3f32659a8..a771fb420 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -95,13 +95,13 @@ class BulkUpdateRequest < ApplicationRecord CurrentUser.scoped(approver) do AliasAndImplicationImporter.new(script, forum_topic_id, "1", true).process! update({ :status => "approved", :approver_id => CurrentUser.id, :skip_secondary_validations => true }, :as => CurrentUser.role) - forum_updater.update("The \"bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} has been approved.", "APPROVED") + forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been approved by @#{approver.name}.", "APPROVED") end rescue Exception => x self.approver = approver CurrentUser.scoped(approver) do - forum_updater.update("The \"Bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} has failed: #{x.to_s}", "FAILED") + forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has failed: #{x.to_s}", "FAILED") end end @@ -119,10 +119,14 @@ class BulkUpdateRequest < ApplicationRecord end end - def reject! - forum_updater.update("The \"bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} has been rejected.", "REJECTED") + def reject!(rejector) + forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been rejected by @#{rejector.name}.", "REJECTED") update_attribute(:status, "rejected") end + + def bulk_update_request_link + %{"bulk update request ##{id}":/bulk_update_requests?search%5Bid%5D=#{id}} + end end module ValidationMethods diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index 38c4fffe8..293487b68 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -3,6 +3,7 @@ class ForumPost < ApplicationRecord attr_accessible :body, :topic_id, :as => [:member, :builder, :gold, :platinum, :admin, :moderator, :default] attr_accessible :is_locked, :is_sticky, :is_deleted, :as => [:admin, :moderator] + attr_accessor :skip_mention_notifications # used by `Mentionable::queue_mention_messages` attr_readonly :topic_id belongs_to :creator, :class_name => "User" belongs_to :updater, :class_name => "User" diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 86c6b82cb..b98a0eb55 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -42,26 +42,6 @@ class TagAlias < TagRelationship delay(:queue => "default").process!(update_topic: update_topic) end end - - def approval_message - "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has been approved." - end - - def failure_message(e = nil) - "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) failed during processing. Reason: #{e}" - end - - def reject_message - "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has been rejected." - end - - def conflict_message - "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has conflicting wiki pages. [[#{consequent_name}]] should be updated to include information from [[#{antecedent_name}]] if necessary." - end - - def date_timestamp - Time.now.strftime("%Y-%m-%d") - end end module ForumMethods @@ -106,7 +86,7 @@ class TagAlias < TagRelationship clear_all_cache ensure_category_consistency update_posts - forum_updater.update(approval_message, "APPROVED") if update_topic + forum_updater.update(approval_message(approver), "APPROVED") if update_topic rename_wiki_and_artist update({ :status => "active", :post_count => consequent_tag.post_count }, :as => CurrentUser.role) end @@ -243,7 +223,7 @@ class TagAlias < TagRelationship def reject! update({ :status => "deleted" }, :as => CurrentUser.role) clear_all_cache - forum_updater.update(reject_message, "REJECTED") + forum_updater.update(reject_message(CurrentUser.user), "REJECTED") destroy end diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index 75c02bfba..9e1b75cde 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -148,7 +148,7 @@ class TagImplication < TagRelationship update_posts update({ :status => "active" }, :as => CurrentUser.role) update_descendant_names_for_parents - forum_updater.update("The tag implication #{antecedent_name} -> #{consequent_name} has been approved.", "APPROVED") if update_topic + forum_updater.update(approval_message(approver), "APPROVED") if update_topic end rescue Exception => e if tries < 5 @@ -157,7 +157,7 @@ class TagImplication < TagRelationship retry end - forum_updater.update("The tag implication #{antecedent_name} -> #{consequent_name} failed during processing. Reason: #{e}", "FAILED") if update_topic + forum_updater.update(failure_message(e), "FAILED") if update_topic update({ :status => "error: #{e}" }, :as => CurrentUser.role) if Rails.env.production? @@ -186,7 +186,7 @@ class TagImplication < TagRelationship def reject! update({ :status => "deleted", }, :as => CurrentUser.role) - forum_updater.update("The tag implication #{antecedent_name} -> #{consequent_name} has been rejected.", "REJECTED") + forum_updater.update(reject_message(CurrentUser.user), "REJECTED") destroy end @@ -210,10 +210,6 @@ class TagImplication < TagRelationship end end - def date_timestamp - Time.now.strftime("%Y-%m-%d") - end - def forum_updater @forum_updater ||= begin post = if forum_topic diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index 21515be19..8a9be2780 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -117,5 +117,37 @@ class TagRelationship < ApplicationRecord end end + module MessageMethods + def relationship + # "TagAlias" -> "tag alias", "TagImplication" -> "tag implication" + self.class.name.underscore.tr("_", " ") + end + + def approval_message(approver) + "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has been approved by @#{approver.name}." + end + + def failure_message(e = nil) + "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} failed during processing. Reason: #{e}" + end + + def reject_message(rejector) + "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has been rejected by @#{rejector.name}." + end + + def conflict_message + "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has conflicting wiki pages. [[#{consequent_name}]] should be updated to include information from [[#{antecedent_name}]] if necessary." + end + + def date_timestamp + Time.now.strftime("%Y-%m-%d") + end + + def forum_link + "(forum ##{forum_post.id})" if forum_post.present? + end + end + extend SearchMethods + include MessageMethods end diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index 654c91ab4..45c467eff 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -127,13 +127,19 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase @req.approver_id = @admin.id assert_difference("ForumPost.count") do - @req.reject! + @req.reject!(@admin) end @topic.reload @post.reload assert_match(/\[REJECTED\]/, @topic.title) end + + should "not send @mention dmails to the approver" do + assert_no_difference("Dmail.count") do + @req.approve!(@admin) + end + end end context "when searching" do diff --git a/test/unit/tag_alias_request_test.rb b/test/unit/tag_alias_request_test.rb index 7d140b04a..23ddff93b 100644 --- a/test/unit/tag_alias_request_test.rb +++ b/test/unit/tag_alias_request_test.rb @@ -46,5 +46,11 @@ class TagAliasRequestTest < ActiveSupport::TestCase tar.create end end + + should "save the forum post id" do + tar = TagAliasRequest.new(:antecedent_name => "aaa", :consequent_name => "bbb", :reason => "reason", :skip_secondary_validations => true) + tar.create + assert_equal(tar.forum_topic.posts.first.id, tar.tag_alias.forum_post.id) + end end end diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index 2333cdfe1..4c779dea8 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -66,11 +66,11 @@ class TagAliasTest < ActiveSupport::TestCase tag1 = FactoryGirl.create(:tag, :name => "aaa") tag2 = FactoryGirl.create(:tag, :name => "bbb") ta = FactoryGirl.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb") - assert_nil(Cache.get("ta:aaa")) + assert_nil(Cache.get("ta:#{Cache.hash("aaa")}")) TagAlias.to_aliased(["aaa"]) - assert_equal("bbb", Cache.get("ta:aaa")) + assert_equal("bbb", Cache.get("ta:#{Cache.hash("aaa")}")) ta.destroy - assert_nil(Cache.get("ta:aaa")) + assert_nil(Cache.get("ta:#{Cache.hash("aaa")}")) end should "move saved searches" do