diff --git a/modules/webhooks/app/workers/represented_webhook_job.rb b/modules/webhooks/app/workers/represented_webhook_job.rb index e56e6aec12c..c42db5e0c01 100644 --- a/modules/webhooks/app/workers/represented_webhook_job.rb +++ b/modules/webhooks/app/workers/represented_webhook_job.rb @@ -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 diff --git a/modules/webhooks/app/workers/work_package_webhook_job.rb b/modules/webhooks/app/workers/work_package_webhook_job.rb index 92312622479..04edb45a57e 100644 --- a/modules/webhooks/app/workers/work_package_webhook_job.rb +++ b/modules/webhooks/app/workers/work_package_webhook_job.rb @@ -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 diff --git a/modules/webhooks/lib/open_project/webhooks/event_resources/work_package.rb b/modules/webhooks/lib/open_project/webhooks/event_resources/work_package.rb index 802ce5f44a5..4e1132b93f4 100644 --- a/modules/webhooks/lib/open_project/webhooks/event_resources/work_package.rb +++ b/modules/webhooks/lib/open_project/webhooks/event_resources/work_package.rb @@ -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 diff --git a/modules/webhooks/spec/workers/work_package_webhook_job_spec.rb b/modules/webhooks/spec/workers/work_package_webhook_job_spec.rb index 51179708a80..85269ab78ce 100644 --- a/modules/webhooks/spec/workers/work_package_webhook_job_spec.rb +++ b/modules/webhooks/spec/workers/work_package_webhook_job_spec.rb @@ -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