Change the default null
value for timestamps
As per discussion, this changes the model generators to specify `null: false` for timestamp columns. A warning is now emitted if `timestamps` is called without a `null` option specified, so we can safely change the behavior when no option is specified in Rails 5.
This commit is contained in:
parent
82e28492e7
commit
ea3ba34506
@ -56,6 +56,19 @@ def default_primary_key
|
||||
end
|
||||
end
|
||||
|
||||
module TimestampDefaultDeprecation # :nodoc:
|
||||
def emit_warning_if_null_unspecified(options)
|
||||
return if options.key?(:null)
|
||||
|
||||
ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc)
|
||||
`timestamp` was called without specifying an option for `null`. In Rails
|
||||
5.0, this behavior will change to `null: false`. You should manually
|
||||
specify `null: true` to prevent the behavior of your existing migrations
|
||||
from changing.
|
||||
MESSAGE
|
||||
end
|
||||
end
|
||||
|
||||
# Represents the schema of an SQL table in an abstract way. This class
|
||||
# provides methods for manipulating the schema representation.
|
||||
#
|
||||
@ -77,6 +90,8 @@ def default_primary_key
|
||||
# The table definitions
|
||||
# The Columns are stored as a ColumnDefinition in the +columns+ attribute.
|
||||
class TableDefinition
|
||||
include TimestampDefaultDeprecation
|
||||
|
||||
# An array of ColumnDefinition objects, representing the column changes
|
||||
# that have been defined.
|
||||
attr_accessor :indexes
|
||||
@ -276,6 +291,7 @@ def index(column_name, options = {})
|
||||
# <tt>:updated_at</tt> to the table.
|
||||
def timestamps(*args)
|
||||
options = args.extract_options!
|
||||
emit_warning_if_null_unspecified(options)
|
||||
column(:created_at, :datetime, options)
|
||||
column(:updated_at, :datetime, options)
|
||||
end
|
||||
@ -405,6 +421,8 @@ def add_column(name, type, options)
|
||||
# end
|
||||
#
|
||||
class Table
|
||||
include TimestampDefaultDeprecation
|
||||
|
||||
def initialize(table_name, base)
|
||||
@table_name = table_name
|
||||
@base = base
|
||||
@ -452,8 +470,9 @@ def rename_index(index_name, new_index_name)
|
||||
# Adds timestamps (+created_at+ and +updated_at+) columns to the table. See SchemaStatements#add_timestamps
|
||||
#
|
||||
# t.timestamps
|
||||
def timestamps
|
||||
@base.add_timestamps(@table_name)
|
||||
def timestamps(options = {})
|
||||
emit_warning_if_null_unspecified(options)
|
||||
@base.add_timestamps(@table_name, options)
|
||||
end
|
||||
|
||||
# Changes the column's definition according to the new options.
|
||||
@ -559,6 +578,5 @@ def native
|
||||
@base.native_database_types
|
||||
end
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
|
@ -838,9 +838,9 @@ def columns_for_distinct(columns, orders) #:nodoc:
|
||||
#
|
||||
# add_timestamps(:suppliers)
|
||||
#
|
||||
def add_timestamps(table_name)
|
||||
add_column table_name, :created_at, :datetime
|
||||
add_column table_name, :updated_at, :datetime
|
||||
def add_timestamps(table_name, options = {})
|
||||
add_column table_name, :created_at, :datetime, options
|
||||
add_column table_name, :updated_at, :datetime, options
|
||||
end
|
||||
|
||||
# Removes the timestamp columns (+created_at+ and +updated_at+) from the table definition.
|
||||
|
@ -764,8 +764,8 @@ def remove_index_sql(table_name, options = {})
|
||||
"DROP INDEX #{index_name}"
|
||||
end
|
||||
|
||||
def add_timestamps_sql(table_name)
|
||||
[add_column_sql(table_name, :created_at, :datetime), add_column_sql(table_name, :updated_at, :datetime)]
|
||||
def add_timestamps_sql(table_name, options = {})
|
||||
[add_column_sql(table_name, :created_at, :datetime, options), add_column_sql(table_name, :updated_at, :datetime, options)]
|
||||
end
|
||||
|
||||
def remove_timestamps_sql(table_name)
|
||||
|
@ -9,7 +9,7 @@ def change
|
||||
<% end -%>
|
||||
<% end -%>
|
||||
<% if options[:timestamps] %>
|
||||
t.timestamps
|
||||
t.timestamps null: false
|
||||
<% end -%>
|
||||
end
|
||||
<% attributes_with_index.each do |attribute| -%>
|
||||
|
@ -105,7 +105,7 @@ def test_remove_timestamps
|
||||
with_real_execute do
|
||||
begin
|
||||
ActiveRecord::Base.connection.create_table :delete_me do |t|
|
||||
t.timestamps
|
||||
t.timestamps null: true
|
||||
end
|
||||
ActiveRecord::Base.connection.remove_timestamps :delete_me
|
||||
assert !column_present?('delete_me', 'updated_at', 'datetime')
|
||||
|
@ -105,7 +105,7 @@ def test_remove_timestamps
|
||||
with_real_execute do
|
||||
begin
|
||||
ActiveRecord::Base.connection.create_table :delete_me do |t|
|
||||
t.timestamps
|
||||
t.timestamps null: true
|
||||
end
|
||||
ActiveRecord::Base.connection.remove_timestamps :delete_me
|
||||
assert !column_present?('delete_me', 'updated_at', 'datetime')
|
||||
|
@ -87,7 +87,7 @@ def test_timestamp_data_type_with_precision
|
||||
|
||||
def test_timestamps_helper_with_custom_precision
|
||||
ActiveRecord::Base.connection.create_table(:foos) do |t|
|
||||
t.timestamps :precision => 4
|
||||
t.timestamps :null => true, :precision => 4
|
||||
end
|
||||
assert_equal 4, activerecord_column_option('foos', 'created_at', 'precision')
|
||||
assert_equal 4, activerecord_column_option('foos', 'updated_at', 'precision')
|
||||
@ -95,7 +95,7 @@ def test_timestamps_helper_with_custom_precision
|
||||
|
||||
def test_passing_precision_to_timestamp_does_not_set_limit
|
||||
ActiveRecord::Base.connection.create_table(:foos) do |t|
|
||||
t.timestamps :precision => 4
|
||||
t.timestamps :null => true, :precision => 4
|
||||
end
|
||||
assert_nil activerecord_column_option("foos", "created_at", "limit")
|
||||
assert_nil activerecord_column_option("foos", "updated_at", "limit")
|
||||
@ -104,14 +104,14 @@ def test_passing_precision_to_timestamp_does_not_set_limit
|
||||
def test_invalid_timestamp_precision_raises_error
|
||||
assert_raises ActiveRecord::ActiveRecordError do
|
||||
ActiveRecord::Base.connection.create_table(:foos) do |t|
|
||||
t.timestamps :precision => 7
|
||||
t.timestamps :null => true, :precision => 7
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_postgres_agrees_with_activerecord_about_precision
|
||||
ActiveRecord::Base.connection.create_table(:foos) do |t|
|
||||
t.timestamps :precision => 4
|
||||
t.timestamps :null => true, :precision => 4
|
||||
end
|
||||
assert_equal '4', pg_datetime_precision('foos', 'created_at')
|
||||
assert_equal '4', pg_datetime_precision('foos', 'updated_at')
|
||||
|
@ -14,6 +14,7 @@ def setup
|
||||
@connection.drop_table :fruits rescue nil
|
||||
@connection.drop_table :nep_fruits rescue nil
|
||||
@connection.drop_table :nep_schema_migrations rescue nil
|
||||
@connection.drop_table :has_timestamps rescue nil
|
||||
ActiveRecord::SchemaMigration.delete_all rescue nil
|
||||
end
|
||||
|
||||
@ -88,5 +89,61 @@ def test_normalize_version
|
||||
assert_equal "017", ActiveRecord::SchemaMigration.normalize_migration_number("0017")
|
||||
assert_equal "20131219224947", ActiveRecord::SchemaMigration.normalize_migration_number("20131219224947")
|
||||
end
|
||||
|
||||
def test_timestamps_without_null_is_deprecated_on_create_table
|
||||
assert_deprecated do
|
||||
ActiveRecord::Schema.define do
|
||||
create_table :has_timestamps do |t|
|
||||
t.timestamps
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_timestamps_without_null_is_deprecated_on_change_table
|
||||
assert_deprecated do
|
||||
ActiveRecord::Schema.define do
|
||||
create_table :has_timestamps
|
||||
|
||||
change_table :has_timestamps do |t|
|
||||
t.timestamps
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_no_deprecation_warning_from_timestamps_on_create_table
|
||||
assert_not_deprecated do
|
||||
ActiveRecord::Schema.define do
|
||||
create_table :has_timestamps do |t|
|
||||
t.timestamps null: true
|
||||
end
|
||||
|
||||
drop_table :has_timestamps
|
||||
|
||||
create_table :has_timestamps do |t|
|
||||
t.timestamps null: false
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_no_deprecation_warning_from_timestamps_on_change_table
|
||||
assert_not_deprecated do
|
||||
ActiveRecord::Schema.define do
|
||||
create_table :has_timestamps
|
||||
change_table :has_timestamps do |t|
|
||||
t.timestamps null: true
|
||||
end
|
||||
|
||||
drop_table :has_timestamps
|
||||
|
||||
create_table :has_timestamps
|
||||
change_table :has_timestamps do |t|
|
||||
t.timestamps null: false, default: Time.now
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -176,8 +176,11 @@ def test_create_table_raises_when_redefining_custom_primary_key_column
|
||||
end
|
||||
|
||||
def test_create_table_with_timestamps_should_create_datetime_columns
|
||||
connection.create_table table_name do |t|
|
||||
t.timestamps
|
||||
# FIXME: Remove the silence when we change the default `null` behavior
|
||||
ActiveSupport::Deprecation.silence do
|
||||
connection.create_table table_name do |t|
|
||||
t.timestamps
|
||||
end
|
||||
end
|
||||
created_columns = connection.columns(table_name)
|
||||
|
||||
|
@ -88,8 +88,8 @@ def test_remove_references_column_type_with_polymorphic_and_type
|
||||
|
||||
def test_timestamps_creates_updated_at_and_created_at
|
||||
with_change_table do |t|
|
||||
@connection.expect :add_timestamps, nil, [:delete_me]
|
||||
t.timestamps
|
||||
@connection.expect :add_timestamps, nil, [:delete_me, null: true]
|
||||
t.timestamps null: true
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -22,7 +22,7 @@ def setup
|
||||
super
|
||||
@connection = ActiveRecord::Base.connection
|
||||
connection.create_table :test_models do |t|
|
||||
t.timestamps
|
||||
t.timestamps null: true
|
||||
end
|
||||
|
||||
TestModel.reset_column_information
|
||||
|
@ -561,7 +561,7 @@ def test_adding_multiple_columns
|
||||
t.string :qualification, :experience
|
||||
t.integer :age, :default => 0
|
||||
t.date :birthdate
|
||||
t.timestamps
|
||||
t.timestamps null: true
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -138,7 +138,7 @@ def except(adapter_names_to_exclude)
|
||||
t.integer :engines_count
|
||||
t.integer :wheels_count
|
||||
t.column :lock_version, :integer, null: false, default: 0
|
||||
t.timestamps
|
||||
t.timestamps null: false
|
||||
end
|
||||
|
||||
create_table :categories, force: true do |t|
|
||||
@ -537,7 +537,7 @@ def except(adapter_names_to_exclude)
|
||||
t.references :best_friend_of
|
||||
t.integer :insures, null: false, default: 0
|
||||
t.timestamp :born_at
|
||||
t.timestamps
|
||||
t.timestamps null: false
|
||||
end
|
||||
|
||||
create_table :peoples_treasures, id: false, force: true do |t|
|
||||
@ -548,7 +548,7 @@ def except(adapter_names_to_exclude)
|
||||
create_table :pets, primary_key: :pet_id, force: true do |t|
|
||||
t.string :name
|
||||
t.integer :owner_id, :integer
|
||||
t.timestamps
|
||||
t.timestamps null: false
|
||||
end
|
||||
|
||||
create_table :pirates, force: true do |t|
|
||||
@ -726,13 +726,13 @@ def except(adapter_names_to_exclude)
|
||||
t.string :parent_title
|
||||
t.string :type
|
||||
t.string :group
|
||||
t.timestamps
|
||||
t.timestamps null: true
|
||||
end
|
||||
|
||||
create_table :toys, primary_key: :toy_id, force: true do |t|
|
||||
t.string :name
|
||||
t.integer :pet_id, :integer
|
||||
t.timestamps
|
||||
t.timestamps null: false
|
||||
end
|
||||
|
||||
create_table :traffic_lights, force: true do |t|
|
||||
|
@ -222,7 +222,7 @@ def test_model_with_polymorphic_belongs_to_attribute_generates_belongs_to_associ
|
||||
|
||||
def test_migration_with_timestamps
|
||||
run_generator
|
||||
assert_migration "db/migrate/create_accounts.rb", /t.timestamps/
|
||||
assert_migration "db/migrate/create_accounts.rb", /t.timestamps null: false/
|
||||
end
|
||||
|
||||
def test_migration_timestamps_are_skipped
|
||||
|
Loading…
Reference in New Issue
Block a user