diff --git a/app/jobs/organization_event_job.rb b/app/jobs/organization_event_job.rb index 44d740d36f..dfa6fc1140 100644 --- a/app/jobs/organization_event_job.rb +++ b/app/jobs/organization_event_job.rb @@ -5,21 +5,31 @@ class OrganizationEventJob < ApplicationJob queue_as :github_event # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/MethodLength def perform(payload_body) return true unless payload_body.dig("action") == "member_removed" github_user_id = payload_body.dig("membership", "user", "id") github_organization_id = payload_body.dig("organization", "id") - @organization = Organization.find_by(github_id: github_organization_id) + organizations = Organization.where(github_id: github_organization_id) - return false if @organization.blank? - return false if @organization.users.count == 1 + return false if organizations.empty? - @user = @organization.users.find_by(uid: github_user_id) + failed_removals = organizations.reject do |org| + user = org.users.find_by(uid: github_user_id) - return true unless @user - TransferAssignmentsService.new(@organization, @user).transfer - @organization.users.delete(@user) + if user + TransferAssignmentsService.new(org, user).transfer + org.users.delete(user) + else + false + end + end + + # This method should only report success (by returning true) if we were able to remove + # the user from all of the organizations tied to the given github_organization_id + failed_removals.empty? end # rubocop:enable Metrics/AbcSize + # rubocop:enable Metrics/MethodLength end diff --git a/lib/tasks/remove_classroom_users_without_github_org_access.rake b/lib/tasks/remove_classroom_users_without_github_org_access.rake new file mode 100644 index 0000000000..6e84d497a8 --- /dev/null +++ b/lib/tasks/remove_classroom_users_without_github_org_access.rake @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# Checks if classroom admins have access to the associated GitHub organization. If not +# the admin is removed from the classroom +task :remove_classroom_users_without_github_org_access do + User.find_in_batches(batch_size: 250).with_index do |group, batch| + puts "Processing batch ##{batch}" + group.each do |user| + github_org_ids = user.organizations.pluck(:github_id).uniq + next unless github_org_ids.any? + + github_org_ids.each do |gh_id| + github_org = GitHubOrganization.new(user.github_client, gh_id) + next if github_org.admin?(user.github_login) + + payload = { + "action": "member_removed", + "membership": { "user": { "id": user.github_user.id } }, + "organization": { "id": gh_id } + } + + puts "Removing #{user.github_login} from org with id ##{gh_id}" + OrganizationEventJob.perform_later(payload) + end + end + end +end diff --git a/spec/jobs/organization_event_job_spec.rb b/spec/jobs/organization_event_job_spec.rb index cefbefd0ac..da95469e11 100644 --- a/spec/jobs/organization_event_job_spec.rb +++ b/spec/jobs/organization_event_job_spec.rb @@ -42,9 +42,23 @@ OrganizationEventJob.perform_now(payload) - expect { @organization.users.find(uid: @user.uid) }.to raise_error(ActiveRecord::RecordNotFound) + expect(@organization.users).not_to include(@user) expect(assignment.reload.creator_id).not_to eq(@user.id) end + + it "deletes user from multiple organizations (classrooms) mapped to the same GitHub organization" do + org2 = create(:organization, github_id: payload.dig("organization", "id")) + @user.organizations << org2 + @user.save + + expect(@organization.users).to include(@user) + expect(org2.users).to include(@user) + + OrganizationEventJob.perform_now(payload) + + expect(@organization.users).not_to include(@user) + expect(org2.users).not_to include(@user) + end end end end