From 3be4924b9972f2de08c20394a2e78e0d6dd45c9d Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 10 May 2024 18:47:12 +0300 Subject: [PATCH] DEV: Move array type custom fields to JSON type in automation (#26939) The automation plugin has 4 custom field types that are array typed. However, array typed custom fields are deprecated and should be migrated to JSON type. This commit does a couple of things: 1. Migrate all four custom fields to JSON 2. Fix a couple of small bugs that have been discovered while migrating the custom fields to JSON (see the comments on this commit's PR for details https://github.com/discourse/discourse/pull/26939) --- app/models/post_custom_field.rb | 13 +-- app/models/topic_custom_field.rb | 7 +- app/models/user_custom_field.rb | 3 +- .../models/discourse_automation/automation.rb | 54 +++++---- ..._topic_automation_custom_fields_to_json.rb | 32 +++++ ...h_user_automation_custom_fields_to_json.rb | 32 +++++ ...ion_triggered_ids_custom_fields_to_json.rb | 32 +++++ ...rop_automation_ids_custom_field_indexes.rb | 15 +++ .../discourse_automation/event_handlers.rb | 7 +- .../discourse_automation/post_extension.rb | 4 +- .../scripts/auto_responder.rb | 4 +- .../discourse_automation/triggers/topic.rb | 14 ++- plugins/automation/plugin.rb | 12 +- .../spec/integration/core_ext_spec.rb | 110 +++++++++++------- .../automation/spec/models/automation_spec.rb | 18 ++- .../automation/spec/triggers/topic_spec.rb | 8 +- .../spec/triggers/user_updated_spec.rb | 2 +- 17 files changed, 264 insertions(+), 103 deletions(-) create mode 100644 plugins/automation/db/post_migrate/20240507112651_switch_topic_automation_custom_fields_to_json.rb create mode 100644 plugins/automation/db/post_migrate/20240507112751_switch_user_automation_custom_fields_to_json.rb create mode 100644 plugins/automation/db/post_migrate/20240507112851_switch_topic_automation_triggered_ids_custom_fields_to_json.rb create mode 100644 plugins/automation/db/post_migrate/20240507112951_drop_automation_ids_custom_field_indexes.rb diff --git a/app/models/post_custom_field.rb b/app/models/post_custom_field.rb index 346711fcd96..750dc2fe9c5 100644 --- a/app/models/post_custom_field.rb +++ b/app/models/post_custom_field.rb @@ -19,11 +19,10 @@ end # # Indexes # -# idx_post_custom_fields_discourse_automation_unique_id_partial (post_id,value) UNIQUE WHERE ((name)::text = 'discourse_automation_ids'::text) -# index_post_custom_fields_on_name_and_value (name, "left"(value, 200)) -# index_post_custom_fields_on_notice (post_id) UNIQUE WHERE ((name)::text = 'notice'::text) -# index_post_custom_fields_on_post_id (post_id) UNIQUE WHERE ((name)::text = 'missing uploads'::text) -# index_post_custom_fields_on_post_id_and_name (post_id,name) -# index_post_custom_fields_on_stalled_wiki_triggered_at (post_id) UNIQUE WHERE ((name)::text = 'stalled_wiki_triggered_at'::text) -# index_post_id_where_missing_uploads_ignored (post_id) UNIQUE WHERE ((name)::text = 'missing uploads ignored'::text) +# index_post_custom_fields_on_name_and_value (name, "left"(value, 200)) +# index_post_custom_fields_on_notice (post_id) UNIQUE WHERE ((name)::text = 'notice'::text) +# index_post_custom_fields_on_post_id (post_id) UNIQUE WHERE ((name)::text = 'missing uploads'::text) +# index_post_custom_fields_on_post_id_and_name (post_id,name) +# index_post_custom_fields_on_stalled_wiki_triggered_at (post_id) UNIQUE WHERE ((name)::text = 'stalled_wiki_triggered_at'::text) +# index_post_id_where_missing_uploads_ignored (post_id) UNIQUE WHERE ((name)::text = 'missing uploads ignored'::text) # diff --git a/app/models/topic_custom_field.rb b/app/models/topic_custom_field.rb index 25afd75a1df..1c5ba784e95 100644 --- a/app/models/topic_custom_field.rb +++ b/app/models/topic_custom_field.rb @@ -19,8 +19,7 @@ end # # Indexes # -# idx_topic_custom_fields_auto_responder_triggered_ids_partial (topic_id,value) UNIQUE WHERE ((name)::text = 'auto_responder_triggered_ids'::text) -# idx_topic_custom_fields_discourse_automation_unique_id_partial (topic_id,value) UNIQUE WHERE ((name)::text = 'discourse_automation_ids'::text) -# index_topic_custom_fields_on_topic_id_and_name (topic_id,name) -# topic_custom_fields_value_key_idx (value,name) WHERE ((value IS NOT NULL) AND (char_length(value) < 400)) +# idx_topic_custom_fields_auto_responder_triggered_ids_partial (topic_id,value) UNIQUE WHERE ((name)::text = 'auto_responder_triggered_ids'::text) +# index_topic_custom_fields_on_topic_id_and_name (topic_id,name) +# topic_custom_fields_value_key_idx (value,name) WHERE ((value IS NOT NULL) AND (char_length(value) < 400)) # diff --git a/app/models/user_custom_field.rb b/app/models/user_custom_field.rb index 931093bf579..e4d9b4f9182 100644 --- a/app/models/user_custom_field.rb +++ b/app/models/user_custom_field.rb @@ -26,6 +26,5 @@ end # # Indexes # -# idx_user_custom_fields_discourse_automation_unique_id_partial (user_id,value) UNIQUE WHERE ((name)::text = 'discourse_automation_ids'::text) -# index_user_custom_fields_on_user_id_and_name (user_id,name) +# index_user_custom_fields_on_user_id_and_name (user_id,name) # diff --git a/plugins/automation/app/models/discourse_automation/automation.rb b/plugins/automation/app/models/discourse_automation/automation.rb index d091e79c9f6..a6c1bc6dca9 100644 --- a/plugins/automation/app/models/discourse_automation/automation.rb +++ b/plugins/automation/app/models/discourse_automation/automation.rb @@ -34,41 +34,38 @@ module DiscourseAutomation MAX_NAME_LENGTH = 30 validates :name, length: { in: MIN_NAME_LENGTH..MAX_NAME_LENGTH } - def attach_custom_field(target) + def add_id_to_custom_field(target, custom_field_key) if ![Topic, Post, User].any? { |m| target.is_a?(m) } raise "Expected an instance of Topic/Post/User." end - now = Time.now - fk = target.custom_fields_fk - row = { - fk => target.id, - :name => DiscourseAutomation::CUSTOM_FIELD, - :value => id, - :created_at => now, - :updated_at => now, - } - - relation = "#{target.class.name}CustomField".constantize - relation.upsert( - row, - unique_by: - "idx_#{target.class.name.downcase}_custom_fields_discourse_automation_unique_id_partial", - ) + change_automation_ids_custom_field_in_mutex(target, custom_field_key) do + target.reload + ids = Array(target.custom_fields[custom_field_key]) + if !ids.include?(self.id) + ids << self.id + ids = ids.compact.uniq + target.custom_fields[custom_field_key] = ids + target.save_custom_fields + end + end end - def detach_custom_field(target) + def remove_id_from_custom_field(target, custom_field_key) if ![Topic, Post, User].any? { |m| target.is_a?(m) } raise "Expected an instance of Topic/Post/User." end - fk = target.custom_fields_fk - relation = "#{target.class.name}CustomField".constantize - relation.where( - fk => target.id, - :name => DiscourseAutomation::CUSTOM_FIELD, - :value => id, - ).delete_all + change_automation_ids_custom_field_in_mutex(target, custom_field_key) do + target.reload + ids = Array(target.custom_fields[custom_field_key]) + if ids.include?(self.id) + ids = ids.compact.uniq + ids.delete(self.id) + target.custom_fields[custom_field_key] = ids + target.save_custom_fields + end + end end def trigger_field(name) @@ -187,5 +184,12 @@ module DiscourseAutomation def validate_trigger_fields !triggerable || triggerable.valid?(self) end + + def change_automation_ids_custom_field_in_mutex(target, key) + DistributedMutex.synchronize( + "automation_custom_field_#{key}_#{target.class.table_name}_#{target.id}", + validity: 5.seconds, + ) { yield } + end end end diff --git a/plugins/automation/db/post_migrate/20240507112651_switch_topic_automation_custom_fields_to_json.rb b/plugins/automation/db/post_migrate/20240507112651_switch_topic_automation_custom_fields_to_json.rb new file mode 100644 index 00000000000..46f755a1531 --- /dev/null +++ b/plugins/automation/db/post_migrate/20240507112651_switch_topic_automation_custom_fields_to_json.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +class SwitchTopicAutomationCustomFieldsToJson < ActiveRecord::Migration[7.0] + def up + results = DB.query(<<~SQL) + SELECT topic_id, ARRAY_AGG(value) AS values + FROM topic_custom_fields + WHERE name = 'discourse_automation_ids' + GROUP BY topic_id + SQL + + execute(<<~SQL) + DELETE FROM topic_custom_fields + WHERE name = 'discourse_automation_ids' + SQL + + results.each do |row| + parsed = row.values.map(&:to_i).uniq + + DB.exec(<<~SQL, topic_id: row.topic_id, value: parsed.to_json) + INSERT INTO topic_custom_fields + (topic_id, name, value, created_at, updated_at) + VALUES + (:topic_id, 'discourse_automation_ids_json', :value, NOW(), NOW()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/automation/db/post_migrate/20240507112751_switch_user_automation_custom_fields_to_json.rb b/plugins/automation/db/post_migrate/20240507112751_switch_user_automation_custom_fields_to_json.rb new file mode 100644 index 00000000000..3a98a2c0b4f --- /dev/null +++ b/plugins/automation/db/post_migrate/20240507112751_switch_user_automation_custom_fields_to_json.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +class SwitchUserAutomationCustomFieldsToJson < ActiveRecord::Migration[7.0] + def up + results = DB.query(<<~SQL) + SELECT user_id, ARRAY_AGG(value) AS values + FROM user_custom_fields + WHERE name = 'discourse_automation_ids' + GROUP BY user_id + SQL + + execute(<<~SQL) + DELETE FROM user_custom_fields + WHERE name = 'discourse_automation_ids' + SQL + + results.each do |row| + parsed = row.values.map(&:to_i).uniq + + DB.exec(<<~SQL, user_id: row.user_id, value: parsed.to_json) + INSERT INTO user_custom_fields + (user_id, name, value, created_at, updated_at) + VALUES + (:user_id, 'discourse_automation_ids_json', :value, NOW(), NOW()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/automation/db/post_migrate/20240507112851_switch_topic_automation_triggered_ids_custom_fields_to_json.rb b/plugins/automation/db/post_migrate/20240507112851_switch_topic_automation_triggered_ids_custom_fields_to_json.rb new file mode 100644 index 00000000000..61fb62a0144 --- /dev/null +++ b/plugins/automation/db/post_migrate/20240507112851_switch_topic_automation_triggered_ids_custom_fields_to_json.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +class SwitchTopicAutomationTriggeredIdsCustomFieldsToJson < ActiveRecord::Migration[7.0] + def up + results = DB.query(<<~SQL) + SELECT topic_id, ARRAY_AGG(value) AS values + FROM topic_custom_fields + WHERE name = 'auto_responder_triggered_ids' + GROUP BY topic_id + SQL + + execute(<<~SQL) + DELETE FROM topic_custom_fields + WHERE name = 'auto_responder_triggered_ids' + SQL + + results.each do |row| + parsed = row.values.map(&:to_i).uniq + + DB.exec(<<~SQL, topic_id: row.topic_id, value: parsed.to_json) + INSERT INTO topic_custom_fields + (topic_id, name, value, created_at, updated_at) + VALUES + (:topic_id, 'auto_responder_triggered_ids_json', :value, NOW(), NOW()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/automation/db/post_migrate/20240507112951_drop_automation_ids_custom_field_indexes.rb b/plugins/automation/db/post_migrate/20240507112951_drop_automation_ids_custom_field_indexes.rb new file mode 100644 index 00000000000..c6e7bdbc459 --- /dev/null +++ b/plugins/automation/db/post_migrate/20240507112951_drop_automation_ids_custom_field_indexes.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class DropAutomationIdsCustomFieldIndexes < ActiveRecord::Migration[7.0] + def change + remove_index :topic_custom_fields, + name: :idx_topic_custom_fields_discourse_automation_unique_id_partial, + if_exists: true + remove_index :user_custom_fields, + name: :idx_user_custom_fields_discourse_automation_unique_id_partial, + if_exists: true + remove_index :post_custom_fields, + name: :idx_post_custom_fields_discourse_automation_unique_id_partial, + if_exists: true + end +end diff --git a/plugins/automation/lib/discourse_automation/event_handlers.rb b/plugins/automation/lib/discourse_automation/event_handlers.rb index 81d6144fe08..ba7f13cc4cf 100644 --- a/plugins/automation/lib/discourse_automation/event_handlers.rb +++ b/plugins/automation/lib/discourse_automation/event_handlers.rb @@ -75,7 +75,9 @@ module DiscourseAutomation .find_each do |automation| once_per_user = automation.trigger_field("once_per_user")["value"] if once_per_user && - UserCustomField.exists?(name: DiscourseAutomation::CUSTOM_FIELD, user_id: user.id) + user.custom_fields[ + DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD + ].presence&.include?(automation.id) next end @@ -135,7 +137,8 @@ module DiscourseAutomation user.save_custom_fields end - automation.attach_custom_field(user) + automation.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation.trigger!("kind" => name, "user" => user, "user_data" => user_data) end end diff --git a/plugins/automation/lib/discourse_automation/post_extension.rb b/plugins/automation/lib/discourse_automation/post_extension.rb index 1e15c2baaf9..e48a2ddd9a7 100644 --- a/plugins/automation/lib/discourse_automation/post_extension.rb +++ b/plugins/automation/lib/discourse_automation/post_extension.rb @@ -10,9 +10,9 @@ module DiscourseAutomation return if !SiteSetting.discourse_automation_enabled return if self.post_type == Post.types[:small_action] return if !topic - return if topic.custom_fields[DiscourseAutomation::CUSTOM_FIELD].blank? + return if topic.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD].blank? - topic.custom_fields[DiscourseAutomation::CUSTOM_FIELD].each do |automation_id| + topic.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD].each do |automation_id| automation = DiscourseAutomation::Automation.find_by(id: automation_id) next if automation&.script != DiscourseAutomation::Scripts::TOPIC_REQUIRED_WORDS diff --git a/plugins/automation/lib/discourse_automation/scripts/auto_responder.rb b/plugins/automation/lib/discourse_automation/scripts/auto_responder.rb index 4a3a910cea6..538f682cffa 100644 --- a/plugins/automation/lib/discourse_automation/scripts/auto_responder.rb +++ b/plugins/automation/lib/discourse_automation/scripts/auto_responder.rb @@ -70,9 +70,7 @@ DiscourseAutomation::Scriptable.add(DiscourseAutomation::Scripts::AUTO_RESPONDER end .join("\n\n") - value = (Array(post.topic.custom_fields[key]) << automation.id).compact.uniq - post.topic.custom_fields[key] = value - post.topic.save_custom_fields + automation.add_id_to_custom_field(post.topic, key) PostCreator.create!( answering_user, diff --git a/plugins/automation/lib/discourse_automation/triggers/topic.rb b/plugins/automation/lib/discourse_automation/triggers/topic.rb index d1b5ad69cdb..a8d8b3deeb8 100644 --- a/plugins/automation/lib/discourse_automation/triggers/topic.rb +++ b/plugins/automation/lib/discourse_automation/triggers/topic.rb @@ -12,17 +12,19 @@ DiscourseAutomation::Triggerable.add(DiscourseAutomation::Triggers::TOPIC) do previous_topic = Topic.find_by(id: previous_topic_id) if previous_topic - TopicCustomField.where( - topic_id: previous_topic_id, - name: DiscourseAutomation::CUSTOM_FIELD, - value: automation.id, - ).delete_all + automation.remove_id_from_custom_field( + previous_topic, + DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ) end end if topic_id topic = Topic.find_by(id: topic_id) - topic&.upsert_custom_fields(DiscourseAutomation::CUSTOM_FIELD => automation.id) + + next if !topic + + automation.add_id_to_custom_field(topic, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) end end end diff --git a/plugins/automation/plugin.rb b/plugins/automation/plugin.rb index c50a17f578c..9bd14646786 100644 --- a/plugins/automation/plugin.rb +++ b/plugins/automation/plugin.rb @@ -14,7 +14,7 @@ register_asset "stylesheets/common/discourse-automation.scss" module ::DiscourseAutomation PLUGIN_NAME = "automation" - CUSTOM_FIELD = "discourse_automation_ids" + AUTOMATION_IDS_CUSTOM_FIELD = "discourse_automation_ids_json" TOPIC_LAST_CHECKED_BY = "discourse_automation_last_checked_by" TOPIC_LAST_CHECKED_AT = "discourse_automation_last_checked_at" @@ -26,7 +26,7 @@ module ::DiscourseAutomation { id: "TL34", name: "discourse_automation.triggerables.user_promoted.trust_levels.TL34" }, ] - AUTO_RESPONDER_TRIGGERED_IDS = "auto_responder_triggered_ids" + AUTO_RESPONDER_TRIGGERED_IDS = "auto_responder_triggered_ids_json" USER_GROUP_MEMBERSHIP_THROUGH_BADGE_BULK_MODIFY_START_COUNT = 1000 def self.set_active_automation(id) @@ -203,15 +203,15 @@ after_initialize do on(:post_created) { |post| DiscourseAutomation::EventHandlers.handle_stalled_topic(post) } - register_topic_custom_field_type(DiscourseAutomation::CUSTOM_FIELD, [:integer]) - register_topic_custom_field_type(DiscourseAutomation::AUTO_RESPONDER_TRIGGERED_IDS, [:integer]) + register_topic_custom_field_type(DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, :json) + register_topic_custom_field_type(DiscourseAutomation::AUTO_RESPONDER_TRIGGERED_IDS, :json) on(:user_updated) { |user| DiscourseAutomation::EventHandlers.handle_user_updated(user) } on(:user_created) do |user| DiscourseAutomation::EventHandlers.handle_user_updated(user, new_user: true) end - register_user_custom_field_type(DiscourseAutomation::CUSTOM_FIELD, [:integer]) - register_post_custom_field_type(DiscourseAutomation::CUSTOM_FIELD, [:integer]) + register_user_custom_field_type(DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, :json) + register_post_custom_field_type(DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, :json) register_post_custom_field_type("stalled_wiki_triggered_at", :string) end diff --git a/plugins/automation/spec/integration/core_ext_spec.rb b/plugins/automation/spec/integration/core_ext_spec.rb index 4a2c115bb37..6a5c9ab7b4d 100644 --- a/plugins/automation/spec/integration/core_ext_spec.rb +++ b/plugins/automation/spec/integration/core_ext_spec.rb @@ -38,90 +38,122 @@ describe "Core extensions" do describe "post custom fields" do it "supports discourse_automation_ids" do post = create_post - automation_1.attach_custom_field(post) + automation_1.add_id_to_custom_field(post, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(post.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq([automation_1.id]) + expect(post.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + [automation_1.id], + ) - automation_2.attach_custom_field(post) + automation_2.add_id_to_custom_field(post, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(post.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq( + expect(post.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( [automation_1.id, automation_2.id], ) - PostCustomField.where(post_id: post.id, name: DiscourseAutomation::CUSTOM_FIELD).delete_all + PostCustomField.where( + post_id: post.id, + name: DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ).delete_all - expect(post.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to be(nil) + expect(post.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to be(nil) - automation_1.attach_custom_field(post) - automation_1.attach_custom_field(post) - automation_1.attach_custom_field(post) - automation_1.attach_custom_field(post) + automation_1.add_id_to_custom_field(post, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(post, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(post, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(post, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(post.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq([automation_1.id]) + expect(post.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + [automation_1.id], + ) - automation_1.detach_custom_field(post) + automation_1.remove_id_from_custom_field( + post, + DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ) - expect(post.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to be(nil) + expect(post.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq([]) end end describe "topic custom fields" do it "supports discourse_automation_ids" do topic = create_topic - automation_1.attach_custom_field(topic) + automation_1.add_id_to_custom_field(topic, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(topic.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq([automation_1.id]) + expect(topic.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + [automation_1.id], + ) - automation_2.attach_custom_field(topic) + automation_2.add_id_to_custom_field(topic, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(topic.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq( + expect(topic.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( [automation_1.id, automation_2.id], ) - TopicCustomField.where(topic_id: topic.id, name: DiscourseAutomation::CUSTOM_FIELD).delete_all + TopicCustomField.where( + topic_id: topic.id, + name: DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ).delete_all - expect(topic.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to be(nil) + expect(topic.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + nil, + ) - automation_1.attach_custom_field(topic) - automation_1.attach_custom_field(topic) - automation_1.attach_custom_field(topic) - automation_1.attach_custom_field(topic) + automation_1.add_id_to_custom_field(topic, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(topic, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(topic, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(topic, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(topic.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq([automation_1.id]) + expect(topic.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + [automation_1.id], + ) - automation_1.detach_custom_field(topic) + automation_1.remove_id_from_custom_field( + topic, + DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ) - expect(topic.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to be(nil) + expect(topic.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq([]) end end describe "user custom fields" do it "supports discourse_automation_ids" do user = create_user - automation_1.attach_custom_field(user) + automation_1.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(user.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq([automation_1.id]) + expect(user.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + [automation_1.id], + ) - automation_2.attach_custom_field(user) + automation_2.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(user.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq( + expect(user.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( [automation_1.id, automation_2.id], ) - UserCustomField.where(user_id: user.id, name: DiscourseAutomation::CUSTOM_FIELD).delete_all + UserCustomField.where( + user_id: user.id, + name: DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ).delete_all - expect(user.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to be(nil) + expect(user.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq(nil) - automation_1.attach_custom_field(user) - automation_1.attach_custom_field(user) - automation_1.attach_custom_field(user) - automation_1.attach_custom_field(user) + automation_1.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) + automation_1.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD) - expect(user.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to eq([automation_1.id]) + expect(user.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + [automation_1.id], + ) - automation_1.detach_custom_field(user) + automation_1.remove_id_from_custom_field( + user, + DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ) - expect(user.reload.custom_fields[DiscourseAutomation::CUSTOM_FIELD]).to be(nil) + expect(user.reload.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq([]) end end end diff --git a/plugins/automation/spec/models/automation_spec.rb b/plugins/automation/spec/models/automation_spec.rb index d006e857f81..f404dbb094e 100644 --- a/plugins/automation/spec/models/automation_spec.rb +++ b/plugins/automation/spec/models/automation_spec.rb @@ -46,19 +46,29 @@ describe DiscourseAutomation::Automation do end end - describe "#detach_custom_field" do + describe "#remove_id_from_custom_field" do fab!(:automation) it "expects a User/Topic/Post instance" do - expect { automation.detach_custom_field(Invite.new) }.to raise_error(RuntimeError) + expect { + automation.remove_id_from_custom_field( + Invite.new, + DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ) + }.to raise_error(RuntimeError) end end - describe "#attach_custom_field" do + describe "#add_id_to_custom_field" do fab!(:automation) it "expects a User/Topic/Post instance" do - expect { automation.attach_custom_field(Invite.new) }.to raise_error(RuntimeError) + expect { + automation.add_id_to_custom_field( + Invite.new, + DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, + ) + }.to raise_error(RuntimeError) end end diff --git a/plugins/automation/spec/triggers/topic_spec.rb b/plugins/automation/spec/triggers/topic_spec.rb index abf9f4ba515..d808915abf1 100644 --- a/plugins/automation/spec/triggers/topic_spec.rb +++ b/plugins/automation/spec/triggers/topic_spec.rb @@ -14,7 +14,9 @@ describe "TopicRequiredWords" do context "when updating trigger" do it "updates the custom field" do automation.upsert_field!("restricted_topic", "text", { value: topic.id }, target: "trigger") - expect(topic.custom_fields["discourse_automation_ids"]).to eq([automation.id]) + expect(topic.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + [automation.id], + ) new_topic = create_topic automation.upsert_field!( @@ -23,7 +25,9 @@ describe "TopicRequiredWords" do { value: new_topic.id }, target: "trigger", ) - expect(new_topic.custom_fields["discourse_automation_ids"]).to eq([automation.id]) + expect(new_topic.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD]).to eq( + [automation.id], + ) end end end diff --git a/plugins/automation/spec/triggers/user_updated_spec.rb b/plugins/automation/spec/triggers/user_updated_spec.rb index 82d09fd67e5..16b0b455126 100644 --- a/plugins/automation/spec/triggers/user_updated_spec.rb +++ b/plugins/automation/spec/triggers/user_updated_spec.rb @@ -70,7 +70,7 @@ describe "UserUpdated" do end it "doesnt trigger if automation already triggered" do - automation.attach_custom_field(user) + UserUpdater.new(user, user).update(location: "Korea", bio_raw: "good") output = capture_contexts { UserUpdater.new(user, user).update(location: "Japan", bio_raw: "fine") }