Commit 732b8b8c authored by Max Woolf's avatar Max Woolf Committed by Stan Hu

Fix group membership CSV export for invited users

Previously, group members that didn't have an associated
user object would cause the CSV export to fail.

Fixes this to return nil if any of the row information
is unknown rather than throwing an error.

Also leaves the rescue statement in place to avoid
any repeated sidekiq failure and to record any further
failures.

Changelog: fixed
parent 662760b9
......@@ -21,8 +21,8 @@ module Groups
def header_to_value_hash
{
'Username' => 'user_username',
'Name' => 'user_name',
'Username' => -> (member) { member&.user&.username },
'Name' => -> (member) { member&.user&.name },
'Access granted' => -> (member) { member.created_at.to_s(:csv) },
'Access expires' => -> (member) { member.expires_at },
'Max role' => 'human_access',
......
......@@ -16,10 +16,6 @@ module Groups
@response = Groups::Memberships::ExportService.new(container: @group, current_user: @current_user).execute
send_email if @response.success?
rescue Module::DelegationError => exception
# TEMPORARY: Rescue when a User record is not available,
# see https://gitlab.com/gitlab-org/gitlab/-/issues/338707
Raven.capture_exception(exception, tags: { group_id: @group.id, user_id: @current_user.id })
end
private
......
......@@ -89,6 +89,10 @@ RSpec.describe Groups::Memberships::ExportService do
before do
create_list(:group_member, 4, group: group)
create(:group_member, group: group, created_at: '2021-02-01', expires_at: '2022-01-01', user: create(:user, username: 'mwoolf', name: 'Max Woolf'))
create(:group_member, :invited, group: group)
create(:group_member, :ldap, group: group)
create(:group_member, :blocked, group: group)
create(:group_member, :minimal_access, group: group)
end
let(:csv) { CSV.parse(service.execute.payload, headers: true) }
......@@ -97,7 +101,7 @@ RSpec.describe Groups::Memberships::ExportService do
end
it 'has the correct number of rows' do
expect(csv.size).to eq(6)
expect(csv.size).to eq(9)
end
context 'a direct user', :aggregate_failures do
......
......@@ -19,18 +19,4 @@ RSpec.describe Groups::ExportMembershipsWorker do
worker.perform(group.id, user.id)
end
context 'when there is a Module::DelegationError' do
before do
allow_next_instance_of(Groups::Memberships::ExportService) do |service|
allow(service).to receive(:execute).and_raise(Module::DelegationError)
end
end
it 'rescues the exception' do
expect(Notify).not_to receive(:memberships_export_email)
expect(Raven).to receive(:capture_exception).and_call_original
worker.perform(group.id, user.id)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment