mirror of
https://github.com/opf/openproject.git
synced 2026-06-14 03:30:14 +00:00
Refactor: Move actor to base class with optional keyword argument
Move actor resolution to the caller and actor representation to RepresentedWebhookJob, keeping WorkPackageWebhookJob simple. The caller extracts the actor from the journal and passes it as an optional `actor:` keyword argument, making the feature available to all webhook types without changing existing method signatures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -31,8 +31,9 @@
|
||||
class RepresentedWebhookJob < WebhookJob
|
||||
attr_reader :resource
|
||||
|
||||
def perform(webhook_id, resource, event_name)
|
||||
def perform(webhook_id, resource, event_name, actor: nil)
|
||||
@resource = resource
|
||||
@actor = actor
|
||||
super(webhook_id, event_name)
|
||||
|
||||
return unless accepted_in_project?
|
||||
@@ -95,6 +96,8 @@ class RepresentedWebhookJob < WebhookJob
|
||||
end
|
||||
|
||||
def actor_payload
|
||||
nil
|
||||
return nil unless @actor
|
||||
|
||||
::API::V3::Users::UserRepresenter.create(@actor, current_user: User.current)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -29,14 +29,6 @@
|
||||
#++
|
||||
|
||||
class WorkPackageWebhookJob < RepresentedWebhookJob
|
||||
attr_reader :journal
|
||||
|
||||
def perform(webhook_id, journal, event_name)
|
||||
@journal = journal
|
||||
@resource = journal.journable
|
||||
super(webhook_id, @resource, event_name)
|
||||
end
|
||||
|
||||
def payload_key
|
||||
:work_package
|
||||
end
|
||||
@@ -44,11 +36,4 @@ class WorkPackageWebhookJob < RepresentedWebhookJob
|
||||
def payload_representer_class
|
||||
::API::V3::WorkPackages::WorkPackageRepresenter
|
||||
end
|
||||
|
||||
def actor_payload
|
||||
user = User.find_by(id: journal.user_id)
|
||||
return nil unless user
|
||||
|
||||
::API::V3::Users::UserRepresenter.create(user, current_user: User.current)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -50,11 +50,13 @@ module OpenProject::Webhooks::EventResources
|
||||
protected
|
||||
|
||||
def handle_notification(payload, event_name)
|
||||
action = payload[:journal].initial? ? "created" : "updated"
|
||||
journal = payload[:journal]
|
||||
action = journal.initial? ? "created" : "updated"
|
||||
event_name = prefixed_event_name(action)
|
||||
work_package = payload[:journal].journable
|
||||
work_package = journal.journable
|
||||
actor = User.find_by(id: journal.user_id)
|
||||
active_webhooks.with_event_name(event_name).pluck(:id).each do |id|
|
||||
WorkPackageWebhookJob.perform_later(id, work_package, event_name)
|
||||
WorkPackageWebhookJob.perform_later(id, work_package, event_name, actor:)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -36,11 +36,11 @@ RSpec.describe WorkPackageWebhookJob, :webmock, type: :model do
|
||||
shared_let(:request_url) { "http://example.net/test/42" }
|
||||
shared_let(:work_package) { create(:work_package, subject: title) }
|
||||
shared_let(:webhook) { create(:webhook, all_projects: true, url: request_url, secret: nil) }
|
||||
shared_let(:journal) { work_package.journals.first }
|
||||
|
||||
shared_examples "a work package webhook call" do
|
||||
let(:event) { "work_package:created" }
|
||||
let(:job) { described_class.perform_now webhook.id, journal, event }
|
||||
let(:actor) { nil }
|
||||
let(:job) { described_class.perform_now webhook.id, work_package, event, actor: }
|
||||
|
||||
let(:stubbed_url) { request_url }
|
||||
|
||||
@@ -172,14 +172,15 @@ RSpec.describe WorkPackageWebhookJob, :webmock, type: :model do
|
||||
let(:author) { create(:user, firstname: "Original", lastname: "Author") }
|
||||
let(:updater) { create(:user, firstname: "Update", lastname: "User") }
|
||||
let(:work_package) { create(:work_package, author:, subject: title) }
|
||||
let(:journal) do
|
||||
|
||||
before do
|
||||
work_package.add_journal(user: updater, notes: "Updated the work package")
|
||||
work_package.save!
|
||||
work_package.journals.last
|
||||
end
|
||||
|
||||
it_behaves_like "a work package webhook call" do
|
||||
let(:event) { "work_package:updated" }
|
||||
let(:actor) { updater }
|
||||
|
||||
it "includes actor matching the journal user, not the work package author" do
|
||||
subject
|
||||
@@ -203,10 +204,10 @@ RSpec.describe WorkPackageWebhookJob, :webmock, type: :model do
|
||||
describe "actor field on created event" do
|
||||
let(:creator) { create(:user, firstname: "Creator", lastname: "Person") }
|
||||
let(:work_package) { User.execute_as(creator) { create(:work_package, author: creator, subject: title) } }
|
||||
let(:journal) { work_package.journals.first }
|
||||
|
||||
it_behaves_like "a work package webhook call" do
|
||||
let(:event) { "work_package:created" }
|
||||
let(:actor) { creator }
|
||||
|
||||
it "includes actor matching the creator" do
|
||||
subject
|
||||
@@ -221,18 +222,10 @@ RSpec.describe WorkPackageWebhookJob, :webmock, type: :model do
|
||||
end
|
||||
end
|
||||
|
||||
describe "actor absent when journal user has been deleted" do
|
||||
let(:updater) { create(:user) }
|
||||
let(:journal) do
|
||||
work_package.add_journal(user: updater, notes: "Updated")
|
||||
work_package.save!
|
||||
work_package.journals.last
|
||||
end
|
||||
|
||||
before { updater.destroy }
|
||||
|
||||
describe "actor absent when actor is nil" do
|
||||
it_behaves_like "a work package webhook call" do
|
||||
let(:event) { "work_package:updated" }
|
||||
let(:actor) { nil }
|
||||
|
||||
it "fires the webhook without an actor key" do
|
||||
expect { subject }.not_to raise_error
|
||||
@@ -245,7 +238,7 @@ RSpec.describe WorkPackageWebhookJob, :webmock, type: :model do
|
||||
end
|
||||
end
|
||||
|
||||
describe "admin custom field regression with journal (PR #16912)" do
|
||||
describe "admin custom field regression with actor (PR #16912)" do
|
||||
shared_let(:project) { work_package.project }
|
||||
shared_let(:custom_field) do
|
||||
create(:project_custom_field, :string, admin_only: true, projects: [project])
|
||||
@@ -257,14 +250,10 @@ RSpec.describe WorkPackageWebhookJob, :webmock, type: :model do
|
||||
value: "wat")
|
||||
end
|
||||
let(:updater) { create(:admin) }
|
||||
let(:journal) do
|
||||
work_package.add_journal(user: updater, notes: "Updated")
|
||||
work_package.save!
|
||||
work_package.journals.last
|
||||
end
|
||||
|
||||
it_behaves_like "a work package webhook call" do
|
||||
let(:event) { "work_package:updated" }
|
||||
let(:actor) { updater }
|
||||
|
||||
it "includes the custom field value" do
|
||||
subject
|
||||
|
||||
Reference in New Issue
Block a user