Webhooks: Fixed failing delete-based events

Due to queue serialization.
Added a test to check a couple of delete events.
Added ApiTokenFactory to support.
Also made a couple of typing/doc updates while there.

Related to #4373

Author: ssddanbrown

app/Activity/DispatchWebhookJob.php

@@ -24,27 +24,23 @@ class DispatchWebhookJob implements ShouldQueue
     use SerializesModels;
 
     protected Webhook $webhook;
-    protected string $event;
     protected User $initiator;
     protected int $initiatedTime;
-
-    /**
-     * @var string|Loggable
-     */
-    protected $detail;
+    protected array $webhookData;
 
     /**
      * Create a new job instance.
      *
      * @return void
      */
-    public function __construct(Webhook $webhook, string $event, $detail)
+    public function __construct(Webhook $webhook, string $event, Loggable|string $detail)
     {
         $this->webhook = $webhook;
-        $this->event = $event;
-        $this->detail = $detail;
         $this->initiator = user();
         $this->initiatedTime = time();
+
+        $themeResponse = Theme::dispatch(ThemeEvents::WEBHOOK_CALL_BEFORE, $event, $this->webhook, $detail, $this->initiator, $this->initiatedTime);
+        $this->webhookData =  $themeResponse ?? WebhookFormatter::getDefault($event, $this->webhook, $detail, $this->initiator, $this->initiatedTime)->format();
     }
 
     /**
@@ -54,15 +50,13 @@ public function __construct(Webhook $webhook, string $event, $detail)
      */
     public function handle()
     {
-        $themeResponse = Theme::dispatch(ThemeEvents::WEBHOOK_CALL_BEFORE, $this->event, $this->webhook, $this->detail, $this->initiator, $this->initiatedTime);
-        $webhookData = $themeResponse ?? WebhookFormatter::getDefault($this->event, $this->webhook, $this->detail, $this->initiator, $this->initiatedTime)->format();
         $lastError = null;
 
         try {
             $response = Http::asJson()
                 ->withOptions(['allow_redirects' => ['strict' => true]])
                 ->timeout($this->webhook->timeout)
-                ->post($this->webhook->endpoint, $webhookData);
+                ->post($this->webhook->endpoint, $this->webhookData);
         } catch (\Exception $exception) {
             $lastError = $exception->getMessage();
             Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\"");

app/Activity/Tools/WebhookFormatter.php

@@ -17,18 +17,14 @@ class WebhookFormatter
     protected string $event;
     protected User $initiator;
     protected int $initiatedTime;
-
-    /**
-     * @var string|Loggable
-     */
-    protected $detail;
+    protected string|Loggable $detail;
 
     /**
      * @var array{condition: callable(string, Model):bool, format: callable(Model):void}[]
      */
     protected $modelFormatters = [];
 
-    public function __construct(string $event, Webhook $webhook, $detail, User $initiator, int $initiatedTime)
+    public function __construct(string $event, Webhook $webhook, string|Loggable $detail, User $initiator, int $initiatedTime)
     {
         $this->webhook = $webhook;
         $this->event = $event;

app/Api/ApiToken.php

@@ -4,6 +4,7 @@
 
 use BookStack\Activity\Models\Loggable;
 use BookStack\Users\Models\User;
+use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Support\Carbon;
@@ -20,6 +21,8 @@
  */
 class ApiToken extends Model implements Loggable
 {
+    use HasFactory;
+
     protected $fillable = ['name', 'expires_at'];
     protected $casts = [
         'expires_at' => 'date:Y-m-d',

app/Theming/ThemeEvents.php

@@ -132,11 +132,12 @@ class ThemeEvents
      * If the listener returns a non-null value, that will be used as the POST data instead
      * of the system default.
      *
-     * @param string                                $event
-     * @param \BookStack\Activity\Models\Webhook            $webhook
+     * @param string                                     $event
+     * @param \BookStack\Activity\Models\Webhook         $webhook
      * @param string|\BookStack\Activity\Models\Loggable $detail
-     * @param \BookStack\Users\Models\User                  $initiator
-     * @param int                                   $initiatedTime
+     * @param \BookStack\Users\Models\User               $initiator
+     * @param int                                        $initiatedTime
+     * @returns array|null
      */
     const WEBHOOK_CALL_BEFORE = 'webhook_call_before';
 }

database/factories/Api/ApiTokenFactory.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Database\Factories\Api;
+
+use BookStack\Api\ApiToken;
+use BookStack\Users\Models\User;
+use Illuminate\Database\Eloquent\Factories\Factory;
+use Illuminate\Support\Carbon;
+use Illuminate\Support\Str;
+
+class ApiTokenFactory extends Factory
+{
+    protected $model = ApiToken::class;
+
+    public function definition(): array
+    {
+        return [
+            'token_id' => Str::random(10),
+            'secret' => Str::random(12),
+            'name' => $this->faker->name(),
+            'expires_at' => Carbon::now()->addYear(),
+            'created_at' => Carbon::now(),
+            'updated_at' => Carbon::now(),
+            'user_id' => User::factory(),
+        ];
+    }
+}

tests/Actions/WebhookCallTest.php

@@ -6,6 +6,8 @@
 use BookStack\Activity\DispatchWebhookJob;
 use BookStack\Activity\Models\Webhook;
 use BookStack\Activity\Tools\ActivityLogger;
+use BookStack\Api\ApiToken;
+use BookStack\Entities\Models\PageRevision;
 use BookStack\Users\Models\User;
 use Illuminate\Http\Client\Request;
 use Illuminate\Support\Facades\Bus;
@@ -46,6 +48,24 @@ public function test_inactive_webhook_not_called_on_event()
         Bus::assertNotDispatched(DispatchWebhookJob::class);
     }
 
+    public function test_webhook_runs_for_delete_actions()
+    {
+        $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']);
+        Http::fake([
+            '*' => Http::response('', 500),
+        ]);
+
+        $user = $this->users->newUser();
+        $resp = $this->asAdmin()->delete($user->getEditUrl());
+        $resp->assertRedirect('/settings/users');
+
+        /** @var ApiToken $apiToken */
+        $editor = $this->users->editor();
+        $apiToken = ApiToken::factory()->create(['user_id' => $editor]);
+        $resp = $this->delete($editor->getEditUrl('/api-tokens/' . $apiToken->id));
+        $resp->assertRedirect($editor->getEditUrl('#api_tokens'));
+    }
+
     public function test_failed_webhook_call_logs_error()
     {
         $logger = $this->withTestLogger();
@@ -120,7 +140,7 @@ protected function runEvent(string $event, $detail = '', ?User $user = null)
         $activityLogger->add($event, $detail);
     }
 
-    protected function newWebhook(array $attrs = [], array $events = ['all']): Webhook
+    protected function newWebhook(array $attrs, array $events): Webhook
     {
         /** @var Webhook $webhook */
         $webhook = Webhook::factory()->create($attrs);