Skip to content

Commit

Permalink
Support ActiveRecord transitions which don't include Statesman::Adapt…
Browse files Browse the repository at this point in the history
…ers::ActiveRecordTransition

In #306, we made the "updated timestamp column" for a transition
configurable through a `class_attribute` on
`Statesman::Adapters::ActiveRecordTransition`, which is generally
`include`d on transition classes.

However, not all transition classes include this module, and
indeed we have [advised][1] in the README not to include it when
using a PostgreSQL JSON column type for metadata, as pointed out
by #309.

This leads to an exception like the following when trying to
perform a transition:

```
NoMethodError: undefined method `updated_timestamp_column' for #<Class:0x00007fdd2ade07b0>
```

This fixes the issue by defaulting to using `:updated_at` as
before if the transition class doesn't have an
`.updated_timestamp_column` method defined.

As a follow-up action, it'd be nice to get all transitions
including the module to have a more consistent and friendly base
for future improvements to Statesman, as discussed in the
[issue comments][2]. This, however, would be a breaking change
and should be introduced separately after more thought.

Fixes #309.

[1]: https://github.com/gocardless/statesman#using-postgresql-json-column
[2]: #309 (comment)
  • Loading branch information
Tim Rogers committed Feb 14, 2018
1 parent dbb3a75 commit 0a2ed03
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 2 deletions.
17 changes: 15 additions & 2 deletions lib/statesman/adapters/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,28 @@ def transition_conflict_error?(e)
end

def with_updated_timestamp(params)
return params if @transition_class.updated_timestamp_column.nil?
# TODO: Once we've set expectations that transition classes should conform to
# the interface of Adapters::ActiveRecordTransition as a breaking change in the
# next major version, we can stop calling `#respond_to?` first and instead
# assume that there is a `.updated_timestamp_column` method we can call.
#
# At the moment, most transition classes will include the module, but not all,
# not least because it doesn't work with PostgreSQL JSON columns for metadata.
column = if @transition_class.respond_to?(:updated_timestamp_column)
@transition_class.updated_timestamp_column
else
ActiveRecordTransition::DEFAULT_UPDATED_TIMESTAMP_COLUMN
end

return params if column.nil?

timestamp = if ::ActiveRecord::Base.default_timezone == :utc
Time.now.utc
else
Time.now
end

params.merge(@transition_class.updated_timestamp_column => timestamp)
params.merge(column => timestamp)
end
end
end
Expand Down
13 changes: 13 additions & 0 deletions spec/statesman/adapters/active_record_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,19 @@
to(change { previous_transition.reload.updated_at })
end

context "for a transition class without an updated timestamp column attribute" do
let!(:adapter) do
described_class.new(MyActiveRecordModelTransitionWithoutInclude,
model,
observer)
end

it "defaults to touching the previous transition's updated_at timestamp" do
expect { Timecop.freeze(Time.now + 5.seconds) { create } }.
to(change { previous_transition.reload.updated_at })
end
end

context "with a custom updated timestamp column set" do
around do |example|
MyActiveRecordModelTransition.updated_timestamp_column.tap do |original_value|
Expand Down
7 changes: 7 additions & 0 deletions spec/support/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ class MyActiveRecordModelTransition < ActiveRecord::Base
serialize :metadata, JSON
end

class MyActiveRecordModelTransitionWithoutInclude < ActiveRecord::Base
self.table_name = "my_active_record_model_transitions"

belongs_to :my_active_record_model
serialize :metadata, JSON
end

class CreateMyActiveRecordModelMigration < MIGRATION_CLASS
def change
create_table :my_active_record_models do |t|
Expand Down

0 comments on commit 0a2ed03

Please sign in to comment.