FIX: supports groups field in post_created_edited (#28773)

To achieve this this commit does the following:
- create a new `groups field, ideally we would have reused the existing group field, but many automations now have the expectation that this field will return a group id and not an array of group ids, which makes it a dangerous change
- alter the code in `post_created_edited` to use this new groups field and change the logic to use an array
- migrate the existing group fields post_created_edited automations to change name from `restricted_group` to `restricted_groups`, the component from `group` to `groups` and the metadata from `{"value": integer}` to `{"value": [integer]}`
This commit is contained in:
Joffrey JAFFEUX
2024-09-06 15:04:19 +02:00
committed by GitHub
parent d642020b0f
commit eb3a1c7217
9 changed files with 201 additions and 22 deletions

View File

@ -10,6 +10,7 @@ import DaCustomFields from "./fields/da-custom-fields";
import DaDateTimeField from "./fields/da-date-time-field"; import DaDateTimeField from "./fields/da-date-time-field";
import DaEmailGroupUserField from "./fields/da-email-group-user-field"; import DaEmailGroupUserField from "./fields/da-email-group-user-field";
import DaGroupField from "./fields/da-group-field"; import DaGroupField from "./fields/da-group-field";
import DaGroupsField from "./fields/da-groups-field";
import DaKeyValueField from "./fields/da-key-value-field"; import DaKeyValueField from "./fields/da-key-value-field";
import DaMessageField from "./fields/da-message-field"; import DaMessageField from "./fields/da-message-field";
import DaPeriodField from "./fields/da-period-field"; import DaPeriodField from "./fields/da-period-field";
@ -41,6 +42,7 @@ const FIELD_COMPONENTS = {
"trust-levels": DaTrustLevelsField, "trust-levels": DaTrustLevelsField,
category: DaCategoryField, category: DaCategoryField,
group: DaGroupField, group: DaGroupField,
groups: DaGroupsField,
choices: DaChoicesField, choices: DaChoicesField,
category_notification_level: DaCategoryNotificationlevelField, category_notification_level: DaCategoryNotificationlevelField,
email_group_user: DaEmailGroupUserField, email_group_user: DaEmailGroupUserField,

View File

@ -0,0 +1,55 @@
import { tracked } from "@glimmer/tracking";
import { hash } from "@ember/helper";
import { action } from "@ember/object";
import Group from "discourse/models/group";
import GroupChooser from "select-kit/components/group-chooser";
import BaseField from "./da-base-field";
import DAFieldDescription from "./da-field-description";
import DAFieldLabel from "./da-field-label";
export default class GroupsField extends BaseField {
@tracked allGroups = [];
constructor() {
super(...arguments);
Group.findAll({
ignore_automatic: this.args.field.extra.ignore_automatic ?? false,
}).then((groups) => {
if (this.isDestroying || this.isDestroyed) {
return;
}
this.allGroups = groups;
});
}
get maximum() {
return this.args.field.extra.maximum ?? 10;
}
@action
setGroupField(groupIds) {
this.mutValue(groupIds);
}
<template>
<section class="field group-field">
<div class="control-group">
<DAFieldLabel @label={{@label}} @field={{@field}} />
<div class="controls">
<GroupChooser
@content={{this.allGroups}}
@value={{@field.metadata.value}}
@labelProperty="name"
@onChange={{this.setGroupField}}
@options={{hash maximum=this.maximum disabled=@field.isDisabled}}
/>
<DAFieldDescription @description={{@description}} />
</div>
</div>
</section>
</template>
}

View File

@ -188,6 +188,12 @@ module DiscourseAutomation
"type" => "integer", "type" => "integer",
}, },
}, },
"groups" => {
"value" => {
"type" => "array",
"items" => [{ type: "integer" }],
},
},
"email_group_user" => { "email_group_user" => {
"value" => { "value" => {
"type" => "array", "type" => "array",

View File

@ -156,6 +156,9 @@ en:
restricted_group: restricted_group:
label: Group label: Group
description: Optional, will trigger only if the post's topic is a private message in this group's inbox description: Optional, will trigger only if the post's topic is a private message in this group's inbox
restricted_groups:
label: Groups
description: Optional, will trigger only if the post's topic is a private message in one of the group inboxes
ignore_group_members: ignore_group_members:
label: Ignore group members label: Ignore group members
description: Skip the trigger if sender is a member of the Group specified above description: Skip the trigger if sender is a member of the Group specified above

View File

@ -0,0 +1,22 @@
# frozen_string_literal: true
class UpdateFieldsForPostCreatedEditedTrigger < ActiveRecord::Migration[7.1]
def up
execute <<-SQL
UPDATE discourse_automation_fields
SET
component = 'groups',
name = 'restricted_groups',
metadata = jsonb_set(metadata, '{value}', to_jsonb(ARRAY[(metadata->>'value')::int]))
FROM discourse_automation_automations
WHERE discourse_automation_fields.automation_id = discourse_automation_automations.id
AND discourse_automation_automations.trigger = 'post_created_edited'
AND discourse_automation_fields.name = 'restricted_group'
AND discourse_automation_fields.component = 'group';
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -54,15 +54,15 @@ module DiscourseAutomation
next if (restricted_tags["value"] & topic.tags.map(&:name)).empty? next if (restricted_tags["value"] & topic.tags.map(&:name)).empty?
end end
restricted_group_id = automation.trigger_field("restricted_group")["value"] restricted_group_ids = automation.trigger_field("restricted_groups")["value"]
if restricted_group_id.present? if restricted_group_ids.present?
next if !topic.private_message? next if !topic.private_message?
target_group_ids = topic.allowed_groups.pluck(:id) target_group_ids = topic.allowed_groups.pluck(:id)
next if restricted_group_id != target_group_ids.first next if (restricted_group_ids & target_group_ids).empty?
ignore_group_members = automation.trigger_field("ignore_group_members") ignore_group_members = automation.trigger_field("ignore_group_members")["value"]
next if ignore_group_members["value"] && post.user.in_any_groups?([restricted_group_id]) next if ignore_group_members && post.user.in_any_groups?(restricted_group_ids)
end end
ignore_automated = automation.trigger_field("ignore_automated") ignore_automated = automation.trigger_field("ignore_automated")

View File

@ -14,7 +14,7 @@ DiscourseAutomation::Triggerable.add(DiscourseAutomation::Triggers::POST_CREATED
} }
field :restricted_category, component: :category field :restricted_category, component: :category
field :restricted_tags, component: :tags field :restricted_tags, component: :tags
field :restricted_group, component: :group field :restricted_groups, component: :groups
field :ignore_automated, component: :boolean field :ignore_automated, component: :boolean
field :ignore_group_members, component: :boolean field :ignore_group_members, component: :boolean
field :valid_trust_levels, component: :"trust-levels" field :valid_trust_levels, component: :"trust-levels"

View File

@ -164,30 +164,50 @@ describe "PostCreatedEdited" do
context "when group is restricted" do context "when group is restricted" do
fab!(:target_group) { Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]) } fab!(:target_group) { Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]) }
fab!(:another_group) { Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]) }
before do before do
automation.upsert_field!( automation.upsert_field!(
"restricted_group", "restricted_groups",
"group", "groups",
{ value: target_group.id }, { value: [target_group.id, another_group.id] },
target: "trigger", target: "trigger",
) )
end end
it "fires the trigger" do context "when PM is not sent to the group" do
list = it "doesnt fire the trigger" do
capture_contexts do list =
PostCreator.create( capture_contexts do
user, PostCreator.create(
basic_topic_params.merge( user,
target_group_names: [target_group.name], basic_topic_params.merge(
archetype: Archetype.private_message, target_group_names: [Fabricate(:group).name],
), archetype: Archetype.private_message,
) ),
end )
end
expect(list.length).to eq(1) expect(list.length).to eq(0)
expect(list[0]["kind"]).to eq("post_created_edited") end
end
context "when PM is sent to the group" do
it "fires the trigger" do
list =
capture_contexts do
PostCreator.create(
user,
basic_topic_params.merge(
target_group_names: [target_group.name],
archetype: Archetype.private_message,
),
)
end
expect(list.length).to eq(1)
expect(list[0]["kind"]).to eq("post_created_edited")
end
end end
context "when the topic is not a PM" do context "when the topic is not a PM" do

View File

@ -0,0 +1,71 @@
import { getOwner } from "@ember/owner";
import { render } from "@ember/test-helpers";
import { hbs } from "ember-cli-htmlbars";
import { module, test } from "qunit";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import pretender, { response } from "discourse/tests/helpers/create-pretender";
import selectKit from "discourse/tests/helpers/select-kit-helper";
import AutomationFabricators from "discourse/plugins/automation/admin/lib/fabricators";
module("Integration | Component | da-groups-field", function (hooks) {
setupRenderingTest(hooks);
hooks.beforeEach(function () {
this.automation = new AutomationFabricators(getOwner(this)).automation();
pretender.get("/groups/search.json", () => {
return response([
{
id: 1,
name: "cats",
flair_url: "fa-bars",
flair_bg_color: "CC000A",
flair_color: "FFFFFA",
},
{
id: 2,
name: "dogs",
flair_url: "fa-bars",
flair_bg_color: "CC000A",
flair_color: "FFFFFA",
},
]);
});
});
test("set value", async function (assert) {
this.field = new AutomationFabricators(getOwner(this)).field({
component: "groups",
});
await render(
hbs`<AutomationField @automation={{this.automation}} @field={{this.field}} />`
);
await selectKit().expand();
await selectKit().selectRowByValue(1);
assert.deepEqual(this.field.metadata.value, [1]);
});
test("supports a maxmimum value", async function (assert) {
this.field = new AutomationFabricators(getOwner(this)).field({
component: "groups",
extra: { maximum: 1 },
});
await render(
hbs`<AutomationField @automation={{this.automation}} @field={{this.field}} />`
);
await selectKit().expand();
await selectKit().selectRowByValue(1);
assert.deepEqual(this.field.metadata.value, [1]);
await selectKit().expand();
await selectKit().selectRowByValue(2);
assert.deepEqual(this.field.metadata.value, [2]);
});
});