FIX: Various improvements to post notices.

- Notices are visible only by poster and trust level 2+ users.
- Notices are not generated for non-human or staged users.
- Notices are deleted when post is deleted.

app/models/post.rb

@@ -188,13 +188,13 @@ class Post < ActiveRecord::Base
 
   def trash!(trashed_by = nil)
     self.topic_links.each(&:destroy)
+    self.delete_post_notices
     super(trashed_by)
   end
 
   def recover!
     super
     update_flagged_posts_count
-    delete_post_notices
     recover_public_post_actions
     TopicLink.extract_from(self)
     QuotedPost.extract_from(self)
@@ -385,6 +385,7 @@ class Post < ActiveRecord::Base
   def delete_post_notices
     self.custom_fields.delete("post_notice_type")
     self.custom_fields.delete("post_notice_time")
+    self.save_custom_fields
   end
 
   def recover_public_post_actions

app/serializers/post_serializer.rb

@@ -370,6 +370,8 @@ class PostSerializer < BasicPostSerializer
   end
 
   def include_post_notice_type?
+    return false if scope.user&.id != object.user_id && !scope.user&.has_trust_level?(TrustLevel[2])
+
     post_notice_type.present?
   end
 
@@ -378,7 +380,7 @@ class PostSerializer < BasicPostSerializer
   end
 
   def include_post_notice_time?
-    post_notice_time.present?
+    include_post_notice_type? && post_notice_time.present?
   end
 
   def locked

lib/post_creator.rb

@@ -514,6 +514,8 @@ class PostCreator
   end
 
   def create_post_notice
+    return if @user.id < 0 || @user.staged
+
     last_post_time = Post.where(user_id: @user.id)
       .order(created_at: :desc)
       .limit(1)

spec/components/post_creator_spec.rb

@@ -1263,29 +1263,35 @@ describe PostCreator do
 
   context "#create_post_notice" do
     let(:user) { Fabricate(:user) }
-    let(:new_user) { Fabricate(:user) }
+    let(:staged) { Fabricate(:staged) }
-    let(:returning_user) { Fabricate(:user) }
 
-    it "generates post notices" do
+    it "generates post notices for new users" do
-      # new users
+      post = PostCreator.create(user, title: "one of my first topics", raw: "one of my first posts")
-      post = PostCreator.create(new_user, title: "one of my first topics", raw: "one of my first posts")
       expect(post.custom_fields["post_notice_type"]).to eq("first")
-      post = PostCreator.create(new_user, title: "another one of my first topics", raw: "another one of my first posts")
+      post = PostCreator.create(user, title: "another one of my first topics", raw: "another one of my first posts")
       expect(post.custom_fields["post_notice_type"]).to eq(nil)
-
-      # returning users
-      SiteSetting.returning_users_days = 30
-      old_post = Fabricate(:post, user: returning_user, created_at: 31.days.ago)
-      post = PostCreator.create(returning_user, title: "this is a returning topic", raw: "this is a post")
-      expect(post.custom_fields["post_notice_type"]).to eq("returning")
-      expect(post.custom_fields["post_notice_time"]).to eq(old_post.created_at.iso8601)
     end
 
-    it "does not generate post notices" do
+    it "generates post notices for returning users" do
-      Fabricate(:post, user: user, created_at: 3.days.ago)
+      SiteSetting.returning_users_days = 30
+      old_post = Fabricate(:post, user: user, created_at: 31.days.ago)
+
+      post = PostCreator.create(user, title: "this is a returning topic", raw: "this is a post")
+      expect(post.custom_fields["post_notice_type"]).to eq("returning")
+      expect(post.custom_fields["post_notice_time"]).to eq(old_post.created_at.iso8601)
+
       post = PostCreator.create(user, title: "this is another topic", raw: "this is my another post")
       expect(post.custom_fields["post_notice_type"]).to eq(nil)
       expect(post.custom_fields["post_notice_time"]).to eq(nil)
     end
+
+    it "does not generate for non-human or staged users" do
+      [Discourse.system_user, staged].each do |user|
+        expect(user.posts.size).to eq(0)
+        post = PostCreator.create(user, title: "#{user.name}'s first topic", raw: "#{user.name}'s first post")
+        expect(post.custom_fields["post_notice_type"]).to eq(nil)
+        expect(post.custom_fields["post_notice_time"]).to eq(nil)
+      end
+    end
   end
 end

spec/models/post_spec.rb

@@ -143,14 +143,9 @@ describe Post do
         post
       }
 
-      before do
-        post.trash!
-        post.reload
-      end
-
       describe 'recovery' do
         it 'deletes notices' do
-          expect { post.recover! }
+          expect { post.trash! }
             .to change { post.custom_fields.length }.from(2).to(0)
         end
       end

spec/serializers/post_serializer_spec.rb

@@ -174,4 +174,29 @@ describe PostSerializer do
 
   end
 
+  context "a post with notices" do
+    let(:user) { Fabricate(:user, trust_level: 1) }
+    let(:user_tl1) { Fabricate(:user, trust_level: 1) }
+    let(:user_tl2) { Fabricate(:user, trust_level: 2) }
+
+    let(:post) {
+      post = Fabricate(:post, user: user)
+      post.custom_fields["post_notice_type"] = "returning"
+      post.custom_fields["post_notice_time"] = 1.day.ago
+      post.save_custom_fields
+      post
+    }
+
+    def json_for_user(user)
+      PostSerializer.new(post, scope: Guardian.new(user), root: false).as_json
+    end
+
+    it "will not show for poster and TL2+ users" do
+      expect(json_for_user(nil)[:post_notice_type]).to eq(nil)
+      expect(json_for_user(user)[:post_notice_type]).to eq("returning")
+      expect(json_for_user(user_tl1)[:post_notice_type]).to eq(nil)
+      expect(json_for_user(user_tl2)[:post_notice_type]).to eq("returning")
+    end
+  end
+
 end