FIX: Clarify User.group_locked_trust_level.

* Rename User.group_locked_trust_level to User.group_granted_trust_level.

* Remove the column from users table.

app/jobs/scheduled/tl3_promotions.rb

@@ -6,10 +6,14 @@ module Jobs
     def execute(args)
       # Demotions
       demoted_user_ids = []
-      User.real.where(
-        trust_level: TrustLevel[3],
-        manual_locked_trust_level: nil
-      ).where("group_locked_trust_level IS NULL OR group_locked_trust_level < ?", TrustLevel[3]).find_each do |u|
+      User.real
+        .joins("LEFT JOIN (SELECT gu.user_id, MAX(g.grant_trust_level) AS group_granted_trust_level FROM groups g, group_users gu WHERE g.id = gu.group_id GROUP BY gu.user_id) tl ON users.id = tl.user_id")
+        .where(
+          trust_level: TrustLevel[3],
+          manual_locked_trust_level: nil
+        )
+        .where("group_granted_trust_level IS NULL OR group_granted_trust_level < ?", TrustLevel[3])
+        .find_each do |u|
         # Don't demote too soon after being promoted
         next if u.on_tl3_grace_period?
 

app/models/group_user.rb

@@ -62,23 +62,12 @@ 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.update!(group_locked_trust_level: group.grant_trust_level)
-    end
-
     TrustLevelGranter.grant(group.grant_trust_level, user)
   end
 
   def recalculate_trust_level
     return if group.grant_trust_level.nil?
 
-    # Find the highest level of the user's remaining groups
-    highest_level = GroupUser
-      .where(user_id: user.id)
-      .includes(:group)
-      .maximum("groups.grant_trust_level")
-
-    user.update!(group_locked_trust_level: highest_level)
     Promotion.recalculate(user)
   end
 end

app/models/user.rb

@@ -21,6 +21,10 @@ class User < ActiveRecord::Base
   include SecondFactorManager
   include HasDestroyedWebHook
 
+  self.ignored_columns = %w{
+    group_locked_trust_level
+  }
+
   has_many :posts
   has_many :notifications, dependent: :destroy
   has_many :topic_users, dependent: :destroy
@@ -342,6 +346,13 @@ class User < ActiveRecord::Base
     find_by(username_lower: username.downcase)
   end
 
+  def group_granted_trust_level
+    GroupUser
+      .where(user_id: id)
+      .includes(:group)
+      .maximum("groups.grant_trust_level")
+  end
+
   def enqueue_welcome_message(message_type)
     return unless SiteSetting.send_welcome_message?
     Jobs.enqueue(:send_system_message, user_id: id, message_type: message_type)

app/services/user_merger.rb

@@ -209,7 +209,6 @@ class UserMerger
         primary_group_id          = COALESCE(t.primary_group_id, s.primary_group_id),
         registration_ip_address   = COALESCE(t.registration_ip_address, s.registration_ip_address),
         first_seen_at             = LEAST(t.first_seen_at, s.first_seen_at),
-        group_locked_trust_level  = GREATEST(t.group_locked_trust_level, s.group_locked_trust_level),
         manual_locked_trust_level = GREATEST(t.manual_locked_trust_level, s.manual_locked_trust_level)
       FROM users AS s
       WHERE t.id = :target_user_id AND s.id = :source_user_id

db/post_migrate/20181012123001_drop_group_locked_trust_level_from_user.rb

@@ -0,0 +1,9 @@
+class DropGroupLockedTrustLevelFromUser < ActiveRecord::Migration[5.2]
+  def up
+    Migration::ColumnDropper.execute_drop(:posts, %i{group_locked_trust_level})
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/promotion.rb

@@ -121,9 +121,11 @@ class Promotion
     end
 
     # Then consider the group locked level
-    if user.group_locked_trust_level
+    user_group_granted_trust_level = user.group_granted_trust_level
+
+    unless user_group_granted_trust_level.blank?
       return user.update!(
-        trust_level: user.group_locked_trust_level
+        trust_level: user_group_granted_trust_level
       )
     end
 

spec/jobs/tl3_promotions_spec.rb

@@ -17,26 +17,29 @@ describe Jobs::Tl3Promotions do
   subject(:run_job) { described_class.new.execute({}) }
 
   it "promotes tl2 user who qualifies for tl3" do
-    _tl2_user = Fabricate(:user, trust_level: TrustLevel[2])
-    create_qualifying_stats(_tl2_user)
+    tl2_user = Fabricate(:user, trust_level: TrustLevel[2])
+    create_qualifying_stats(tl2_user)
     TrustLevel3Requirements.any_instance.stubs(:requirements_met?).returns(true)
     Promotion.any_instance.expects(:change_trust_level!).with(TrustLevel[3], anything).once
     run_job
   end
 
-  it "promotes a qualifying tl2 user who has a group_locked_trust_level" do
-    _group_locked_user = Fabricate(:user, trust_level: TrustLevel[2], group_locked_trust_level: TrustLevel[1])
-    create_qualifying_stats(_group_locked_user)
+  it "promotes a qualifying tl2 user who has a group_granted_trust_level" do
+    group = Fabricate(:group, grant_trust_level: 1)
+    group_locked_user = Fabricate(:user, trust_level: TrustLevel[2])
+    group.add(group_locked_user)
+
+    create_qualifying_stats(group_locked_user)
     TrustLevel3Requirements.any_instance.stubs(:requirements_met?).returns(true)
     Promotion.any_instance.expects(:change_trust_level!).with(TrustLevel[3], anything).once
     run_job
   end
 
   it "doesn't promote tl1 and tl0 users who have met tl3 requirements" do
-    _tl1_user = Fabricate(:user, trust_level: TrustLevel[1])
-    _tl0_user = Fabricate(:user, trust_level: TrustLevel[0])
-    create_qualifying_stats(_tl1_user)
-    create_qualifying_stats(_tl0_user)
+    tl1_user = Fabricate(:user, trust_level: TrustLevel[1])
+    tl0_user = Fabricate(:user, trust_level: TrustLevel[0])
+    create_qualifying_stats(tl1_user)
+    create_qualifying_stats(tl0_user)
     TrustLevel3Requirements.any_instance.expects(:requirements_met?).never
     Promotion.any_instance.expects(:change_trust_level!).never
     run_job
@@ -91,10 +94,12 @@ describe Jobs::Tl3Promotions do
       expect(user.reload.trust_level).to eq(TrustLevel[3])
     end
 
-    it "demotes a user with a group_locked_trust_level of 2" do
+    it "demotes a user with a group_granted_trust_level of 2" do
+      group = Fabricate(:group, grant_trust_level: 2)
       user = nil
       freeze_time(4.days.ago) do
-        user = Fabricate(:user, trust_level: TrustLevel[3], group_locked_trust_level: TrustLevel[2])
+        user = Fabricate(:user, trust_level: TrustLevel[3])
+        group.add(user)
       end
       TrustLevel3Requirements.any_instance.stubs(:requirements_met?).returns(false)
       TrustLevel3Requirements.any_instance.stubs(:requirements_lost?).returns(true)
@@ -103,10 +108,12 @@ describe Jobs::Tl3Promotions do
 
     end
 
-    it "doesn't demote user if their group_locked_trust_level is 3" do
+    it "doesn't demote user if their group_granted_trust_level is 3" do
+      group = Fabricate(:group, grant_trust_level: 3)
       user = nil
       freeze_time(4.days.ago) do
-        user = Fabricate(:user, trust_level: TrustLevel[3], group_locked_trust_level: TrustLevel[3])
+        user = Fabricate(:user, trust_level: TrustLevel[3])
+        group.add(user)
       end
       TrustLevel3Requirements.any_instance.stubs(:requirements_met?).returns(false)
       TrustLevel3Requirements.any_instance.stubs(:requirements_lost?).returns(true)

spec/models/group_spec.rb

@@ -588,28 +588,28 @@ describe Group do
 
       # Add a group without one to consider `NULL` check
       g2.add(user)
-      expect(user.group_locked_trust_level).to be_nil
+      expect(user.group_granted_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)
-      expect(user.group_locked_trust_level).to eq(2)
+      expect(user.group_granted_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)
-      expect(user.group_locked_trust_level).to eq(3)
+      expect(user.group_granted_trust_level).to eq(3)
       expect(user.manual_locked_trust_level).to be_nil
 
       g1.remove(user)
       expect(user.reload.trust_level).to eq(2)
-      expect(user.group_locked_trust_level).to eq(2)
+      expect(user.group_granted_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.group_granted_trust_level).to be_nil
       expect(user.trust_level).to eq(0)
     end
   end

spec/models/user_spec.rb

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