Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Preserve order when generating CSV #3575

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Preserve order when generating CSV #3575

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d994430
preserve order when generating CSV
pcai Nov 26, 2022
d90183f
Merge branch 'railsadminteam:master' into order-csv-exports
pcai Jul 12, 2023
b1343c2
code review suggestions
pcai Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions lib/rails_admin/support/csv_converter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,22 @@ def export_field_for(method, model_config = @model_config)

def generate_csv_string(options)
generator_options = (options[:generator] || {}).symbolize_keys.delete_if { |_, value| value.blank? }
method = @objects.respond_to?(:find_each) ? :find_each : :each

CSV.generate(**generator_options) do |csv|
csv << generate_csv_header unless options[:skip_header]

@objects.send(method) do |object|
csv << generate_csv_row(object)
if @objects.respond_to?(:page)
pcai marked this conversation as resolved.
Show resolved Hide resolved
page_num = 1
batch = @objects.page(page_num)
while batch.any?
batch.each { |object| csv << generate_csv_row(object) }
page_num += 1
batch = @objects.page(page_num)
end
else
@objects.each do |object|
csv << generate_csv_row(object)
end
end
end
end
Expand Down
27 changes: 27 additions & 0 deletions spec/rails_admin/support/csv_converter_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,5 +151,32 @@
expect(subject[2]).to eq("\n")
end
end

context 'when more than one page of objects' do
before do
FactoryBot.create_list :player, 100
end

let(:objects) { Player.all }
let(:options) { {} }

it 'generates a csv with all objects' do
expect(subject[2].split("\n").count).to be 101
end
end

context 'when objects are ordered' do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is okay to you, please also consider matching whole records. This will make sure of order even for other code changes.

context 'when objects are ordered' do
  # or use before block as before
  let!(players) do
    FactoryBot.create_list :player, 2 do |player, index|
      player.name = "Player #{index}"
    end
  end

  let(:objects) { Player.all.order(name: :desc) }
  let(:options) { {} }

  # modify the subject block to return the array of objects
  it 'preserves the ordering' do
    expect(subject[2].split("\n")[1]).to eq('Player 1', 'Player 2')
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I incorporated some of your suggestions, but matching the whole record is a bit nontrivial because the pk ids are not predictable when randomizing test seed

before do
FactoryBot.create_list :player, 30
FactoryBot.create :player, name: 'Player zzz'
end

let(:objects) { Player.all.order('name desc') }
let(:options) { {} }

it 'preserves the ordering' do
expect(subject[2].split("\n")[1]).to include('Player zzz')
end
end
end
end