Transitions#set_initial_state is forcibly inserting the "state" attribute

[ozydingo]

I've traced my issue down to to the set_initial_state method, which is set as an after_initialize callback. Specifically, the line:

self[transitions_state_column_name] = self.class.get_state_machine.initial_state.to_s

causes the attribute_names method to always include transitions_state_column_name (usually "state") name even when "state" is not selected in the SQL query. This mismatch is causing downstream code to raise an ActiveModel::MissingAttributeError: missing attribute: state.

To illustrate:

Job.select("id").first.attribute_names

=> ["state", "id"]

While a class that does not include Transitions gives

NlpAlgorithm.select("id").first.attribute_names

=> ["id"]

The downstream code that is breaking is looping through attribute_names to pluck only the data that is specified in the SELECT query. I propose that the Transitions gem only set this field if it is SELECTed:

self[transitions_state_column_name] = self.class.get_state_machine.initial_state.to_s if self.attribute_names.include?(transitions_state_column_name)

but I'm not confident enough that this wouldn't break anything, so I'm avoiding creating a pull request in favor of discussing it first.

[troessner]

The downstream code that is breaking is looping through attribute_names to pluck only the data that is specified in the SELECT query. I propose that the Transitions gem only set this field if it is SELECTed:

self[transitions_state_column_name] = self.class.get_state_machine.initial_state.to_s if self.attribute_names.include?(transitions_state_column_name)

but I'm not confident enough that this wouldn't break anything, so I'm avoiding creating a pull request in favor of discussing it first.

Yes, that sounds like a plan! Pull request would be highly welcome.:)

[ozydingo]

So, working on this, seems like it's not Transitions' fault. You already have
if self.has_attribute?(transitions_state_column_name) && state_not_set?
which should have us covered. Except for what I now believe is a Rails bug, at least as of 4.2.3:

[1] pry(#)> self.attribute_names
=> ["id"]
[2] pry(#)> transitions_state_column_name
=> :state
[3] pry(#)> self.attribute_names
=> ["id"]
[4] pry(#)> self.has_attribute?(transitions_state_column_name)
=> false
[5] pry(#)> self.attribute_names
=> ["state", "id"]

Digging into the Rails code, this comes from:

[3] pry(#)> self.attribute_names
=> ["id"]
[4] pry(#)> @attributes.key?(transitions_state_column_name.to_s)
=> false
[5] pry(#)> self.attribute_names
=> ["state", "id"]
[6] pry(#)> @attributes.key?(transitions_state_column_name.to_s)
=> false

(6) here is even more confusing, because now has_attribute? and attribute_names.include? will return different results. I'll look into if this is in fact a bug, and if so if it still a current bug, in Rails; in the meantime, I'm going to create a pull request changing our if statement here to use self.attribute_names.include? since this does not introduce the bug:

[1] pry(#)> self.attribute_names
=> ["id"]
[2] pry(#)> self.attribute_names.include?(transitions_state_column_name.to_s)
=> false
[3] pry(#)> self.attribute_names
=> ["id"]

[ozydingo]

I've confirmed that this bug does NOT happen in Rails 5. So the question is if we want the proposed solution in order to support this edge case for Rails 4 users who use custom SELECTs on models that include Transitions (like me!). I don't see a downside other than cleanliness, but barely that.

[troessner]

#144 is merged, does this fix this issue?

[ozydingo]

Yup, all good! Thanks!

[ozydingo]

Yup, all good!

On Thu, Aug 20, 2015 at 5:19 AM, Timo Rößner notifications@github.com
wrote:

#144 #144 is merged, does
this fix this issue?

—
Reply to this email directly or view it on GitHub
#143 (comment)
.