Merge pull request #4277 from tgxworld/fix_bug_when_post_creator_returns_nil

Fix bug when post creator returns nil

app/models/topic.rb

@@ -546,13 +546,12 @@ class Topic < ActiveRecord::Base
                               topic_id: self.id,
                               skip_validations: true,
                               custom_fields: opts[:custom_fields])
-    new_post = creator.create
-    increment!(:moderator_posts_count) if new_post.persisted?
 
-    if new_post.present?
+    if (new_post = creator.create) && new_post.present?
+      increment!(:moderator_posts_count) if new_post.persisted?
       # If we are moving posts, we want to insert the moderator post where the previous posts were
       # in the stream, not at the end.
-      new_post.update_attributes(post_number: opts[:post_number], sort_order: opts[:post_number]) if opts[:post_number].present?
+      new_post.update_attributes!(post_number: opts[:post_number], sort_order: opts[:post_number]) if opts[:post_number].present?
 
       # Grab any links that are present
       TopicLink.extract_from(new_post)

lib/guardian.rb

@@ -25,6 +25,7 @@ class Guardian
     def moderator?; false; end
     def approved?; false; end
     def staged?; false; end
+    def blocked?; false; end
     def secure_category_ids; []; end
     def topic_create_allowed_category_ids; []; end
     def has_trust_level?(level); false; end
@@ -62,6 +63,10 @@ class Guardian
     @user.moderator?
   end
 
+  def is_blocked?
+    @user.blocked?
+  end
+
   def is_developer?
     @user &&
     is_admin? &&
@@ -112,7 +117,7 @@ class Guardian
   end
 
   def can_moderate?(obj)
-    obj && authenticated? && (is_staff? || (obj.is_a?(Topic) && @user.has_trust_level?(TrustLevel[4])))
+    obj && authenticated? && !is_blocked? && (is_staff? || (obj.is_a?(Topic) && @user.has_trust_level?(TrustLevel[4])))
   end
   alias :can_move_posts? :can_moderate?
   alias :can_see_flags? :can_moderate?
@@ -269,7 +274,7 @@ class Guardian
     # Can't send PMs to suspended users
     (is_staff? || target.is_a?(Group) || !target.suspended?) &&
     # Blocked users can only send PM to staff
-    (!@user.blocked? || target.staff?)
+    (!is_blocked? || target.staff?)
   end
 
   def can_see_emails?

spec/components/guardian_spec.rb

@@ -1208,6 +1208,14 @@ describe Guardian do
       expect(Guardian.new(user).can_moderate?(nil)).to be_falsey
     end
 
+    context 'when user is blocked' do
+      it 'returns false' do
+        user.toggle!(:blocked)
+        expect(Guardian.new(user).can_moderate?(post)).to be(false)
+        expect(Guardian.new(user).can_moderate?(topic)).to be(false)
+      end
+    end
+
     context 'a Topic' do
 
       it 'returns false when not logged in' do

spec/models/topic_spec.rb

@@ -6,6 +6,7 @@ require_dependency 'post_destroyer'
 describe Topic do
 
   let(:now) { Time.zone.local(2013,11,20,8,0) }
+  let(:user) { Fabricate(:user) }
 
   it { is_expected.to validate_presence_of :title }
 
@@ -312,8 +313,6 @@ describe Topic do
       end
 
       context "secure categories" do
-
-        let(:user) { Fabricate(:user) }
         let(:category) { Fabricate(:category, read_restricted: true) }
 
         before do
@@ -518,20 +517,36 @@ describe Topic do
   end
 
   context 'moderator posts' do
-    before do
-      @moderator = Fabricate(:moderator)
-      @topic = Fabricate(:topic)
-      @mod_post = @topic.add_moderator_post(@moderator, "Moderator did something. http://discourse.org", post_number: 999)
-    end
+    let(:moderator) { Fabricate(:moderator) }
+    let(:topic) { Fabricate(:topic) }
 
     it 'creates a moderator post' do
-      expect(@mod_post).to be_present
-      expect(@mod_post.post_type).to eq(Post.types[:moderator_action])
-      expect(@mod_post.post_number).to eq(999)
-      expect(@mod_post.sort_order).to eq(999)
-      expect(@topic.topic_links.count).to eq(1)
-      @topic.reload
-      expect(@topic.moderator_posts_count).to eq(1)
+      mod_post = topic.add_moderator_post(
+        moderator,
+        "Moderator did something. http://discourse.org",
+        post_number: 999
+      )
+
+      expect(mod_post).to be_present
+      expect(mod_post.post_type).to eq(Post.types[:moderator_action])
+      expect(mod_post.post_number).to eq(999)
+      expect(mod_post.sort_order).to eq(999)
+      expect(topic.topic_links.count).to eq(1)
+      expect(topic.reload.moderator_posts_count).to eq(1)
+    end
+
+    context "when moderator post fails to be created" do
+      before do
+        user.toggle!(:blocked)
+      end
+
+      it "should not increment moderator_posts_count" do
+        expect(topic.moderator_posts_count).to eq(0)
+
+        topic.add_moderator_post(user, "winter is never coming")
+
+        expect(topic.moderator_posts_count).to eq(0)
+      end
     end
   end