ActiveRecord is being converted to Hash unexpectedly

rubyonrails-talk
1

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?

Code:

class Objective < ActiveRecord::Base   belongs_to :user   has_many :next_actions, :order => 'id DESC', :conditions => {:completed => false}   has_many :action_items, :order => 'position'   has_many :progress_updates

  serialize :freq_value

validates_presence_of :name, :due_date, :completion, :pr_u_frequency_id   validates_numericality_of :completion, :only_integer => true, :greater_than_or_equal_to => 0, :less_than_or_equal_to => 100

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

end

class ObjectivesController < ApplicationController   before_filter :require_user

  layout false

  def create     objective = Objective.create(params[:objective], current_user.id)     Rails.logger.info("****objective.class = #{objective.class}****")   end end

2

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.

3

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~

4

Thanks for your reply.

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

5

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.

6

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...

7

maybe you should use another method name and try again~~

8

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.

Fred

9

> 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.

I see where you are coming from.

Would still be interesting to find exactly what is going on.

Yes, very much so!

10

Try this:

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?

-- W

11

Wes wrote:

Try this:

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

over*ride*

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?

Good catch!

-- W

Best,

12

Wes wrote: > Try this:

> 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.

Why not?

> And as others have said, don't overwrite

over*ride*

> AR class methods like > 'create', otherwise you will enter a world of pain.

Thanks for the good practice tip.

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?

Good catch!

It will eventually be used, just not yet.

13

Wes wrote: > Try this:

> 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.

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

over*ride*

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

-- W

14

> Wes wrote: > > Try this:

> > 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.

Why not?

There's no reason not to. In fact it's a Rails convention to do so.

> > And as others have said, don't overwrite

> over*ride*

> > AR class methods like > > 'create', otherwise you will enter a world of pain.

Thanks for the good practice tip.

> 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?

> 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).

-- W

15

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 :).