Skip to content

Commit

Permalink
Change the default null value for timestamps to false
Browse files Browse the repository at this point in the history
  • Loading branch information
rafaelfranca committed Jan 4, 2015
1 parent 94c8715 commit a939506
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 81 deletions.
4 changes: 4 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
* Change the default `null` value for `timestamps` to `false`.

*Rafael Mendonça França*

* Return an array of pools from `connection_pools`.

*Rafael Mendonça França*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,6 @@ def default_primary_key
end
end

module TimestampDefaultDeprecation # :nodoc:
def emit_warning_if_null_unspecified(options)
return if options.key?(:null)

ActiveSupport::Deprecation.warn(<<-MSG.squish)
`#timestamp` was called without specifying an option for `null`. In Rails 5,
this behavior will change to `null: false`. You should manually specify
`null: true` to prevent the behavior of your existing migrations from changing.
MSG
end
end

class ReferenceDefinition # :nodoc:
def initialize(
name,
Expand Down Expand Up @@ -167,8 +155,6 @@ def foreign_table_name
# 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
Expand Down Expand Up @@ -375,7 +361,9 @@ def foreign_key(table_name, options = {}) # :nodoc:
# t.timestamps null: false
def timestamps(*args)
options = args.extract_options!
emit_warning_if_null_unspecified(options)

options[:null] = false if options[:null].nil?

column(:created_at, :datetime, options)
column(:updated_at, :datetime, options)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -851,14 +851,14 @@ def columns_for_distinct(columns, orders) #:nodoc:
columns
end

include TimestampDefaultDeprecation
# Adds timestamps (+created_at+ and +updated_at+) columns to +table_name+.
# Additional options (like <tt>null: false</tt>) are forwarded to #add_column.
#
# add_timestamps(:suppliers, null: false)
#
def add_timestamps(table_name, options = {})
emit_warning_if_null_unspecified(options)
options[:null] = false if options[:null].nil?

add_column table_name, :created_at, :datetime, options
add_column table_name, :updated_at, :datetime, options
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ module ConnectionAdapters # :nodoc:
autoload :TableDefinition
autoload :Table
autoload :AlterTable
autoload :TimestampDefaultDeprecation
end

autoload_at 'active_record/connection_adapters/abstract/connection_pool' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def change
<% end -%>
<% end -%>
<% if options[:timestamps] %>
t.timestamps null: false
t.timestamps
<% end -%>
end
<% attributes_with_index.each do |attribute| -%>
Expand Down
71 changes: 20 additions & 51 deletions activerecord/test/cases/ar_schema_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,69 +93,38 @@ def test_normalize_version
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
def test_timestamps_without_null_set_null_to_false_on_create_table
ActiveRecord::Schema.define do
create_table :has_timestamps do |t|
t.timestamps
end
end

assert !@connection.columns(:has_timestamps).find { |c| c.name == 'created_at' }.null
assert !@connection.columns(:has_timestamps).find { |c| c.name == 'updated_at' }.null
end

def test_timestamps_without_null_is_deprecated_on_change_table
assert_deprecated do
ActiveRecord::Schema.define do
create_table :has_timestamps
def test_timestamps_without_null_set_null_to_false_on_change_table
ActiveRecord::Schema.define do
create_table :has_timestamps

change_table :has_timestamps do |t|
t.timestamps
end
change_table :has_timestamps do |t|
t.timestamps default: Time.now
end
end
end

def test_timestamps_without_null_is_deprecated_on_add_timestamps
assert_deprecated do
ActiveRecord::Schema.define do
create_table :has_timestamps
add_timestamps :has_timestamps
end
end
assert !@connection.columns(:has_timestamps).find { |c| c.name == 'created_at' }.null
assert !@connection.columns(:has_timestamps).find { |c| c.name == 'updated_at' }.null
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
def test_timestamps_without_null_set_null_to_false_on_add_timestamps
ActiveRecord::Schema.define do
create_table :has_timestamps
add_timestamps :has_timestamps, default: Time.now
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
assert !@connection.columns(:has_timestamps).find { |c| c.name == 'created_at' }.null
assert !@connection.columns(:has_timestamps).find { |c| c.name == 'updated_at' }.null
end
end
end
17 changes: 7 additions & 10 deletions activerecord/test/cases/migration/change_schema_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,32 +195,29 @@ def test_create_table_raises_when_redefining_custom_primary_key_column
end

def test_create_table_with_timestamps_should_create_datetime_columns
# 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
connection.create_table table_name do |t|
t.timestamps
end
created_columns = connection.columns(table_name)

created_at_column = created_columns.detect {|c| c.name == 'created_at' }
updated_at_column = created_columns.detect {|c| c.name == 'updated_at' }

assert created_at_column.null
assert updated_at_column.null
assert !created_at_column.null
assert !updated_at_column.null
end

def test_create_table_with_timestamps_should_create_datetime_columns_with_options
connection.create_table table_name do |t|
t.timestamps :null => false
t.timestamps null: true
end
created_columns = connection.columns(table_name)

created_at_column = created_columns.detect {|c| c.name == 'created_at' }
updated_at_column = created_columns.detect {|c| c.name == 'updated_at' }

assert !created_at_column.null
assert !updated_at_column.null
assert created_at_column.null
assert updated_at_column.null
end

def test_create_table_without_a_block
Expand Down
2 changes: 1 addition & 1 deletion railties/test/generators/model_generator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,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 null: false/
assert_migration "db/migrate/create_accounts.rb", /t.timestamps/
end

def test_migration_timestamps_are_skipped
Expand Down

0 comments on commit a939506

Please sign in to comment.