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.

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

Would be nice to have a :module option to decorates_association #809

Open
itkin opened this issue Jun 12, 2017 · 2 comments · May be fixed by #835
Open

Would be nice to have a :module option to decorates_association #809

itkin opened this issue Jun 12, 2017 · 2 comments · May be fixed by #835

Comments

@itkin
Copy link

itkin commented Jun 12, 2017

Hello,

I use drapper to decorates models in the context of my app and its admin interface.

Say for a model class Document i have 2 different decorators DocumentDecorator and Admin::DocumentDecorator.

On the other hand i heavily use STI. Currently decorates_association handle STI pretty well but not when you have more than one decorator per STI class.

Would be nice to have a :module option to decorates_association, eventually settable at the decorator class level for all associations, in order to handle STI associations more nicely :

class UserDecorator < ApplicationDecorator
  decorates_association :documents
end

class DocumentDecorator  < ApplicationDecorator
end

class BillingDocumentDecorator  < DocumentDecorator
end

class KycDocumentDecorator  < DocumentDecorator
end

class Admin::UserDecorator < UserDecorator
  decorates_association :documents, module: "Admin"
end

class Admin::DocumentDecorator  < DocumentDecorator
end

class Admin::BillingDocumentDecorator  < Admin::DocumentDecorator
end

class Admin::KycDocumentDecorator  < Admin::DocumentDecorator
end

> user = user.decorate(with: Admin::UserDecorator)
> user.documents
=> [admin_kyc_document_decorator_instance, admin_billing_document_decorator_instance, ..]

I could try to send you a pull request if you find the feature interesting let me know, thx for this nice gem.

EDIT : prefixed example instance variables with 'admin_'

@codebycliff
Copy link
Collaborator

Thanks for the suggestion!

I can see the use case you are after. The inference this gem is currently doing causes quite a bit of headaches, so I'm hesitant to add more magic. However, I feel like we can do a better job of handling this case. It seems like it should be possible to infer the module without the addition of a module option. In your example above, we could potentially infer the Admin module because the UserDecorator is under the Admin namespace.

I'd be happy to entertain a pull request if you are interested in submitting one.

@Alexander-Senko
Copy link
Collaborator

Alexander-Senko commented Aug 31, 2024

In your example above, we could potentially infer the Admin module because the UserDecorator is under the Admin namespace.

👍 to that.

If one wants to be explicit about decorator used, I'd suggest passing of with: parameter to .decorates_association.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants