mirror of
https://github.com/discourse/discourse.git
synced 2024-11-22 15:25:35 +08:00
UX: logs when an automation is destroyed (#29565)
Some checks are pending
Licenses / run (push) Waiting to run
Linting / run (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (frontend, themes) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, chat) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, plugins) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, themes) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Chrome) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Firefox ESR) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Firefox Evergreen) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (annotations, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (backend, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (backend, plugins) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (frontend, plugins) (push) Waiting to run
Some checks are pending
Licenses / run (push) Waiting to run
Linting / run (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (frontend, themes) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, chat) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, plugins) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, themes) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Chrome) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Firefox ESR) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Firefox Evergreen) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (annotations, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (backend, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (backend, plugins) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (frontend, plugins) (push) Waiting to run
A `UserHistory` entry will now be created when an automation is destroyed. This is visible in `/admin/logs/staff_action_logs`. id, name, trigger and script will be logged. This commit also creates a service `DestroyAutomation` to hold all the destroy automation logic. --------- Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
This commit is contained in:
parent
a43bd24c67
commit
932bd6ba85
|
@ -81,9 +81,11 @@ module DiscourseAutomation
|
||||||
end
|
end
|
||||||
|
|
||||||
def destroy
|
def destroy
|
||||||
automation = DiscourseAutomation::Automation.find(params[:id])
|
DiscourseAutomation::DestroyAutomation.call(service_params) do
|
||||||
automation.destroy!
|
on_success { render(json: success_json) }
|
||||||
render json: success_json
|
on_model_not_found(:automation) { raise Discourse::NotFound }
|
||||||
|
on_failed_policy(:can_destroy_automation) { raise Discourse::InvalidAccess }
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
|
@ -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
|
|
@ -1,5 +1,10 @@
|
||||||
en:
|
en:
|
||||||
js:
|
js:
|
||||||
|
admin:
|
||||||
|
logs:
|
||||||
|
staff_actions:
|
||||||
|
actions:
|
||||||
|
delete_automation: delete automation
|
||||||
discourse_automation:
|
discourse_automation:
|
||||||
title: Automation
|
title: Automation
|
||||||
create: Create
|
create: Create
|
||||||
|
|
|
@ -24,7 +24,7 @@ DiscourseAutomation::Engine.routes.draw do
|
||||||
get "/triggerables" => "admin_triggerables#index"
|
get "/triggerables" => "admin_triggerables#index"
|
||||||
get "/automations" => "admin_automations#index"
|
get "/automations" => "admin_automations#index"
|
||||||
get "/automations/:id" => "admin_automations#show"
|
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"
|
put "/automations/:id" => "admin_automations#update"
|
||||||
post "/automations" => "admin_automations#create"
|
post "/automations" => "admin_automations#create"
|
||||||
end
|
end
|
||||||
|
|
|
@ -235,6 +235,13 @@ describe DiscourseAutomation::AdminAutomationsController do
|
||||||
delete "/admin/plugins/discourse-automation/automations/#{automation.id}.json"
|
delete "/admin/plugins/discourse-automation/automations/#{automation.id}.json"
|
||||||
expect(DiscourseAutomation::Automation.find_by(id: automation.id)).to eq(nil)
|
expect(DiscourseAutomation::Automation.find_by(id: automation.id)).to eq(nil)
|
||||||
end
|
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
|
end
|
||||||
|
|
||||||
context "when logged in as a regular user" do
|
context "when logged in as a regular user" do
|
||||||
|
|
43
plugins/automation/spec/services/destroy_automation_spec.rb
Normal file
43
plugins/automation/spec/services/destroy_automation_spec.rb
Normal file
|
@ -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
|
Loading…
Reference in New Issue
Block a user