Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"missing attribute: state" when state attribute is not included in the query #76

Closed
davout opened this issue Feb 12, 2013 · 29 comments
Closed

Comments

@davout
Copy link

davout commented Feb 12, 2013

When selecting aggregates with custom SQL the error is raised.
It worked perfectly in 0.1.4 but broke in 0.1.5

@troessner
Copy link
Owner

Can you give me an example model and corresponding query to reproduce this?

@davout
Copy link
Author

davout commented Feb 12, 2013

http://pastie.org/private/n3r0focjcdges7durjog
The state machine is defined at line 85, the failing method at 160.

@davout
Copy link
Author

davout commented Feb 12, 2013

It fails here.

transitions (0.1.5) lib/active_model/transitions.rb:66:in `set_initial_state'
activesupport (3.2.12) lib/active_support/callbacks.rb:405:in `_run__3282874665777803106__initialize__1090282513155503724__callbacks'
activesupport (3.2.12) lib/active_support/callbacks.rb:405:in `__run_callback'
activesupport (3.2.12) lib/active_support/callbacks.rb:385:in `_run_initialize_callbacks'
activesupport (3.2.12) lib/active_support/callbacks.rb:81:in `run_callbacks'
activerecord (3.2.12) lib/active_record/base.rb:523:in `init_with'
activerecord (3.2.12) lib/active_record/inheritance.rb:68:in `instantiate'
activerecord (3.2.12) lib/active_record/querying.rb:38:in `block (2 levels) in find_by_sql'
activerecord (3.2.12) lib/active_record/querying.rb:38:in `collect!'
activerecord (3.2.12) lib/active_record/querying.rb:38:in `block in find_by_sql'
activerecord (3.2.12) lib/active_record/explain.rb:40:in `logging_query_plan'
activerecord (3.2.12) lib/active_record/querying.rb:37:in `find_by_sql'
newrelic_rpm (3.5.6.46) lib/new_relic/agent/method_tracer.rb:491:in `block in find_by_sql_with_trace_ActiveRecord_self_name_find_by_sql'
newrelic_rpm (3.5.6.46) lib/new_relic/agent/method_tracer.rb:240:in `trace_execution_scoped'
newrelic_rpm (3.5.6.46) lib/new_relic/agent/method_tracer.rb:486:in `find_by_sql_with_trace_ActiveRecord_self_name_find_by_sql'
activerecord (3.2.12) lib/active_record/relation.rb:171:in `exec_queries'
activerecord (3.2.12) lib/active_record/relation.rb:160:in `block in to_a'
activerecord (3.2.12) lib/active_record/explain.rb:40:in `logging_query_plan'
activerecord (3.2.12) lib/active_record/relation.rb:159:in `to_a'
activerecord (3.2.12) lib/active_record/relation/finder_methods.rb:159:in `all'
app/models/trade_order.rb:161:in `get_orders'

@troessner
Copy link
Owner

Thanks for the report, I'll look into this as soon as I can.

@maser
Copy link

maser commented Feb 19, 2013

This was broken in bf5d2b6 by adding Mongoid support. Using respond_to? does not work as the state and state= methods still exist even when the attribute was not loaded. I would fix it, but I don’t know how to do that without breaking Mongoid support.

We now just use 0.1.4.

@troessner
Copy link
Owner

I just reverted: #68
and released 0.1.6.
Please let me know if that works for you guys.

@davout
Copy link
Author

davout commented Mar 1, 2013

Hey, thanks a lot, looks like it's working.
Except now my tests are throwing

WARNING: Transitions::Machine#new_update or Transitions::Machine#new_initialize already defined. This can possibly break ActiveModel::Transitions

multiple times.

@troessner
Copy link
Owner

Not sure why you get that but this was added in a recent pull request:

https://github.com/troessner/transitions/pull/73/files#L1R47

The same here for me:

WARNING: Transitions::Machine#new_update or Transitions::Machine#new_initialize already defined. This can possibly break ActiveModel::Transitions.
WARNING: Transitions::Machine#new_update or Transitions::Machine#new_initialize already defined. This can possibly break ActiveModel::Transitions.
...................................................................................................................*.....................................................................................................

I have a more or less vanilla rails app - seems like this happens because more than one model use transitions, but that is just a shot in the blue from my side.

@cmw do you see what's going wrong here?

@davout
Copy link
Author

davout commented Mar 1, 2013

@troessner yeah, a bunch of my models include AM::T

@maser
Copy link

maser commented Mar 3, 2013

I get a nice SystemStackError with 0.1.6:

/usr/local/lib/ruby/gems/1.8/gems/transitions-0.1.6/lib/active_model/transitions.rb:36:in `old_initialize': stack level too deep (SystemStackError)
        from /usr/local/lib/ruby/gems/1.8/gems/transitions-0.1.6/lib/active_model/transitions.rb:36:in `initialize'
        from /usr/local/lib/ruby/gems/1.8/gems/transitions-0.1.6/lib/transitions.rb:45:in `new'
        from /usr/local/lib/ruby/gems/1.8/gems/transitions-0.1.6/lib/transitions.rb:45:in `state_machine'
        from /vagrant/rails_app/app/models/news_feed.rb:15

@troessner
Copy link
Owner

@maser interesting. So I guess you'll stick with 0.1.4 for now, right?
Can you provide me with a test case on how to reproduce that behaviour?

Besides that, it seems like this error was introduced with: #73

@cmw any ideas on that?

@troessner
Copy link
Owner

@maser can you do me a favor and narrow it down for me a litte?

This means:

  • Clone this repo
  • Adjust your Gemfile to use your local instead of the installed one
  • Check out commit

commit b2e64fe
Merge: ab8e1bb 98dc77d
Author: Timo Rößner timo.roessner@googlemail.com
Date: Thu Jan 31 03:08:15 2013 -0800

and check if it's working again.

  • Then check out

commit 1304a15
Author: Christian M. Weis christian.weis@gmail.com
Date: Fri Feb 1 19:06:03 2013 +0100
Support for configurable column names

and see if it fails.

@maser
Copy link

maser commented Mar 4, 2013

@troessner Yes, we are using 0.1.4 for now. b2e64fe and 1304a15 work as you expected them to, so your fix works. Thanks!

There is a new arning, though: lib/transitions/version.rb:2: warning: already initialized constant VERSION.

@troessner
Copy link
Owner

Wait, what? Now I am confused - from your error report I thought that 1304a15 introduced a bug which you were experiencing now. So I was expecting that b2e64fe works for you but 1304a15 not. So both those commits work for you?

@troessner
Copy link
Owner

Also from what you said I am not sure if this still affects the current version. Does

so your fix works.

mean that it works for you now with v 0.1.6? If not, can you tell me how to reproduce this error? Because I'm using 0.1.6 in multiple rails apps and have no problems so far.

@maser
Copy link

maser commented Mar 5, 2013

@troessner Sorry, I wasn’t clear enough. b2e64fe works, 1304a15 does not, neither does 0.1.6.

I can try to reproduce the error in a clean app tomorrow.

@troessner
Copy link
Owner

b2e64fe works, 1304a15 does not, neither does 0.1.6.

Ah, good to know.

I can try to reproduce the error in a clean app tomorrow.

That would be very helpful, thanks!

@maser
Copy link

maser commented Mar 5, 2013

I cannot reproduce the problem in a new Rails application. I’ll have to test if there is an incompatibility with some other part of the application (it is large enough for that to take some time though).

@cmw
Copy link
Collaborator

cmw commented Mar 5, 2013

The warning

WARNING: Transitions::Machine#new_update or Transitions::Machine#new_initialize already defined. This can possibly break ActiveModel::Transitions

just means that AM::T got loaded more than once and the condition prevented the aliasing block from producing endless loops.

I only included the warning because it's conceivable, that Transitions::Machine#new_initialize could be defined before the module is added. This will potentially break something, so, if things don't seem to work right, this is a pointer to where the problem might be.

@davout
Copy link
Author

davout commented Mar 5, 2013

@cmw I get the warning multiple times each time Rails is loaded.

I have multiple models with state machines, all the models include ìnclude ActiveModel::Transitions`.
Tried

ActiveSupport.on_load(:active_record) do
  ActiveRecord::Base.send(:extend, ActiveModel::Transitions)
end

as a workaround, but apparently that fails too because the columns are not yet defined on the model at that point and AM::T tries to find the state column.

Trying to extend AR later in the lifecycle doesn't work either as the state_machine call fails inside the model rightfully complaining that the method is undefined.

@troessner
Copy link
Owner

@cmw I see your point, however this warning can get annoying pretty quick - how about removing this warning and use some method name we're sure nobody else uses, like new_transitions_initialize?

@davout
Copy link
Author

davout commented Mar 5, 2013

@troessner would that solve the warning popping when AM::T is included multiple times in different models ?

@cmw
Copy link
Collaborator

cmw commented Mar 5, 2013

We wouldn't necessarily have to display the warning in that case (though we still need the check to protect from endless loop). Not a bad idea, though of course we can't really know that nobody uses a method with that name.

@troessner
Copy link
Owner

@troessner would that solve the warning popping when AM::T is included multiple times in different models ?

Yes.

We wouldn't necessarily have to display the warning in that case (though we still need the check to protect from endless loop).

Yeah, then let's drop it.

Not a bad idea, though of course we can't really know that nobody uses a method with that name.

True, but I'd say the chances are so low that we should go with it ..:)

Do you have the time to whip up a pull request? I can do it as well but unfortunately I'm pretty loaded for the next few days.

@cmw
Copy link
Collaborator

cmw commented Mar 6, 2013

@troessner You got it :)

@troessner
Copy link
Owner

Alright, I just released 1.0.7 with all the latest and greatest..;)

@davout the warnings should be gone now
@maser please let me know if you have still that SystemStackError with that version or if this problem in general just has gone away - and if so, please open up a dedicated issue.

-> Closing this.

@davout
Copy link
Author

davout commented Mar 7, 2013

Works perfect. Thanks a lot!

@maser
Copy link

maser commented Mar 7, 2013

@troessner I still get it, but I won’t open an issue before I know how to reproduce this.

@tony-pizza
Copy link

Not sure if I should open a new issue, but I'm getting this again.

ruby 2.0.0
rails 4.0.0 + postgres 9.2
transitions (0.1.9 f8a0b94)

2.0.0 (main):0 > a = Auction.select('id').last
  Auction Load (0.6ms)  SELECT id FROM "auctions" ORDER BY auctions.ends_at DESC LIMIT 1
ActiveModel::MissingAttributeError: missing attribute: state
from /Users/Ian/.rvm/gems/ruby-2.0.0-p195/bundler/gems/rails-c119faa86458/activerecord/lib/active_record/attribute_methods/read.rb:89:in `block (2 levels) in read_attribute'

jmazzi pushed a commit to jmazzi/transitions that referenced this issue Nov 8, 2013
This reverts commit bf5d2b6.
because of: troessner#76
Conflicts:
	lib/active_model/transitions.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants