Reduced complexity of admin flags controller, split up into methods, moved reports into model.

app/controllers/admin/flags_controller.rb

@@ -3,98 +3,13 @@ class Admin::FlagsController < Admin::AdminController
 
     # we may get out of sync, fix it here
     PostAction.update_flagged_posts_count
+    posts, users = PostAction.flagged_posts_report(params[: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
+    if posts.blank?
-from posts p
-join topics t on t.id = topic_id
-join (
-  select
-    post_id,
-    count(*) as cnt,
-    max(created_at) max,
-    min(created_at) min
-    from post_actions
-    /*where2*/
-    group by post_id
-) as a on a.post_id = p.id
-/*where*/
-/*order_by*/
-limit 100
-"
-
-    sql.where2 "post_action_type_id in (:flag_types)", flag_types: PostActionType.notify_flag_types.values
-
-
-    # it may make sense to add a view that shows flags on deleted posts,
-    # we don't clear the flags on post deletion, just supress counts
-    #   they may have deleted_at on the action not set
-    if params[:filter] == 'old'
-      sql.where2 "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"
-    end
-
-    if params[:filter] == 'old'
-      sql.order_by "max desc"
-    else
-      sql.order_by "cnt desc, max asc"
-    end
-
-    posts = sql.exec.to_a
-
-    if posts.length == 0
       render json: {users: [], posts: []}
-      return
-    end
-
-    map = {}
-    users = Set.new
-
-    posts.each{ |p|
-      users << p["user_id"]
-      p["excerpt"] = Post.excerpt(p["cooked"])
-      p.delete "cooked"
-      p[:topic_slug] = Slug.for(p["title"])
-      map[p["id"]] = p
-    }
-
-    sql = SqlBuilder.new "select a.id, a.user_id, post_action_type_id, a.created_at, post_id, a.message, p.topic_id, t.slug
-from post_actions a
-left join posts p on p.id = related_post_id
-left join topics t on t.id = p.topic_id
-/*where*/
-"
-    sql.where("post_action_type_id in (:flag_types)", flag_types: PostActionType.notify_flag_types.values)
-    sql.where("post_id in (:posts)", posts: posts.map{|p| p["id"].to_i})
-
-    if params[:filter] == 'old'
-      sql.where('a.deleted_at is not null')
     else
-      sql.where('a.deleted_at is null')
+      render json: MultiJson.dump({users: serialize_data(users, BasicUserSerializer), posts: posts})
     end
-
-    sql.exec.each do |action|
-      action["permalink"] = Topic.url(action["topic_id"],action["slug"]) if action["slug"].present?
-      p = map[action["post_id"]]
-      p[:post_actions] ||= []
-      p[:post_actions] << action
-
-      users << action["user_id"]
-    end
-
-    sql =
-"select id, username, name, email from users
-where id in (?)"
-
-    users = User.exec_sql(sql, users.to_a).to_a
-
-    users.each { |u|
-      u["avatar_template"] = User.avatar_template(u["email"])
-      u.delete("email")
-    }
-
-    render json: MultiJson.dump({users: users, posts: posts})
   end
 
   def clear

app/controllers/application_controller.rb

@@ -122,21 +122,24 @@ class ApplicationController < ActionController::Base
     @guardian ||= Guardian.new(current_user)
   end
 
+
+  def serialize_data(obj, serializer, opts={})
+    # If it's an array, apply the serializer as an each_serializer to the elements
+    serializer_opts = {scope: guardian}.merge!(opts)
+    if obj.is_a?(Array)
+      serializer_opts[:each_serializer] = serializer
+      ActiveModel::ArraySerializer.new(obj, serializer_opts).as_json
+    else
+      serializer.new(obj, serializer_opts).as_json
+    end
+  end
+
   # This is odd, but it seems that in Rails `render json: obj` is about
   # 20% slower than calling MultiJSON.dump ourselves. I'm not sure why
   # Rails doesn't call MultiJson.dump when you pass it json: obj but
   # it seems we don't need whatever Rails is doing.
   def render_serialized(obj, serializer, opts={})
-
+    render_json_dump(serialize_data(obj, serializer, opts))
-    # If it's an array, apply the serializer as an each_serializer to the elements
-    serializer_opts = {scope: guardian}.merge!(opts)
-    if obj.is_a?(Array)
-      serializer_opts[:each_serializer] = serializer
-      render_json_dump(ActiveModel::ArraySerializer.new(obj, serializer_opts).as_json)
-    else
-      render_json_dump(serializer.new(obj, serializer_opts).as_json)
-    end
-
   end
 
   def render_json_dump(obj)

app/models/post_action.rb

@@ -13,6 +13,7 @@ class PostAction < ActiveRecord::Base
   belongs_to :post
   belongs_to :user
   belongs_to :post_action_type
+  belongs_to :related_post, class_name: 'Post'
 
   rate_limit :post_action_rate_limiter
 
@@ -20,7 +21,7 @@ class PostAction < ActiveRecord::Base
 
   def self.update_flagged_posts_count
     posts_flagged_count = PostAction.joins(post: :topic)
-                                    .where('post_actions.post_action_type_id' => PostActionType.notify_flag_types.values,
+                                    .where('post_actions.post_action_type_id' => PostActionType.notify_flag_type_ids,
                                            'posts.deleted_at' => nil,
                                            'topics.deleted_at' => nil)
                                     .count('DISTINCT posts.id')
@@ -221,7 +222,7 @@ class PostAction < ActiveRecord::Base
     Topic.update_all ["#{column} = #{column} + ?", delta], id: post.topic_id
 
 
-    if PostActionType.notify_flag_types.values.include?(post_action_type_id)
+    if PostActionType.notify_flag_type_ids.include?(post_action_type_id)
       PostAction.update_flagged_posts_count
     end
 
@@ -245,8 +246,77 @@ class PostAction < ActiveRecord::Base
     end
   end
 
+  def self.flagged_posts_report(filter)
+    posts = flagged_posts(filter)
+    return nil if posts.blank?
+
+    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
+    end
+
+    post_actions = PostAction.includes({:related_post => :topic})
+                              .where(post_action_type_id: PostActionType.notify_flag_type_ids)
+                              .where(post_id: post_lookup.keys)
+    post_actions = post_actions.with_deleted if filter == 'old'
+
+    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')
+      if (pa.related_post && pa.related_post.topic)
+        action.merge!(topic_id: pa.related_post.topic_id,
+                     slug: pa.related_post.topic.slug,
+                     permalink: pa.related_post.topic.url)
+      end
+      post[:post_actions] << action
+      users << pa.user_id
+    end
+
+    [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"
+
+    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 supress counts
+    #   they may have deleted_at on the action not set
+    if filter == 'old'
+      sql.where2 "deleted_at is not null"
+      sql.order_by "max desc"
+    else
+      sql.where "p.deleted_at is null and t.deleted_at is null"
+      sql.where2 "deleted_at is null"
+      sql.order_by "cnt desc, max asc"
+    end
+
+    sql.exec.to_a
+  end
+
   def self.target_moderators
     Group[:moderators].name
   end

app/models/post_action_type.rb

@@ -22,8 +22,8 @@ class PostActionType < ActiveRecord::Base
     end
 
     # flags resulting in mod notifications
-    def notify_flag_types
+    def notify_flag_type_ids
-      @notify_flag_types ||= types.only(:off_topic, :spam, :inappropriate, :notify_moderators)
+      @notify_flag_type_ids ||= types.only(:off_topic, :spam, :inappropriate, :notify_moderators).values
     end
 
     def is_flag?(sym)