diff --git a/plugins/automation/app/controllers/discourse_automation/admin_automations_controller.rb b/plugins/automation/app/controllers/discourse_automation/admin_automations_controller.rb index f0d9ba1bd6e..cdaceb3a61d 100644 --- a/plugins/automation/app/controllers/discourse_automation/admin_automations_controller.rb +++ b/plugins/automation/app/controllers/discourse_automation/admin_automations_controller.rb @@ -81,9 +81,11 @@ module DiscourseAutomation end def destroy - automation = DiscourseAutomation::Automation.find(params[:id]) - automation.destroy! - render json: success_json + DiscourseAutomation::DestroyAutomation.call(service_params) do + on_success { render(json: success_json) } + on_model_not_found(:automation) { raise Discourse::NotFound } + on_failed_policy(:can_destroy_automation) { raise Discourse::InvalidAccess } + end end private diff --git a/plugins/automation/app/services/discourse_automation/destroy_automation.rb b/plugins/automation/app/services/discourse_automation/destroy_automation.rb new file mode 100644 index 00000000000..339c8d8da8d --- /dev/null +++ b/plugins/automation/app/services/discourse_automation/destroy_automation.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module DiscourseAutomation + class DestroyAutomation + include ::Service::Base + + # @!method self.call(guardian:, params:) + # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :automation_id + # @return [Service::Base::Context] + params do + attribute :automation_id, :integer + validates :automation_id, presence: true + end + + model :automation + policy :can_destroy_automation + transaction do + step :log_action + step :destroy_automation + end + + private + + def fetch_automation(params:) + DiscourseAutomation::Automation.find_by(id: params.automation_id) + end + + def can_destroy_automation(guardian:) + guardian.is_admin? + end + + def log_action(automation:, guardian:) + StaffActionLogger.new(guardian.user).log_custom( + "delete_automation", + id: automation.id, + name: automation.name, + script: automation.script, + trigger: automation.trigger, + ) + end + + def destroy_automation(automation:) + automation.destroy! + end + end +end diff --git a/plugins/automation/config/locales/client.en.yml b/plugins/automation/config/locales/client.en.yml index aa0540b433d..0e2151a3a95 100644 --- a/plugins/automation/config/locales/client.en.yml +++ b/plugins/automation/config/locales/client.en.yml @@ -1,5 +1,10 @@ en: js: + admin: + logs: + staff_actions: + actions: + delete_automation: delete automation discourse_automation: title: Automation create: Create diff --git a/plugins/automation/config/routes.rb b/plugins/automation/config/routes.rb index 79185e91fb3..e778e471968 100644 --- a/plugins/automation/config/routes.rb +++ b/plugins/automation/config/routes.rb @@ -24,7 +24,7 @@ DiscourseAutomation::Engine.routes.draw do get "/triggerables" => "admin_triggerables#index" get "/automations" => "admin_automations#index" get "/automations/:id" => "admin_automations#show" - delete "/automations/:id" => "admin_automations#destroy" + delete "/automations/:automation_id" => "admin_automations#destroy" put "/automations/:id" => "admin_automations#update" post "/automations" => "admin_automations#create" end diff --git a/plugins/automation/spec/requests/admin_discourse_automation_automations_spec.rb b/plugins/automation/spec/requests/admin_discourse_automation_automations_spec.rb index 79647fb9e17..625089cf7b6 100644 --- a/plugins/automation/spec/requests/admin_discourse_automation_automations_spec.rb +++ b/plugins/automation/spec/requests/admin_discourse_automation_automations_spec.rb @@ -235,6 +235,13 @@ describe DiscourseAutomation::AdminAutomationsController do delete "/admin/plugins/discourse-automation/automations/#{automation.id}.json" expect(DiscourseAutomation::Automation.find_by(id: automation.id)).to eq(nil) end + + context "when the automation is not found" do + it "raises a 404" do + delete "/admin/plugins/discourse-automation/automations/999.json" + expect(response.status).to eq(404) + end + end end context "when logged in as a regular user" do diff --git a/plugins/automation/spec/services/destroy_automation_spec.rb b/plugins/automation/spec/services/destroy_automation_spec.rb new file mode 100644 index 00000000000..6f317604dae --- /dev/null +++ b/plugins/automation/spec/services/destroy_automation_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +RSpec.describe DiscourseAutomation::DestroyAutomation do + describe described_class::Contract, type: :model do + subject(:contract) { described_class.new } + + it { is_expected.to validate_presence_of :automation_id } + end + + describe ".call" do + subject(:result) { described_class.call(params:, **dependencies) } + + fab!(:user) { Fabricate(:admin) } + fab!(:automation) { Fabricate(:automation) } + + let(:guardian) { user.guardian } + let(:params) { { automation_id: automation.id } } + let(:dependencies) { { guardian: } } + + it "logs the action" do + expect { result }.to change { UserHistory.count }.by(1) + expect(UserHistory.last.details).to eq( + "id: #{automation.id}\nname: #{automation.name}\nscript: #{automation.script}\ntrigger: #{automation.trigger}", + ) + end + + it "destroys the automation" do + expect { result }.to change { DiscourseAutomation::Automation.count }.by(-1) + end + + context "when the automation is not found" do + before { params[:automation_id] = 999 } + + it { is_expected.to fail_to_find_a_model(:automation) } + end + + context "when user can't destroy the automation" do + fab!(:user) { Fabricate(:user) } + + it { is_expected.to fail_a_policy(:can_destroy_automation) } + end + end +end