Should I Move My Custom Methods to Model from Controller

Should I move my custom methods to model from controller?

Consider moving your logic into a service object. I think shoving controller logic into the model is simply moving the problem to a different location. Yes, you do isolate the logic to a single area, but there are instances where you end up moving logic to models because of convention rather than the fact that it really belongs there.

A service object can help you reduce duplication and isolate your business logic without getting the model too involved in things it does not need to know about (your repeated json response, for example).

class OrderService
  def initialize(legacy_alphanumeric_product_number)
    # do stuff
  end
  # do more stuff
end

From the controller, you can just call

def check_whatever
  @order = OrderService.new(params[:some_product_code])
  @order.check_something
  # do more stuff
end

Take a look at 7 Patterns to Refactor Fat ActiveRecord Models. I found it very helpful. There is also a RailsCasts episode on service objects (requires pro subscription).

Calling model custom method in controller

Change the class method:

def self.add_manager(params)

To an instance method:

def add_manager(params)

as @rally is an instance of your Rally class

(answered in comments above - removing from unanswered list)

accessing custom method in model

Use local scope instand

public function scopeEmployee($query)
{
   return $query->where('user_type', 'employee');

}

Your controller can be as it was !

    public function index(){
     $users = User::latest()->employee()->get();
     return ProductsResource::collection($users);
}

How to write a custom function in a model?

Guess you are asking about the static functions:

class Order extends Model {
     public static function myMethod() {
     }
}

and you can call it anywhere like

Order::myMethod();

Does it belong to controller or model?

A simple test (of whether it's a controller / model action) is to look at what the action is meant to do - does it work with data or params?

Rails models are meant to be "locked away" from much of the higher-level logic in your application; it's meant to provide a base-level set of actions to manipulate the data being inserted into your db; anything requiring request-level data should be handled in the controller.

As such...

redirect_to store_settings_store_path(id: current_store.id)

... because you're manipulating the flow of your app, your logic should stay in the controller.

If you weren't redirecting (IE were only manipulating data), you'd be able to put this functionality into a before_create callback in your model.

Moving method logic with SQL to model in rails project

You beat me to it (and went, essentially, with my second option). But, I'll post this anyway.

When you do this:

def array_of_disbursable_invoices
  report = report.array_of_disbursable_invoices
  render json: report
end

You're calling array_of_disbursable_invoices on an instance. But, you don't instantiate Report - thus, the undefined method 'array_of_disbursable_invoices' for nil:NilClass error.

So, I think you have two choices:

(1) You could call the method on an instance, something like:

report = Invoice.new.array_of_disbursable_invoices

(2) You could make the method a class method, something like:

class Invoice < ActiveModel::Base
  class << self
    def array_of_disbursable_invoices
      sql = "SELECT MIN(departure_date), ch_invoice.invoice_id
      FROM ch_invoice
      INNER JOIN ch_trip
      ON ch_invoice.invoice_id = ch_trip.invoice_id
      WHERE departure_date <= (SELECT SYS_EXTRACT_UTC(SYSTIMESTAMP)FROM DUAL)
      AND service_rendered = 0
      AND paid = 1
      Group By ch_invoice.invoice_id"

      connection.exec_query(sql)
    end
  end
end

I think I'd recommend (1). Also, personally, I'd use the ActiveRecord query interface instead of the SQL (assuming you have all the associations set up in your models). But, that's a personal preference.

Where should I put my custom code when it doesn't fit into the model or controller?

The logical place to put it would be in the model, so I think having something like Order::import_from_excel would be a good idea. Disclaimer: I'm an advocate of the fat model, slim controller philosophy, so other people might hold different opinions about this.

However, you should encapsulate the functionality into a seperate class in lib/ that gets instantiated and called from import_from_excel. This helps testability, decreases coupling and will probably help you if the requirement to import other things from Excel arises in the future.

Is it okay to hit the database from a custom model binder?

I've decided to edit my original answer given my thinking on these types of things has evolved since early 2010.

In my original answer, I basically expressed that, while my instincts told me you shouldn't do this, I was uncomfortable saying we shouldn't without being able to articulate why.

Now, I'd recommend against this on the grounds that the responsibility of a Model Binder is to translate a user request into a Request Model and that retrieving data outside of what can be derived from the request goes beyond this scope of responsibility.

---

Related Topics