Skip to content

Commit

Permalink
Fix list import concurrently creating lists of the same name (#26372)
Browse files Browse the repository at this point in the history
  • Loading branch information
ClearlyClaire committed Aug 7, 2023
1 parent cd6f2b3 commit 30c64bf
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
7 changes: 5 additions & 2 deletions app/services/bulk_import_service.rb
Expand Up @@ -162,10 +162,9 @@ def import_bookmarks!

def import_lists!
rows = @import.rows.to_a
included_lists = rows.map { |row| row.data['list_name'] }.uniq

if @import.overwrite?
included_lists = rows.map { |row| row.data['list_name'] }.uniq

@account.owned_lists.where.not(title: included_lists).destroy_all

# As list membership changes do not retroactively change timeline
Expand All @@ -175,6 +174,10 @@ def import_lists!
end
end

included_lists.each do |title|
@account.owned_lists.find_or_create_by!(title: title)
end

Import::RowWorker.push_bulk(rows) do |row|
[row.id]
end
Expand Down
6 changes: 6 additions & 0 deletions spec/services/bulk_import_row_service_spec.rb
Expand Up @@ -161,6 +161,12 @@
end

include_examples 'common behavior'

it 'does not create a new list' do
account.follow!(target_account)

expect { subject.call(import_row) }.to_not(change { List.where(title: 'my list').count })
end
end
end
end
Expand Down

0 comments on commit 30c64bf

Please sign in to comment.