diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 791ce8b98f9..ba26c096481 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -190,8 +190,7 @@ module PostGuardian # Can't delete the first post return false if post.is_first_post? - can_moderate = can_moderate_topic?(post.topic) - return true if can_moderate + return true if can_moderate_topic?(post.topic) # Can't delete posts in archived topics unless you are staff return false if post.topic&.archived? diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index bf17ca08459..81d36c02c28 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -17,7 +17,7 @@ module TopicGuardian return false if anonymous? || topic.nil? return true if is_staff? - is_category_group_moderator?(topic.category) + can_perform_action_available_to_group_moderators?(topic) end alias :can_moderate_topic? :can_review_topic? @@ -132,7 +132,6 @@ module TopicGuardian !Post.where(topic_id: topic.id, post_number: 1).where.not(locked_by_id: nil).exists? end - # Recovery Method def can_recover_topic?(topic) if is_staff? || (topic&.category && is_category_group_moderator?(topic.category)) !!(topic && topic.deleted_at) @@ -191,12 +190,9 @@ module TopicGuardian def filter_allowed_categories(records) unless is_admin? - allowed_ids = allowed_category_ids - if allowed_ids.length > 0 - records = records.where('topics.category_id IS NULL or topics.category_id IN (?)', allowed_ids) - else - records = records.where('topics.category_id IS NULL') - end + records = allowed_category_ids.size == 0 ? + records.where('topics.category_id IS NULL') : + records.where('topics.category_id IS NULL or topics.category_id IN (?)', allowed_category_ids) records = records.references(:categories) end records diff --git a/lib/post_merger.rb b/lib/post_merger.rb index 1cfabc3fbbe..81356f3a500 100644 --- a/lib/post_merger.rb +++ b/lib/post_merger.rb @@ -9,12 +9,13 @@ class PostMerger end def merge - return unless ensure_at_least_two_posts + return if @posts.count < 2 + ensure_same_topic! ensure_same_user! guardian = Guardian.new(@user) - ensure_staff_user!(guardian) + ensure_can_merge!(guardian) posts = @posts.sort_by do |post| guardian.ensure_can_delete!(post) @@ -38,29 +39,25 @@ class PostMerger private - def ensure_at_least_two_posts - @posts.count >= 2 - end - def ensure_same_topic! - unless @posts.map(&:topic_id).uniq.length == 1 + if @posts.map(&:topic_id).uniq.size != 1 raise CannotMergeError.new(I18n.t("merge_posts.errors.different_topics")) end end def ensure_same_user! - unless @posts.map(&:user_id).uniq.length == 1 + if @posts.map(&:user_id).uniq.size != 1 raise CannotMergeError.new(I18n.t("merge_posts.errors.different_users")) end end - def ensure_staff_user!(guardian) - raise Discourse::InvalidAccess unless guardian.is_staff? + def ensure_can_merge!(guardian) + raise Discourse::InvalidAccess unless guardian.can_moderate_topic?(@posts[0].topic) end def ensure_max_post_length!(raw) value = StrippedLengthValidator.get_sanitized_value(raw) - if value.length > SiteSetting.max_post_length + if value.size > SiteSetting.max_post_length raise CannotMergeError.new(I18n.t("merge_posts.errors.max_post_length")) end end diff --git a/spec/components/post_merger_spec.rb b/spec/components/post_merger_spec.rb index be1b97a4d1c..e8efd4b19d1 100644 --- a/spec/components/post_merger_spec.rb +++ b/spec/components/post_merger_spec.rb @@ -44,11 +44,27 @@ describe PostMerger do expect { PostMerger.new(admin, [reply2, post, reply1]).merge }.to raise_error(Discourse::InvalidAccess) end - it "should only allow staff user to merge posts" do + it "should only allow staff or TL4 user to merge posts" do reply1 = create_post(topic: topic, post_number: post.post_number, user: user) reply2 = create_post(topic: topic, post_number: post.post_number, user: user) - expect { PostMerger.new(user, [reply2, reply1]).merge }.to raise_error(Discourse::InvalidAccess) + merged_raw = reply1.raw + "\n\n" + reply2.raw + + tl1 = Fabricate(:user, trust_level: 1) + tl2 = Fabricate(:user, trust_level: 2) + tl3 = Fabricate(:user, trust_level: 3) + tl4 = Fabricate(:user, trust_level: 4) + + expect { PostMerger.new(tl1, [reply2, reply1]).merge }.to raise_error(Discourse::InvalidAccess) + expect { PostMerger.new(tl2, [reply2, reply1]).merge }.to raise_error(Discourse::InvalidAccess) + expect { PostMerger.new(tl3, [reply2, reply1]).merge }.to raise_error(Discourse::InvalidAccess) + + PostMerger.new(tl4, [reply2, reply1]).merge + + expect(reply1.trashed?).to eq(true) + expect(reply2.trashed?).to eq(false) + + expect(reply2.raw).to eq(merged_raw) end it "should not allow posts from different topics to be merged" do