Reputation: 2285
I have a Subject model which represents a treeview with parent children nodes.
To move a subject to another branch/node, I need to have a 2 subject id's that represent from and to values.
I have started by putting all of the logic into the controller but am now wanting to reuse the copy method and would like to set it up in the model.
Here is some of my controller code.
def copy
from = Subject.find(params[:from])
to = Subject.find(params[:to])
if to.is_descendant_of? from
render :json => {:error => ["Can't move branch because the target is a descendant."]}.to_json, :status => :bad_request
return
end
if to.read_only?
render :json => {:error => ["Can't copy to this branch as it is read only." ]}.to_json, :status => :bad_request
return
end
if params[:subjects] == 'copy'
subject = Subject.create(:name => from.name, :description => from.description, :parent_id => to.id)
#recursively walk the tree
copy_tree(from, subject)
else
#move the tree
if !(from.read_only or to.read_only)
to.children << from
end
end
end
Here is what I started doing in my model
class Subject < ActiveRecord::Base
def self.copy(from, to, operations)
from = Subject.find(from)
to = Subject.find(to)
if to.is_descendant_of? from
#how do I add validation errors on this static method?
end
end
end
My first concern is how to add errors to a static method in a model?
I'm not sure if I'm going about it the right way by using a static method or an instance method.
Anyone able to give me a bit of help in refactoring this code?
Upvotes: 1
Views: 470
Reputation: 64363
You have three possible solutions. ( I prefer the 3rd approach)
1) Return nil on success, error string on failure
# model code
def self.copy(from, to, operations)
if to.is_descendant_of? from
return "Can't move branch because the target is a descendant."
end
end
# controller code
error = Subject.copy(params[:from], params[:to], ..)
if (error)
return render(:json => {:error => [error]}, :status => :bad_request)
end
2) Throw exception on error
def self.copy(from, to, operations)
if to.is_descendant_of? from
throw "Can't move branch because the target is a descendant."
end
end
# controller code
begin
Subject.copy(params[:from], params[:to], ..)
rescue Exception => ex
return render(:json => {:error => [ex.to_s]}, :status => :bad_request)
end
3) Add an instance method on Subject
class.
def copy_from(from, operations)
if is_descendant_of? from
errors.add_to_base("Can't move branch because the target is a descendant.")
return false
end
return true #upon success
end
# controller code
from = Subject.find(params[:from]) #resolve from and to
to = Subject.find(params[:to])
if to.copy_from(from)
# success
else
# errors
return render(:json => {:error => [to.errors]}, :status => :bad_request)
end
Upvotes: 2