FIX: Group can't be deleted if certain users are demoted.

app/models/group_user.rb

@@ -82,15 +82,8 @@ class GroupUser < ActiveRecord::Base
       .includes(:group)
       .maximum("groups.grant_trust_level")
 
-    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.update!(group_locked_trust_level: nil)
-      Promotion.recalculate(user)
-    else
-      user.update!(group_locked_trust_level: highest_level)
-      user.change_trust_level!(highest_level)
-    end
+    user.update!(group_locked_trust_level: highest_level)
+    Promotion.recalculate(user)
   end
 
 end

lib/promotion.rb

@@ -111,16 +111,16 @@ class Promotion
   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
+      return user.update!(
+        trust_level: user.manual_locked_trust_level
+      )
     end
 
     # Then consider the group locked level
     if user.group_locked_trust_level
-      user.trust_level = user.group_locked_trust_level
-      user.save
-      return
+      return user.update!(
+        trust_level: user.group_locked_trust_level
+      )
     end
 
     user.update_column(:trust_level, TrustLevel[0])

spec/models/group_spec.rb

@@ -541,50 +541,83 @@ describe Group do
     end
   end
 
-  it "correctly grants a trust level to members" do
-    group = Fabricate(:group, grant_trust_level: 2)
-    u0 = Fabricate(:user, trust_level: 0)
-    u3 = Fabricate(:user, trust_level: 3)
+  describe 'trust level management' do
+    it "correctly grants a trust level to members" do
+      group = Fabricate(:group, grant_trust_level: 2)
+      u0 = Fabricate(:user, trust_level: 0)
+      u3 = Fabricate(:user, trust_level: 3)
 
-    group.add(u0)
-    expect(u0.reload.trust_level).to eq(2)
+      group.add(u0)
+      expect(u0.reload.trust_level).to eq(2)
 
-    group.add(u3)
-    expect(u3.reload.trust_level).to eq(3)
-  end
+      group.add(u3)
+      expect(u3.reload.trust_level).to eq(3)
+    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)
+    describe 'when a user has qualified for trust level 1' do
+      let(:user) do
+        Fabricate(:user,
+          trust_level: 1,
+          created_at: Time.zone.now - 10.years
+        )
+      end
 
-    user = Fabricate(:user, trust_level: 0)
+      let(:group) { Fabricate(:group, grant_trust_level: 1) }
+      let(:group2) { Fabricate(:group, grant_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
+      before do
+        user.user_stat.update!(
+          topics_entered: 999,
+          posts_read_count: 999,
+          time_read: 999
+        )
+      end
 
-    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
+      it "should not demote the user" do
+        group.add(user)
+        group2.add(user)
 
-    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
+        expect(user.reload.trust_level).to eq(1)
 
-    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
+        group.remove(user)
 
-    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)
+        expect(user.reload.trust_level).to eq(0)
+      end
+    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)
+
+      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)
+      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)
+      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(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
   end
 
   it 'should cook the bio' do