simplify flag reporting and correct it so it properly displays old flags

app/models/post_action.rb

@@ -138,7 +138,10 @@ class PostAction < ActiveRecord::Base
   end
 
   def self.remove_act(user, post, post_action_type_id)
-    if action = where(post_id: post.id, user_id: user.id, post_action_type_id: post_action_type_id).first
+    if action = where(post_id: post.id,
+                      user_id: user.id,
+                      post_action_type_id:
+                      post_action_type_id).first
       action.trash!
       action.run_callbacks(:save)
     end
@@ -297,32 +300,44 @@ class PostAction < ActiveRecord::Base
   end
 
   def self.flagged_posts_report(filter)
-    posts = flagged_posts(filter)
-    return nil if posts.blank?
+
+    actions = flagged_post_actions(filter)
+
+    post_ids = actions.limit(300).pluck(:post_id).uniq
+    return nil if post_ids.blank?
+
+    posts = SqlBuilder.new("SELECT p.id, t.title, p.cooked, p.user_id,
+      p.topic_id, p.post_number, p.hidden, t.visible topic_visible
+      FROM posts p
+      JOIN topics t ON t.id = p.topic_id
+      WHERE p.id in (:post_ids)").map_exec(OpenStruct, post_ids: post_ids)
 
     post_lookup = {}
     users = Set.new
 
     posts.each do |p|
-      users << p["user_id"]
-      p["excerpt"] = Post.excerpt(p.delete("cooked"))
-      p[:topic_slug] = Slug.for(p["title"])
-      post_lookup[p["id"].to_i] = p
+      users << p.user_id
+      p.excerpt = Post.excerpt(p.cooked)
+      p.topic_slug = Slug.for(p.title)
+      post_lookup[p.id] = p
     end
 
-    post_actions = PostAction.includes({:related_post => :topic})
-                              .where(post_action_type_id: PostActionType.notify_flag_type_ids)
-                              .where(post_id: post_lookup.keys)
-    if filter == 'old'
-      post_actions = post_actions.with_deleted.where('deleted_at IS NOT NULL OR defer = true')
-    else
-      post_actions = post_actions.where('defer IS NULL OR defer = false')
-    end
+    # maintain order
+    posts = post_ids.map{|id| post_lookup[id]}
+
+    post_actions = actions.where(:post_id => post_ids)
+                          .select('post_actions.id,
+                                   post_actions.user_id,
+                                   post_action_type_id,
+                                   post_actions.created_at,
+                                   post_actions.post_id,
+                                   post_actions.message')
+                          .to_a
 
     post_actions.each do |pa|
       post = post_lookup[pa.post_id]
       post[:post_actions] ||= []
-      action = pa.attributes.slice('id', 'user_id', 'post_action_type_id', 'created_at', 'post_id', 'message')
+      action = pa.attributes
       if (pa.related_post && pa.related_post.topic)
         action.merge!(topic_id: pa.related_post.topic_id,
                      slug: pa.related_post.topic.slug,
@@ -332,42 +347,34 @@ class PostAction < ActiveRecord::Base
       users << pa.user_id
     end
 
+    # TODO add serializer so we can skip this
+    posts.map!(&:to_h)
     [posts, User.select([:id, :username, :email]).where(id: users.to_a).all]
   end
 
   protected
 
-  def self.flagged_posts(filter)
-    sql = SqlBuilder.new "select p.id, t.title, p.cooked, p.user_id, p.topic_id, p.post_number, p.hidden, t.visible topic_visible
-                          from posts p
-                          join topics t on t.id = topic_id
-                          join (
-                            select
-                              post_id,
-                              count(*) as cnt,
-                              max(created_at) max
-                              from post_actions
-                              /*where2*/
-                              group by post_id
-                          ) as a on a.post_id = p.id
-                          /*where*/
-                          /*order_by*/
-                          limit 100"
+  def self.flagged_post_actions(filter)
+    post_actions = PostAction
+                      .includes({:related_post => :topic})
+                      .where(post_action_type_id: PostActionType.notify_flag_type_ids)
+                      .joins(:post => :topic)
+                      .order('post_actions.created_at DESC')
 
-    sql.where2 "post_action_type_id in (:flag_types)", flag_types: PostActionType.notify_flag_type_ids
-
-    # it may make sense to add a view that shows flags on deleted posts,
-    # we don't clear the flags on post deletion, just suppress counts
     if filter == 'old'
-      sql.where2 "deleted_at is not null OR defer = true"
-      sql.order_by "max desc"
+      post_actions
+        .with_deleted
+        .where('post_actions.deleted_at IS NOT NULL OR
+                defer = true OR
+                topics.deleted_at IS NOT NULL OR
+                posts.deleted_at IS NOT NULL')
     else
-      sql.where "p.deleted_at is null and t.deleted_at is null"
-      sql.where2 "deleted_at is null and (defer IS NULL OR defer = false)"
-      sql.order_by "cnt desc, max asc"
+      post_actions
+        .where('defer IS NULL OR
+                defer = false')
+        .where('posts.deleted_at IS NULL AND
+                topics.deleted_at IS NULL')
     end
-
-    sql.exec.to_a
   end
 
   def self.target_moderators

spec/models/post_action_spec.rb

@@ -12,6 +12,16 @@ describe PostAction do
   let(:post) { Fabricate(:post) }
   let(:bookmark) { PostAction.new(user_id: post.user_id, post_action_type_id: PostActionType.types[:bookmark] , post_id: post.id) }
 
+  describe "flagged_posts_report" do
+    it "operates correctly" do
+      PostAction.act(codinghorror, post, PostActionType.types[:spam])
+      posts, users = PostAction.flagged_posts_report("")
+      p posts
+      posts.count.should == 1
+      users.count.should == 2
+    end
+  end
+
   describe "messaging" do
 
     it "notify moderators integration test" do