← Back to Upcase

Where to put common functionality for different controllers?


(Patrik Bóna) #1

I have something like this in my controllers. Both methods are private for the actual controller:

def new
  ...
  restore_user_data_from_cookie
end

def create
  ...
  save_used_data_in_cookie
end

So basically user needs to enter some data just once and then they are stored in cookies and automatically filled in the new form.

Until now I had this functionality just in one controller for one model. But I’ve just added another controller and model and I can feel code smell.

I would like to avoid duplication, but I am not sure where to extract this code. Ideally I would like to have methods like these:

save_data_to_cookies(@model, [:attr1, attr2, ...])
restore_data_from_cookies(@model)

But I am not sure where to put these methods. In pure ruby script, I would create separate class for handling this. But what is the rails way?

Should I put it to ApplicationController? Or create some concern? Module? Also how should I test it? Just integration tests?

Many thanks in advance for all your answers!


(Derek Prior) #2

I’d probably create a module and include it into the controllers that need that behavior. I like to name these ending in -able. It reads really nicely, but it’s not required, obviously. Maybe restorable here or something?

If you use an ActiveSupport::Concern, for your module, you can very easily add the restore_user_data... and save_user_data... calls as filters on the appropriate actions using the included block.

Because the filters will need access to the instance variable, I would add a template method to the module. Something like:

def resource
  raise NotImplementedError, "Restorables must implement #resource`
end

Then your filters in the module use this resource method rther than an instance variable. The controllers that include your module would then have to override resource to provide the right variable.


(Ben Orenstein) #3

Like you, my default move in this situation is to extract a class.

A module would work as well, but comes with some pitfalls. Ruby Science has great discussion on what those are.

As for testing, I’d have unit coverage for the extracted class, plus integration coverage that hits one of the controllers.


(Patrik Bóna) #4

Hello Guys,

Many thanks for your answers!

@derekprior I like your suggestion because for me it is a Rails way. So I’ve implemented something like this:

module Restorable
  private

  def save_user_data
    attributes_to_save.each do |attribute|
      cookies[cookie_name(attribute)] = { value: resource.send(attribute), expires: 3.months.from_now }
    end
  end

  def restore_user_data
    attributes_to_save.each do |attribute|
      resource.send("#{attribute}=", cookies[cookie_name(attribute)])
    end
  end

  def resource
    raise NotImplementedError, 'Restorables must implement #resource'
  end

  def attributes_to_save
    raise NotImplementedError, 'Restorables must implement #attributes_to_save'
  end

  def cookie_name(attribute)
    "#{resource.class.name.downcase}_#{attribute}"
  end
end

Then in my controller I just need to include Restorable and define attributes_to_save and resource methods. Also I need to call restore_user_data in new action and save_user_data in create action.

I’ve wanted to use before_filter to do it automatically as you suggested, but it is not working because instance variable for resource is not set before action is called.

@benorenstein I definitely wanted to try your solution as first. But I was not able to do it. I was even not able to start with tests… Issue is that I have no idea how to access cookies in extracted class. I’ve tried different solutions which came to my mind, do some googling, but without any success.

Could you please point me to right direction? How would you test/implement it?


(Derek Prior) #5

What does the code for the controllers that include restorable look like? You should be able to refactor such that the ivar gets set with memoization in its implementation of resource.


(Patrik Bóna) #6

Well @derekprior. What seemed impossible for me before is now done! :grin: Thanks!

I’ve created version which is working with before filters.

Fo anyone interested, here it is:

module Restorable
  extend ActiveSupport::Concern

  included do
    before_filter :restore_user_data, only: [:new]
    before_filter :save_user_data, only: [:create, :update]
  end  

  private

  def save_user_data
    if resource.valid?
      attributes_to_save.each do |attribute|
        cookies[cookie_name(attribute)] = { value: resource.send(attribute), expires: 3.months.from_now }
      end  
    end  
  end  

  def restore_user_data
    attributes_to_save.each do |attribute|
      resource.send("#{attribute}=", cookies[cookie_name(attribute)])
    end  
  end  

  def resource
    raise NotImplementedError, 'Restorables must implement #resource'
  end  

  def attributes_to_save
    raise NotImplementedError, 'Restorables must implement #attributes_to_save'
  end  

  def cookie_name(attribute)
    "#{resource.class.name.downcase}_#{attribute}"
  end  
end

And I’ve implemented these two additional private methods in my controller:

  def resource
    if params[:solver]
      @solver ||= issue.solvers.build(solver_params)
    else
      @solver ||= issue.solvers.build
    end
  end
    
  def attributes_to_save
    [:first_name, :last_name, :email]
  end

Is there more elegant way how to implement resource? Because in solver_params I require some parameters and it is not working when they are empty…

Anyway I would also like to try out solution suggested by Ben. So If anyone of you can point me to right direction it would be great! Thanks.


(Ben Orenstein) #7

I didn’t say I actually knew how to do this :smile:

I was just suggesting the general approach I’d try first. I’d have to do some digging around in Rails to figure out how to pass the cookie in.


(Patrik Bóna) #8

haha, tricky one :wink:

No issues, thanks for your help anyway.