diff --git a/app/models/discourse_connect.rb b/app/models/discourse_connect.rb index ff256985858..ede77372f82 100644 --- a/app/models/discourse_connect.rb +++ b/app/models/discourse_connect.rb @@ -187,15 +187,17 @@ class DiscourseConnect < DiscourseConnectBase def synchronize_groups(user) names = (groups || "").split(",").map(&:downcase) - to_be_added = Group.where('LOWER(NAME) in (?) AND NOT automatic', names) - to_be_removed = Group.joins(:group_users).where(automatic: false, group_users: { user_id: user.id }) + current_groups = user.groups.where(automatic: false) + desired_groups = Group.where('LOWER(NAME) in (?) AND NOT automatic', names) - if to_be_added.present? - to_be_removed = to_be_removed.where("groups.id NOT IN (?)", to_be_added.map(&:id)).to_a + to_be_added = desired_groups + if current_groups.present? + to_be_added = to_be_added.where("groups.id NOT IN (?)", current_groups.map(&:id)) end - if to_be_removed.present? - to_be_added = to_be_added.where("groups.id NOT IN (?)", to_be_removed.map(&:id)).to_a + to_be_removed = current_groups + if desired_groups.present? + to_be_removed = to_be_removed.where("groups.id NOT IN (?)", desired_groups.map(&:id)) end if to_be_added.present? || to_be_removed.present? @@ -406,12 +408,8 @@ class DiscourseConnect < DiscourseConnectBase def add_user_to_groups(user, groups) groups.each do |group| - begin - GroupUser.create!(user_id: user.id, group_id: group.id) - GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(user) - rescue Exception => e - Discourse.warn_exception(e, message: "User already in group") - end + GroupUser.create!(user_id: user.id, group_id: group.id) + GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(user) end end diff --git a/spec/models/discourse_connect_spec.rb b/spec/models/discourse_connect_spec.rb index a55a50b519a..81a679ba33e 100644 --- a/spec/models/discourse_connect_spec.rb +++ b/spec/models/discourse_connect_spec.rb @@ -206,7 +206,7 @@ RSpec.describe DiscourseConnect do expect(group2.usernames).to eq("") group1.add(user) - group1.save + group1.save! sso.lookup_or_create_user(ip_address) expect(group1.usernames).to eq("") @@ -216,6 +216,16 @@ RSpec.describe DiscourseConnect do sso.lookup_or_create_user(ip_address) expect(group1.usernames).to eq("") expect(group2.usernames).to eq("") + + group1.add(user) + group1.save! + group2.add(user) + group2.save! + sso.groups = "#{group1.name},badname,trust_level_4" + + sso.lookup_or_create_user(ip_address) + expect(group1.usernames).to eq(user.username) + expect(group2.usernames).to eq("") end it "can specify groups" do