Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 14 additions & 9 deletions app/views/rails_admin/main/_delete_notice.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@
- else
= wording
%ul
- @abstract_model.each_associated_children(object) do |association, child|
%li
- child_config = RailsAdmin.config(child)
= @abstract_model.model.human_attribute_name association.name
- wording = child.send(child_config.object_label_method)
- if child.id && (show_action = action(:show, child_config.abstract_model, child))
= link_to(wording, url_for(action: show_action.action_name, model_name: child_config.abstract_model.to_param, id: child.id), class: 'pjax')
- else
= wording
- @abstract_model.each_associated_children(object) do |association, children|
- humanized_association = @abstract_model.model.human_attribute_name association.name
- limit = children.count > 12 ? 10 : children.count
- children.first(limit).each do |child|
= content_tag_for :li, child do
- child_config = RailsAdmin.config(child)
= humanized_association.singularize
- wording = child.send(child_config.object_label_method)
- if child.id && (show_action = action(:show, child_config.abstract_model, child))
= link_to(wording, url_for(action: show_action.action_name, model_name: child_config.abstract_model.to_param, id: child.id), class: 'pjax')
- else
= wording
- if children.count > limit
%li= t('admin.misc.more', count: children.count - limit, models_name: humanized_association)
1 change: 1 addition & 0 deletions config/locales/rails_admin.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ en:
navigation_static_label: "Links"
log_out: "Log out"
ago: "ago"
more: "Plus %{count} more %{models_name}"
flash:
successful: "%{name} successfully %{action}"
error: "%{name} failed to be %{action}"
Expand Down
7 changes: 3 additions & 4 deletions lib/rails_admin/abstract_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,11 @@ def each_associated_children(object)
case association.type
when :has_one
if child = object.send(association.name)
yield(association, child)
yield(association, [child])
end
when :has_many
object.send(association.name).each do |child| # rubocop:disable ShadowingOuterLocalVariable
yield(association, child)
end
children = object.send(association.name)
yield(association, Array.new(children))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The problem with this is that it still fetches all children from the database.
I implemented a quick and dirty fix for this here: https://github.com/alexandergitter/rails_admin/commit/6a172da72c98bfd5ca7285b356478a16e7a7a064

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think that children are fetched from database here.
children variable contains a CollectionProxy returned to _delete_notice.html.haml where count and 'first(limit) is called. First n children are fetched here.
Am I wrong ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mhh that's interesting, I didn't notice that.
In my tests Active Record did an unlimited SELECT though, not sure if there's something else that would trigger that... maybe it's because children gets wrapped in a new array? Would that eagerly fetch all children?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, wrapping children in a a new array may fetch database values.

end
end
end
Expand Down
13 changes: 13 additions & 0 deletions spec/integration/basic/delete/rails_admin_basic_delete_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,17 @@
is_expected.to have_link(@player.name, href: "/admin/player/#{@player.id}")
end
end

describe 'delete an object which has many associated item' do
before do
comments = FactoryGirl.create_list :comment, 20
@player = FactoryGirl.create :player, comments: comments
visit delete_path(model_name: 'player', id: @player.id)
end

it 'shows only ten first plus x mores', skip_mongoid: true do
is_expected.to have_selector('.comment', count: 10)
is_expected.to have_content('Plus 10 more Comments')
end
end
end
20 changes: 20 additions & 0 deletions spec/rails_admin/abstract_model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,24 @@
@abstract_model.all(sort: PK_COLUMN, page: 1, per: 2)
end
end

describe 'each_associated_children' do
before do
@abstract_model = RailsAdmin::AbstractModel.new('Player')
@draft = FactoryGirl.build :draft
@comments = FactoryGirl.build_list :comment, 2
@player = FactoryGirl.build :player, draft: @draft, comments: @comments
end

it 'should return has_one and has_many associations with its children' do
@abstract_model.each_associated_children(@player) do |association, children|
expect(children).to eq case association.name
when :draft
[@draft]
when :comments
@comments
end
end
end
end
end