diff --git a/services/nginx/app/classes/slack.php b/services/nginx/app/classes/slack.php index 27fbfc8b..2d050b56 100644 --- a/services/nginx/app/classes/slack.php +++ b/services/nginx/app/classes/slack.php @@ -33,17 +33,17 @@ class slack implements notification_i public function send_department_booking_notification(int $department_id, $message): self { // Get the departments webhook - $webhook = self::get_department_webhook($department_id); + $webhook = static::get_department_webhook($department_id); // Check if the webhook is empty if (empty($webhook)) { throw new \Exception('Department webhook is empty'); } // Send the notification to the department - self::add_log(self::send_webhook_message($message, $webhook)); + self::add_log(static::send_webhook_message($message, $webhook)); return $this; } - private function get_department_webhook(int $department_id): string|null + protected function get_department_webhook(int $department_id): string|null { // Check if the department webhook is cached $webhook = redis->get_department_webhook($department_id); @@ -134,6 +134,68 @@ class slack implements notification_i . "Status: $status"; } + /** + * Send a new-booking notification to the department's Slack webhook. + * + * Filter: only PICKUP bookings trigger a Slack notification. Drop-off + * bookings (pickup_bool === false) are intentionally silenced per + * Mikkel's SENERE 14 / TRU-106 request — drop-offs are noise in the + * channel. Other delivery channels (SMS, email) are unaffected. + * + * Returns true if a Slack message was sent, false if it was filtered + * out (drop-off) or the department has no Slack webhook configured. + * + * @throws \Exception If the department lookup or webhook send fails. + */ + public function send_new_booking_notification( + $id, + $customer_number, + string $wash_type, + string $contact_email, + string $reference_number, + string $regNrTraekker, + string $regNrTrailer, + string $washCertificateEmail, + string $date, + int $department, + bool $pickup_bool, + string $notes, + string $washCertificateStatus, + string $washCertificateUrl, + string $status + ): bool { + // TRU-106: drop-off bookings must not post to Slack. + if (!$pickup_bool) { + return false; + } + + $webhook = static::get_department_webhook($department); + if (empty($webhook)) { + return false; + } + + $message = static::format_new_booking( + $id, + $customer_number, + $wash_type, + $contact_email, + $reference_number, + $regNrTraekker, + $regNrTrailer, + $washCertificateEmail, + $date, + $department, + $pickup_bool, + $notes, + $washCertificateStatus, + $washCertificateUrl, + $status + ); + + self::add_log(static::send_webhook_message($message, $webhook)); + return true; + } + public function send_message(string $string, ?string $module = null): void { global $SLACK_DEFAULT_WEBHOOK; diff --git a/services/nginx/app/objects/bookings_o.php b/services/nginx/app/objects/bookings_o.php index 42cdbbff..d46a2ec2 100644 --- a/services/nginx/app/objects/bookings_o.php +++ b/services/nginx/app/objects/bookings_o.php @@ -205,10 +205,12 @@ class bookings_o extends db $sql = "SELECT * FROM $this->table WHERE id = $id"; $result = $db->query($sql); if ($db->num_rows($result) === 0) { - // Send a department webhook if the booking is new + // Send a department webhook if the booking is new. + // TRU-106: send_new_booking_notification() filters out drop-offs + // (pickup_bool = 0) so only pickup bookings post to Slack. $slack = new slack(); try { - $slack->send_department_booking_notification($department, $slack->format_new_booking( + $slack->send_new_booking_notification( $id, $customer_number, $wash_type, @@ -219,12 +221,12 @@ class bookings_o extends db $washCertificateEmail, $date, $department, - $pickup_bool, + (bool)$pickup_bool, $notes, $washCertificateStatus, $washCertificateUrl, $status - )); + ); } catch (Exception $e) { // Log the error $logs = new logs_o(); @@ -313,11 +315,13 @@ class bookings_o extends db !$deliverSlack // Only send email if slack is not available ); // Check if the department has a slack webhook + // TRU-106: send_new_booking_notification() filters out drop-offs + // (pickup_bool = false) so only pickup bookings post to Slack. if ($deliverSlack) { // Send a notification to the department $slack = new slack(); try { - $slack->send_department_booking_notification($department->id, $slack->format_new_booking( + $slack->send_new_booking_notification( $this->id, $customer_array['customer_number'], self::formatWashTypeFromServices(json_decode($this->data->value(), true)), @@ -333,7 +337,7 @@ class bookings_o extends db $this->washCertificateStatus->value(), $this->washCertificateUrl->value(), $this->status->value() - )); + ); } catch (Exception $e) { // Previously this bare call would crash the entire // notifyNewBooking() flow if Slack returned non-2xx, so diff --git a/services/nginx/app/tests/Unit/Slack/SlackNewBookingPickupFilterTest.php b/services/nginx/app/tests/Unit/Slack/SlackNewBookingPickupFilterTest.php new file mode 100644 index 00000000..7f206e43 --- /dev/null +++ b/services/nginx/app/tests/Unit/Slack/SlackNewBookingPickupFilterTest.php @@ -0,0 +1,157 @@ + use $this->webhook, '' => empty, etc. + public string $sendResult = 'Message sent successfully. Response: ok'; + + public function __construct() + { + // Skip parent config loading for unit isolation. + } + + protected function get_department_webhook(int $department_id): string + { + return $this->webhookOverride ?? $this->webhook; + } + + public function send_webhook_message(string $message, string $webhook): string + { + $this->messages[] = [ + 'message' => $message, + 'webhook' => $webhook, + ]; + + return $this->sendResult; + } + + /** + * Stub format_new_booking so unit tests don't need a live redis/db + * (the real implementation calls departments_o::getDepartmentName, + * which dereferences the global `redis` object that is not loaded + * in the unit test bootstrap). + */ + public function format_new_booking( + $id, + $customer_number, + string $wash_type, + string $contact_email, + string $reference_number, + string $regNrTraekker, + string $regNrTrailer, + string $washCertificateEmail, + string $date, + int $department, + $pickup_bool, + string $notes, + string $washCertificateStatus, + string $washCertificateUrl, + string $status + ): string { + $pickupLabel = $pickup_bool ? '1' : '0'; + return "*Ny booking oprettet* ( ID: {$id} )\n" + . "Kunde: ({$customer_number})\n" + . "Type: {$wash_type}\n" + . "Reference nummer: {$reference_number}\n" + . "RegNr Traekker: {$regNrTraekker}\n" + . "RegNr Trailer: {$regNrTrailer}\n" + . "Dato: {$date}\n" + . "Hentning: {$pickupLabel}\n" + . "Noter: {$notes}"; + } +} + +/** + * Sample booking data used by all the tests below. + */ +function tru106_sample_booking(): array +{ + return [ + 'id' => 4242, + 'customer_number' => 1001, + 'wash_type' => 'Standard wash', + 'contact_email' => 'dispatcher@example.com', + 'reference_number' => 'REF-001', + 'regNrTraekker' => 'AB12345', + 'regNrTrailer' => 'CD67890', + 'washCertificateEmail' => '', + 'date' => '2026-08-16 09:00:00', + 'department' => 4, + 'notes' => 'No notes', + 'washCertificateStatus' => '', + 'washCertificateUrl' => '', + 'status' => 'pending', + ]; +} + +function tru106_call_send_new_booking_notification(slack $slack, array $b, bool $pickup): bool +{ + return $slack->send_new_booking_notification( + $b['id'], + $b['customer_number'], + $b['wash_type'], + $b['contact_email'], + $b['reference_number'], + $b['regNrTraekker'], + $b['regNrTrailer'], + $b['washCertificateEmail'], + $b['date'], + $b['department'], + $pickup, + $b['notes'], + $b['washCertificateStatus'], + $b['washCertificateUrl'], + $b['status'] + ); +} + +it('posts a Slack notification when the new booking is a pickup (TRU-106)', function (): void { + $slack = new SlackNewBookingPickupFilterFake(); + $sent = tru106_call_send_new_booking_notification($slack, tru106_sample_booking(), true); + + expect($sent)->toBeTrue() + ->and($slack->messages)->toHaveCount(1) + ->and($slack->messages[0]['webhook'])->toBe('https://hooks.slack.test/services/TRU-106-pickup-filter') + ->and($slack->messages[0]['message'])->toContain('Ny booking oprettet') + ->and($slack->messages[0]['message'])->toContain('ID: 4242') + ->and($slack->messages[0]['message'])->toContain('Kunde:') + ->and(json_encode($slack->get_log(), JSON_UNESCAPED_SLASHES))->toContain('sent successfully'); +}); + +it('does NOT post a Slack notification when the new booking is a drop-off (TRU-106)', function (): void { + $slack = new SlackNewBookingPickupFilterFake(); + $sent = tru106_call_send_new_booking_notification($slack, tru106_sample_booking(), false); + + expect($sent)->toBeFalse() + ->and($slack->messages)->toBe([]) + ->and($slack->get_log())->toBe([]); +}); + +it('does NOT post a Slack notification when the department has no webhook configured (TRU-106)', function (): void { + $slack = new SlackNewBookingPickupFilterFake(); + $slack->webhookOverride = ''; + $sent = tru106_call_send_new_booking_notification($slack, tru106_sample_booking(), true); + + expect($sent)->toBeFalse() + ->and($slack->messages)->toBe([]) + ->and($slack->get_log())->toBe([]); +}); + +it('does not leak the webhook URL into the log payload for a pickup (TRU-106)', function (): void { + $slack = new SlackNewBookingPickupFilterFake(); + tru106_call_send_new_booking_notification($slack, tru106_sample_booking(), true); + + $logDump = json_encode($slack->get_log(), JSON_UNESCAPED_SLASHES); + expect($logDump)->not->toContain('hooks.slack.test') + ->and($logDump)->toContain('sent successfully'); +});