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 cdaceb3a61d..85a3e440d0a 100644 --- a/plugins/automation/app/controllers/discourse_automation/admin_automations_controller.rb +++ b/plugins/automation/app/controllers/discourse_automation/admin_automations_controller.rb @@ -81,7 +81,7 @@ module DiscourseAutomation end def destroy - DiscourseAutomation::DestroyAutomation.call(service_params) do + DiscourseAutomation::Destroy.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 } diff --git a/plugins/automation/app/services/discourse_automation/destroy.rb b/plugins/automation/app/services/discourse_automation/destroy.rb new file mode 100644 index 00000000000..0584c4160de --- /dev/null +++ b/plugins/automation/app/services/discourse_automation/destroy.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +class DiscourseAutomation::Destroy + include Service::Base + + # @!method self.call(guardian:, params:) + # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :automation_id + # @return [Service::Base::Context] + + policy :can_destroy_automation + + params do + attribute :automation_id, :integer + validates :automation_id, presence: true + end + + model :automation + + transaction do + step :log_action + step :destroy_automation + end + + private + + def can_destroy_automation(guardian:) + guardian.is_admin? + end + + def fetch_automation(params:) + DiscourseAutomation::Automation.find_by(id: params.automation_id) + end + + def log_action(automation:, guardian:) + StaffActionLogger.new(guardian.user).log_custom( + "delete_automation", + **automation.slice(:id, :name, :script, :trigger), + ) + end + + def destroy_automation(automation:) + automation.destroy! + end +end diff --git a/plugins/automation/app/services/discourse_automation/destroy_automation.rb b/plugins/automation/app/services/discourse_automation/destroy_automation.rb deleted file mode 100644 index 339c8d8da8d..00000000000 --- a/plugins/automation/app/services/discourse_automation/destroy_automation.rb +++ /dev/null @@ -1,48 +0,0 @@ -# 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/spec/services/destroy_automation_spec.rb b/plugins/automation/spec/services/discourse_automation/destroy_spec.rb similarity index 57% rename from plugins/automation/spec/services/destroy_automation_spec.rb rename to plugins/automation/spec/services/discourse_automation/destroy_spec.rb index 6f317604dae..09662498612 100644 --- a/plugins/automation/spec/services/destroy_automation_spec.rb +++ b/plugins/automation/spec/services/discourse_automation/destroy_spec.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true -RSpec.describe DiscourseAutomation::DestroyAutomation do +RSpec.describe DiscourseAutomation::Destroy do describe described_class::Contract, type: :model do - subject(:contract) { described_class.new } - it { is_expected.to validate_presence_of :automation_id } end @@ -17,15 +15,16 @@ RSpec.describe DiscourseAutomation::DestroyAutomation do 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}", - ) + context "when user can't destroy the automation" do + fab!(:user) { Fabricate(:user) } + + it { is_expected.to fail_a_policy(:can_destroy_automation) } end - it "destroys the automation" do - expect { result }.to change { DiscourseAutomation::Automation.count }.by(-1) + context "when data is invalid" do + before { params[:automation_id] = nil } + + it { is_expected.to fail_a_contract } end context "when the automation is not found" do @@ -34,10 +33,17 @@ RSpec.describe DiscourseAutomation::DestroyAutomation do it { is_expected.to fail_to_find_a_model(:automation) } end - context "when user can't destroy the automation" do - fab!(:user) { Fabricate(:user) } + context "when everything's ok" do + 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 { is_expected.to fail_a_policy(:can_destroy_automation) } + it "destroys the automation" do + expect { result }.to change { DiscourseAutomation::Automation.count }.by(-1) + end end end end