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

Breaking change: Remove custom timestamp feature #861

Merged
merged 1 commit into from
Sep 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).

### Breaking Changes

- None
- `timestamp_field=` removed without replacement. It is no longer configurable. The
timestamp field in the `versions` table must now be named `created_at`.

### Deprecated

Expand Down
14 changes: 6 additions & 8 deletions lib/paper_trail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,12 @@ def enabled_for_model?(model)

# Set the field which records when a version was created.
# @api public
def timestamp_field=(field_name)
PaperTrail.config.timestamp_field = field_name
end

# Returns the field which records when a version was created.
# @api public
def timestamp_field
PaperTrail.config.timestamp_field
def timestamp_field=(_field_name)
raise(
"PaperTrail.timestamp_field= has been removed, without replacement. " \
"It is no longer configurable. The timestamp field in the versions table " \
"must now be named created_at."
)
end

# Sets who is responsible for any changes that occur. You would normally use
Expand Down
2 changes: 1 addition & 1 deletion lib/paper_trail/cleaner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def gather_versions(item_id = nil, date = :all)
# versions.
# @api private
def group_versions_by_date(versions)
versions.group_by { |v| v.send(PaperTrail.timestamp_field).to_date }
versions.group_by { |v| v.created_at.to_date }
end
end
end
3 changes: 1 addition & 2 deletions lib/paper_trail/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module PaperTrail
# configuration can be found in `paper_trail.rb`, others in `controller.rb`.
class Config
include Singleton
attr_accessor :timestamp_field, :serializer, :version_limit
attr_accessor :serializer, :version_limit
attr_writer :track_associations

def initialize
Expand All @@ -15,7 +15,6 @@ def initialize
@enabled = true

# Variables which affect all threads, whose access is *not* synchronized.
@timestamp_field = :created_at
@serializer = PaperTrail::Serializers::YAML
end

Expand Down
10 changes: 1 addition & 9 deletions lib/paper_trail/record_history.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def sequence
@versions.select(primary_key).order(primary_key.asc)
else
@versions.
select([timestamp, primary_key]).
select([table[:created_at], primary_key]).
order(@version_class.timestamp_sort_order)
end
end
Expand All @@ -45,13 +45,5 @@ def primary_key
def table
@version_class.arel_table
end

# @return - Arel::Attribute - Attribute representing the timestamp column
# of the version table, usually named `created_at` (the rails convention)
# but not always.
# @api private
def timestamp
table[PaperTrail.timestamp_field]
end
end
end
8 changes: 3 additions & 5 deletions lib/paper_trail/record_trail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def record_create
whodunnit: PaperTrail.whodunnit
}
if @record.respond_to?(:updated_at)
data[PaperTrail.timestamp_field] = @record.updated_at
data[:created_at] = @record.updated_at
end
if record_object_changes? && changed_notably?
data[:object_changes] = recordable_object_changes
Expand Down Expand Up @@ -220,7 +220,7 @@ def record_update(force)
whodunnit: PaperTrail.whodunnit
}
if @record.respond_to?(:updated_at)
data[PaperTrail.timestamp_field] = @record.updated_at
data[:created_at] = @record.updated_at
end
if record_object_changes?
data[:object_changes] = recordable_object_changes
Expand Down Expand Up @@ -376,9 +376,7 @@ def version_at(timestamp, reify_options = {})
# Returns the objects (not Versions) as they were between the given times.
def versions_between(start_time, end_time)
versions = send(@record.class.versions_association_name).between(start_time, end_time)
versions.collect { |version|
version_at(version.send(PaperTrail.timestamp_field))
}
versions.collect { |version| version_at(version.created_at) }
end

# Executes the given method or block without creating a new version.
Expand Down
14 changes: 7 additions & 7 deletions lib/paper_trail/version_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ def subsequent(obj, timestamp_arg = false)
return where(arel_table[primary_key].gt(obj.id)).order(arel_table[primary_key].asc)
end

obj = obj.send(PaperTrail.timestamp_field) if obj.is_a?(self)
where(arel_table[PaperTrail.timestamp_field].gt(obj)).order(timestamp_sort_order)
obj = obj.send(:created_at) if obj.is_a?(self)
where(arel_table[:created_at].gt(obj)).order(timestamp_sort_order)
end

# Returns versions before `obj`.
Expand All @@ -90,22 +90,22 @@ def preceding(obj, timestamp_arg = false)
return where(arel_table[primary_key].lt(obj.id)).order(arel_table[primary_key].desc)
end

obj = obj.send(PaperTrail.timestamp_field) if obj.is_a?(self)
where(arel_table[PaperTrail.timestamp_field].lt(obj)).
obj = obj.send(:created_at) if obj.is_a?(self)
where(arel_table[:created_at].lt(obj)).
order(timestamp_sort_order("desc"))
end

def between(start_time, end_time)
where(
arel_table[PaperTrail.timestamp_field].gt(start_time).
and(arel_table[PaperTrail.timestamp_field].lt(end_time))
arel_table[:created_at].gt(start_time).
and(arel_table[:created_at].lt(end_time))
).order(timestamp_sort_order)
end

# Defaults to using the primary key as the secondary sort order if
# possible.
def timestamp_sort_order(direction = "asc")
[arel_table[PaperTrail.timestamp_field].send(direction.downcase)].tap do |array|
[arel_table[:created_at].send(direction.downcase)].tap do |array|
array << arel_table[primary_key].send(direction.downcase) if primary_key_is_int?
end
end
Expand Down
27 changes: 0 additions & 27 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,6 @@ def assert_changes_equal(expected, actual)
end
end

#
# Helpers
#

def change_schema
ActiveRecord::Migration.verbose = false
ActiveRecord::Schema.define do
add_column :versions, :custom_created_at, :datetime
end
ActiveRecord::Migration.verbose = true
reset_version_class_column_info!
end

# Wrap args in a hash to support the ActionController::TestCase and
# ActionDispatch::Integration HTTP request method switch to keyword args
# (see https://github.com/rails/rails/blob/master/actionpack/CHANGELOG.md)
Expand All @@ -111,17 +98,3 @@ def params_wrapper(args)
args
end
end

def reset_version_class_column_info!
PaperTrail::Version.connection.schema_cache.clear!
PaperTrail::Version.reset_column_information
end

def restore_schema
ActiveRecord::Migration.verbose = false
ActiveRecord::Schema.define do
remove_column :versions, :custom_created_at
end
ActiveRecord::Migration.verbose = true
reset_version_class_column_info!
end
37 changes: 0 additions & 37 deletions test/unit/cleaner_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,41 +148,4 @@ def populate_db!
end
end
end # clean_versions! method

context "Custom timestamp field" do
setup do
change_schema
populate_db!
# now mess with the timestamps
@animals.each do |animal|
animal.versions.reverse.each_with_index do |version, index|
version.update_attribute(:custom_created_at, Time.now.utc + index.days)
end
end
PaperTrail.timestamp_field = :custom_created_at
@animals.map { |a| a.versions.reload } # reload the `versions` association for each animal
end

teardown do
PaperTrail.timestamp_field = :created_at
restore_schema
end

should "Baseline" do
assert_equal 9, PaperTrail::Version.count
@animals.each do |animal|
assert_equal 3, animal.versions.size
animal.versions.each_cons(2) do |a, b|
assert_equal a.created_at.to_date, b.created_at.to_date
assert_not_equal a.custom_created_at.to_date, b.custom_created_at.to_date
end
end
end

should "group by `PaperTrail.timestamp_field` when seperating the versions by date to clean" do
assert_equal 9, PaperTrail::Version.count
PaperTrail.clean_versions!
assert_equal 9, PaperTrail::Version.count
end
end
end
6 changes: 2 additions & 4 deletions test/unit/inheritance_column_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,12 @@ class InheritanceColumnTest < ActiveSupport::TestCase

# For some reason `@dog.versions` doesn't include the final `destroy` version.
# Neither do `@dog.versions.scoped` nor `@dog.versions(true)` nor `@dog.versions.reload`.
dog_versions = PaperTrail::Version.where(item_id: @dog.id).
order(PaperTrail.timestamp_field)
dog_versions = PaperTrail::Version.where(item_id: @dog.id).order(:created_at)
assert_equal 4, dog_versions.count
assert_nil dog_versions.first.reify
assert_equal %w(NilClass Dog Dog Dog), dog_versions.map { |v| v.reify.class.name }

cat_versions = PaperTrail::Version.where(item_id: @cat.id).
order(PaperTrail.timestamp_field)
cat_versions = PaperTrail::Version.where(item_id: @cat.id).order(:created_at)
assert_equal 4, cat_versions.count
assert_nil cat_versions.first.reify
assert_equal %w(NilClass Cat Cat Cat), cat_versions.map { |v| v.reify.class.name }
Expand Down
7 changes: 0 additions & 7 deletions test/unit/model_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -517,16 +517,9 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase

context "after a column is removed from the record's schema" do
setup do
change_schema
Widget.connection.schema_cache.clear!
Widget.reset_column_information
@last = @widget.versions.last
end

teardown do
restore_schema
end

should "reify previous version" do
assert_kind_of Widget, @last.reify
end
Expand Down
41 changes: 0 additions & 41 deletions test/unit/timestamp_test.rb

This file was deleted.

7 changes: 0 additions & 7 deletions test/unit/version_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,10 @@
module PaperTrail
class VersionTest < ActiveSupport::TestCase
setup do
change_schema
@animal = Animal.create
assert Version.creates.present?
end

teardown do
restore_schema
Animal.connection.schema_cache.clear!
Animal.reset_column_information
end

context ".creates" do
should "return only create events" do
Version.creates.each do |version|
Expand Down