I am running into a very odd problem. I have an ActiveRecord called
Objective with a public create method. When this method is called in
a controller the return value is a Hash instead of Objective. When I
test the method in Rspec for the model it returns as Objective.
Does anyone have an idea about why the return type would be different
and if so how to fix it so it always returns Objective?
I should mention that the Hash does contain the Objective’s data. the
reason it is important that Objective is returned instead of Hash is
because I want to be able to call methods on it.
hi,drewB
first,I don’t agree about the code of create method,use another
method name seems better.
second,you mentioned that “objective.class” will return Hash? It’s
impossible,Objective#create return a instance of Objective or nil~~
finally,show out your code about Rspec,please~
hi,drewB
first,I don’t agree about the code of create method,use another
method name seems better.
Why don’t you agree?
second,you mentioned that “objective.class” will return Hash? It’s
impossible,Objective#create return a instance of Objective or nil~~
I agree it is very strange but the log file clearly shows that it is
returning a Hash (so are the error messages I get when I try to call a
method).
finally,show out your code about Rspec,please~
#I am using machinist. Objective.plan returns a hash with values I
specify elsewhere
it “should return objective when create called” do
obj = Objective.create(Objective.plan, 5)
obj.class.to_s.should == “Objective”
end
def self.create(attributes, user_id) #create new record with user_id correctly set
attributes.delete :freq_options #deletes this unused key
obj = self.new(attributes)
obj.user_id = user_id
obj.save ? obj : nil
end
I don’t know if this is directly related to your problem, but why are
you overriding AR::Base.create ? You shouldn’t need to; just do @user.objectives.create(attributes). You’re trying to reinvent what AR
already gives you for free.
It is overwritten because the objective requires a user_id which comes
from a different place than the other attributes. I could simply use
create and add more code at the controller level but I prefer to push
that down to the model.
On Oct 25, 2:15 pm, Marnen Laibow-Koser <rails-mailing-l…@andreas-
Marnen Laibow-Koser had pointed out the reason.
put more code at the model is best practice,but in this
situation,ActiveRecord already gives you a brief solution～
your problem seems so strange…I will write codes to test it…
On Oct 26, 12:49 pm, Marnen Laibow-Koser <rails-mailing-l…@andreas- s.net> wrote:
Then create a method with a different name – say, create_with_user_id
– or use a before_create callback to set the user_id. Don’t override
create.
or in this case, using associations and writing
some_user.objectives.create(…) would be even nicer. Would still be
interesting to find exactly what is going on.
because the objective requires a user_id which comes
from a different place than the other attributes. I could simply use
create and add more code at the controller level
This is the sort of very simple logic that is appropriate in the
controller.
but I prefer to push
that down to the model.
Then create a method with a different name – say, create_with_user_id
– or use a before_create callback to set the user_id. Don’t override
create.
On Oct 25, 2:15�pm, Marnen Laibow-Koser <rails-mailing-l…@andreas-
class ObjectivesController < ApplicationController
before_filter :require_user
layout false
def create
objective = current_user.objectives.create(params[:objective])
end
end
And as others have said, don’t overwrite AR class methods like
‘create’, otherwise you will enter a world of pain.
Also, you are deleting a param called ‘freq_options’, if the model
does not need it then it shouldn’t be in the hash. If the controller
needs it, then pass it as params[:freq_options] rather than params
[:objective][:freq_options]. If it’s not used there, then why pass it
at all?
On Oct 26, 9:51 am, Marnen Laibow-Koser <rails-mailing-l…@andreas- s.net> wrote:
No! Create shouldn’t be a controller method.
Why not?
And as others have said, don’t overwrite
override
AR class methods like
‘create’, otherwise you will enter a world of pain.
Thanks for the good practice tip.
Also, you are deleting a param called ‘freq_options’, if the model
does not need it then it shouldn’t be in the hash. If the controller
needs it, then pass it as params[:freq_options] rather than params
[:objective][:freq_options]. If it’s not used there, then why pass it
at all?
class ObjectivesController < ApplicationController
before_filter :require_user
layout false
def create
objective = current_user.objectives.create(params[:objective])
end
end
No! Create shouldn’t be a controller method.
And as others have said, don’t overwrite
override
AR class methods like
‘create’, otherwise you will enter a world of pain.
Yeah. If you must override, it’s probably best to do something like
def create(attributes, user_id)
super(attributes.merge :user_id => user_id)
end
but even this is kind of bad.
Also, you are deleting a param called ‘freq_options’, if the model
does not need it then it shouldn’t be in the hash. If the controller
needs it, then pass it as params[:freq_options] rather than params
[:objective][:freq_options]. If it’s not used there, then why pass it
at all?
There’s no reason not to. In fact it’s a Rails convention to do so.
Also, you are deleting a param called ‘freq_options’, if the model
does not need it then it shouldn’t be in the hash. If the controller
needs it, then pass it as params[:freq_options] rather than params
[:objective][:freq_options]. If it’s not used there, then why pass it
at all?
Good catch!
It will eventually be used, just not yet.
Ok, if you intend to use it later, then you can just specify an
accessor in your model, that way you can pass it in without error.
class Objective < ActiveRecord::Base
attr_accessor :freq_options
end
Then later if you add this as a db attribute, you can remove the
accessor. Otherwise you may want to do some manipulation of other data
in the model based on the value of this accessor (via callbacks).
On Oct 26, 12:51 pm, Marnen Laibow-Koser <rails-mailing-l…@andreas- s.net> wrote:
No! Create shouldn’t be a controller method.
Huh? It’s rails convention. A restful controller has 6 actions (index,
show, create, edit, update, destroy).
And as others have said, don’t overwrite
override
Wow, why so pedantic? The intent was clear. Did I kick your puppy or
something?
AR class methods like
‘create’, otherwise you will enter a world of pain.
Yeah. If you must override, it’s probably best to do something like
def create(attributes, user_id)
super(attributes.merge :user_id => user_id)
end
but even this is kind of bad.
That’s a really bad example as the additional parameter is handled via
an association. Besides, if an attribute is not in the request
attributes hash, it’s much easier to just add it in the controller,
since that’s what you’re already doing in a backwards sort of way.
Ex: Foo.create(params[:foo].merge(:my_var => ‘bar’)) vs. Foo.create
(params[:foo], ‘bar’) + an ugly overridden method in the class
Thanks everyone for the help. I didn’t end up figuring out what was
causing the problem but changing the code to current_user.Objective…
fixed the problem and makes a lot more sense.
I hope not too many puppies were hurt during this exchange :).
On Oct 26, 10:56 am, Marnen Laibow-Koser <rails-mailing-l…@andreas-
On Oct 26, 9:51�am, Marnen Laibow-Koser <rails-mailing-l…@andreas- s.net> wrote:
No! �Create shouldn’t be a controller method.
Why not?
Sorry, that was a flash of abject stupidity on my part. Please feel
free to ignore it.
And as others have said, don’t overwrite
override
AR class methods like
‘create’, otherwise you will enter a world of pain.
Thanks for the good practice tip.
Also, you are deleting a param called ‘freq_options’, if the model
does not need it then it shouldn’t be in the hash. If the controller
needs it, then pass it as params[:freq_options] rather than params
[:objective][:freq_options]. If it’s not used there, then why pass it
at all?
Please feel