Handmade decorators in Ruby on Rails. When Drapper causes more problems than solves

Question

I'm using Draper gem in my Rails project to obtain decorator pattern functionality. I thought that gem will help to increase my productivity by separating model logic and view logic.

But overall workflow has become terrifying. I should always add .decorate to the model in every controller.

My decorators classes are a net of delegate_all between each other. Also when I started to use ActiveModel Serializers, I had figured that serializers can't work with my decorators.

I don't like this approach and decided not to use a separate decorator class, but provide a Decorator Module for each model.

Is it OK? What problems could arise with this solution?

module UserDecorator
  extend ActiveSupport::Concern
  
  included do
    def full_name
      "{first_name} #{last_name}"
    end
  end
end

First of all, it's very simple. And I solve the next problems:

1. I don't need to work with decorated inherited class of model. I just provide decorator methods for the existing model class.

2. ActiveSerializer can now use decorator methods.

But I feel that something is wrong with this solution. Please, give me some critics!

Answer 1

Either way works.

Using an included decorator concern on those models that share those concerns is fine.

Using a decorator class for each model that needs decorating is also fine.

The code doesn't "smell" right when you say that you "always add .decorate to [each] model in every controller". The README on the Draper gem says that you should defer decoration as late as possible; that is, within the view.

You can pass a decoration from the controller to the corresponding view, but you can also pass the original, undecorated object, and let the view choose when decorations are needed. Just because you can decorate every object doesn't mean you should.

Having decorator classes with a "net of delegate_all between each other" (with cross-model decorations) also do not have a good "code smell".

It is fairly common to have a parent class decorator make reference to associated object decorations; that's just good OO design. Eg:

report = Report.where("date < ?", selected_date)
report.decorate
report.line_items.decorate

However, if you end up having a lot of common decorations shared across many models, DRYing up your code suggests using a common decorator somewhere -- either in a parent decorator class, or in a shared model concern.

I don't think you should be trying to serialize a decoration; the serialization should be occurring on the model itself. Alternatively, use a distinct decoration for the attributes that require special-case serialization.