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

Draper changes the behaviour of url_for when using STI models #886

Open
daniel-rikowski opened this issue Jul 22, 2020 · 1 comment
Open

Comments

@daniel-rikowski
Copy link

daniel-rikowski commented Jul 22, 2020

I'm using Draper 4.0.1 and I have these models:

class Document < AR::Base; end;
class InputDocument < Document; end;
class OutputDocument < Document; end;

I also have a DocumentDecorator, but no dedicated decorators for the STI sub classes.

When I call url_for using a decorated InputDocument instance the wrong path helper is generated/called.

Example:

doc = InputDocument.find(1).decorate
doc.class # => DocumentDecorator
doc.object.class # => InputDocument
url_for(doc.object) # => /input_documents (EXPECTED)
url_for(doc) # => /documents (NOT EXPECTED)

I traced the url_for call through the various Rails methods. Eventually doc.model_name gets called, which seems to be the cause of this bug:

Draper doesn't delegate the model_name call to doc.object.class but to doc.class.object_class instead.

doc.object.model_name # => InputDocument
doc.object.class.model_name # => InputDocument
doc.class.object_class.model_name # => Document

Subsequentially Rails gets the wrong ActiveModel::Name and generates the wrong path helper name.

IMHO model_name should be delegated to the decorated object's actual class (or even to the instance itself?) and not the class inferred by the decorator, but I'm not sure this is the best course of action...

@Alexander-Senko
Copy link
Collaborator

Alexander-Senko commented Sep 4, 2024

url_for(doc) # => /documents (NOT EXPECTED)

I'd rather say that's what I expected in this case 🤔

Reason 1

Decorators encapsulate models isolating them from the outer code. Here, the outer code sees a document. If you'd like it to see an input one, just define InputDocumentDecorator.

Reason 2

A quite common use case for decorators is to follow controllers hierarchy. Thus, one model class can be decorated with different decorators depending on the controller scope. In this case it seems quite natural for URLs to match decorator names and not the model ones.

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

No branches or pull requests

2 participants