From d9944301b0b7fad676c6898e8f1bb33ff629f3fe Mon Sep 17 00:00:00 2001 From: Peter Cai <222655+pcai@users.noreply.github.com> Date: Sat, 26 Nov 2022 22:36:35 +0000 Subject: [PATCH 1/2] preserve order when generating CSV --- lib/rails_admin/support/csv_converter.rb | 15 ++++++++--- .../rails_admin/support/csv_converter_spec.rb | 27 +++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/lib/rails_admin/support/csv_converter.rb b/lib/rails_admin/support/csv_converter.rb index ae9a20c888..5bda506360 100644 --- a/lib/rails_admin/support/csv_converter.rb +++ b/lib/rails_admin/support/csv_converter.rb @@ -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) + 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 diff --git a/spec/rails_admin/support/csv_converter_spec.rb b/spec/rails_admin/support/csv_converter_spec.rb index 5baeb951a4..c5a4c418db 100644 --- a/spec/rails_admin/support/csv_converter_spec.rb +++ b/spec/rails_admin/support/csv_converter_spec.rb @@ -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 + 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 From b1343c285f4bc0aa90f8078fca7cde9bf64c5da4 Mon Sep 17 00:00:00 2001 From: Peter Cai <222655+pcai@users.noreply.github.com> Date: Wed, 12 Jul 2023 16:07:18 +0000 Subject: [PATCH 2/2] code review suggestions --- lib/rails_admin/support/csv_converter.rb | 11 +++++------ spec/rails_admin/support/csv_converter_spec.rb | 6 ++++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/rails_admin/support/csv_converter.rb b/lib/rails_admin/support/csv_converter.rb index 431fe234b0..df4f75a1cd 100644 --- a/lib/rails_admin/support/csv_converter.rb +++ b/lib/rails_admin/support/csv_converter.rb @@ -76,16 +76,15 @@ def generate_csv_string(options) if @objects.respond_to?(:page) page_num = 1 - batch = @objects.page(page_num) - while batch.any? + loop do + batch = @objects.page(page_num) + break if batch.blank? + 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 + @objects.each { |object| csv << generate_csv_row(object) } end end end diff --git a/spec/rails_admin/support/csv_converter_spec.rb b/spec/rails_admin/support/csv_converter_spec.rb index 21be44fe65..021fc1fc14 100644 --- a/spec/rails_admin/support/csv_converter_spec.rb +++ b/spec/rails_admin/support/csv_converter_spec.rb @@ -166,11 +166,13 @@ context 'when objects are ordered' do before do - FactoryBot.create_list :player, 30 + FactoryBot.create_list :player, 2 do |player, index| + player.name = "Player #{index}" + end FactoryBot.create :player, name: 'Player zzz' end - let(:objects) { Player.all.order('name desc') } + let(:objects) { Player.all.order(name: :desc) } let(:options) { {} } it 'preserves the ordering' do