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

Consider implement #preload in Draper::CollectionDecorator #812

Open
teohm opened this issue Jun 22, 2017 · 2 comments · May be fixed by #662
Open

Consider implement #preload in Draper::CollectionDecorator #812

teohm opened this issue Jun 22, 2017 · 2 comments · May be fixed by #662
Assignees
@Alexander-Senko
Labels
suggestion

Comments

@teohm
Copy link

teohm commented Jun 22, 2017

We're using datagrid gem together with draper decorators.

They works very well so far, until recently datagrid introduces preload feature, where it actively preloads associations when rendering a ActiveRecord resource.

To fix this breaking change, we have to implement #preload in our Draper::CollectionDecorator subclasses:

class MyCollectionDecorator < Draper::CollectionDecorator
  def preload(*args)
    object.preload(*args).decorate
  end
end

I wonder if it makes sense to move this #preload implementation into Draper::CollectionDecorator?

What do you all think?
@codebycliff
Copy link
Collaborator

Thanks for the suggestion!

I've seen a few issues related to this functionality. Let me look into those and get back to you. At first glance I don't see a problem with adding this functionality, but I'll want to get a little more context before I give you a full answer.

@PragTob PragTob mentioned this issue Dec 11, 2017
Closed
@brunohkbx brunohkbx mentioned this issue Feb 20, 2019
Merged
codebycliff pushed a commit that referenced this issue Feb 25, 2019
@brunohkbx @codebycliff
feat: Include ORM associations in CollectionDecorator (#845)
6b3e9bc 
## Description
Include all QueryMethods from the ORM in CollectionDecorator. The default adapter is :active_record

* Why was this change required?
It was necessary to delegate or define a method of the ORM which you are using in your decorator to make an instance of CollectionDecorator able to call it.
* Is there something you aren't happy with or that needs extra attention?
In order to support other ORM associations, we'll need to write a method `allowed?` for each strategy at `lib/draper/query_methods/load_strategy.rb`

## Testing
1. Create a decorator for the model and its association
```ruby
class OrderHistoryDecorator < Draper::Decorator
  delegate_all
end

class OrderDecorator < Draper::Decorator
  delegate_all

  decorates_association :order_histories, with: OrderHistoryDecorator
end
```

2. Call any query method in the decorated instance
```ruby
pry(main)> Order.last.decorate.order_histories.includes(:user)
```

## References
* Issue #702 
* Issue #812
@Alexander-Senko
Copy link
Collaborator

I wonder if it makes sense to move this #preload implementation into Draper::CollectionDecorator?

IMO, it doesn't as Draper::CollectionDecorator may decorate collections not backed by a DB. It's not meant to be used solely with Active Record, as I can see it.

Instead, there is a proposal of RelationDecorator that could solve this.

@Alexander-Senko Alexander-Senko self-assigned this Sep 7, 2024
@Alexander-Senko Alexander-Senko linked a pull request Sep 7, 2024 that will close this issue
Add better support for activerecord relations and mongoid criteria #662
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Alexander-Senko Alexander-Senko

Labels
suggestion
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add better support for activerecord relations and mongoid criteria donaldpiret/draper
3 participants
@teohm @codebycliff @Alexander-Senko