From 4b7059417307f2b40d90715e9fe68f743a729ba9 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Fri, 12 Aug 2022 15:45:09 +0300
Subject: [PATCH] FIX: Reset flair group if user is removed from group (#17862)
The flair used to stay set even if the user was removed from the group.
---
app/models/group_user.rb | 12 ++++++++----
...0_reset_flair_group_id_if_not_group_member.rb | 16 ++++++++++++++++
spec/models/group_user_spec.rb | 5 +++--
3 files changed, 27 insertions(+), 6 deletions(-)
create mode 100644 db/migrate/20220811170600_reset_flair_group_id_if_not_group_member.rb
diff --git a/app/models/group_user.rb b/app/models/group_user.rb
index babfaade4e6..5ee988eaa70 100644
--- a/app/models/group_user.rb
+++ b/app/models/group_user.rb
@@ -8,7 +8,7 @@ class GroupUser < ActiveRecord::Base
after_destroy :grant_other_available_title
after_save :set_primary_group
- after_destroy :remove_primary_group, :recalculate_trust_level
+ after_destroy :remove_primary_and_flair_group, :recalculate_trust_level
before_create :set_notification_level
after_save :grant_trust_level
@@ -84,10 +84,14 @@ class GroupUser < ActiveRecord::Base
user.update!(primary_group: group) if group.primary_group
end
- def remove_primary_group
- return if user.primary_group_id != group_id
+ def remove_primary_and_flair_group
return if self.destroyed_by_association&.active_record == User # User is being destroyed, so don't try to update
- user.update_attribute(:primary_group_id, nil)
+
+ updates = {}
+ updates[:primary_group_id] = nil if user.primary_group_id == group_id
+ updates[:flair_group_id] = nil if user.flair_group_id == group_id
+
+ user.update(updates) if updates.present?
end
def grant_other_available_title
diff --git a/db/migrate/20220811170600_reset_flair_group_id_if_not_group_member.rb b/db/migrate/20220811170600_reset_flair_group_id_if_not_group_member.rb
new file mode 100644
index 00000000000..a29db0a5a69
--- /dev/null
+++ b/db/migrate/20220811170600_reset_flair_group_id_if_not_group_member.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class ResetFlairGroupIdIfNotGroupMember < ActiveRecord::Migration[7.0]
+ def change
+ execute <<~SQL
+ UPDATE users
+ SET flair_group_id = NULL
+ WHERE flair_group_id IS NOT NULL AND NOT EXISTS (
+ SELECT 1
+ FROM group_users
+ WHERE group_users.user_id = users.id
+ AND group_users.group_id = users.flair_group_id
+ )
+ SQL
+ end
+end
diff --git a/spec/models/group_user_spec.rb b/spec/models/group_user_spec.rb
index 1ceb91f5960..16b9404b1e7 100644
--- a/spec/models/group_user_spec.rb
+++ b/spec/models/group_user_spec.rb
@@ -226,8 +226,8 @@ RSpec.describe GroupUser do
describe '#destroy!' do
fab!(:group) { Fabricate(:group) }
- it "removes `primary_group_id` and exec `match_primary_group_changes` method on user model" do
- user = Fabricate(:user, primary_group: group)
+ it "removes `primary_group_id`, `flair_group_id` and exec `match_primary_group_changes` method on user model" do
+ user = Fabricate(:user, primary_group: group, flair_group: group)
group_user = Fabricate(:group_user, group: group, user: user)
user.expects(:match_primary_group_changes).once
@@ -235,6 +235,7 @@ RSpec.describe GroupUser do
user.reload
expect(user.primary_group_id).to be_nil
+ expect(user.flair_group_id).to be_nil
end
end
end