Lots bounce emails related fixes

- Show bounce score on user admin page
- Added reset bounce score button on user admin page
- Only whitelisted email types are sent to emails with high bounce score
- FIX: properly detect bounces even when there is no TO: header in the email
- Don't desactivate a user when reaching the bounce threshold

app/assets/javascripts/admin/models/admin-user.js.es6

@@ -1,3 +1,4 @@
+import computed from 'ember-addons/ember-computed-decorators';
 import { propertyNotEqual } from 'discourse/lib/computed';
 import { popupAjaxError } from 'discourse/lib/ajax-error';
 import ApiKey from 'admin/models/api-key';
@@ -6,11 +7,42 @@ import TL3Requirements from 'admin/models/tl3-requirements';
 
 const AdminUser = Discourse.User.extend({
 
-  customGroups: Em.computed.filter("groups", (g) => !g.automatic && Group.create(g)),
+  customGroups: Ember.computed.filter("groups", g => !g.automatic && Group.create(g)),
-  automaticGroups: Em.computed.filter("groups", (g) => g.automatic && Group.create(g)),
+  automaticGroups: Ember.computed.filter("groups", g => g.automatic && Group.create(g)),
 
   canViewProfile: Ember.computed.or("active", "staged"),
 
+  @computed("bounce_score", "reset_bounce_score_after")
+  bounceScore(bounce_score, reset_bounce_score_after) {
+    if (bounce_score > 0) {
+      return `${bounce_score} - ${moment(reset_bounce_score_after).format('LL')}`;
+    } else {
+      return bounce_score;
+    }
+  },
+
+  @computed("bounce_score")
+  bounceScoreExplanation(bounce_score) {
+    if (bounce_score === 0) {
+      return I18n.t("admin.user.bounce_score_explanation.none");
+    } else if (bounce_score < Discourse.SiteSettings.bounce_score_threshold) {
+      return I18n.t("admin.user.bounce_score_explanation.some");
+    } else {
+      return I18n.t("admin.user.bounce_score_explanation.threshold_reached");
+    }
+  },
+
+  canResetBounceScore: Ember.computed.gt("bounce_score", 0),
+
+  resetBounceScore() {
+    return Discourse.ajax(`/admin/users/${this.get("id")}/reset_bounce_score`, {
+      type: 'POST'
+    }).then(() => this.setProperties({
+      "bounce_score": 0,
+      "reset_bounce_score_after": null
+    }));
+  },
+
   generateApiKey() {
     const self = this;
     return Discourse.ajax("/admin/users/" + this.get('id') + "/generate_api_key", {

app/assets/javascripts/admin/templates/email-bounced.hbs

@@ -6,7 +6,6 @@
         <th>{{i18n 'admin.email.user'}}</th>
         <th>{{i18n 'admin.email.to_address'}}</th>
         <th>{{i18n 'admin.email.email_type'}}</th>
-        <th>{{i18n 'admin.email.skipped_reason'}}</th>
       </tr>
     </thead>
 
@@ -15,7 +14,6 @@
       <td>{{text-field value=filter.user placeholderKey="admin.email.logs.filters.user_placeholder"}}</td>
       <td>{{text-field value=filter.address placeholderKey="admin.email.logs.filters.address_placeholder"}}</td>
       <td>{{text-field value=filter.type placeholderKey="admin.email.logs.filters.type_placeholder"}}</td>
-      <td>{{text-field value=filter.skipped_reason placeholderKey="admin.email.logs.filters.skipped_reason_placeholder"}}</td>
     </tr>
 
     {{#each model as |l|}}
@@ -31,16 +29,9 @@
         </td>
         <td><a href='mailto:{{unbound l.to_address}}'>{{l.to_address}}</a></td>
         <td>{{l.email_type}}</td>
-        <td>
-          {{#if l.post_url}}
-            <a href="{{l.post_url}}">{{l.skipped_reason}}</a>
-          {{else}}
-            {{l.skipped_reason}}
-          {{/if}}
-        </td>
       </tr>
     {{else}}
-      <tr><td colspan="5">{{i18n 'admin.email.logs.none'}}</td></tr>
+      <tr><td colspan="4">{{i18n 'admin.email.logs.none'}}</td></tr>
     {{/each}}
 
   </table>

app/assets/javascripts/admin/templates/user-index.hbs

@@ -350,7 +350,20 @@
   <div class="display-row">
     <div class='field'>{{i18n 'admin.user.staged'}}</div>
     <div class='value'>{{model.staged}}</div>
-    <div class='controls'>{{i18n 'admin.user.stage_explanation'}}</div>
+    <div class='controls'>{{i18n 'admin.user.staged_explanation'}}</div>
+  </div>
+
+  <div class="display-row">
+    <div class='field'>{{i18n 'admin.user.bounce_score'}}</div>
+    <div class='value'>{{model.bounceScore}}</div>
+    <div class='controls'>
+      {{#if model.canResetBounceScore}}
+        <button class='btn' {{action "resetBounceScore" target="content"}} title={{i18n "admin.user.reset_bounce_score.title"}}>
+          {{i18n "admin.user.reset_bounce_score.label"}}
+        </button>
+      {{/if}}
+      {{model.bounceScoreExplanation}}
+    </div>
   </div>
 </section>
 

app/controllers/admin/users_controller.rb

@@ -23,7 +23,8 @@ class Admin::UsersController < Admin::AdminController
                                     :primary_group,
                                     :generate_api_key,
                                     :revoke_api_key,
-                                    :anonymize]
+                                    :anonymize,
+                                    :reset_bounce_score]
 
   def index
     users = ::AdminUserIndexQuery.new(params).find_users
@@ -355,6 +356,12 @@ class Admin::UsersController < Admin::AdminController
     end
   end
 
+  def reset_bounce_score
+    guardian.ensure_can_reset_bounce_score!(@user)
+    @user.user_stat.update_columns(bounce_score: 0, reset_bounce_score_after: nil)
+    render json: success_json
+  end
+
   private
 
     def fetch_user

app/jobs/regular/user_email.rb

@@ -48,7 +48,24 @@ module Jobs
       @skip_context = { type: type, user_id: user_id, to_address: to_address, post_id: post_id }
     end
 
-    NOTIFICATIONS_SENT_BY_MAILING_LIST ||= Set.new %w{posted replied mentioned group_mentioned quoted}
+    NOTIFICATIONS_SENT_BY_MAILING_LIST ||= Set.new %w{
+      posted
+      replied
+      mentioned
+      group_mentioned
+      quoted
+    }
+
+    CRITICAL_EMAIL_TYPES = Set.new %i{
+      account_created
+      admin_login
+      confirm_new_email
+      confirm_old_email
+      forgot_password
+      notify_old_email
+      signup
+      signup_after_approval
+    }
 
     def message_for_email(user, post, type, notification,
                          notification_type=nil, notification_data_hash=nil,
@@ -109,7 +126,7 @@ module Jobs
         email_args[:email_token] = email_token
       end
 
-      if type == 'notify_old_email'
+      if type == :notify_old_email
         email_args[:new_email] = user.email
       end
 
@@ -117,7 +134,7 @@ module Jobs
         return skip_message(I18n.t('email_log.exceeded_emails_limit'))
       end
 
-      if (user.user_stat.try(:bounce_score) || 0) >= SiteSetting.bounce_score_threshold
+      if !CRITICAL_EMAIL_TYPES.include?(type) && user.user_stat.bounce_score >= SiteSetting.bounce_score_threshold
         return skip_message(I18n.t('email_log.exceeded_bounces_limit'))
       end
 

app/jobs/scheduled/poll_mailbox.rb

@@ -30,7 +30,7 @@ module Jobs
 
         set_incoming_email_rejection_message(
           receiver.incoming_email,
-          I18n.t("email.incoming.errors.bounced_email_report")
+          I18n.t("email.incoming.errors.bounced_email_error")
         )
       rescue Email::Receiver::AutoGeneratedEmailReplyError => e
         log_email_process_failure(mail_string, e)

app/serializers/admin_detailed_user_serializer.rb

@@ -20,7 +20,9 @@ class AdminDetailedUserSerializer < AdminUserSerializer
              :primary_group_id,
              :badge_count,
              :warnings_received_count,
-             :user_fields
+             :user_fields,
+             :bounce_score,
+             :reset_bounce_score_after
 
   has_one :approved_by, serializer: BasicUserSerializer, embed: :objects
   has_one :api_key, serializer: ApiKeySerializer, embed: :objects
@@ -76,4 +78,12 @@ class AdminDetailedUserSerializer < AdminUserSerializer
     object.user_fields.present?
   end
 
+  def bounce_score
+    object.user_stat.bounce_score
+  end
+
+  def reset_bounce_score_after
+    object.user_stat.reset_bounce_score_after
+  end
+
 end

config/locales/client.en.yml

@@ -2593,10 +2593,18 @@ en:
         block_failed: 'There was a problem blocking the user.'
         block_confirm: 'Are you sure you want to block this user? They will not be able to create any new topics or posts.'
         block_accept: 'Yes, block this user'
+        bounce_score: "Bounce Score"
+        reset_bounce_score:
+          label: "Reset"
+          title: "Reset bounce score back to 0"
         deactivate_explanation: "A deactivated user must re-validate their email."
         suspended_explanation: "A suspended user can't log in."
         block_explanation: "A blocked user can't post or start topics."
-        stage_explanation: "A staged user can only post via email in specific topics."
+        staged_explanation: "A staged user can only post via email in specific topics."
+        bounce_score_explanation:
+          none: "No bounces were received recently from that email."
+          some: "Some bounces were received recently from that email."
+          threshold_reached: "Received too many bounces from that email."
         trust_level_change_failed: "There was a problem changing the user's trust level."
         suspend_modal_title: "Suspend User"
         trust_level_2_users: "Trust Level 2 Users"

config/locales/server.en.yml

@@ -64,7 +64,7 @@ en:
         reply_user_not_matching_error: "Happens when a reply came in from a different email address the notification was sent to."
         topic_not_found_error: "Happens when a reply came in but the related topic has been deleted."
         topic_closed_error: "Happens when a reply came in but the related topic has been closed."
-        bounced_email_report: "Email is a bounced email report."
+        bounced_email_error: "Email is a bounced email report."
         auto_generated_email_reply: "Email contains a reply to an auto generated email."
         screened_email_error: "Happens when the sender's email address was already screened."
 

config/routes.rb

@@ -108,6 +108,7 @@ Discourse::Application.routes.draw do
       get "leader_requirements" => "users#tl3_requirements"
       get "tl3_requirements"
       put "anonymize"
+      post "reset_bounce_score"
     end
     get "users/:id.json" => 'users#show', defaults: {format: 'json'}
     get 'users/:id/:username' => 'users#show'

config/site_settings.yml

@@ -572,6 +572,7 @@ email:
     type: list
   block_auto_generated_emails: true
   bounce_score_threshold:
+    client: true
     default: 4
     min: 1
 

lib/email/receiver.rb

@@ -156,7 +156,7 @@ module Email
     end
 
     def verp
-      @verp ||= @mail.destinations.select { |to| to[/\+verp-\h{32}@/] }.first
+      @verp ||= all_destinations.select { |to| to[/\+verp-\h{32}@/] }.first
     end
 
     def update_bounce_score(email, score)
@@ -171,10 +171,8 @@ module Email
           user.user_stat.reset_bounce_score_after = 30.days.from_now
           user.user_stat.save
 
-          if user.active && user.user_stat.bounce_score >= SiteSetting.bounce_score_threshold
+          if user.user_stat.bounce_score >= SiteSetting.bounce_score_threshold
-            user.deactivate
             StaffActionLogger.new(Discourse.system_user).log_revoke_email(user)
-            EmailToken.where(email: user.email, confirmed: true).update_all(confirmed: false)
           end
         end
 
@@ -292,14 +290,16 @@ module Email
       user
     end
 
-    def destinations
+    def all_destinations
-      [  @mail.destinations,
+      @all_destinations ||= [
+        @mail.destinations,
         [@mail[:x_forwarded_to]].flatten.compact.map(&:decoded),
         [@mail[:delivered_to]].flatten.compact.map(&:decoded),
-      ].flatten
+      ].flatten.select(&:present?).uniq.lazy
-       .select(&:present?)
+    end
-       .uniq
+
-       .lazy
+    def destinations
+      all_destinations
         .map { |d| check_address(d) }
         .drop_while(&:blank?)
     end

lib/guardian/user_guardian.rb

@@ -51,6 +51,10 @@ module UserGuardian
     is_staff? && !user.nil? && !user.staff?
   end
 
+  def can_reset_bounce_score?(user)
+    user && is_staff?
+  end
+
   def can_check_emails?(user)
     is_admin? || (is_staff? && SiteSetting.show_email_on_profile)
   end

spec/components/email/receiver_spec.rb

@@ -68,7 +68,7 @@ describe Email::Receiver do
 
     let(:bounce_key) { "14b08c855160d67f2e0c2f8ef36e251e" }
     let(:bounce_key_2) { "b542fb5a9bacda6d28cc061d18e4eb83" }
-    let!(:user) { Fabricate(:user, email: "foo@bar.com", active: true) }
+    let!(:user) { Fabricate(:user, email: "foo@bar.com") }
     let!(:email_log) { Fabricate(:email_log, user: user, bounce_key: bounce_key) }
     let!(:email_log_2) { Fabricate(:email_log, user: user, bounce_key: bounce_key_2) }
 
@@ -82,7 +82,6 @@ describe Email::Receiver do
 
       email_log.reload
       expect(email_log.bounced).to eq(true)
-      expect(email_log.user.active).to eq(true)
       expect(email_log.user.user_stat.bounce_score).to eq(1)
     end
 
@@ -91,7 +90,6 @@ describe Email::Receiver do
 
       email_log.reload
       expect(email_log.bounced).to eq(true)
-      expect(email_log.user.active).to eq(true)
       expect(email_log.user.user_stat.bounce_score).to eq(2)
 
       Timecop.freeze(2.days.from_now) do
@@ -99,7 +97,6 @@ describe Email::Receiver do
 
         email_log_2.reload
         expect(email_log_2.bounced).to eq(true)
-        expect(email_log_2.user.active).to eq(false)
         expect(email_log_2.user.user_stat.bounce_score).to eq(4)
       end
     end