REFACTOR: Track manual locked user levels separately from groups

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

@@ -288,7 +288,7 @@
     </div>
     <div class="controls">
       {{#if model.canLockTrustLevel}}
-        {{#if model.trust_level_locked}}
+        {{#if model.manual_locked_trust_level}}
           {{d-icon "lock" title="admin.user.trust_level_locked_tip"}}
           {{d-button action="lockTrustLevel" actionParam=false label="admin.user.unlock_trust_level"}}
         {{else}}

app/controllers/admin/users_controller.rb

@@ -200,14 +200,14 @@ class Admin::UsersController < Admin::AdminController
     guardian.ensure_can_change_trust_level!(@user)
     level = params[:level].to_i
 
-    if !@user.trust_level_locked && [0, 1, 2].include?(level) && Promotion.send("tl#{level + 1}_met?", @user)
-      @user.trust_level_locked = true
-      @user.save
-    end
-
-    if !@user.trust_level_locked && level == 3 && Promotion.tl3_lost?(@user)
-      @user.trust_level_locked = true
-      @user.save
+    if @user.manual_locked_trust_level.nil?
+      if [0, 1, 2].include?(level) && Promotion.send("tl#{level + 1}_met?", @user)
+        @user.manual_locked_trust_level = level
+        @user.save
+      elsif level == 3 && Promotion.tl3_lost?(@user)
+        @user.manual_locked_trust_level = level
+        @user.save
+      end
     end
 
     @user.change_trust_level!(level, log_action_for: current_user)
@@ -225,19 +225,11 @@ class Admin::UsersController < Admin::AdminController
       return render_json_error I18n.t('errors.invalid_boolean')
     end
 
-    @user.trust_level_locked = new_lock == "true"
+    @user.manual_locked_trust_level = (new_lock == "true") ? @user.trust_level : nil
     @user.save
 
     StaffActionLogger.new(current_user).log_lock_trust_level(@user)
-
-    unless @user.trust_level_locked
-      p = Promotion.new(@user)
-      2.times { p.review }
-      p.review_tl2
-      if @user.trust_level == 3 && Promotion.tl3_lost?(@user)
-        @user.change_trust_level!(2, log_action_for: current_user)
-      end
-    end
+    Promotion.recalculate(@user, current_user)
 
     render body: nil
   end

app/jobs/scheduled/tl3_promotions.rb

@@ -6,7 +6,11 @@ module Jobs
     def execute(args)
       # Demotions
       demoted_user_ids = []
-      User.real.where(trust_level: TrustLevel[3], trust_level_locked: false).find_each do |u|
+      User.real.where(
+        trust_level: TrustLevel[3],
+        manual_locked_trust_level: nil,
+        group_locked_trust_level: nil
+      ).find_each do |u|
         # Don't demote too soon after being promoted
         next if u.on_tl3_grace_period?
 
@@ -17,9 +21,11 @@ module Jobs
       end
 
       # Promotions
-      User.real.where(trust_level: TrustLevel[2],
-                      trust_level_locked: false)
-        .where.not(id: demoted_user_ids).find_each do |u|
+      User.real.where(
+        trust_level: TrustLevel[2],
+        manual_locked_trust_level: nil,
+        group_locked_trust_level: nil
+      ).where.not(id: demoted_user_ids).find_each do |u|
         Promotion.new(u).review_tl2
       end
     end

app/models/group_user.rb

@@ -65,12 +65,15 @@ class GroupUser < ActiveRecord::Base
 
   def grant_trust_level
     return if group.grant_trust_level.nil?
+    if (user.group_locked_trust_level || 0) < group.grant_trust_level
+      user.group_locked_trust_level = group.grant_trust_level
+      user.save
+    end
     TrustLevelGranter.grant(group.grant_trust_level, user)
   end
 
   def recalculate_trust_level
     return if group.grant_trust_level.nil?
-    return unless SiteSetting.group_removes_trust_level?
 
     # Find the highest level of the user's remaining groups
     highest_level = GroupUser
@@ -81,12 +84,14 @@ class GroupUser < ActiveRecord::Base
     if highest_level.nil?
       # If the user no longer has a group with a trust level,
       # unlock them, start at 0 and consider promotions.
-      user.trust_level_locked = false
-      user.trust_level = 0
+      user.group_locked_trust_level = nil
       user.save
 
-      Promotion.new(user).review
+      Promotion.recalculate(user)
     else
+      user.group_locked_trust_level = highest_level
+      user.save
+
       user.change_trust_level!(highest_level)
     end
   end

app/models/post.rb

@@ -775,6 +775,8 @@ end
 #  public_version          :integer          default(1), not null
 #  action_code             :string
 #  image_url               :string
+#  trolling_count          :integer          default(0), not null
+#  real_life_threat_count  :integer          default(0), not null
 #
 # Indexes
 #

app/models/topic.rb

@@ -1288,56 +1288,51 @@ end
 #
 # Table name: topics
 #
-#  id                            :integer          not null, primary key
-#  title                         :string           not null
-#  last_posted_at                :datetime
-#  created_at                    :datetime         not null
-#  updated_at                    :datetime         not null
-#  views                         :integer          default(0), not null
-#  posts_count                   :integer          default(0), not null
-#  user_id                       :integer
-#  last_post_user_id             :integer          not null
-#  reply_count                   :integer          default(0), not null
-#  featured_user1_id             :integer
-#  featured_user2_id             :integer
-#  featured_user3_id             :integer
-#  avg_time                      :integer
-#  deleted_at                    :datetime
-#  highest_post_number           :integer          default(0), not null
-#  image_url                     :string
-#  like_count                    :integer          default(0), not null
-#  incoming_link_count           :integer          default(0), not null
-#  category_id                   :integer
-#  visible                       :boolean          default(TRUE), not null
-#  moderator_posts_count         :integer          default(0), not null
-#  closed                        :boolean          default(FALSE), not null
-#  archived                      :boolean          default(FALSE), not null
-#  bumped_at                     :datetime         not null
-#  has_summary                   :boolean          default(FALSE), not null
-#  vote_count                    :integer          default(0), not null
-#  archetype                     :string           default("regular"), not null
-#  featured_user4_id             :integer
-#  notify_moderators_count       :integer          default(0), not null
-#  spam_count                    :integer          default(0), not null
-#  pinned_at                     :datetime
-#  score                         :float
-#  percent_rank                  :float            default(1.0), not null
-#  subtype                       :string
-#  slug                          :string
-#  auto_close_at                 :datetime
-#  auto_close_user_id            :integer
-#  auto_close_started_at         :datetime
-#  deleted_by_id                 :integer
-#  participant_count             :integer          default(1)
-#  word_count                    :integer
-#  excerpt                       :string(1000)
-#  pinned_globally               :boolean          default(FALSE), not null
-#  auto_close_based_on_last_post :boolean          default(FALSE)
-#  auto_close_hours              :float
-#  pinned_until                  :datetime
-#  fancy_title                   :string(400)
-#  highest_staff_post_number     :integer          default(0), not null
-#  featured_link                 :string
+#  id                        :integer          not null, primary key
+#  title                     :string           not null
+#  last_posted_at            :datetime
+#  created_at                :datetime         not null
+#  updated_at                :datetime         not null
+#  views                     :integer          default(0), not null
+#  posts_count               :integer          default(0), not null
+#  user_id                   :integer
+#  last_post_user_id         :integer          not null
+#  reply_count               :integer          default(0), not null
+#  featured_user1_id         :integer
+#  featured_user2_id         :integer
+#  featured_user3_id         :integer
+#  avg_time                  :integer
+#  deleted_at                :datetime
+#  highest_post_number       :integer          default(0), not null
+#  image_url                 :string
+#  like_count                :integer          default(0), not null
+#  incoming_link_count       :integer          default(0), not null
+#  category_id               :integer
+#  visible                   :boolean          default(TRUE), not null
+#  moderator_posts_count     :integer          default(0), not null
+#  closed                    :boolean          default(FALSE), not null
+#  archived                  :boolean          default(FALSE), not null
+#  bumped_at                 :datetime         not null
+#  has_summary               :boolean          default(FALSE), not null
+#  vote_count                :integer          default(0), not null
+#  archetype                 :string           default("regular"), not null
+#  featured_user4_id         :integer
+#  notify_moderators_count   :integer          default(0), not null
+#  spam_count                :integer          default(0), not null
+#  pinned_at                 :datetime
+#  score                     :float
+#  percent_rank              :float            default(1.0), not null
+#  subtype                   :string
+#  slug                      :string
+#  deleted_by_id             :integer
+#  participant_count         :integer          default(1)
+#  word_count                :integer
+#  excerpt                   :string(1000)
+#  pinned_globally           :boolean          default(FALSE), not null
+#  pinned_until              :datetime
+#  fancy_title               :string(400)
+#  highest_staff_post_number :integer          default(0), not null
+#  featured_link             :string
 #
 # Indexes
 #

app/models/trust_level3_requirements.rb

@@ -63,7 +63,7 @@ class TrustLevel3Requirements
   end
 
   def trust_level_locked
-    @user.trust_level_locked
+    !@user.manual_locked_trust_level.nil?
   end
 
   def on_grace_period

app/models/user.rb

@@ -1136,47 +1136,49 @@ end
 #
 # Table name: users
 #
-#  id                      :integer          not null, primary key
-#  username                :string(60)       not null
-#  created_at              :datetime         not null
-#  updated_at              :datetime         not null
-#  name                    :string
-#  seen_notification_id    :integer          default(0), not null
-#  last_posted_at          :datetime
-#  password_hash           :string(64)
-#  salt                    :string(32)
-#  active                  :boolean          default(FALSE), not null
-#  username_lower          :string(60)       not null
-#  last_seen_at            :datetime
-#  admin                   :boolean          default(FALSE), not null
-#  last_emailed_at         :datetime
-#  trust_level             :integer          not null
-#  approved                :boolean          default(FALSE), not null
-#  approved_by_id          :integer
-#  approved_at             :datetime
-#  previous_visit_at       :datetime
-#  suspended_at            :datetime
-#  suspended_till          :datetime
-#  date_of_birth           :date
-#  views                   :integer          default(0), not null
-#  flag_level              :integer          default(0), not null
-#  ip_address              :inet
-#  moderator               :boolean          default(FALSE)
-#  silenced                :boolean          default(FALSE)
-#  title                   :string
-#  uploaded_avatar_id      :integer
-#  locale                  :string(10)
-#  primary_group_id        :integer
-#  registration_ip_address :inet
-#  trust_level_locked      :boolean          default(FALSE), not null
-#  staged                  :boolean          default(FALSE), not null
-#  first_seen_at           :datetime
+#  id                        :integer          not null, primary key
+#  username                  :string(60)       not null
+#  created_at                :datetime         not null
+#  updated_at                :datetime         not null
+#  name                      :string
+#  seen_notification_id      :integer          default(0), not null
+#  last_posted_at            :datetime
+#  password_hash             :string(64)
+#  salt                      :string(32)
+#  active                    :boolean          default(FALSE), not null
+#  username_lower            :string(60)       not null
+#  last_seen_at              :datetime
+#  admin                     :boolean          default(FALSE), not null
+#  last_emailed_at           :datetime
+#  trust_level               :integer          not null
+#  approved                  :boolean          default(FALSE), not null
+#  approved_by_id            :integer
+#  approved_at               :datetime
+#  previous_visit_at         :datetime
+#  suspended_at              :datetime
+#  suspended_till            :datetime
+#  date_of_birth             :date
+#  views                     :integer          default(0), not null
+#  flag_level                :integer          default(0), not null
+#  ip_address                :inet
+#  moderator                 :boolean          default(FALSE)
+#  title                     :string
+#  uploaded_avatar_id        :integer
+#  locale                    :string(10)
+#  primary_group_id          :integer
+#  registration_ip_address   :inet
+#  trust_level_locked        :boolean          default(FALSE), not null
+#  staged                    :boolean          default(FALSE), not null
+#  first_seen_at             :datetime
+#  blizzard_avatar           :string
+#  silenced_till             :datetime
+#  group_locked_trust_level  :integer
+#  manual_locked_trust_level :integer
 #
 # Indexes
 #
 #  idx_users_admin                    (id)
 #  idx_users_moderator                (id)
-#  index_users_on_email               (lower((email)::text)) UNIQUE
 #  index_users_on_last_posted_at      (last_posted_at)
 #  index_users_on_last_seen_at        (last_seen_at)
 #  index_users_on_uploaded_avatar_id  (uploaded_avatar_id)

app/models/user_option.rb

@@ -176,7 +176,7 @@ end
 #  theme_key                        :string
 #  theme_key_seq                    :integer          default(0), not null
 #  allow_private_messages           :boolean          default(TRUE), not null
-#  homepage_id                      :integer          default(null)
+#  homepage_id                      :integer
 #
 # Indexes
 #

app/models/user_visit.rb

@@ -43,9 +43,11 @@ end
 #  visited_at :date             not null
 #  posts_read :integer          default(0)
 #  mobile     :boolean          default(FALSE)
+#  time_read  :integer          default(0), not null
 #
 # Indexes
 #
-#  index_user_visits_on_user_id_and_visited_at  (user_id,visited_at) UNIQUE
-#  index_user_visits_on_visited_at_and_mobile   (visited_at,mobile)
+#  index_user_visits_on_user_id_and_visited_at                (user_id,visited_at) UNIQUE
+#  index_user_visits_on_user_id_and_visited_at_and_time_read  (user_id,visited_at,time_read)
+#  index_user_visits_on_visited_at_and_mobile                 (visited_at,mobile)
 #

app/serializers/admin_user_list_serializer.rb

@@ -12,7 +12,7 @@ class AdminUserListSerializer < BasicUserSerializer
              :created_at_age,
              :username_lower,
              :trust_level,
-             :trust_level_locked,
+             :manual_locked_trust_level,
              :flag_level,
              :username,
              :title,

app/services/anonymous_shadow_creator.rb

@@ -39,7 +39,7 @@ class AnonymousShadowCreator
         username: username,
         active: true,
         trust_level: 1,
-        trust_level_locked: true,
+        manual_locked_trust_level: 1,
         created_at: 1.day.ago # bypass new user restrictions
       )
 

app/services/staff_action_logger.rb

@@ -89,7 +89,7 @@ class StaffActionLogger
 
   def log_lock_trust_level(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user && user.is_a?(User)
-    UserHistory.create!(params(opts).merge(action: UserHistory.actions[user.trust_level_locked ? :lock_trust_level : :unlock_trust_level],
+    UserHistory.create!(params(opts).merge(action: UserHistory.actions[user.manual_locked_trust_level.nil? ? :unlock_trust_level : :lock_trust_level],
                                            target_user_id: user.id))
   end
 

app/services/trust_level_granter.rb

@@ -11,7 +11,6 @@ class TrustLevelGranter
   def grant
     if @user.trust_level < @trust_level
       @user.change_trust_level!(@trust_level)
-      @user.trust_level_locked = true
       @user.save!
     end
   end

config/locales/server.en.yml

@@ -1040,7 +1040,6 @@ en:
 
     max_topics_in_first_day: "The maximum number of topics a user is allowed to create in the 24 hour period after creating their first post"
     max_replies_in_first_day: "The maximum number of replies a user is allowed to create in the 24 hour period after creating their first post"
-    group_removes_trust_level: "When a user is removed grom a group, their adjusted trust level should be reverted."
 
     tl2_additional_likes_per_day_multiplier: "Increase limit of likes per day for tl2 (member) by multiplying with this number"
     tl3_additional_likes_per_day_multiplier: "Increase limit of likes per day for tl3 (regular) by multiplying with this number"

config/site_settings.yml

@@ -908,8 +908,6 @@ trust:
   tl3_links_no_follow:
     default: false
     client: true
-  group_removes_trust_level:
-    default: false
 
 security:
   force_https:

db/fixtures/009_users.rb

@@ -83,6 +83,17 @@ ColumnDropper.drop(
   }
 )
 
+ColumnDropper.drop(
+  table: 'users',
+  after_migration: 'AddTrustLevelLocksToUsers',
+  columns: %w[
+    trust_level_locked
+  ],
+  on_drop: ->() {
+    STDERR.puts 'Removing user trust_level_locked!'
+  }
+)
+
 # User for the smoke tests
 if ENV["SMOKE"] == "1"
   UserEmail.seed do |ue|

db/migrate/20171123200157_add_trust_level_locks_to_users.rb

@@ -0,0 +1,27 @@
+class AddTrustLevelLocksToUsers < ActiveRecord::Migration[5.1]
+  def up
+    add_column :users, :group_locked_trust_level, :integer, null: true
+    add_column :users, :manual_locked_trust_level, :integer, null: true
+
+    execute <<~SQL
+      UPDATE users SET manual_locked_trust_level = trust_level WHERE trust_level_locked
+    SQL
+
+    execute <<~SQL
+      UPDATE users SET group_locked_trust_level = x.tl
+      FROM users AS u
+      INNER JOIN (
+        SELECT gu.user_id, MAX(g.grant_trust_level) AS tl
+        FROM group_users AS gu
+        INNER JOIN groups AS g ON gu.group_id = g.id
+          WHERE g.grant_trust_level IS NOT NULL
+        GROUP BY gu.user_id
+      ) AS x ON x.user_id = u.id
+      WHERE users.id = u.id
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/promotion.rb

@@ -11,7 +11,7 @@ class Promotion
   # Returns true if the user was promoted, false otherwise.
   def review
     # nil users are never promoted
-    return false if @user.blank? || @user.trust_level_locked
+    return false if @user.blank? || !@user.manual_locked_trust_level.nil?
 
     # Promotion beyond basic requires some expensive queries, so don't do that here.
     return false if @user.trust_level >= TrustLevel[2]
@@ -40,7 +40,7 @@ class Promotion
     old_level = @user.trust_level
     new_level = level
 
-    if new_level < old_level && !@user.trust_level_locked
+    if new_level < old_level && @user.manual_locked_trust_level.nil?
       next_up = new_level + 1
       key = "tl#{next_up}_met?"
       if self.class.respond_to?(key) && self.class.send(key, @user)
@@ -107,4 +107,31 @@ class Promotion
     TrustLevel3Requirements.new(user).requirements_lost?
   end
 
+  # Figure out what a user's trust level should be from scratch
+  def self.recalculate(user, performed_by = nil)
+    # First, use the manual locked level
+    unless user.manual_locked_trust_level.nil?
+      user.trust_level = user.manual_locked_trust_level
+      user.save
+      return
+    end
+
+    # Then consider the group locked level
+    if user.group_locked_trust_level
+      user.trust_level = user.group_locked_trust_level
+      user.save
+      return
+    end
+
+    user.update_column(:trust_level, TrustLevel[0])
+
+    p = Promotion.new(user)
+    p.review_tl0
+    p.review_tl1
+    p.review_tl2
+    if user.trust_level == 3 && Promotion.tl3_lost?(user)
+      user.change_trust_level!(2, log_action_for: performed_by || Discourse.system_user)
+    end
+  end
+
 end

spec/controllers/admin/users_controller_spec.rb

@@ -372,12 +372,14 @@ describe Admin::UsersController do
         @another_user.update_attributes(trust_level: TrustLevel[1])
 
         put :trust_level, params: {
-          user_id: @another_user.id, level: TrustLevel[0]
+          user_id: @another_user.id,
+          level: TrustLevel[0]
         }, format: :json
 
         expect(response).to be_success
         @another_user.reload
-        expect(@another_user.trust_level_locked).to eq(true)
+        expect(@another_user.trust_level).to eq(TrustLevel[0])
+        expect(@another_user.manual_locked_trust_level).to eq(TrustLevel[0])
       end
     end
 

spec/fabricators/user_fabricator.rb

@@ -97,7 +97,7 @@ Fabricator(:anonymous, from: :user) do
   username { sequence(:username) { |i| "anonymous#{i}" } }
   email { sequence(:email) { |i| "anonymous#{i}@anonymous.com" } }
   trust_level TrustLevel[1]
-  trust_level_locked true
+  manual_locked_trust_level TrustLevel[1]
 
   before_create do |user|
     user.custom_fields["master_id"] = 1

spec/models/group_spec.rb

@@ -468,65 +468,38 @@ describe Group do
     expect(u3.reload.trust_level).to eq(3)
   end
 
-  describe "group_removes_trust_level" do
+  it "adjusts the user trust level" do
+    g0 = Fabricate(:group, grant_trust_level: 2)
+    g1 = Fabricate(:group, grant_trust_level: 3)
+    g2 = Fabricate(:group)
 
-    context "set to false" do
-      before do
-        SiteSetting.group_removes_trust_level = false
-      end
+    user = Fabricate(:user, trust_level: 0)
 
-      it "maintains the chosen trust level" do
-        g0 = Fabricate(:group, grant_trust_level: 2)
-        g1 = Fabricate(:group, grant_trust_level: 3)
-        user = Fabricate(:user, trust_level: 0)
+    # Add a group without one to consider `NULL` check
+    g2.add(user)
+    expect(user.group_locked_trust_level).to be_nil
+    expect(user.manual_locked_trust_level).to be_nil
 
-        g0.add(user)
-        expect(user.reload.trust_level).to eq(2)
+    g0.add(user)
+    expect(user.reload.trust_level).to eq(2)
+    expect(user.group_locked_trust_level).to eq(2)
+    expect(user.manual_locked_trust_level).to be_nil
 
-        g1.add(user)
-        expect(user.reload.trust_level).to eq(3)
+    g1.add(user)
+    expect(user.reload.trust_level).to eq(3)
+    expect(user.group_locked_trust_level).to eq(3)
+    expect(user.manual_locked_trust_level).to be_nil
 
-        g1.remove(user)
-        expect(user.reload.trust_level).to eq(3)
-
-        g0.remove(user)
-        expect(user.reload.trust_level).to eq(3)
-      end
-    end
-
-    context "set to true" do
-      before do
-        SiteSetting.group_removes_trust_level = true
-      end
-
-      it "adjusts the user trust level" do
-        g0 = Fabricate(:group, grant_trust_level: 2)
-        g1 = Fabricate(:group, grant_trust_level: 3)
-        g2 = Fabricate(:group)
-
-        # Add a group without one to consider `NULL` check
-        g2.add(user)
-
-        user = Fabricate(:user, trust_level: 0)
-
-        g0.add(user)
-        expect(user.reload.trust_level).to eq(2)
-        expect(user.trust_level_locked?).to eq(true)
-
-        g1.add(user)
-        expect(user.reload.trust_level).to eq(3)
-        expect(user.trust_level_locked?).to eq(true)
-
-        g1.remove(user)
-        expect(user.reload.trust_level).to eq(2)
-        expect(user.trust_level_locked?).to eq(true)
-
-        g0.remove(user)
-        expect(user.reload.trust_level).to eq(0)
-        expect(user.trust_level_locked?).to eq(false)
-      end
-    end
+    g1.remove(user)
+    expect(user.reload.trust_level).to eq(2)
+    expect(user.group_locked_trust_level).to eq(2)
+    expect(user.manual_locked_trust_level).to be_nil
 
+    g0.remove(user)
+    user.reload
+    expect(user.manual_locked_trust_level).to be_nil
+    expect(user.group_locked_trust_level).to be_nil
+    expect(user.trust_level).to eq(0)
   end
 
   it 'should cook the bio' do

spec/models/user_spec.rb

@@ -1270,7 +1270,8 @@ describe User do
 
       expect(user.title).to eq("bars and wats")
       expect(user.trust_level).to eq(1)
-      expect(user.trust_level_locked).to eq(true)
+      expect(user.manual_locked_trust_level).to be_nil
+      expect(user.group_locked_trust_level).to eq(1)
     end
   end
 

spec/services/staff_action_logger_spec.rb

@@ -385,12 +385,12 @@ describe StaffActionLogger do
     end
 
     it "creates a new UserHistory record" do
-      user.trust_level_locked = true
+      user.manual_locked_trust_level = 3
       expect { logger.log_lock_trust_level(user) }.to change { UserHistory.count }.by(1)
       user_history = UserHistory.last
       expect(user_history.action).to eq(UserHistory.actions[:lock_trust_level])
 
-      user.trust_level_locked = false
+      user.manual_locked_trust_level = nil
       expect { logger.log_lock_trust_level(user) }.to change { UserHistory.count }.by(1)
       user_history = UserHistory.last
       expect(user_history.action).to eq(UserHistory.actions[:unlock_trust_level])