Fix Groups Import issue that caused old groups memberships to persist

closes VICE-2382

flag=none

test plan:
  - Specs pass
  - Go to ticket and follow reproduce steps
  - After each import, groups membership should be as
      stated on the CSV.

qa risk: low

Change-Id: Ibaa98e165f4f5284caae0bc02011c2c76106ebd7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282770
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Drake Harper <drake.harper@instructure.com>
Product-Review: Drake Harper <drake.harper@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
This commit is contained in:
Omar Gerardo Soto-Fortuño 2022-01-13 10:55:45 -05:00 committed by Omar Soto-Fortuño
parent 233bb6b2e9
commit 0d9fc3aa7d
2 changed files with 47 additions and 1 deletions

View File

@ -109,7 +109,7 @@ class GroupAndMembershipImporter < ActiveRecord::Base
def validate_user(user)
# if they have any memberships, we are moving them via delete and add
GroupMembership.where(group_id: group_category.groups.select(:id), user_id: user.id).take&.destroy
GroupMembership.where(group_id: group_category.groups.select(:id), user_id: user.id).destroy_all
end
def user_from_row(row)

View File

@ -90,6 +90,52 @@ describe GroupAndMembershipImporter do
expect(progress.workflow_state).to eq "completed"
end
it "works multiple times" do
import_csv_data(%(user_id,group_name
user_0, first group
user_1, second group
user_2, third group
user_3, third group
user_4, first group))
expect(Pseudonym.where(user: gc1.groups.where(name: "first group").take.users).pluck(:sis_user_id).sort).to eq %w[user_0 user_4]
expect(Pseudonym.where(user: gc1.groups.where(name: "second group").take.users).pluck(:sis_user_id).sort).to eq ["user_1"]
expect(Pseudonym.where(user: gc1.groups.where(name: "third group").take.users).pluck(:sis_user_id).sort).to eq %w[user_2 user_3]
import_csv_data(%(user_id,group_name
user_0, first group
user_1, first group
user_2, first group
user_3, third group
user_4, third group))
expect(Pseudonym.where(user: gc1.groups.where(name: "first group").take.users).pluck(:sis_user_id).sort).to eq %w[user_0 user_1 user_2]
expect(Pseudonym.where(user: gc1.groups.where(name: "second group").take.users).pluck(:sis_user_id).sort).to eq []
expect(Pseudonym.where(user: gc1.groups.where(name: "third group").take.users).pluck(:sis_user_id).sort).to eq %w[user_3 user_4]
import_csv_data(%(user_id,group_name
user_0, third group
user_1, third group
user_2, third group
user_3, third group
user_4, third group))
expect(Pseudonym.where(user: gc1.groups.where(name: "first group").take.users).pluck(:sis_user_id).sort).to eq []
expect(Pseudonym.where(user: gc1.groups.where(name: "second group").take.users).pluck(:sis_user_id).sort).to eq []
expect(Pseudonym.where(user: gc1.groups.where(name: "third group").take.users).pluck(:sis_user_id).sort).to eq %w[user_0 user_1 user_2 user_3 user_4]
import_csv_data(%(user_id,group_name
user_0, first group
user_1, first group
user_2, first group
user_3, first group
user_4, first group))
expect(Pseudonym.where(user: gc1.groups.where(name: "first group").take.users).pluck(:sis_user_id).sort).to eq %w[user_0 user_1 user_2 user_3 user_4]
expect(Pseudonym.where(user: gc1.groups.where(name: "second group").take.users).pluck(:sis_user_id).sort).to eq []
expect(Pseudonym.where(user: gc1.groups.where(name: "third group").take.users).pluck(:sis_user_id).sort).to eq []
end
it "skips invalid_users" do
progress = import_csv_data(%(user_id,group_name
user_0, first group