Add :namespace option to decorate

[n-rodriguez]

This PR is a rebase on master branch of @ivan-denysov work. It also contains some fixes to finalize the implementation. See https://github.com/drapergem/draper/pull/835#issuecomment-571708445

ping @codebycliff

it closes

• https://github.com/drapergem/draper/pull/835
• https://github.com/drapergem/draper/issues/809

[dan-jensen]

@n-rodriguez huge respect for the work you've put into this, it looks like you've done a great job. I was just struggling with decorating a namespaced model for a while and came across this PR.

I wanted to comment because ended up solving my problem a very simple way: by namespacing the decorator the same way as the model. In my case, app/decorators/help/article_decorator.rb defines class Help::ArticleDecorator < Draper::Decorator. Given that this works, I wonder if it's the best solution because 1) it already exists, 2) it relies on convention over configuration, and 3) it plays nice with Rails autoloaders (both classic and zeitwerk both complain if the file namespace doesn't match the class namespace - "expected {FILE} to define constant {CLASS}"). But I realize your code may have benefits I'm not aware of. What do you think?

I just pushed a PR with a namespace example. (I don't believe it conflicts with this PR in any way.) Would love to hear your comments on it because you know so much about this area 🙂

[n-rodriguez]

by namespacing the decorator the same way as the model. But I realize your code may have benefits I'm not aware of. What do you think?

It is indeed a solution. But you may not want to do that (namespacing the decorator the same way as the model) but instead use more "conceptual" namespaces.

For instance, my Rails app is splited into 3 different websites (1 Rails app with 2 engines) sharing the same database. So there are 3 decorators namespaces : one for each websites and those namespaces are not bound to a specific model.

It looks like :

module BackOffice
  # resides in Rails app : rails_root/app/decorators/back_office/application_decorator.rb
  class ApplicationDecorator < Draper::Decorator
  end

  class UserDecorator < ApplicationDecorator
  end
end

module FrenchSite
  # resides in engine : engine/french_site/app/decorators/french_site/application_decorator.rb
  class ApplicationDecorator < Draper::Decorator
  end

  class FooDecorator < ApplicationDecorator
  end
end

module SpanishSite
  # resides in engine : engine/spanish_site/app/decorators/spanish_site/application_decorator.rb
  class ApplicationDecorator < Draper::Decorator
  end

  class BarDecorator < ApplicationDecorator
  end
end

You can do that only with this PR.

[n-rodriguez]

Ping @codebycliff

[n-rodriguez]

Hi there! Any news?

[n-rodriguez]

Ping @codebycliff. It would be nice to have some news...

I hope I've not worked for nothing....

[n-rodriguez]

ping @codebycliff

[n-rodriguez]

ping @codebycliff

[n-rodriguez]

is it possible to have some fucking news?

[codebycliff]

@n-rodriguez While I understand your frustration and apologize for the delay, I don't appreciate the language and don't really think anyone in the industry would either. We are all volunteers here – where we are in fact working for nothing. The reason for my delay is two fold:

The first being I simply don't have the time to maintain this gem anymore. My work and personal responsibilities have changed significantly since I took on the maintenance of this gem and I don't have the same amount of free time that I once did.

The second reason is the nature of this change. This pull request is adding more functionality around inference, which is the largest headache of this gem. When I originally took over maintenance of this gem, the original author (@jcasimir) really wanted me to remove the inference entirely per this issue. While I never was able to lead up that effort, I'm hesitant to introduce more functionality around the #decorate method. I alluded to this hesitancy in the original issue that was opened. The direction the original author wanted to go was adding functionality like this through a contrib package that adds the functionality. I'd recommend this approach if you need to get this functionality out into the wild quickly. Otherwise, the little time I have to dedicate to this gem will be focused on bug fixes and releasing new versions.

If you (or anyone else) would like to volunteer to helping maintain the repository going forward, that is something we can discuss. I just don't want to merge new features and not be around to support them. Let me know your thoughts. Again, I apologize for the delay.

[tovodeverett]

From the sidelines, a few observations:

• I totally understand the frustration @n-rodriguez expressed (3.5 months is a long time), and while I agree with @codebycliff that the language used in the comment was inappropriate, I can also understand where it comes from.
• I totally understand how life can take over and make it hard to find time for even short efforts. I don't know what's going on in @codebycliff's life, but things changed dramatically for me when I started having kids. If I can't work on something during paid time, it's a struggle to decide between scratching a personal itch and spending time with my kids so that they can do things we love to do (skiing, mountain biking, etc.).
• Hopefully @codebycliff's measured response will lead to a de-escalation and attract some new blood into helming the project so that @codebycliff can share the reins with someone who has the time to shepherd the project. That would be an awesome outcome for those of us who rely upon draper, but definitely don't have the time or skills to manage it.

[n-rodriguez]

FYI I've made a fork that implements this feature : https://github.com/jbox-web/draper

[Alexander-Senko]

We'd better had a class lookup here.

I guess, it's a core feature we could start with. It doesn't have that great impact of the new namespaces API and can be implemented on it's own resolving #809.

[Alexander-Senko]

A longer history of the subject: #373 → #480 → #494 → #499.