FIX: Don't allow a user to become TL3 if they've ever been penalized

Previously the code would only check if they were *currently* suspended
or silenced.

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

@@ -5,7 +5,6 @@ import { propertyNotEqual } from 'discourse/lib/computed';
 import { popupAjaxError } from 'discourse/lib/ajax-error';
 import ApiKey from 'admin/models/api-key';
 import Group from 'discourse/models/group';
-import TL3Requirements from 'admin/models/tl3-requirements';
 import { userPath } from 'discourse/lib/url';
 
 const wrapAdmin = user => user ? AdminUser.create(user) : null;
@@ -472,11 +471,12 @@ const AdminUser = Discourse.User.extend({
     });
   },
 
-  tl3Requirements: function() {
+  @computed('tl3_requirements')
-    if (this.get('tl3_requirements')) {
+  tl3Requirements(requirements) {
-      return TL3Requirements.create(this.get('tl3_requirements'));
+    if (requirements) {
+      return this.store.createRecord('tl3Requirements', requirements);
     }
-  }.property('tl3_requirements'),
+  },
 
   @computed('suspended_by')
   suspendedBy: wrapAdmin,

app/assets/javascripts/admin/models/tl3-requirements.js.es6

@@ -1,11 +1,15 @@
-const TL3Requirements = Discourse.Model.extend({
+import computed from 'ember-addons/ember-computed-decorators';
-  days_visited_percent: function() {
-    return Math.round((this.get('days_visited') * 100) / this.get('time_period'));
-  }.property('days_visited', 'time_period'),
 
-  min_days_visited_percent: function() {
+export default Discourse.Model.extend({
-    return Math.round((this.get('min_days_visited') * 100) / this.get('time_period'));
+  @computed('days_visited', 'time_period')
-  }.property('min_days_visited', 'time_period'),
+  days_visited_percent(daysVisited, timePeriod) {
+    return Math.round((daysVisited * 100) / timePeriod);
+  },
+
+  @computed('min_days_visited', 'time_period')
+  min_days_visited_percent(minDaysVisited, timePeriod) {
+    return Math.round((minDaysVisited * 100) / timePeriod);
+  },
 
   met: function() {
     return {
@@ -21,22 +25,26 @@ const TL3Requirements = Discourse.Model.extend({
       likes_received: this.get('num_likes_received') >= this.get('min_likes_received'),
       likes_received_days: this.get('num_likes_received_days') >= this.get('min_likes_received_days'),
       likes_received_users: this.get('num_likes_received_users') >= this.get('min_likes_received_users'),
-      level_locked: this.get('trust_level_locked')
+      level_locked: this.get('trust_level_locked'),
+      silenced: this.get('penalty_counts.silenced') === 0,
+      suspended: this.get('penalty_counts.suspended') === 0
     };
-  }.property('days_visited', 'min_days_visited',
+  }.property(
-             'num_topics_replied_to', 'min_topics_replied_to',
+    'days_visited', 'min_days_visited',
-             'topics_viewed', 'min_topics_viewed',
+    'num_topics_replied_to', 'min_topics_replied_to',
-             'posts_read', 'min_posts_read',
+    'topics_viewed', 'min_topics_viewed',
-             'num_flagged_posts', 'max_flagged_posts',
+    'posts_read', 'min_posts_read',
-             'topics_viewed_all_time', 'min_topics_viewed_all_time',
+    'num_flagged_posts', 'max_flagged_posts',
-             'posts_read_all_time', 'min_posts_read_all_time',
+    'topics_viewed_all_time', 'min_topics_viewed_all_time',
-             'num_flagged_by_users', 'max_flagged_by_users',
+    'posts_read_all_time', 'min_posts_read_all_time',
-             'num_likes_given', 'min_likes_given',
+    'num_flagged_by_users', 'max_flagged_by_users',
-             'num_likes_received', 'min_likes_received',
+    'num_likes_given', 'min_likes_given',
-             'num_likes_received', 'min_likes_received',
+    'num_likes_received', 'min_likes_received',
-             'num_likes_received_days', 'min_likes_received_days',
+    'num_likes_received', 'min_likes_received',
-             'num_likes_received_users', 'min_likes_received_users',
+    'num_likes_received_days', 'min_likes_received_days',
-             'trust_level_locked')
+    'num_likes_received_users', 'min_likes_received_users',
+    'trust_level_locked',
+    'penalty_counts.silenced',
+    'penalty_counts.suspended'
+  )
 });
-
-export default TL3Requirements;

app/assets/javascripts/admin/routes/admin-user-tl3-requirements.js.es6

@@ -1,5 +1,5 @@
 export default Discourse.Route.extend({
-  model: function() {
+  model() {
     return this.modelFor('adminUser');
   }
 });

app/assets/javascripts/admin/templates/user-tl3-requirements.hbs

@@ -96,6 +96,18 @@
         <td>{{model.tl3Requirements.num_likes_received_users}}</td>
         <td>{{model.tl3Requirements.min_likes_received_users}}</td>
       </tr>
+      <tr>
+        <th>{{i18n 'admin.user.tl3_requirements.silenced'}}</th>
+        <td>{{check-icon model.tl3Requirements.met.silenced}}</td>
+        <td>{{model.tl3Requirements.penalty_counts.silenced}}</td>
+        <td>0</td>
+      </tr>
+      <tr>
+        <th>{{i18n 'admin.user.tl3_requirements.suspended'}}</th>
+        <td>{{check-icon model.tl3Requirements.met.suspended}}</td>
+        <td>{{model.tl3Requirements.penalty_counts.suspended}}</td>
+        <td>0</td>
+      </tr>
     </tbody>
   </table>
 

app/models/trust_level3_requirements.rb

@@ -2,6 +2,19 @@
 # the Leader (3) trust level.
 class TrustLevel3Requirements
 
+  class PenaltyCounts
+    attr_reader :silenced, :suspended
+
+    def initialize(row)
+      @silenced = row['silence_count'] || 0
+      @suspended = row['suspend_count'] || 0
+    end
+
+    def total
+      @silenced + @suspended
+    end
+  end
+
   include ActiveModel::Serialization
 
   LOW_WATER_MARK = 0.9
@@ -29,6 +42,7 @@ class TrustLevel3Requirements
 
     (!@user.suspended?) &&
       (!@user.silenced?) &&
+      penalty_counts.total == 0 &&
       days_visited >= min_days_visited &&
       num_topics_replied_to >= min_topics_replied_to &&
       topics_viewed >= min_topics_viewed &&
@@ -48,6 +62,7 @@ class TrustLevel3Requirements
 
     @user.suspended? ||
       @user.silenced? ||
+      penalty_counts.total > 0 ||
       days_visited < min_days_visited * LOW_WATER_MARK ||
       num_topics_replied_to < min_topics_replied_to * LOW_WATER_MARK ||
       topics_viewed < min_topics_viewed * LOW_WATER_MARK ||
@@ -78,6 +93,38 @@ class TrustLevel3Requirements
     @user.user_visits.where("visited_at > ? and posts_read > 0", time_period.days.ago).count
   end
 
+  def penalty_counts
+    args = {
+      user_id: @user.id,
+      silence_user: UserHistory.actions[:silence_user],
+      unsilence_user: UserHistory.actions[:unsilence_user],
+      suspend_user: UserHistory.actions[:suspend_user],
+      unsuspend_user: UserHistory.actions[:unsuspend_user]
+    }
+
+    sql = <<~SQL
+      SELECT SUM(
+          CASE
+            WHEN action = :silence_user THEN 1
+            WHEN action = :unsilence_user THEN -1
+            ELSE 0
+          END
+        ) AS silence_count,
+        SUM(
+          CASE
+            WHEN action = :suspend_user THEN 1
+            WHEN action = :unsuspend_user THEN -1
+            ELSE 0
+          END
+        ) AS suspend_count
+      FROM user_histories AS uh
+      WHERE uh.target_user_id = :user_id
+        AND uh.action IN (:silence_user, :unsilence_user, :suspend_user, :unsuspend_user)
+    SQL
+
+    PenaltyCounts.new(UserHistory.exec_sql(sql, args).first)
+  end
+
   def min_days_visited
     SiteSetting.tl3_requires_days_visited
   end

app/serializers/penalty_counts_serializer.rb

@@ -0,0 +1,11 @@
+class PenaltyCountsSerializer < ApplicationSerializer
+  attributes :silenced, :suspended
+
+  def silenced
+    object.silenced
+  end
+
+  def suspended
+    object.suspended
+  end
+end

app/serializers/trust_level3_requirements_serializer.rb

@@ -1,4 +1,9 @@
+require_dependency 'penalty_counts_serializer'
+
 class TrustLevel3RequirementsSerializer < ApplicationSerializer
+
+  has_one :penalty_counts, embed: :object, serializer: PenaltyCountsSerializer
+
   attributes :time_period,
              :requirements_met,
              :requirements_lost,

config/locales/client.en.yml

@@ -3667,6 +3667,8 @@ en:
           likes_received: "Likes Received"
           likes_received_days: "Likes Received: unique days"
           likes_received_users: "Likes Received: unique users"
+          suspended: "Suspended (all time)"
+          silenced: "Silenced (all time)"
           qualifies: "Qualifies for trust level 3."
           does_not_qualify: "Doesn't qualify for trust level 3."
           will_be_promoted: "Will be promoted soon."

spec/models/trust_level3_requirements_spec.rb

@@ -4,6 +4,7 @@ describe TrustLevel3Requirements do
 
   let(:user) { Fabricate.build(:user) }
   subject(:tl3_requirements) { described_class.new(user) }
+  let(:moderator) { Fabricate(:moderator) }
 
   before do
     described_class.clear_cache
@@ -14,6 +15,38 @@ describe TrustLevel3Requirements do
   end
 
   describe "requirements" do
+
+    describe "penalty_counts" do
+
+      it "returns if the user has ever been silenced" do
+        expect(tl3_requirements.penalty_counts.silenced).to eq(0)
+        expect(tl3_requirements.penalty_counts.total).to eq(0)
+        UserSilencer.new(user, moderator).silence
+        expect(tl3_requirements.penalty_counts.silenced).to eq(1)
+        expect(tl3_requirements.penalty_counts.total).to eq(1)
+        UserSilencer.new(user, moderator).unsilence
+        expect(tl3_requirements.penalty_counts.silenced).to eq(0)
+        expect(tl3_requirements.penalty_counts.total).to eq(0)
+      end
+
+      it "returns if the user has ever been suspended" do
+        user.save
+
+        expect(tl3_requirements.penalty_counts.suspended).to eq(0)
+        expect(tl3_requirements.penalty_counts.total).to eq(0)
+
+        UserHistory.create(target_user_id: user.id, action: UserHistory.actions[:suspend_user])
+
+        expect(tl3_requirements.penalty_counts.suspended).to eq(1)
+        expect(tl3_requirements.penalty_counts.total).to eq(1)
+
+        UserHistory.create(target_user_id: user.id, action: UserHistory.actions[:unsuspend_user])
+
+        expect(tl3_requirements.penalty_counts.suspended).to eq(0)
+        expect(tl3_requirements.penalty_counts.total).to eq(0)
+      end
+    end
+
     it "time_period uses site setting" do
       SiteSetting.tl3_time_period = 80
       expect(tl3_requirements.time_period).to eq(80)
@@ -366,6 +399,18 @@ describe TrustLevel3Requirements do
       expect(tl3_requirements.requirements_met?).to eq(false)
     end
 
+    it "are not met if previously silenced" do
+      user.save
+      UserHistory.create(target_user_id: user.id, action: UserHistory.actions[:silence_user])
+      expect(tl3_requirements.requirements_met?).to eq(false)
+    end
+
+    it "are not met if previously suspended" do
+      user.save
+      UserHistory.create(target_user_id: user.id, action: UserHistory.actions[:suspend_user])
+      expect(tl3_requirements.requirements_met?).to eq(false)
+    end
+
     it "are lost if not enough likes received on different days" do
       tl3_requirements.stubs(:num_likes_received_days).returns(4)
       expect(tl3_requirements.requirements_lost?).to eq(true)