diff --git a/changelogs/unreleased/32054-rails-should-use-timestamptz-database-type-for-postgresql.yml b/changelogs/unreleased/32054-rails-should-use-timestamptz-database-type-for-postgresql.yml new file mode 100644 index 0000000000000000000000000000000000000000..7fc9e0a4f0e7887baf16c69ef1adc251e26a159f --- /dev/null +++ b/changelogs/unreleased/32054-rails-should-use-timestamptz-database-type-for-postgresql.yml @@ -0,0 +1,4 @@ +--- +title: Add database helpers 'add_timestamps_with_timezone' and 'timestamps_with_timezone' +merge_request: 11229 +author: @blackst0ne diff --git a/config/initializers/active_record_data_types.rb b/config/initializers/active_record_data_types.rb new file mode 100644 index 0000000000000000000000000000000000000000..beb97c6fce0603bdca474c8633ad9a3cebc07523 --- /dev/null +++ b/config/initializers/active_record_data_types.rb @@ -0,0 +1,24 @@ +# ActiveRecord custom data type for storing datetimes with timezone information. +# See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11229 + +if Gitlab::Database.postgresql? + require 'active_record/connection_adapters/postgresql_adapter' + + module ActiveRecord + module ConnectionAdapters + class PostgreSQLAdapter + NATIVE_DATABASE_TYPES.merge!(datetime_with_timezone: { name: 'timestamptz' }) + end + end + end +elsif Gitlab::Database.mysql? + require 'active_record/connection_adapters/mysql2_adapter' + + module ActiveRecord + module ConnectionAdapters + class AbstractMysqlAdapter + NATIVE_DATABASE_TYPES.merge!(datetime_with_timezone: { name: 'timestamp' }) + end + end + end +end diff --git a/config/initializers/active_record_table_definition.rb b/config/initializers/active_record_table_definition.rb new file mode 100644 index 0000000000000000000000000000000000000000..4f59e35f4da026a0623639c0eeb17c9fc0639f44 --- /dev/null +++ b/config/initializers/active_record_table_definition.rb @@ -0,0 +1,24 @@ +# ActiveRecord custom method definitions with timezone information. +# See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11229 + +require 'active_record/connection_adapters/abstract/schema_definitions' + +# Appends columns `created_at` and `updated_at` to a table. +# +# It is used in table creation like: +# create_table 'users' do |t| +# t.timestamps_with_timezone +# end +module ActiveRecord + module ConnectionAdapters + class TableDefinition + def timestamps_with_timezone(**options) + options[:null] = false if options[:null].nil? + + [:created_at, :updated_at].each do |column_name| + column(column_name, :datetime_with_timezone, options) + end + end + end + end +end diff --git a/db/migrate/20160314114439_add_requested_at_to_members.rb b/db/migrate/20160314114439_add_requested_at_to_members.rb index 273819d4cd8069f160d0a360bfa72f1610d16f03..76c8b8a1a24fc26be9a91d283fbb601e89ba5dff 100644 --- a/db/migrate/20160314114439_add_requested_at_to_members.rb +++ b/db/migrate/20160314114439_add_requested_at_to_members.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class AddRequestedAtToMembers < ActiveRecord::Migration def change add_column :members, :requested_at, :datetime diff --git a/db/migrate/20160415062917_create_personal_access_tokens.rb b/db/migrate/20160415062917_create_personal_access_tokens.rb index ce0b33f32bd0d07dd6550eaab095e4651f4b91ca..c7b49870bf7501aea1576c5555b3102a9d554899 100644 --- a/db/migrate/20160415062917_create_personal_access_tokens.rb +++ b/db/migrate/20160415062917_create_personal_access_tokens.rb @@ -1,3 +1,5 @@ +# rubocop:disable Migration/Datetime +# rubocop:disable Migration/Timestamps class CreatePersonalAccessTokens < ActiveRecord::Migration def change create_table :personal_access_tokens do |t| diff --git a/db/migrate/20160610204157_add_deployments.rb b/db/migrate/20160610204157_add_deployments.rb index cb144ea8a6d82c7cd8a005e8f0ee433100787bec..0e7e6e747a369c2aadf5c89c40341f31463b728e 100644 --- a/db/migrate/20160610204157_add_deployments.rb +++ b/db/migrate/20160610204157_add_deployments.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Datetime class AddDeployments < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160610204158_add_environments.rb b/db/migrate/20160610204158_add_environments.rb index e1c71d173c41758e32e7c945c4e0e094a6a13ee8..699cee2b246220ad3f781bb995007a45e72bf855 100644 --- a/db/migrate/20160610204158_add_environments.rb +++ b/db/migrate/20160610204158_add_environments.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Datetime class AddEnvironments < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160705054938_add_protected_branches_push_access.rb b/db/migrate/20160705054938_add_protected_branches_push_access.rb index f27295524e1b12cee8b96fd5c5c0c6650a405704..97aaaf9d2c8c0f23022ba755a1e02298dfb84539 100644 --- a/db/migrate/20160705054938_add_protected_branches_push_access.rb +++ b/db/migrate/20160705054938_add_protected_branches_push_access.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Timestamps class AddProtectedBranchesPushAccess < ActiveRecord::Migration DOWNTIME = false diff --git a/db/migrate/20160705054952_add_protected_branches_merge_access.rb b/db/migrate/20160705054952_add_protected_branches_merge_access.rb index 32adfa266cd7c252583f536d5235ea249e818a8b..51a52a5ac17a7b55dc9a7e27457cd92bf1e6d2c5 100644 --- a/db/migrate/20160705054952_add_protected_branches_merge_access.rb +++ b/db/migrate/20160705054952_add_protected_branches_merge_access.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Timestamps class AddProtectedBranchesMergeAccess < ActiveRecord::Migration DOWNTIME = false diff --git a/db/migrate/20160724205507_add_resolved_to_notes.rb b/db/migrate/20160724205507_add_resolved_to_notes.rb index b8ebcdbd156019255d771a1097601d8b90a8698f..3aca272a3f7bca8c6f2b029c030aa59f05d40ffd 100644 --- a/db/migrate/20160724205507_add_resolved_to_notes.rb +++ b/db/migrate/20160724205507_add_resolved_to_notes.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class AddResolvedToNotes < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160727163552_create_user_agent_details.rb b/db/migrate/20160727163552_create_user_agent_details.rb index ed4ccfedc0a4a73b7765e562a851f832c4d2c939..3eb36f8464f828441ef51051188ba92b194a6e6a 100644 --- a/db/migrate/20160727163552_create_user_agent_details.rb +++ b/db/migrate/20160727163552_create_user_agent_details.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateUserAgentDetails < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160727191041_create_boards.rb b/db/migrate/20160727191041_create_boards.rb index 56afbd4e030004fc0910add68f10b06d9ccaf0e1..9ec8df1b8e8cac54930f73cc2fa4b254857c3f9b 100644 --- a/db/migrate/20160727191041_create_boards.rb +++ b/db/migrate/20160727191041_create_boards.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateBoards < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160727193336_create_lists.rb b/db/migrate/20160727193336_create_lists.rb index 61d501215f21472717867c75d38e23dab1507a6e..3fd95dc8cfc41ed0dae9edbe843c0f8a19b80961 100644 --- a/db/migrate/20160727193336_create_lists.rb +++ b/db/migrate/20160727193336_create_lists.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateLists < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb b/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb index 30d98a0124e811229852c216896c3b79b1fbb9e2..404c253e18bfa53dbad2f2cba716cfa4c1f27094 100644 --- a/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb +++ b/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime # rubocop:disable RemoveIndex class AddDeletedAtToNamespaces < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160824124900_add_table_issue_metrics.rb b/db/migrate/20160824124900_add_table_issue_metrics.rb index e9bb79b3c628f10f3b86ac9ea58d8451f00f8424..30d35ef1db2b678375c948b931b90567e1afff71 100644 --- a/db/migrate/20160824124900_add_table_issue_metrics.rb +++ b/db/migrate/20160824124900_add_table_issue_metrics.rb @@ -1,6 +1,8 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Datetime +# rubocop:disable Migration/Timestamps class AddTableIssueMetrics < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160825052008_add_table_merge_request_metrics.rb b/db/migrate/20160825052008_add_table_merge_request_metrics.rb index e01cc5038b900efa9cfee7b6421750e1eeb11859..56b39634dfde312bfeec8f868f6141d8fb9e247d 100644 --- a/db/migrate/20160825052008_add_table_merge_request_metrics.rb +++ b/db/migrate/20160825052008_add_table_merge_request_metrics.rb @@ -1,6 +1,8 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Datetime +# rubocop:disable Migration/Timestamps class AddTableMergeRequestMetrics < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160831214002_create_project_features.rb b/db/migrate/20160831214002_create_project_features.rb index 343953826f0bcce4bba763f77ee0146097b03f5c..7ac6c8ec654295c08576649601aca08ceed36071 100644 --- a/db/migrate/20160831214002_create_project_features.rb +++ b/db/migrate/20160831214002_create_project_features.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateProjectFeatures < ActiveRecord::Migration DOWNTIME = false diff --git a/db/migrate/20160915042921_create_merge_requests_closing_issues.rb b/db/migrate/20160915042921_create_merge_requests_closing_issues.rb index 94874a853dae6e15df78dd78a7ebfbb10a3be4ed..10c5604bb5c4caccf20a9acbf71e47c9b1f0c935 100644 --- a/db/migrate/20160915042921_create_merge_requests_closing_issues.rb +++ b/db/migrate/20160915042921_create_merge_requests_closing_issues.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Timestamps class CreateMergeRequestsClosingIssues < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161014173530_create_label_priorities.rb b/db/migrate/20161014173530_create_label_priorities.rb index 2c22841c28a27af37d73a525033d80e5ec82aa84..28937c81e026455bec689e2d2123dccf0f196d4b 100644 --- a/db/migrate/20161014173530_create_label_priorities.rb +++ b/db/migrate/20161014173530_create_label_priorities.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateLabelPriorities < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161113184239_create_user_chat_names_table.rb b/db/migrate/20161113184239_create_user_chat_names_table.rb index 97b597654f78040cbfe8fe6da42b706341581d7e..62ccb599f2eba82c0ec3e18c83bd5813c37945a4 100644 --- a/db/migrate/20161113184239_create_user_chat_names_table.rb +++ b/db/migrate/20161113184239_create_user_chat_names_table.rb @@ -1,3 +1,5 @@ +# rubocop:disable Migration/Datetime +# rubocop:disable Migration/Timestamps class CreateUserChatNamesTable < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161124111402_add_routes_table.rb b/db/migrate/20161124111402_add_routes_table.rb index a02e046a18e7d47b1940710667b2533963a29aee..f5241d906d18ae98a3b08150f57fc34509ef1ed1 100644 --- a/db/migrate/20161124111402_add_routes_table.rb +++ b/db/migrate/20161124111402_add_routes_table.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Timestamps class AddRoutesTable < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161221152132_add_last_used_at_to_key.rb b/db/migrate/20161221152132_add_last_used_at_to_key.rb index fb2b15817de096b926737c547f41d9d8c667e313..86dc78702478956c83450104c6c6cc0afefc798b 100644 --- a/db/migrate/20161221152132_add_last_used_at_to_key.rb +++ b/db/migrate/20161221152132_add_last_used_at_to_key.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class AddLastUsedAtToKey < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161223034646_create_timelogs_ce.rb b/db/migrate/20161223034646_create_timelogs_ce.rb index 66d9cd823fb646f1d338e76b290c3de34101453a..1e894cc916113afd9895a5062df3b37e5f2034c4 100644 --- a/db/migrate/20161223034646_create_timelogs_ce.rb +++ b/db/migrate/20161223034646_create_timelogs_ce.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateTimelogsCe < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161228124936_change_expires_at_to_date_in_personal_access_tokens.rb b/db/migrate/20161228124936_change_expires_at_to_date_in_personal_access_tokens.rb index af1bac897cc87fdc5ced6c9f867c09d33a394096..16f7cc487cef12afcee5d31fcbecb20e5e2e7428 100644 --- a/db/migrate/20161228124936_change_expires_at_to_date_in_personal_access_tokens.rb +++ b/db/migrate/20161228124936_change_expires_at_to_date_in_personal_access_tokens.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/Datetime class ChangeExpiresAtToDateInPersonalAccessTokens < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170120131253_create_chat_teams.rb b/db/migrate/20170120131253_create_chat_teams.rb index 7995d383986393a3e9b67a14f49639b2c4f747fc..522088219112bfaa5bc5dc344b8498e28ba3473d 100644 --- a/db/migrate/20170120131253_create_chat_teams.rb +++ b/db/migrate/20170120131253_create_chat_teams.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateChatTeams < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170130221926_create_uploads.rb b/db/migrate/20170130221926_create_uploads.rb index 6f06c5dd84082bfe37a11118c0c61d72036956e1..4d9fa0bb69299ac2898865b64116b746594ca97a 100644 --- a/db/migrate/20170130221926_create_uploads.rb +++ b/db/migrate/20170130221926_create_uploads.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class CreateUploads < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170222143317_drop_ci_projects.rb b/db/migrate/20170222143317_drop_ci_projects.rb index 4db8658f36fbe1c3edcc128ebc492174eb14ab22..9973e53501cb5db3cd894e5229e12f2db3dfc165 100644 --- a/db/migrate/20170222143317_drop_ci_projects.rb +++ b/db/migrate/20170222143317_drop_ci_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class DropCiProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170301205639_remove_unused_ci_tables_and_columns.rb b/db/migrate/20170301205639_remove_unused_ci_tables_and_columns.rb index 69dd15b8b4e5a0eb5fc8931410c1eaa07a6c1f6e..1a77d5934a3eca0a8ce7c836308ef79a485ec48e 100644 --- a/db/migrate/20170301205639_remove_unused_ci_tables_and_columns.rb +++ b/db/migrate/20170301205639_remove_unused_ci_tables_and_columns.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class RemoveUnusedCiTablesAndColumns < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170309173138_create_protected_tags.rb b/db/migrate/20170309173138_create_protected_tags.rb index 796f3c903443b1009c1e23b2940896d7d4c939a5..4684c9964c496eaf74532e6478c1f57e61e189d9 100644 --- a/db/migrate/20170309173138_create_protected_tags.rb +++ b/db/migrate/20170309173138_create_protected_tags.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateProtectedTags < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170314082049_create_system_note_metadata.rb b/db/migrate/20170314082049_create_system_note_metadata.rb index dd1e6cf8172a164c003fd45bba1bc7f26ee448d8..fee47e9605327a79a50d9347668f40fa35381788 100644 --- a/db/migrate/20170314082049_create_system_note_metadata.rb +++ b/db/migrate/20170314082049_create_system_note_metadata.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateSystemNoteMetadata < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170315194013_add_closed_at_to_issues.rb b/db/migrate/20170315194013_add_closed_at_to_issues.rb index 1326118cc8dacec0e9a47b641e86b48978a81036..34a1bd7ca8c86a333ff9a49d3a832f7789bd2a81 100644 --- a/db/migrate/20170315194013_add_closed_at_to_issues.rb +++ b/db/migrate/20170315194013_add_closed_at_to_issues.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class AddClosedAtToIssues < ActiveRecord::Migration DOWNTIME = false diff --git a/db/migrate/20170322013926_create_container_repository.rb b/db/migrate/20170322013926_create_container_repository.rb index 91540bc88bd5927af4bb496fa0f5fd677fa745b5..242f7b8d17df0a99f01c6113b80480a114353a1b 100644 --- a/db/migrate/20170322013926_create_container_repository.rb +++ b/db/migrate/20170322013926_create_container_repository.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateContainerRepository < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170329095907_create_ci_trigger_schedules.rb b/db/migrate/20170329095907_create_ci_trigger_schedules.rb index cfcfa27ebb53881861dfd4d1428e937a41082f3d..06a2010db23f248f8a9122ae59c5539f5ad41fbc 100644 --- a/db/migrate/20170329095907_create_ci_trigger_schedules.rb +++ b/db/migrate/20170329095907_create_ci_trigger_schedules.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class CreateCiTriggerSchedules < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170425112128_create_pipeline_schedules_table.rb b/db/migrate/20170425112128_create_pipeline_schedules_table.rb index 3612a796ae869e8c8b97008199f7320a461fd16a..57df47f5f428ea8d727f4ac48a224b1dbbe755db 100644 --- a/db/migrate/20170425112128_create_pipeline_schedules_table.rb +++ b/db/migrate/20170425112128_create_pipeline_schedules_table.rb @@ -1,3 +1,5 @@ +# rubocop:disable Migration/Datetime +# rubocop:disable Migration/Timestamps class CreatePipelineSchedulesTable < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170427215854_create_redirect_routes.rb b/db/migrate/20170427215854_create_redirect_routes.rb index 2bf086b3e30fcfdaea571ee6b22e8c241ece4359..6db508e5db452b3ecdc8fbee7b0a54c491dd3ef3 100644 --- a/db/migrate/20170427215854_create_redirect_routes.rb +++ b/db/migrate/20170427215854_create_redirect_routes.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateRedirectRoutes < ActiveRecord::Migration # Set this constant to true if this migration requires downtime. DOWNTIME = false diff --git a/db/migrate/20170503004125_add_last_repository_updated_at_to_projects.rb b/db/migrate/20170503004125_add_last_repository_updated_at_to_projects.rb index 00c685cf342fee2fad345d516c1f163d97adc088..2ea49f627428f111c8bdbe06824e5a6d3bd6fe87 100644 --- a/db/migrate/20170503004125_add_last_repository_updated_at_to_projects.rb +++ b/db/migrate/20170503004125_add_last_repository_updated_at_to_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class AddLastRepositoryUpdatedAtToProjects < ActiveRecord::Migration DOWNTIME = false diff --git a/db/migrate/20170523121229_create_conversational_development_index_metrics.rb b/db/migrate/20170523121229_create_conversational_development_index_metrics.rb index 9f9ec5260551155abc4cd111319fa966d7bd30bc..7026a867ae16493d3c96d55f7f6ef1c7142d88f1 100644 --- a/db/migrate/20170523121229_create_conversational_development_index_metrics.rb +++ b/db/migrate/20170523121229_create_conversational_development_index_metrics.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreateConversationalDevelopmentIndexMetrics < ActiveRecord::Migration DOWNTIME = false diff --git a/db/migrate/20170525132202_create_pipeline_stages.rb b/db/migrate/20170525132202_create_pipeline_stages.rb index 25656f2a2c28f65d300d5e582d30a179ddfe4a00..825993aa41e435d365204bca90ba5393fbb4ccd1 100644 --- a/db/migrate/20170525132202_create_pipeline_stages.rb +++ b/db/migrate/20170525132202_create_pipeline_stages.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Timestamps class CreatePipelineStages < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/post_migrate/20170425130047_drop_ci_trigger_schedules_table.rb b/db/post_migrate/20170425130047_drop_ci_trigger_schedules_table.rb index 24750c58ef00810b7319e65cae049f683b05b605..159b533eaaa86e3765d9851efcc605d9bc1c95c0 100644 --- a/db/post_migrate/20170425130047_drop_ci_trigger_schedules_table.rb +++ b/db/post_migrate/20170425130047_drop_ci_trigger_schedules_table.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/Datetime class DropCiTriggerSchedulesTable < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 77ba2a5fd87802fa434018f373fce2978fa68b47..161d2544169fca36d04aedbbcf059d46ed80d0cd 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -122,7 +122,7 @@ limit can vary from installation to installation. As a result it's recommended you do not use more than 32 threads in a single migration. Usually 4-8 threads should be more than enough. -## Removing indices +## Removing indexes When removing an index make sure to use the method `remove_concurrent_index` instead of the regular `remove_index` method. The `remove_concurrent_index` method @@ -142,7 +142,7 @@ class MyMigration < ActiveRecord::Migration end ``` -## Adding indices +## Adding indexes If you need to add a unique index please keep in mind there is the possibility of existing duplicates being present in the database. This means that should @@ -222,6 +222,41 @@ add_column_with_default(:projects, :foo, :integer, default: 10, limit: 8) add_column(:projects, :foo, :integer, default: 10, limit: 8) ``` +## Timestamp column type + +By default, Rails uses the `timestamp` data type that stores timestamp data without timezone information. +The `timestamp` data type is used by calling either the `add_timestamps` or the `timestamps` method. +Also Rails converts the `:datetime` data type to the `timestamp` one. + +Example: + +```ruby +# timestamps +create_table :users do |t| + t.timestamps +end + +# add_timestamps +def up + add_timestamps :users +end + +# :datetime +def up + add_column :users, :last_sign_in, :datetime +end +``` + +Instead of using these methods one should use the following methods to store timestamps with timezones: + +* `add_timestamps_with_timezone` +* `timestamps_with_timezone` + +This ensures all timestamps have a time zone specified. This in turn means existing timestamps won't +suddenly use a different timezone when the system's timezone changes. It also makes it very clear which +timezone was used in the first place. + + ## Testing Make sure that your migration works with MySQL and PostgreSQL with data. An diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index a412bb6dbd2cc9d17d929d9e19ab00a65c671263..cd85f961242c086c98e898206864c6c05b884b54 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -1,6 +1,39 @@ module Gitlab module Database module MigrationHelpers + # Adds `created_at` and `updated_at` columns with timezone information. + # + # This method is an improved version of Rails' built-in method `add_timestamps`. + # + # Available options are: + # default - The default value for the column. + # null - When set to `true` the column will allow NULL values. + # The default is to not allow NULL values. + def add_timestamps_with_timezone(table_name, options = {}) + options[:null] = false if options[:null].nil? + + [:created_at, :updated_at].each do |column_name| + if options[:default] && transaction_open? + raise '`add_timestamps_with_timezone` with default value cannot be run inside a transaction. ' \ + 'You can disable transactions by calling `disable_ddl_transaction!` ' \ + 'in the body of your migration class' + end + + # If default value is presented, use `add_column_with_default` method instead. + if options[:default] + add_column_with_default( + table_name, + column_name, + :datetime_with_timezone, + default: options[:default], + allow_null: options[:null] + ) + else + add_column(table_name, column_name, :datetime_with_timezone, options) + end + end + end + # Creates a new index, concurrently when supported # # On PostgreSQL this method creates an index concurrently, on MySQL this diff --git a/rubocop/cop/migration/add_timestamps.rb b/rubocop/cop/migration/add_timestamps.rb new file mode 100644 index 0000000000000000000000000000000000000000..08ddd91e54d339052fb8c91d2deac2d7ccbeeeb5 --- /dev/null +++ b/rubocop/cop/migration/add_timestamps.rb @@ -0,0 +1,25 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if 'add_timestamps' method is called with timezone information. + class AddTimestamps < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = 'Do not use `add_timestamps`, use `add_timestamps_with_timezone` instead'.freeze + + # Check methods. + def on_send(node) + return unless in_migration?(node) + + add_offense(node, :selector) if method_name(node) == :add_timestamps + end + + def method_name(node) + node.children[1] + end + end + end + end +end diff --git a/rubocop/cop/migration/datetime.rb b/rubocop/cop/migration/datetime.rb new file mode 100644 index 0000000000000000000000000000000000000000..651935dd53e642b0dbcedd306e99a4ceaba62bb2 --- /dev/null +++ b/rubocop/cop/migration/datetime.rb @@ -0,0 +1,36 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if datetime data type is added with timezone information. + class Datetime < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = 'Do not use the `datetime` data type, use `datetime_with_timezone` instead'.freeze + + # Check methods in table creation. + def on_def(node) + return unless in_migration?(node) + + node.each_descendant(:send) do |send_node| + add_offense(send_node, :selector) if method_name(send_node) == :datetime + end + end + + # Check methods. + def on_send(node) + return unless in_migration?(node) + + node.each_descendant do |descendant| + add_offense(node, :expression) if descendant.type == :sym && descendant.children.last == :datetime + end + end + + def method_name(node) + node.children[1] + end + end + end + end +end diff --git a/rubocop/cop/migration/timestamps.rb b/rubocop/cop/migration/timestamps.rb new file mode 100644 index 0000000000000000000000000000000000000000..71a9420cc3bca8574d031d1baa13444c43d4dce4 --- /dev/null +++ b/rubocop/cop/migration/timestamps.rb @@ -0,0 +1,27 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if 'timestamps' method is called with timezone information. + class Timestamps < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = 'Do not use `timestamps`, use `timestamps_with_timezone` instead'.freeze + + # Check methods in table creation. + def on_def(node) + return unless in_migration?(node) + + node.each_descendant(:send) do |send_node| + add_offense(send_node, :selector) if method_name(send_node) == :timestamps + end + end + + def method_name(node) + node.children[1] + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index dae30969abf35ba8af0f5690109a99371edcbe84..6b8d127dde60ec8a9f34ab6c4584162968ebac78 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -8,7 +8,10 @@ require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' +require_relative 'cop/migration/add_timestamps' +require_relative 'cop/migration/datetime' require_relative 'cop/migration/remove_concurrent_index' require_relative 'cop/migration/remove_index' require_relative 'cop/migration/reversible_add_column_with_default' +require_relative 'cop/migration/timestamps' require_relative 'cop/migration/update_column_in_batches' diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 3fdafd867da3d145d7b7d6cd3e1e1de4ceeb2ac3..30aa463faf88a9617092e12e1302cba302849140 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -7,7 +7,42 @@ ) end - before { allow(model).to receive(:puts) } + before do + allow(model).to receive(:puts) + end + + describe '#add_timestamps_with_timezone' do + before do + allow(model).to receive(:transaction_open?).and_return(false) + end + + context 'using PostgreSQL' do + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + allow(model).to receive(:disable_statement_timeout) + end + + it 'adds "created_at" and "updated_at" fields with the "datetime_with_timezone" data type' do + expect(model).to receive(:add_column).with(:foo, :created_at, :datetime_with_timezone, { null: false }) + expect(model).to receive(:add_column).with(:foo, :updated_at, :datetime_with_timezone, { null: false }) + + model.add_timestamps_with_timezone(:foo) + end + end + + context 'using MySQL' do + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + end + + it 'adds "created_at" and "updated_at" fields with "datetime_with_timezone" data type' do + expect(model).to receive(:add_column).with(:foo, :created_at, :datetime_with_timezone, { null: false }) + expect(model).to receive(:add_column).with(:foo, :updated_at, :datetime_with_timezone, { null: false }) + + model.add_timestamps_with_timezone(:foo) + end + end + end describe '#add_concurrent_index' do context 'outside a transaction' do diff --git a/spec/rubocop/cop/migration/add_timestamps_spec.rb b/spec/rubocop/cop/migration/add_timestamps_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..18df62dec3e5846288d2b421107480846f2614ee --- /dev/null +++ b/spec/rubocop/cop/migration/add_timestamps_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/add_timestamps' + +describe RuboCop::Cop::Migration::AddTimestamps do + include CopHelper + + subject(:cop) { described_class.new } + let(:migration_with_add_timestamps) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_timestamps(:users) + end + end + ) + end + + let(:migration_without_add_timestamps) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + end + end + ) + end + + let(:migration_with_add_timestamps_with_timezone) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_timestamps_with_timezone(:users) + end + end + ) + end + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when the "add_timestamps" method is used' do + inspect_source(cop, migration_with_add_timestamps) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([7]) + end + end + + it 'does not register an offense when the "add_timestamps" method is not used' do + inspect_source(cop, migration_without_add_timestamps) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + + it 'does not register an offense when the "add_timestamps_with_timezone" method is used' do + inspect_source(cop, migration_with_add_timestamps_with_timezone) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, migration_with_add_timestamps) + inspect_source(cop, migration_without_add_timestamps) + inspect_source(cop, migration_with_add_timestamps_with_timezone) + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/rubocop/cop/migration/datetime_spec.rb b/spec/rubocop/cop/migration/datetime_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..388b086ce6ad5689f45e7378d6d5754f2e30bcbe --- /dev/null +++ b/spec/rubocop/cop/migration/datetime_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/datetime' + +describe RuboCop::Cop::Migration::Datetime do + include CopHelper + + subject(:cop) { described_class.new } + let(:migration_with_datetime) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_column(:users, :last_sign_in, :datetime) + end + end + ) + end + + let(:migration_without_datetime) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + end + end + ) + end + + let(:migration_with_datetime_with_timezone) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_column(:users, :last_sign_in, :datetime_with_timezone) + end + end + ) + end + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when the ":datetime" data type is used' do + inspect_source(cop, migration_with_datetime) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([7]) + end + end + + it 'does not register an offense when the ":datetime" data type is not used' do + inspect_source(cop, migration_without_datetime) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + + it 'does not register an offense when the ":datetime_with_timezone" data type is used' do + inspect_source(cop, migration_with_datetime_with_timezone) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, migration_with_datetime) + inspect_source(cop, migration_without_datetime) + inspect_source(cop, migration_with_datetime_with_timezone) + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/rubocop/cop/migration/timestamps_spec.rb b/spec/rubocop/cop/migration/timestamps_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..cdf1423d0c53ce561818cd18dcc8d025dd923b56 --- /dev/null +++ b/spec/rubocop/cop/migration/timestamps_spec.rb @@ -0,0 +1,99 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/timestamps' + +describe RuboCop::Cop::Migration::Timestamps do + include CopHelper + + subject(:cop) { described_class.new } + let(:migration_with_timestamps) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.timestamps null: true + t.string :password + end + end + end + ) + end + + let(:migration_without_timestamps) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.string :password + end + end + end + ) + end + + let(:migration_with_timestamps_with_timezone) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.timestamps_with_timezone null: true + t.string :password + end + end + end + ) + end + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when the "timestamps" method is used' do + inspect_source(cop, migration_with_timestamps) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([8]) + end + end + + it 'does not register an offense when the "timestamps" method is not used' do + inspect_source(cop, migration_without_timestamps) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + + it 'does not register an offense when the "timestamps_with_timezone" method is used' do + inspect_source(cop, migration_with_timestamps_with_timezone) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, migration_with_timestamps) + inspect_source(cop, migration_without_timestamps) + inspect_source(cop, migration_with_timestamps_with_timezone) + + expect(cop.offenses.size).to eq(0) + end + end +end