mirror of
https://github.com/paper-trail-gem/paper_trail.git
synced 2022-11-09 11:33:19 -05:00
Breaking change: Remove custom timestamp feature
A little over four years ago, a feature was added to allow users to change the name of the `versions.created_at` column. No reason was given at the time. https://github.com/airblade/paper_trail/pull/129 There are three reasons why this should not be configurable: 1. The standard column name in rails is `created_at`. 2. The majority of the `versions` table is, and should be, an implementation detail of PT. 3. This configuration option added moderate complexity to the library, and severe complexity to the test suite.
This commit is contained in:
parent
1da4763c85
commit
f963752703
13 changed files with 23 additions and 156 deletions
|
@ -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
|
||||
|
||||
|
|
|
@ -72,14 +72,12 @@ module PaperTrail
|
|||
|
||||
# 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
|
||||
|
|
|
@ -52,7 +52,7 @@ module PaperTrail
|
|||
# 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
|
||||
|
|
|
@ -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
|
||||
|
@ -15,7 +15,6 @@ module PaperTrail
|
|||
@enabled = true
|
||||
|
||||
# Variables which affect all threads, whose access is *not* synchronized.
|
||||
@timestamp_field = :created_at
|
||||
@serializer = PaperTrail::Serializers::YAML
|
||||
end
|
||||
|
||||
|
|
|
@ -26,7 +26,7 @@ module PaperTrail
|
|||
@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
|
||||
|
@ -45,13 +45,5 @@ module PaperTrail
|
|||
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
|
||||
|
|
|
@ -170,7 +170,7 @@ module PaperTrail
|
|||
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
|
||||
|
@ -220,7 +220,7 @@ module PaperTrail
|
|||
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
|
||||
|
@ -376,9 +376,7 @@ module PaperTrail
|
|||
# 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.
|
||||
|
|
|
@ -74,8 +74,8 @@ module PaperTrail
|
|||
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`.
|
||||
|
@ -90,22 +90,22 @@ module PaperTrail
|
|||
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
|
||||
|
|
|
@ -88,19 +88,6 @@ module ActiveSupport
|
|||
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)
|
||||
|
@ -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
|
||||
|
|
|
@ -148,41 +148,4 @@ class PaperTrailCleanerTest < ActiveSupport::TestCase
|
|||
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
|
||||
|
|
|
@ -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 }
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -1,41 +0,0 @@
|
|||
require "test_helper"
|
||||
|
||||
class TimestampTest < ActiveSupport::TestCase
|
||||
setup do
|
||||
PaperTrail.timestamp_field = :custom_created_at
|
||||
change_schema
|
||||
|
||||
Fluxor.instance_eval <<-END
|
||||
has_paper_trail
|
||||
END
|
||||
|
||||
@fluxor = Fluxor.create name: "Some text."
|
||||
@fluxor.update_attributes name: "Some more text."
|
||||
@fluxor.update_attributes name: "Even more text."
|
||||
end
|
||||
|
||||
teardown do
|
||||
PaperTrail.timestamp_field = :created_at
|
||||
restore_schema
|
||||
end
|
||||
|
||||
test "versions works with custom timestamp field" do
|
||||
# Normal behaviour
|
||||
assert_equal 3, @fluxor.versions.length
|
||||
assert_nil @fluxor.versions[0].reify
|
||||
assert_equal "Some text.", @fluxor.versions[1].reify.name
|
||||
assert_equal "Some more text.", @fluxor.versions[2].reify.name
|
||||
|
||||
# Tinker with custom timestamps.
|
||||
now = Time.now.utc
|
||||
@fluxor.versions.reverse.each_with_index do |version, index|
|
||||
version.update_attribute :custom_created_at, (now + index.seconds)
|
||||
end
|
||||
|
||||
# Test we are ordering by custom timestamps.
|
||||
@fluxor.versions.reload # reload association
|
||||
assert_nil @fluxor.versions[2].reify
|
||||
assert_equal "Some text.", @fluxor.versions[1].reify.name
|
||||
assert_equal "Some more text.", @fluxor.versions[0].reify.name
|
||||
end
|
||||
end
|
|
@ -3,17 +3,10 @@ require "test_helper"
|
|||
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|
|
||||
|
|
Loading…
Reference in a new issue