From e0e321b8e2df9e5922d7a4f4b75a1154946ea553 Mon Sep 17 00:00:00 2001 From: Mazen Khalil Date: Thu, 26 Mar 2026 03:51:06 +0300 Subject: [PATCH] fix: Annotaterb model annotation incomplete migration (#13132) This pull request fixes the model annotation tooling due to previous incomplete migration from `annotate` to `annotaterb` gem (#12600). It also improves the handling of serialized values in the `InstallationConfig` model by ensuring a default value is set, simplifying the code, and removing a workaround for YAML deserialization. **Annotation tooling updates:** * Added `.annotaterb.yml` to configure the `annotate_rb` gem with project-specific options, centralizing annotation settings. * Replaced the custom `auto_annotate_models.rake` task with the standard rake task from `annotate_rb`, and added `lib/tasks/annotate_rb.rake` to load annotation tasks in development environments. [[1]](diffhunk://#diff-9450d2359e45f1db407b3871dde787a25d60bb721aed179a65ffd2692e95fb4bL1-L61) [[2]](diffhunk://#diff-578cdfc7ad56637e42472ea891ea286dff8803d9a1750afdbfeafec164d9b8b2R1-R8) **Model serialization improvements:** * Updated the `InstallationConfig` model to set a default value for the `serialized_value` attribute, ensuring it always has a hash with indifferent access and removing the need for a deserialization workaround in the `value` method. [[1]](diffhunk://#diff-b4bdde42c1ad0f584073818bd43dbd865b1b3b50d4701b131979f900d7c68297L22-R22) [[2]](diffhunk://#diff-b4bdde42c1ad0f584073818bd43dbd865b1b3b50d4701b131979f900d7c68297L36-L39) --------- Co-authored-by: Sojan Jose --- .annotaterb.yml | 65 +++++++++++++++++++++++++ app/models/csat_survey_response.rb | 34 +++++++------ app/models/installation_config.rb | 6 +-- app/models/reporting_event.rb | 15 +++--- app/models/reporting_events_rollup.rb | 22 ++++----- app/models/webhook.rb | 1 + lib/tasks/annotate_rb.rake | 8 +++ lib/tasks/auto_annotate_models.rake | 61 ----------------------- spec/models/installation_config_spec.rb | 12 +++++ 9 files changed, 125 insertions(+), 99 deletions(-) create mode 100644 .annotaterb.yml create mode 100644 lib/tasks/annotate_rb.rake delete mode 100644 lib/tasks/auto_annotate_models.rake diff --git a/.annotaterb.yml b/.annotaterb.yml new file mode 100644 index 000000000..07162a22d --- /dev/null +++ b/.annotaterb.yml @@ -0,0 +1,65 @@ +--- +:position: before +:position_in_additional_file_patterns: before +:position_in_class: before +:position_in_factory: before +:position_in_fixture: before +:position_in_routes: before +:position_in_serializer: before +:position_in_test: before +:classified_sort: true +:exclude_controllers: true +:exclude_factories: true +:exclude_fixtures: true +:exclude_helpers: true +:exclude_scaffolds: true +:exclude_serializers: true +:exclude_sti_subclasses: false +:exclude_tests: true +:force: false +:format_markdown: false +:format_rdoc: false +:format_yard: false +:frozen: false +:grouped_polymorphic: false +:ignore_model_sub_dir: false +:ignore_unknown_models: false +:include_version: false +:show_check_constraints: false +:show_complete_foreign_keys: false +:show_foreign_keys: true +:show_indexes: true +:show_indexes_include: false +:simple_indexes: false +:sort: false +:timestamp: false +:trace: false +:with_comment: true +:with_column_comments: true +:with_table_comments: true +:position_of_column_comment: :with_name +:active_admin: false +:command: +:debug: false +:hide_default_column_types: json,jsonb,hstore +:hide_limit_column_types: integer,bigint,boolean +:timestamp_columns: +- created_at +- updated_at +:ignore_columns: +:ignore_routes: +:models: true +:routes: false +:skip_on_db_migrate: false +:target_action: :do_annotations +:wrapper: +:wrapper_close: +:wrapper_open: +:classes_default_to_s: [] +:additional_file_patterns: [] +:model_dir: +- app/models +- enterprise/app/models +:require: [] +:root_dir: +- '' diff --git a/app/models/csat_survey_response.rb b/app/models/csat_survey_response.rb index 804dfd4b7..212530493 100644 --- a/app/models/csat_survey_response.rb +++ b/app/models/csat_survey_response.rb @@ -2,24 +2,28 @@ # # Table name: csat_survey_responses # -# id :bigint not null, primary key -# feedback_message :text -# rating :integer not null -# created_at :datetime not null -# updated_at :datetime not null -# account_id :bigint not null -# assigned_agent_id :bigint -# contact_id :bigint not null -# conversation_id :bigint not null -# message_id :bigint not null +# id :bigint not null, primary key +# csat_review_notes :text +# feedback_message :text +# rating :integer not null +# review_notes_updated_at :datetime +# created_at :datetime not null +# updated_at :datetime not null +# account_id :bigint not null +# assigned_agent_id :bigint +# contact_id :bigint not null +# conversation_id :bigint not null +# message_id :bigint not null +# review_notes_updated_by_id :bigint # # Indexes # -# index_csat_survey_responses_on_account_id (account_id) -# index_csat_survey_responses_on_assigned_agent_id (assigned_agent_id) -# index_csat_survey_responses_on_contact_id (contact_id) -# index_csat_survey_responses_on_conversation_id (conversation_id) -# index_csat_survey_responses_on_message_id (message_id) UNIQUE +# index_csat_survey_responses_on_account_id (account_id) +# index_csat_survey_responses_on_assigned_agent_id (assigned_agent_id) +# index_csat_survey_responses_on_contact_id (contact_id) +# index_csat_survey_responses_on_conversation_id (conversation_id) +# index_csat_survey_responses_on_message_id (message_id) UNIQUE +# index_csat_survey_responses_on_review_notes_updated_by_id (review_notes_updated_by_id) # class CsatSurveyResponse < ApplicationRecord belongs_to :account diff --git a/app/models/installation_config.rb b/app/models/installation_config.rb index a7400460c..19252de50 100644 --- a/app/models/installation_config.rb +++ b/app/models/installation_config.rb @@ -19,7 +19,7 @@ class InstallationConfig < ApplicationRecord # https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017 # FIX ME : fixes breakage of installation config. we need to migrate. # Fix configuration in application.rb - serialize :serialized_value, coder: YAML, type: ActiveSupport::HashWithIndifferentAccess + serialize :serialized_value, coder: YAML, type: ActiveSupport::HashWithIndifferentAccess, default: {}.with_indifferent_access before_validation :set_lock validates :name, presence: true @@ -33,10 +33,6 @@ class InstallationConfig < ApplicationRecord after_commit :clear_cache def value - # This is an extra hack again cause of the YAML serialization, in case of new object initialization in super admin - # It was throwing error as the default value of column '{}' was failing in deserialization. - return {}.with_indifferent_access if new_record? && @attributes['serialized_value']&.value_before_type_cast == '{}' - serialized_value[:value] end diff --git a/app/models/reporting_event.rb b/app/models/reporting_event.rb index f083e32a1..6c2c10b01 100644 --- a/app/models/reporting_event.rb +++ b/app/models/reporting_event.rb @@ -17,13 +17,14 @@ # # Indexes # -# index_reporting_events_on_account_id (account_id) -# index_reporting_events_on_conversation_id (conversation_id) -# index_reporting_events_on_created_at (created_at) -# index_reporting_events_on_inbox_id (inbox_id) -# index_reporting_events_on_name (name) -# index_reporting_events_on_user_id (user_id) -# reporting_events__account_id__name__created_at (account_id,name,created_at) +# index_reporting_events_for_response_distribution (account_id,name,inbox_id,created_at) +# index_reporting_events_on_account_id (account_id) +# index_reporting_events_on_conversation_id (conversation_id) +# index_reporting_events_on_created_at (created_at) +# index_reporting_events_on_inbox_id (inbox_id) +# index_reporting_events_on_name (name) +# index_reporting_events_on_user_id (user_id) +# reporting_events__account_id__name__created_at (account_id,name,created_at) # class ReportingEvent < ApplicationRecord diff --git a/app/models/reporting_events_rollup.rb b/app/models/reporting_events_rollup.rb index a9e345cba..9ad1b5135 100644 --- a/app/models/reporting_events_rollup.rb +++ b/app/models/reporting_events_rollup.rb @@ -2,17 +2,17 @@ # # Table name: reporting_events_rollups # -# id :bigint not null, primary key -# count :bigint default(0), not null -# date :date not null -# dimension_id :bigint not null -# dimension_type :string not null -# metric :string not null -# sum_value :float default(0.0), not null -# sum_value_business_hours :float default(0.0), not null -# created_at :datetime not null -# updated_at :datetime not null -# account_id :integer not null +# id :bigint not null, primary key +# count :bigint default(0), not null +# date :date not null +# dimension_type :string not null +# metric :string not null +# sum_value :float default(0.0), not null +# sum_value_business_hours :float default(0.0), not null +# created_at :datetime not null +# updated_at :datetime not null +# account_id :integer not null +# dimension_id :bigint not null # # Indexes # diff --git a/app/models/webhook.rb b/app/models/webhook.rb index 6b36c4bbd..9586e1053 100644 --- a/app/models/webhook.rb +++ b/app/models/webhook.rb @@ -4,6 +4,7 @@ # # id :bigint not null, primary key # name :string +# secret :string # subscriptions :jsonb # url :text # webhook_type :integer default("account_type") diff --git a/lib/tasks/annotate_rb.rake b/lib/tasks/annotate_rb.rake new file mode 100644 index 000000000..2c54aa32e --- /dev/null +++ b/lib/tasks/annotate_rb.rake @@ -0,0 +1,8 @@ +# This rake task was added by annotate_rb gem. + +# Can set `ANNOTATERB_SKIP_ON_DB_TASKS` to be anything to skip this +if Rails.env.development? && ENV['ANNOTATERB_SKIP_ON_DB_TASKS'].nil? + require 'annotate_rb' + + AnnotateRb::Core.load_rake_tasks +end diff --git a/lib/tasks/auto_annotate_models.rake b/lib/tasks/auto_annotate_models.rake deleted file mode 100644 index 9dcd13126..000000000 --- a/lib/tasks/auto_annotate_models.rake +++ /dev/null @@ -1,61 +0,0 @@ -# NOTE: only doing this in development as some production environments (Heroku) -# NOTE: are sensitive to local FS writes, and besides -- it's just not proper -# NOTE: to have a dev-mode tool do its thing in production. -if Rails.env.development? - require 'annotate_rb' - - AnnotateRb::Core.load_rake_tasks - - task :set_annotation_options do - # You can override any of these by setting an environment variable of the - # same name. - AnnotateRb::Options.set_defaults( - 'additional_file_patterns' => [], - 'routes' => 'false', - 'models' => 'true', - 'position_in_routes' => 'before', - 'position_in_class' => 'before', - 'position_in_test' => 'before', - 'position_in_fixture' => 'before', - 'position_in_factory' => 'before', - 'position_in_serializer' => 'before', - 'show_foreign_keys' => 'true', - 'show_complete_foreign_keys' => 'false', - 'show_indexes' => 'true', - 'simple_indexes' => 'false', - 'model_dir' => [ - 'app/models', - 'enterprise/app/models', - ], - 'root_dir' => '', - 'include_version' => 'false', - 'require' => '', - 'exclude_tests' => 'true', - 'exclude_fixtures' => 'true', - 'exclude_factories' => 'true', - 'exclude_serializers' => 'true', - 'exclude_scaffolds' => 'true', - 'exclude_controllers' => 'true', - 'exclude_helpers' => 'true', - 'exclude_sti_subclasses' => 'false', - 'ignore_model_sub_dir' => 'false', - 'ignore_columns' => nil, - 'ignore_routes' => nil, - 'ignore_unknown_models' => 'false', - 'hide_limit_column_types' => 'integer,bigint,boolean', - 'hide_default_column_types' => 'json,jsonb,hstore', - 'skip_on_db_migrate' => 'false', - 'format_bare' => 'true', - 'format_rdoc' => 'false', - 'format_markdown' => 'false', - 'sort' => 'false', - 'force' => 'false', - 'frozen' => 'false', - 'classified_sort' => 'true', - 'trace' => 'false', - 'wrapper_open' => nil, - 'wrapper_close' => nil, - 'with_comment' => 'true' - ) - end -end diff --git a/spec/models/installation_config_spec.rb b/spec/models/installation_config_spec.rb index 49270f2a8..49b58f79e 100644 --- a/spec/models/installation_config_spec.rb +++ b/spec/models/installation_config_spec.rb @@ -3,5 +3,17 @@ require 'rails_helper' RSpec.describe InstallationConfig do + subject(:installation_config) { described_class.new(name: 'INSTALLATION_NAME') } + it { is_expected.to validate_presence_of(:name) } + + describe 'new record defaults' do + it 'initializes serialized_value with indifferent access' do + expect(installation_config.serialized_value).to eq({}.with_indifferent_access) + end + + it 'returns nil for value before assignment' do + expect(installation_config.value).to be_nil + end + end end