Rework moderators activity query (#6230)

* Order rows in query

* Don't increment revisions when moderator revises their own post

app/models/report.rb

@@ -762,152 +762,144 @@ class Report
     ]
 
     report.modes = [:table]
-
     report.data = []
-    mod_data = {}
 
-    User.real.where(moderator: true).find_each do |u|
+    query = <<~SQL
-      mod_data[u.id] = {
+    WITH mods AS (
-        user_id: u.id,
+    SELECT
-        username: u.username_lower,
+    id AS user_id,
-        user_avatar_template: u.avatar_template,
+    username_lower AS username,
-      }
+    uploaded_avatar_id
-    end
+    FROM users u
-
+    WHERE u.moderator = 'true'
-    time_read_query = <<~SQL
+    AND u.id > 0
+    ),
+    time_read AS (
     SELECT SUM(uv.time_read) AS time_read,
     uv.user_id
-    FROM user_visits uv
+    FROM mods m
-    JOIN users u
+    JOIN user_visits uv
-    ON u.id = uv.user_id
+    ON m.user_id = uv.user_id
-    WHERE u.moderator = 'true'
+    WHERE uv.visited_at >= '#{report.start_date}'
-    AND u.id > 0
-    AND uv.visited_at >= '#{report.start_date}'
     AND uv.visited_at <= '#{report.end_date}'
     GROUP BY uv.user_id
-    SQL
-
-    flag_count_query = <<~SQL
-    WITH period_actions AS (
-    SELECT agreed_by_id,
-    disagreed_by_id
-    FROM post_actions
-    WHERE post_action_type_id IN (#{PostActionType.flag_types_without_custom.values.join(',')})
-    AND created_at >= '#{report.start_date}'
-    AND created_at <= '#{report.end_date}'
     ),
-    agreed_flags AS (
+    flag_count AS (
-    SELECT pa.agreed_by_id AS user_id,
+        WITH period_actions AS (
-    COUNT(*) AS flag_count
+        SELECT agreed_by_id,
-    FROM period_actions pa
+        disagreed_by_id
-    JOIN users u
+        FROM post_actions
-    ON u.id = pa.agreed_by_id
+        WHERE post_action_type_id IN (#{PostActionType.flag_types_without_custom.values.join(',')})
-    WHERE u.moderator = 'true'
+        AND created_at >= '#{report.start_date}'
-    AND u.id > 0
+        AND created_at <= '#{report.end_date}'
-    GROUP BY agreed_by_id
+        ),
-    ),
+        agreed_flags AS (
-    disagreed_flags AS (
+        SELECT pa.agreed_by_id AS user_id,
-    SELECT pa.disagreed_by_id AS user_id,
+        COUNT(*) AS flag_count
-    COUNT(*) AS flag_count
+        FROM mods m
-    FROM period_actions pa
+        JOIN period_actions pa
-    JOIN users u
+        ON pa.agreed_by_id = m.user_id
-    ON u.id = pa.disagreed_by_id
+        GROUP BY agreed_by_id
-    WHERE u.moderator = 'true'
+        ),
-    AND u.id > 0
+        disagreed_flags AS (
-    GROUP BY disagreed_by_id
+        SELECT pa.disagreed_by_id AS user_id,
-    )
+        COUNT(*) AS flag_count
+        FROM mods m
+        JOIN period_actions pa
+        ON pa.disagreed_by_id = m.user_id
+        GROUP BY disagreed_by_id
+        )
     SELECT
     COALESCE(af.user_id, df.user_id) AS user_id,
     COALESCE(af.flag_count, 0) + COALESCE(df.flag_count, 0) AS flag_count
     FROM agreed_flags af
     FULL OUTER JOIN disagreed_flags df
     ON df.user_id = af.user_id
-    SQL
+    ),
-
+    revision_count AS (
-    revision_count_query = <<~SQL
     SELECT pr.user_id,
     COUNT(*) AS revision_count
-    FROM post_revisions pr
+    FROM mods m
-    JOIN users u
+    JOIN post_revisions pr
-    ON u.id = pr.user_id
+    ON pr.user_id = m.user_id
-    WHERE u.moderator = 'true'
+    JOIN posts p
-    AND u.id > 0
+    ON p.id = pr.post_id
-    AND pr.created_at >= '#{report.start_date}'
+    WHERE pr.created_at >= '#{report.start_date}'
     AND pr.created_at <= '#{report.end_date}'
+    AND p.user_id <> pr.user_id
     GROUP BY pr.user_id
-    SQL
+    ),
-
+    topic_count AS (
-    topic_count_query = <<~SQL
     SELECT t.user_id,
     COUNT(*) AS topic_count
-    FROM topics t
+    FROM mods m
-    JOIN users u
+    JOIN topics t
-    ON u.id = t.user_id
+    ON t.user_id = m.user_id
-    WHERE u.moderator = 'true'
+    WHERE t.archetype = 'regular'
-    AND u.id > 0
-    AND t.archetype = 'regular'
     AND t.created_at >= '#{report.start_date}'
     AND t.created_at <= '#{report.end_date}'
     GROUP BY t.user_id
-    SQL
+    ),
-
+    post_count AS (
-    post_count_query = <<~SQL
     SELECT p.user_id,
     COUNT(*) AS post_count
-    FROM posts p
+    FROM mods m
-    JOIN users u
+    JOIN posts p
-    ON u.id = p.user_id
+    ON p.user_id = m.user_id
     JOIN topics t
     ON t.id = p.topic_id
-    WHERE u.moderator = 'true'
+    WHERE t.archetype = 'regular'
-    AND u.id > 0
-    AND t.archetype = 'regular'
     AND p.created_at >= '#{report.start_date}'
     AND p.created_at <= '#{report.end_date}'
     GROUP BY p.user_id
-    SQL
+    ),
-
+    pm_count AS (
-    pm_count_query = <<~SQL
     SELECT p.user_id,
     COUNT(*) AS pm_count
-    FROM posts p
+    FROM mods m
-    JOIN users u
+    JOIN posts p
-    ON u.id = p.user_id
+    ON p.user_id = m.user_id
     JOIN topics t
     ON t.id = p.topic_id
-    WHERE u.moderator = 'true'
+    WHERE t.archetype = 'private_message'
-    AND u.id > 0
-    AND t.archetype = 'private_message'
     AND p.created_at >= '#{report.start_date}'
     AND p.created_at <= '#{report.end_date}'
     GROUP BY p.user_id
+    )
+
+    SELECT
+    m.user_id,
+    m.username,
+    m.uploaded_avatar_id,
+    tr.time_read,
+    fc.flag_count,
+    rc.revision_count,
+    tc.topic_count,
+    pc.post_count,
+    pmc.pm_count
+    FROM mods m
+    LEFT JOIN time_read tr ON tr.user_id = m.user_id
+    LEFT JOIN flag_count fc ON fc.user_id = m.user_id
+    LEFT JOIN revision_count rc ON rc.user_id = m.user_id
+    LEFT JOIN topic_count tc ON tc.user_id = m.user_id
+    LEFT JOIN post_count pc ON pc.user_id = m.user_id
+    LEFT JOIN pm_count pmc ON pmc.user_id = m.user_id
+    ORDER BY m.username
     SQL
 
-    DB.query(time_read_query).each do |row|
+    DB.query(query).each do |row|
-      mod_data[row.user_id][:time_read] = row.time_read
+      mod = {}
+      mod[:username] = row.username
+      mod[:user_id] = row.user_id
+      mod[:user_avatar_template] = User.avatar_template(row.username, row.uploaded_avatar_id)
+      mod[:time_read] = row.time_read
+      mod[:flag_count] = row.flag_count
+      mod[:revision_count] = row.revision_count
+      mod[:topic_count] = row.topic_count
+      mod[:post_count] = row.post_count
+      mod[:pm_count] = row.pm_count
+      report.data << mod
     end
-
-    DB.query(flag_count_query).each do |row|
-      mod_data[row.user_id][:flag_count] = row.flag_count
-    end
-
-    DB.query(revision_count_query).each do |row|
-      mod_data[row.user_id][:revision_count] = row.revision_count
-    end
-
-    DB.query(topic_count_query).each do |row|
-      mod_data[row.user_id][:topic_count] = row.topic_count
-    end
-
-    DB.query(post_count_query).each do |row|
-      mod_data[row.user_id][:post_count] = row.post_count
-    end
-
-    DB.query(pm_count_query).each do |row|
-      mod_data[row.user_id][:pm_count] = row.pm_count
-    end
-
-    report.data = mod_data.values
   end
 
   def self.report_flags_status(report)

spec/models/report_spec.rb

@@ -563,6 +563,18 @@ describe Report do
         freeze_time(Date.today)
       end
 
+      context "moderators order" do
+        before do
+          Fabricate(:post, user: sam)
+          Fabricate(:post, user: jeff)
+        end
+
+        it "returns the moderators in alphabetical order" do
+          expect(report.data[0][:username]).to eq('jeff')
+          expect(report.data[1][:username]).to eq('sam')
+        end
+      end
+
       context "time read" do
         before do
           sam.user_visits.create(visited_at: 2.days.ago, time_read: 200)
@@ -575,10 +587,10 @@ describe Report do
         end
 
         it "returns the correct read times" do
-          expect(report.data[0][:username]).to eq('sam')
+          expect(report.data[0][:username]).to eq('jeff')
-          expect(report.data[0][:time_read]).to eq(300)
+          expect(report.data[0][:time_read]).to eq(3000)
-          expect(report.data[1][:username]).to eq('jeff')
+          expect(report.data[1][:username]).to eq('sam')
-          expect(report.data[1][:time_read]).to eq(3000)
+          expect(report.data[1][:time_read]).to eq(300)
         end
       end
 
@@ -589,7 +601,7 @@ describe Report do
           PostAction.agree_flags!(flagged_post, jeff)
         end
 
-        it "returns the correct read times" do
+        it "returns the correct flag counts" do
           expect(report.data.count).to eq(1)
           expect(report.data[0][:flag_count]).to eq(1)
           expect(report.data[0][:username]).to eq("jeff")
@@ -604,10 +616,10 @@ describe Report do
         end
 
         it "returns the correct topic count" do
-          expect(report.data[0][:topic_count]).to eq(2)
+          expect(report.data[0][:topic_count]).to eq(1)
-          expect(report.data[0][:username]).to eq('sam')
+          expect(report.data[0][:username]).to eq('jeff')
-          expect(report.data[1][:topic_count]).to eq(1)
+          expect(report.data[1][:topic_count]).to eq(2)
-          expect(report.data[1][:username]).to eq('jeff')
+          expect(report.data[1][:username]).to eq('sam')
         end
 
         context "private messages" do
@@ -616,8 +628,8 @@ describe Report do
           end
 
           it "doesn’t count private topic" do
-            expect(report.data[0][:topic_count]).to eq(2)
+            expect(report.data[0][:topic_count]).to eq(1)
-            expect(report.data[1][:topic_count]).to eq(1)
+            expect(report.data[1][:topic_count]).to eq(2)
           end
         end
       end
@@ -630,10 +642,10 @@ describe Report do
         end
 
         it "returns the correct topic count" do
-          expect(report.data[0][:topic_count]).to eq(2)
+          expect(report.data[0][:topic_count]).to eq(1)
-          expect(report.data[0][:username]).to eq('sam')
+          expect(report.data[0][:username]).to eq('jeff')
-          expect(report.data[1][:topic_count]).to eq(1)
+          expect(report.data[1][:topic_count]).to eq(2)
-          expect(report.data[1][:username]).to eq('jeff')
+          expect(report.data[1][:username]).to eq('sam')
         end
 
         context "private messages" do
@@ -642,8 +654,8 @@ describe Report do
           end
 
           it "doesn’t count private post" do
-            expect(report.data[0][:post_count]).to eq(2)
+            expect(report.data[0][:post_count]).to eq(1)
-            expect(report.data[1][:post_count]).to eq(1)
+            expect(report.data[1][:post_count]).to eq(2)
           end
         end
       end
@@ -657,10 +669,11 @@ describe Report do
         end
 
         it "returns the correct topic count" do
-          expect(report.data[0][:pm_count]).to be_blank
+          expect(report.data[0][:pm_count]).to eq(1)
-          expect(report.data[0][:username]).to eq('sam')
+          expect(report.data[0][:username]).to eq('jeff')
-          expect(report.data[1][:pm_count]).to eq(1)
+          expect(report.data[1][:pm_count]).to be_blank
-          expect(report.data[1][:username]).to eq('jeff')
+          expect(report.data[1][:username]).to eq('sam')
+
         end
       end
 
@@ -678,15 +691,11 @@ describe Report do
         context "revise own post" do
           before do
             post = Fabricate(:post, user: sam)
-            Fabricate(:post, user: sam)
+            post.revise(sam, raw: 'updated body')
-              .revise(sam, raw: 'updated body', edit_reason: 'not cool')
-
-            Fabricate(:post)
-              .revise(sam, raw: 'updated body', edit_reason: 'not cool')
           end
 
-          it "doesnt count a revison on your own post" do
+          it "doesn't count a revison on your own post" do
-            expect(report.data[0][:revision_count]).to eq(2)
+            expect(report.data[0][:revision_count]).to eq(1)
             expect(report.data[0][:username]).to eq('sam')
           end
         end