DEV: Move array type custom fields to JSON type in automation #26939
Conversation
OsamaSayegh
changed the title
| if once_per_user && | ||
| UserCustomField.exists?(name: DiscourseAutomation::CUSTOM_FIELD, user_id: user.id) |
There was a problem hiding this comment.
Here's one bug that will be fixed in this PR. This line should also check that the custom field's value matches the triggered automation's ID. Without doing this, it means that the once_per_user option in an automation rule will affect other automation rules for a given user.
| end | ||
| end | ||
|
|
||
| if topic_id | ||
| topic = Topic.find_by(id: topic_id) | ||
| topic&.upsert_custom_fields(DiscourseAutomation::CUSTOM_FIELD => automation.id) |
There was a problem hiding this comment.
Here's another bug that will fixed in this PR. The usage of Topic#upsert_custom_fields on a custom field of type array is incorrect because the method finds an existing record and updates its value which means the custom field will always be a single-value array. The right thing here would've been to use Automion#attach_custom_field which creates a new record for every new value in the array, but with the migration to JSON type, the right thing now is to maintain a single record that holds multiple values.
plugins/automation/app/models/discourse_automation/automation.rb
Outdated
Show resolved
Hide resolved
…rse#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 discourse#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 PR migrates all four custom fields to JSON and fixes a couple of bugs that are related to the 4 custom fields. I will leave details of the bugs in comments to the relevant lines.