From 0b947b6aabcec59ed0160e525d0eaac98bbb3d33 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 7 May 2024 11:06:31 +0800 Subject: [PATCH] DEV: Improve code comment about when ignored columns can be removed (#26894) Ignored columns can only be dropped when its associated post-deploy migration has been promoted to a regular migration. This is so because Discourse doesn't rely on a schema file system to setup a brand new database and thus the column information will be loaded by the application first before the post-deploy migration runs. --- app/models/category.rb | 6 +++--- app/models/directory_column.rb | 3 +-- app/models/email_token.rb | 3 +-- app/models/embeddable_host.rb | 3 +-- app/models/group.rb | 3 +-- app/models/invite.rb | 3 +-- app/models/post.rb | 4 ++-- app/models/tag.rb | 2 +- app/models/topic.rb | 4 ++-- app/models/topic_user.rb | 2 +- app/models/user_api_key.rb | 2 +- app/models/user_option.rb | 3 +-- app/models/user_profile.rb | 3 +-- app/models/user_stat.rb | 3 +-- lib/migration/safe_migrate.rb | 2 +- 15 files changed, 19 insertions(+), 27 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index 907bc1e42e5..b2fa9d56e05 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -4,9 +4,9 @@ class Category < ActiveRecord::Base RESERVED_SLUGS = ["none"] self.ignored_columns = [ - :suppress_from_latest, # TODO(2020-11-18): remove - :required_tag_group_id, # TODO(2023-04-01): remove - :min_tags_from_required_group, # TODO(2023-04-01): remove + :suppress_from_latest, # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy + :required_tag_group_id, # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy + :min_tags_from_required_group, # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy ] include Searchable diff --git a/app/models/directory_column.rb b/app/models/directory_column.rb index 73a0762a1e8..0722be0fd95 100644 --- a/app/models/directory_column.rb +++ b/app/models/directory_column.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true class DirectoryColumn < ActiveRecord::Base - # TODO(2021-06-18): Remove automatic column - self.ignored_columns = ["automatic"] + self.ignored_columns = ["automatic"] # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy self.inheritance_column = nil enum type: { automatic: 0, user_field: 1, plugin: 2 }, _scopes: false diff --git a/app/models/email_token.rb b/app/models/email_token.rb index 5cfcfc446d8..796db3affc0 100644 --- a/app/models/email_token.rb +++ b/app/models/email_token.rb @@ -40,8 +40,7 @@ class EmailToken < ActiveRecord::Base end end - # TODO(2022-01-01): Remove - self.ignored_columns = %w[token] + self.ignored_columns = %w[token] # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy def self.scopes @scopes ||= Enum.new(signup: 1, password_reset: 2, email_login: 3, email_update: 4) diff --git a/app/models/embeddable_host.rb b/app/models/embeddable_host.rb index 285c6fd1b79..2b25be6e581 100644 --- a/app/models/embeddable_host.rb +++ b/app/models/embeddable_host.rb @@ -10,8 +10,7 @@ class EmbeddableHost < ActiveRecord::Base self.host.sub!(%r{/.*\z}, "") end - # TODO(2021-07-23): Remove - self.ignored_columns = ["path_whitelist"] + self.ignored_columns = ["path_whitelist"] # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy def self.record_for_url(uri) if uri.is_a?(String) diff --git a/app/models/group.rb b/app/models/group.rb index 5e0928c11a5..3a7f2a4f5d9 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -3,8 +3,7 @@ require "net/imap" class Group < ActiveRecord::Base - # TODO(2021-05-26): remove - self.ignored_columns = %w[flair_url] + self.ignored_columns = %w[flair_url] # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy include HasCustomFields include AnonCacheInvalidator diff --git a/app/models/invite.rb b/app/models/invite.rb index 2aa7c91f6fd..43b75133583 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -13,8 +13,7 @@ class Invite < ActiveRecord::Base include RateLimiter::OnCreateRecord include Trashable - # TODO(2021-05-22): remove - self.ignored_columns = %w[user_id redeemed_at] + self.ignored_columns = %w[user_id redeemed_at] # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy BULK_INVITE_EMAIL_LIMIT = 200 DOMAIN_REGEX = diff --git a/app/models/post.rb b/app/models/post.rb index 0d103d15e73..52ee157a6df 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -11,8 +11,8 @@ class Post < ActiveRecord::Base include LimitedEdit self.ignored_columns = [ - "avg_time", # TODO(2021-01-04): remove - "image_url", # TODO(2021-06-01): remove + "avg_time", # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy + "image_url", # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy ] cattr_accessor :plugin_permitted_create_params, :plugin_permitted_update_params diff --git a/app/models/tag.rb b/app/models/tag.rb index b258cea67fd..7a935be68af 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -6,7 +6,7 @@ class Tag < ActiveRecord::Base include HasSanitizableFields self.ignored_columns = [ - "topic_count", # TODO(tgxworld): Remove on 1 July 2023 + "topic_count", # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy ] RESERVED_TAGS = [ diff --git a/app/models/topic.rb b/app/models/topic.rb index 233a9aed103..c7d943e3033 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -16,8 +16,8 @@ class Topic < ActiveRecord::Base EXTERNAL_ID_MAX_LENGTH = 50 self.ignored_columns = [ - "avg_time", # TODO(2021-01-04): remove - "image_url", # TODO(2021-06-01): remove + "avg_time", # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy + "image_url", # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy ] def_delegator :featured_users, :user_ids, :featured_user_ids diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index cc9ce74b5c5..9d35a5f8c7d 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -2,7 +2,7 @@ class TopicUser < ActiveRecord::Base self.ignored_columns = [ - :highest_seen_post_number, # Remove after 01 Jan 2022 + :highest_seen_post_number, # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy ] belongs_to :user diff --git a/app/models/user_api_key.rb b/app/models/user_api_key.rb index 59815a5c70c..9d004cf3bd9 100644 --- a/app/models/user_api_key.rb +++ b/app/models/user_api_key.rb @@ -2,7 +2,7 @@ class UserApiKey < ActiveRecord::Base self.ignored_columns = [ - "scopes", # TODO(2020-12-18): remove + "scopes", # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy ] REVOKE_MATCHER = RouteMatcher.new(actions: "user_api_keys#revoke", methods: :post, params: [:id]) diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 18024366a12..0ee8e71472e 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -14,8 +14,7 @@ class UserOption < ActiveRecord::Base } self.ignored_columns = [ - "disable_jump_reply", # Remove once 20210706091905 is promoted from post_deploy to regular migration - "sidebar_list_destination", # TODO(osama): Remove in January 2024 + "sidebar_list_destination", # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy ] self.primary_key = :user_id diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index 91920dd9d70..5ee6ce6e9d8 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true class UserProfile < ActiveRecord::Base - # TODO Remove `badge_granted_title` after 2023-09-01 - self.ignored_columns = ["badge_granted_title"] + self.ignored_columns = ["badge_granted_title"] # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy BAKED_VERSION = 1 diff --git a/app/models/user_stat.rb b/app/models/user_stat.rb index 32e00681bd9..65f0f98aa9e 100644 --- a/app/models/user_stat.rb +++ b/app/models/user_stat.rb @@ -3,8 +3,7 @@ class UserStat < ActiveRecord::Base belongs_to :user after_save :trigger_badges - # TODO(2021-05-13): Remove - self.ignored_columns = ["topic_reply_count"] + self.ignored_columns = ["topic_reply_count"] # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy def self.ensure_consistency!(last_seen = 1.hour.ago) reset_bounce_scores diff --git a/lib/migration/safe_migrate.rb b/lib/migration/safe_migrate.rb index def669ff037..7806a3044cb 100644 --- a/lib/migration/safe_migrate.rb +++ b/lib/migration/safe_migrate.rb @@ -141,7 +141,7 @@ class Migration::SafeMigrate or rename columns. Note, to minimize disruption use self.ignored_columns = ["column name"] on your - ActiveRecord model, this can be removed 6 months or so later. + ActiveRecord model, this can be removed after the post deployment migration has been promoted to a regular migration. This protection is in place to protect us against dropping columns that are currently in use by live applications.