From 7bc223ef7cacf150ee2e1a8ce6c39e5f2f4186f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Tue, 23 Sep 2014 18:37:31 +0200
Subject: [PATCH] FEATURE: improve flags reminder email

---
 app/mailers/pending_flags_mailer.rb           | 23 +++++++----
 app/models/user.rb                            |  1 -
 .../pending_flags_mailer/notify.html.erb      | 38 +++++++++----------
 .../pending_flags_mailer/notify.text.erb      |  4 +-
 4 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/app/mailers/pending_flags_mailer.rb b/app/mailers/pending_flags_mailer.rb
index 597f1ca8855..c0f67d2bf29 100644
--- a/app/mailers/pending_flags_mailer.rb
+++ b/app/mailers/pending_flags_mailer.rb
@@ -7,24 +7,33 @@ class PendingFlagsMailer < ActionMailer::Base
   def notify
     return unless SiteSetting.contact_email
 
-    @posts, users = FlagQuery.flagged_posts_report(Discourse.system_user, 'active', 0, 20)
+    @posts, @topics, @users = FlagQuery.flagged_posts_report(Discourse.system_user, 'active', 0, 20)
 
     @posts.each do |post| # Note: post is a Hash, not a Post.
+      topic = @topics.select { |t| t[:id] == post[:topic_id] }.first
+
+      post[:title] = topic[:title]
+      post[:url] = "#{Discourse.base_url}#{Post.url(topic[:slug], topic[:id], post[:post_number])}"
+      post[:user] = @users.select { |u| u[:id] == post[:user_id] }.first
+
       counts = flag_reason_counts(post)
-      post[:reason_counts] = counts.map do |reason, count|
-        "#{I18n.t('post_action_types.' + reason.to_s + '.title')}: #{count}"
-      end.join(', ')
+      post[:reason_counts] = counts.map { |reason, count| "#{I18n.t('post_action_types.' + reason.to_s + '.title')}: #{count}" }.join(', ')
+      post[:html_reason_counts] = counts.map { |reason, count| "<strong>#{I18n.t('post_action_types.' + reason.to_s + '.title')}:</strong> #{count}" }.join(', ')
     end
 
     @hours = SiteSetting.notify_about_flags_after
 
-    build_email( SiteSetting.contact_email,
-                 subject: "[#{SiteSetting.title}] " + I18n.t('flags_reminder.subject_template', {count: PostAction.flagged_posts_count}) )
+    subject = "[#{SiteSetting.title}] " + I18n.t('flags_reminder.subject_template', { count: PostAction.flagged_posts_count })
+    build_email(SiteSetting.contact_email, subject: subject)
   end
 
   private
 
   def flag_reason_counts(post)
-    post[:post_actions].inject({}) {|h,v| h[v[:name_key]] ||= 0; h[v[:name_key]] += 1; h }
+    post[:post_actions].inject({}) do |h,v|
+      h[v[:name_key]] ||= 0
+      h[v[:name_key]] += 1
+      h
+    end
   end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 96ee20ba7e2..d6613f53d56 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -351,7 +351,6 @@ class User < ActiveRecord::Base
     "//www.gravatar.com/avatar/#{email_hash}.png?s={size}&r=pg&d=identicon"
   end
 
-
   # Don't pass this up to the client - it's meant for server side use
   # This is used in
   #   - self oneboxes in open graph data
diff --git a/app/views/pending_flags_mailer/notify.html.erb b/app/views/pending_flags_mailer/notify.html.erb
index 836c1d73fdf..4e31f21114f 100644
--- a/app/views/pending_flags_mailer/notify.html.erb
+++ b/app/views/pending_flags_mailer/notify.html.erb
@@ -1,27 +1,27 @@
 <p>
-  <%=t 'flags_reminder.flags_were_submitted', count: @hours %>
-  <a href="<%= Discourse.base_url + '/admin/flags/active' %>"><%=t 'flags_reminder.please_review' %></a>
+  <%= t 'flags_reminder.flags_were_submitted', count: @hours %>
+  <a href="<%= Discourse.base_url + '/admin/flags/active' %>"><%= t 'flags_reminder.please_review' %></a>
 </p>
 
-<hr/>
-
-<table>
-  <tr>
-    <td></td>
-    <td><%=t 'flags_reminder.post_number' %></td>
-    <td></td>
-  </tr>
+<table style="border-collapse: collapse; border-spacing: 0;">
   <% @posts.each do |post| %>
-  <tr>
-    <td><%= link_to post[:title], "#{Discourse.base_url}/t/#{post[:topic_slug]}/#{post[:topic_id]}/#{post[:post_number]}" %></td>
-    <td align="center"><%= post[:post_number] %></td>
-    <td><%= post[:reason_counts] %></td>
-  </tr>
+    <tr style="border-bottom: 1px solid #e9e9e9; border-top: 1px solid #e9e9e9;">
+      <td style="width: 25px; padding: 8px 0 0 0; text-align: center; vertical-align: top;">
+        <a href='<%= Discourse.base_url + "/admin/users/#{post[:user].username.downcase}" %>'>
+          <img width="25" height="25" src="<%= Discourse.base_url + post[:user].avatar_template.gsub("{size}", "45") %>" title="<%= post[:user].username %>">
+        </a>
+      </td>
+      <td style="padding: 8px; word-wrap: break-word; vertical-align: top;">
+        <h3 style="margin: 0 !important;"><%= link_to post[:title], post[:url] %></h3>
+        <p style="margin: 0 2px;"><%= raw post[:excerpt] %></p>
+      </td>
+      <td style="vertical-align: middle; min-width: 120px;">
+        <%= raw post[:html_reason_counts] %>
+      </td>
+    </tr>
   <% end %>
 </table>
 
-<hr/>
-
 <p>
-  <%=t 'flags_reminder.how_to_disable' %>
-</p>
\ No newline at end of file
+  <%= t 'flags_reminder.how_to_disable' %>
+</p>
diff --git a/app/views/pending_flags_mailer/notify.text.erb b/app/views/pending_flags_mailer/notify.text.erb
index 3f3f9eb5d09..c4564fddc02 100644
--- a/app/views/pending_flags_mailer/notify.text.erb
+++ b/app/views/pending_flags_mailer/notify.text.erb
@@ -3,7 +3,7 @@
 <%= Discourse.base_url + '/admin/flags/active' %>
 
 <% @posts.each do |post| %>
-<%= post[:title] %>: <%=t 'flags_reminder.post_number' %> <%= post[:post_number] %> - <%= post[:reason_counts] %>
+ - <%= post[:title] %>: <%=t 'flags_reminder.post_number' %> <%= post[:post_number] %> - <%= post[:reason_counts] %>
 <% end %>
 
-<%=t 'flags_reminder.how_to_disable' %>
\ No newline at end of file
+<%=t 'flags_reminder.how_to_disable' %>