Discourse/discourse
Discourse/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-08-17 18:04:11 +08:00
Code Packages Activity Actions 4

FIX: Improve automation on/off toggle (#33482)

Browse source 
This commit replaces the "enabled" checkbox on the automation page with a
toggle at the top of the automation page. We've made this change to make
the automation edit page consistent with the automations list page where
a toggle is used for turning on/off automations, and also consistent
with the rest of admin pages where we've been using toggle for
controlling the enable/disable state.

Internal topic: t/153523.
This commit is contained in:
Osama Sayegh 2025-08-14 10:16:20 +03:00 • committed by GitHub
parent 5dfa0f3cb6
commit 91e33664ae
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 522 additions and 51 deletions
Download patch file Download diff file Expand all files Collapse all files

21
plugins/automation/admin/assets/javascripts/admin/components/automation-enabled-toggle.gjs Normal file
View file

@ -0,0 +1,21 @@
import { on } from "@ember/modifier";
import DToggleSwitch from "discourse/components/d-toggle-switch";
import { i18n } from "discourse-i18n";
import DTooltip from "float-kit/components/d-tooltip";
const AutomationEnabledToggle = <template>
{{#if @canBeEnabled}}
<DToggleSwitch @state={{@automation.enabled}} {{on "click" @onToggle}} />
{{else}}
<DTooltip @identifier="automation-enabled-toggle">
<:trigger>
<DToggleSwitch disabled={{true}} @state={{@automation.enabled}} />
</:trigger>
<:content>
{{i18n "discourse_automation.models.automation.enable_toggle_disabled"}}
</:content>
</DTooltip>
{{/if}}
</template>;
export default AutomationEnabledToggle;

17
plugins/automation/admin/assets/javascripts/admin/components/automation-list.gjs
View file

@ -4,15 +4,16 @@ import { on } from "@ember/modifier";
import { action } from "@ember/object";
import { LinkTo } from "@ember/routing";
import { service } from "@ember/service";
import { or } from "truth-helpers";
import DButton from "discourse/components/d-button";
import DPageSubheader from "discourse/components/d-page-subheader";
import DToggleSwitch from "discourse/components/d-toggle-switch";
import avatar from "discourse/helpers/avatar";
import formatDate from "discourse/helpers/format-date";
import { popupAjaxError } from "discourse/lib/ajax-error";
import { escapeExpression } from "discourse/lib/utilities";
import { i18n } from "discourse-i18n";
import AdminConfigAreaEmptyList from "admin/components/admin-config-area-empty-list";
import AutomationEnabledToggle from "discourse/plugins/automation/admin/components/automation-enabled-toggle";
// number of runs required to show the runs count for the period
const RUN_THRESHOLD = 10;
@ -201,10 +202,16 @@ export default class AutomationList extends Component {
"discourse_automation.models.automation.enabled.label"
}}
</div>
<DToggleSwitch
@state={{automation.enabled}}
{{on "click" (fn this.toggleEnabled automation)}}
/>
<span class="enabled-toggle-with-tooltip">
<AutomationEnabledToggle
@automation={{automation}}
@canBeEnabled={{or
automation.enabled
automation.canBeEnabled
}}
@onToggle={{fn this.toggleEnabled automation}}
/>
</span>
</td>
{{/if}}

56
plugins/automation/admin/assets/javascripts/admin/models/discourse-automation-automation.js
View file

@ -1,8 +1,12 @@
import { tracked } from "@glimmer/tracking";
import RestModel from "discourse/models/rest";
import Field from "discourse/plugins/automation/admin/models/discourse-automation-field";
const ATTRIBUTES = ["name", "script", "fields", "trigger", "id"];
export default class Automation extends RestModel {
@tracked enabled;
updateProperties() {
return {
id: this.id,
@ -20,4 +24,56 @@ export default class Automation extends RestModel {
createProperties() {
return this.getProperties(ATTRIBUTES);
}
get canBeEnabled() {
for (const target of ["script", "trigger"]) {
if (!this[target]) {
return false;
}
for (const template of this[target].templates) {
if (!template.is_required) {
continue;
}
const field = this[target].fields.find(
(f) => f.name === template.name && f.component === template.component
);
if (!field) {
return false;
}
const val = field.metadata?.value;
if (val === undefined || val === null || val.length === 0) {
return false;
}
}
}
return true;
}
triggerFields() {
return this.#fieldsForTarget("trigger");
}
scriptFields() {
return this.#fieldsForTarget("script");
}
#fieldsForTarget(target) {
return (this[target]?.templates || []).map((template) => {
const jsonField = this[target].fields.find(
(f) => f.name === template.name && f.component === template.component
);
return Field.create(
template,
{
name: this[target].id,
type: target,
},
jsonField
);
});
}
}

18
plugins/automation/admin/assets/javascripts/discourse/controllers/admin-plugins/show/automation/edit.js
View file

@ -4,7 +4,7 @@ import { filterBy, reads } from "@ember/object/computed";
import { schedule } from "@ember/runloop";
import { service } from "@ember/service";
import { ajax } from "discourse/lib/ajax";
import { extractError } from "discourse/lib/ajax-error";
import { extractError, popupAjaxError } from "discourse/lib/ajax-error";
import { i18n } from "discourse-i18n";
export default class AutomationEdit extends Controller {
@ -27,6 +27,22 @@ export default class AutomationEdit extends Controller {
}
}
get disableEnabledToggle() {
return !(this.automation.canBeEnabled || this.automation.enabled);
}
@action
async toggleEnabled() {
const automation = this.model.automation;
automation.enabled = !automation.enabled;
try {
await automation.save({ enabled: automation.enabled });
} catch (e) {
popupAjaxError(e);
automation.enabled = !automation.enabled;
}
}
@action
saveAutomation(routeToIndex = false) {
this.setProperties({ error: null, isUpdatingAutomation: true });

21
plugins/automation/admin/assets/javascripts/discourse/routes/admin-plugins-show-automation-edit.js
View file

@ -2,7 +2,6 @@ import { action } from "@ember/object";
import { hash } from "rsvp";
import { ajax } from "discourse/lib/ajax";
import DiscourseRoute from "discourse/routes/discourse";
import Field from "discourse/plugins/automation/admin/models/discourse-automation-field";
export default class AutomationEdit extends DiscourseRoute {
model(params) {
@ -17,22 +16,6 @@ export default class AutomationEdit extends DiscourseRoute {
});
}
_fieldsForTarget(automation, target) {
return (automation[target].templates || []).map((template) => {
const jsonField = automation[target].fields.find(
(f) => f.name === template.name && f.component === template.component
);
return Field.create(
template,
{
name: automation[target].id,
type: target,
},
jsonField
);
});
}
setupController(controller, model) {
const automation = model.automation;
controller.setProperties({
@ -43,9 +26,7 @@ export default class AutomationEdit extends DiscourseRoute {
enabled: automation.enabled,
trigger: automation.trigger?.id,
script: automation.script?.id,
fields: this._fieldsForTarget(automation, "script").concat(
this._fieldsForTarget(automation, "trigger")
),
fields: automation.scriptFields().concat(automation.triggerFields()),
},
});
}

44
plugins/automation/admin/assets/javascripts/discourse/templates/admin-plugins/show/automation/edit.gjs
View file

@ -1,7 +1,6 @@
import { Input } from "@ember/component";
import { fn, hash } from "@ember/helper";
import RouteTemplate from "ember-route-template";
import { and } from "truth-helpers";
import { and, not } from "truth-helpers";
import BackButton from "discourse/components/back-button";
import DButton from "discourse/components/d-button";
import TextField from "discourse/components/text-field";
@ -9,6 +8,7 @@ import withEventValue from "discourse/helpers/with-event-value";
import { i18n } from "discourse-i18n";
import AdminConfigAreaCard from "admin/components/admin-config-area-card";
import ComboBox from "select-kit/components/combo-box";
import AutomationEnabledToggle from "discourse/plugins/automation/admin/components/automation-enabled-toggle";
import AutomationField from "discourse/plugins/automation/admin/components/automation-field";
import FormError from "discourse/plugins/automation/admin/components/form-error";
@ -22,6 +22,27 @@ export default RouteTemplate(
@route="adminPlugins.show.automation.index"
class="discourse-automation-back"
/>
{{#if @controller.automationForm.trigger}}
<AdminConfigAreaCard class="automation-enabled-card">
<:content>
<div class="control-group automation-enabled">
<label>{{i18n
"discourse_automation.models.automation.enabled.label"
}}</label>
<span class="enabled-toggle-with-tooltip">
<AutomationEnabledToggle
@automation={{@controller.model.automation}}
@canBeEnabled={{not @controller.disableEnabledToggle}}
@onToggle={{@controller.toggleEnabled}}
/>
</span>
</div>
</:content>
</AdminConfigAreaCard>
{{/if}}
<AdminConfigAreaCard @heading="discourse_automation.select_script">
<:content>
<form class="form-horizontal">
@ -179,25 +200,6 @@ export default RouteTemplate(
</section>
{{/if}}
{{#if @controller.automationForm.trigger}}
<div
class="control-group automation-enabled alert
{{if
@controller.automationForm.enabled
'alert-info'
'alert-warning'
}}"
>
<span>{{i18n
"discourse_automation.models.automation.enabled.label"
}}</span>
<Input
@type="checkbox"
@checked={{@controller.automationForm.enabled}}
/>
</div>
{{/if}}
<div class="control-group">
<DButton
@isLoading={{@controller.isUpdatingAutomation}}

3
plugins/automation/app/controllers/discourse_automation/admin_automations_controller.rb
View file

@ -56,6 +56,9 @@ module DiscourseAutomation
automation =
DiscourseAutomation::Automation.includes(:fields, :pending_automations).find(params[:id])
automation.perform_required_fields_validation = true
if automation.scriptable.forced_triggerable
params[:trigger] = automation.scriptable.forced_triggerable[:triggerable].to_s
end

45
plugins/automation/app/models/discourse_automation/automation.rb
View file

@ -23,12 +23,14 @@ module DiscourseAutomation
validates :script, presence: true
validate :validate_trigger_fields
validate :required_fields_complete, if: :perform_required_fields_validation?
after_destroy do |automation|
UserCustomField.where(name: automation.new_user_custom_field_name).destroy_all
end
attr_accessor :running_in_background
attr_accessor :perform_required_fields_validation
def trigger=(new_trigger)
@triggerable = nil
@ -201,6 +203,49 @@ module DiscourseAutomation
validity: 5.seconds,
) { yield }
end
def perform_required_fields_validation?
!!@perform_required_fields_validation && enabled?
end
def required_fields_complete
if scriptable.blank?
errors.add(
:base,
I18n.t("discourse_automation.models.automations.validations.script_blank"),
)
return
end
if triggerable.blank?
errors.add(
:base,
I18n.t("discourse_automation.models.automations.validations.trigger_blank"),
)
return
end
if missing_fields = scriptable.missing_required_fields.presence
errors.add(
:base,
I18n.t(
"discourse_automation.models.automations.validations.script_missing_required_fields",
fields: missing_fields.join(", "),
),
)
return
end
if missing_fields = triggerable.missing_required_fields.presence
errors.add(
:base,
I18n.t(
"discourse_automation.models.automations.validations.trigger_missing_required_fields",
fields: missing_fields.join(", "),
),
)
end
end
end
end

16
plugins/automation/assets/stylesheets/common/discourse-automation.scss
View file

@ -135,6 +135,16 @@
}
.discourse-automation-form {
.automation-enabled-card {
margin-bottom: 1em;
}
.enabled-toggle-with-tooltip {
display: flex;
align-items: center;
gap: var(--space-2);
}
.scriptables,
.triggerables {
.select-kit-body {
@ -267,7 +277,11 @@
display: flex;
align-items: center;
justify-content: flex-start;
padding: 1em;
gap: 1em;
label {
margin: 0;
}
.ember-checkbox {
margin: 0 0 0 1em;

3
plugins/automation/config/locales/client.en.yml
View file

@ -442,7 +442,7 @@ en:
status:
label: Status
enabled:
label: Enabled
label: "Enable automation"
disabled:
label: Disabled
placeholders:
@ -453,3 +453,4 @@ en:
label: Last run
last_updated_by:
label: Updated by
enable_toggle_disabled: "You can enable this automation once all required fields are complete."

6
plugins/automation/config/locales/server.en.yml
View file

@ -3,6 +3,12 @@ en:
discourse_automation_enabled: "Enable discourse-automation plugin"
discourse_automation:
models:
automations:
validations:
script_blank: "Script cannot be blank"
trigger_blank: "Trigger cannot be blank"
script_missing_required_fields: "Script is missing required fields: %{fields}"
trigger_missing_required_fields: "Trigger is missing required fields: %{fields}"
fields:
required_field: Field `%{name}` must be filled on `%{target}:%{target_name}`.
invalid_field: Field’s component `%{component}` is not usable on `%{target}:%{target_name}`.

25
plugins/automation/lib/discourse_automation/scriptable.rb
View file

@ -137,6 +137,31 @@ module DiscourseAutomation
fields.map { |f| f[:component] }.uniq
end
def missing_required_fields
if automation.blank?
raise RuntimeError.new("`missing_required_fields` cannot be called without `@automation`")
end
required = Set.new
fields.each do |field|
next if !field[:required]
required << field[:name].to_s
end
return [] if required.empty?
automation
.fields
.where(target: "script", name: required)
.pluck(:name, :metadata)
.each do |name, metadata|
required.delete(name) if metadata.present? && metadata["value"].present?
end
required.to_a
end
def utils
Utils
end

25
plugins/automation/lib/discourse_automation/triggerable.rb
View file

@ -105,6 +105,31 @@ module DiscourseAutomation
end
end
def missing_required_fields
if automation.blank?
raise RuntimeError.new("`missing_required_fields` cannot be called without `@automation`")
end
required = Set.new
fields.each do |field|
next if !field[:required]
required << field[:name].to_s
end
return [] if required.empty?
automation
.fields
.where(target: "trigger", name: required)
.pluck(:name, :metadata)
.each do |name, metadata|
required.delete(name) if metadata.present? && metadata["value"].present?
end
required.to_a
end
def self.add(identifier, &block)
@all_triggers = nil
define_method("__triggerable_#{identifier}", block || proc {})

194
plugins/automation/spec/models/automation_spec.rb
View file

@ -191,4 +191,198 @@ describe DiscourseAutomation::Automation do
expect(automation).to be_valid
end
end
describe "#required_fields_complete" do
fab!(:automation) do
Fabricate(
:automation,
enabled: false,
script: "required_cats",
trigger: "required_cats_trigger",
)
end
before do
DiscourseAutomation::Scriptable.add("required_cats") do
field :cat, component: :text, required: true
field :dog, component: :text, required: false
end
DiscourseAutomation::Triggerable.add("required_cats_trigger") do
field :litterbox, component: :text, required: true
end
end
context "when the perform_required_fields_validation flag is true" do
before { automation.perform_required_fields_validation = true }
context "when a script required field is missing" do
before do
automation.fields.create!(
name: "litterbox",
component: "text",
metadata: {
value: "kittytoilet",
},
target: "trigger",
)
end
it "blocks enabling the automation and indicates the missing fields" do
automation.enabled = true
expect(automation.save).to eq(false)
expect(automation.errors.full_messages).to contain_exactly(
I18n.t(
"discourse_automation.models.automations.validations.script_missing_required_fields",
fields: "cat",
),
)
cat_field =
automation.fields.create!(
name: "cat",
component: "text",
metadata: {
value: "kitty",
},
target: "script",
)
automation.reload
automation.enabled = true
expect(automation.save).to eq(true)
automation.update!(enabled: false)
cat_field.update_attribute(:metadata, { value: "" })
automation.enabled = true
expect(automation.save).to eq(false)
expect(automation.errors.full_messages).to contain_exactly(
I18n.t(
"discourse_automation.models.automations.validations.script_missing_required_fields",
fields: "cat",
),
)
end
it "doesn't block disabling the automation" do
automation.fields.destroy_all
automation.update_attribute(:enabled, true)
automation.reload
expect(automation.enabled).to eq(true)
automation.enabled = false
expect(automation.save).to eq(true)
expect(automation.errors.full_messages).to be_empty
end
end
context "when a trigger required field is missing" do
before do
automation.fields.create!(
name: "cat",
component: "text",
metadata: {
value: "hellokitty",
},
target: "script",
)
end
it "blocks enabling the automation and indicates the missing fields" do
automation.enabled = true
expect(automation.save).to eq(false)
expect(automation.errors.full_messages).to contain_exactly(
I18n.t(
"discourse_automation.models.automations.validations.trigger_missing_required_fields",
fields: "litterbox",
),
)
litterbox_field =
automation.fields.create!(
name: "litterbox",
component: "text",
metadata: {
value: "kitty",
},
target: "trigger",
)
automation.reload
automation.enabled = true
expect(automation.save).to eq(true)
automation.update!(enabled: false)
litterbox_field.update_attribute(:metadata, { value: "" })
automation.enabled = true
expect(automation.save).to eq(false)
expect(automation.errors.full_messages).to contain_exactly(
I18n.t(
"discourse_automation.models.automations.validations.trigger_missing_required_fields",
fields: "litterbox",
),
)
end
it "doesn't block disabling the automation" do
automation.fields.destroy_all
automation.update_attribute(:enabled, true)
automation.reload
expect(automation.enabled).to eq(true)
automation.enabled = false
expect(automation.save).to eq(true)
expect(automation.errors.full_messages).to be_empty
end
end
context "when all required fields are filled" do
it "allows enabling the automation" do
automation.fields.create!(
name: "cat",
component: "text",
metadata: {
value: "kitty",
},
target: "script",
)
automation.fields.create!(
name: "litterbox",
component: "text",
metadata: {
value: "kittytoilet",
},
target: "trigger",
)
automation.enabled = true
expect(automation.save).to eq(true)
expect(automation.errors.full_messages).to be_empty
end
end
context "when trigger is blank" do
before { automation.update!(trigger: nil) }
it "blocks enabling the automation and indicates the trigger is blank" do
automation.enabled = true
expect(automation.save).to eq(false)
expect(automation.errors.full_messages).to contain_exactly(
I18n.t("discourse_automation.models.automations.validations.trigger_blank"),
)
end
end
end
context "when the perform_required_fields_validation flag is false" do
before { automation.perform_required_fields_validation = false }
it "doesn't block enabling the automation even if there are missing required fields" do
automation.enabled = true
expect(automation.save).to eq(true)
end
end
end
end

2
plugins/automation/spec/requests/admin_discourse_automation_automations_spec.rb
View file

@ -1,7 +1,7 @@
# frozen_string_literal: true
describe DiscourseAutomation::AdminAutomationsController do
fab!(:automation)
fab!(:automation) { Fabricate(:automation, enabled: true) }
before do
SiteSetting.discourse_automation_enabled = true

60
plugins/automation/spec/system/automation_edit_page_spec.rb Normal file
View file

@ -0,0 +1,60 @@
# frozen_string_literal: true
describe "DiscourseAutomation | Edit page", type: :system do
fab!(:admin)
fab!(:automation) { Fabricate(:automation, enabled: true) }
before do
DiscourseAutomation::Scriptable.add("required_cats") do
field :cat, component: :text, required: true
field :dog, component: :text, required: false
end
SiteSetting.discourse_automation_enabled = true
sign_in(admin)
end
let(:automation_page) { PageObjects::Pages::Automation.new }
let(:tooltip) { PageObjects::Components::Tooltips.new("automation-enabled-toggle") }
describe "the enabled toggle" do
it "can turn on/off the automation" do
automation_page.visit(automation)
expect(automation_page.enabled_toggle).to be_checked
automation_page.enabled_toggle.toggle
expect(automation_page.enabled_toggle).to be_unchecked
automation_page.enabled_toggle.toggle
expect(automation_page.enabled_toggle).to be_checked
end
it "is disabled when the automation has missing required fields" do
automation.update!(enabled: false, script: "required_cats")
automation_page.visit(automation)
expect(automation_page.enabled_toggle).to be_disabled
automation_page.enabled_toggle.label_component.hover
expect(tooltip).to be_present(
text: I18n.t("js.discourse_automation.models.automation.enable_toggle_disabled"),
)
end
it "is enabled when the automation is enabled even with missing required fields" do
automation.update_attribute(:script, "required_cats")
automation.fields.destroy_all
automation_page.visit(automation)
expect(automation_page.enabled_toggle).to be_enabled
automation_page.enabled_toggle.toggle
expect(automation_page.enabled_toggle).to be_unchecked
expect(automation_page.enabled_toggle).to be_disabled
automation_page.enabled_toggle.label_component.hover
expect(tooltip).to be_present(
text: I18n.t("js.discourse_automation.models.automation.enable_toggle_disabled"),
)
end
end
end

4
plugins/automation/spec/system/page_objects/discourse_automation/automation.rb
View file

@ -33,6 +33,10 @@ module PageObjects
self
end
def enabled_toggle
PageObjects::Components::DToggleSwitch.new(".d-toggle-switch .d-toggle-switch__checkbox")
end
def form
@form ||= find(".discourse-automation-edit")
end

4
plugins/automation/spec/system/smoke_test_spec.rb
View file

@ -5,6 +5,8 @@ describe "DiscourseAutomation | smoke test", type: :system do
fab!(:group) { Fabricate(:group, name: "test") }
fab!(:badge) { Fabricate(:badge, name: "badge") }
let(:automation_page) { PageObjects::Pages::Automation.new }
before do
SiteSetting.discourse_automation_enabled = true
sign_in(admin)
@ -70,7 +72,7 @@ describe "DiscourseAutomation | smoke test", type: :system do
select_kit = PageObjects::Components::SelectKit.new(".group-chooser")
select_kit.expand
select_kit.select_row_by_name("test")
find(".automation-enabled input").click
automation_page.enabled_toggle.toggle
find(".update-automation").click
expect(page).to have_css(".automations__name", text: "aaaaa")

9
spec/system/page_objects/components/d_toggle_switch.rb
View file

@ -27,6 +27,15 @@ module PageObjects
visible: :all,
)
end
def disabled?
label_component.has_css?(".d-toggle-switch__checkbox[disabled]", visible: :all)
end
def enabled?
label_component.has_no_css?(".d-toggle-switch__checkbox[disabled]", visible: :all) &&
label_component.has_css?(".d-toggle-switch__checkbox", visible: :all)
end
end
end
end