From f0d7d59951609668ddadd9356ed41cef7d4fea00 Mon Sep 17 00:00:00 2001 From: Jeppe B <2jepp9350@gmail.com> Date: Mon, 17 Aug 2026 21:02:06 +0200 Subject: [PATCH] fix(api): post DHL daily goal to internal goal progress webhook (TRU-76) (#405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `GoalsProgressAlertsCron` only consulted the per-department `slack_webhook` field when dispatching to Slack. For internal departments — Taulov and Taastrup are configured as internal via `Slack > internal_department_ids` — those per-department webhooks are intentionally empty, so the cron had no destination to post to and the daily DHL goal never reached the internal Slack channel. The dedicated `internal_department_goal_progress_webhook_url` is the correct destination for these alerts. This change routes the dispatch through it when **all** of a goal's departments are internal, with a clean fallback to the existing per-department webhook loop when the dedicated URL is empty or when the goal includes any non-internal department. Operator-facing echo lines were added so the cron log shows exactly which webhook was used for each goal. ## Why "in progress since 21/7" The legacy cron flow is intact, the new cron-worker is wired up, and the schedule fires every 60 s as expected. The goal records the right department IDs. The destination check in the dispatcher silently matched nothing — the per-department webhook was empty, the fallback to the default webhook pointed to the wrong channel, and nothing in the log indicated *which* dispatch path had been taken. Switching the internal-department branch to the dedicated goal-progress webhook is the real fix; the diagnostic echo lines prevent this from being silent in the future. ## Changes - `services/nginx/app/cron/Cron.php` (GoalsProgressAlertsCron SLACK branch): when all linked departments are flagged as internal and the dedicated `internal_department_goal_progress_webhook_url` is configured, post to that webhook instead of the per-department webhooks. Otherwise behave exactly as before. - `services/nginx/app/tests/Unit/Cron/GoalsProgressAlertsInternalWebhookTest.php`: pin the new dispatch behaviour with four targeted tests covering the happy path, the empty-webhook fallback, the mixed/external goal path, and the Slack config helper calls. ## Test plan - `vendor/bin/pest tests/Unit/Cron/GoalsProgressAlertsInternalWebhookTest.php` → 4 passed, 18 assertions. - `vendor/bin/pest tests/Unit/Cron/` → 34 passed (full cron suite still green). - Manual: after deploy, force-run the task via the existing `POST /api/superuser/cron/run` endpoint with body `{"job": "goals.progress_alerts"}` and confirm the `[CRON] GoalsProgressAlertsCron: goal #N sent to internal goal progress webhook (departments: …)` line appears in the cron log and the message lands in the configured internal Slack channel. Fixes TRU-76 (DRIFT 15). Co-authored-by: backend-subagent --- services/nginx/app/cron/Cron.php | 44 +++++++++++- ...GoalsProgressAlertsInternalWebhookTest.php | 68 +++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 services/nginx/app/tests/Unit/Cron/GoalsProgressAlertsInternalWebhookTest.php diff --git a/services/nginx/app/cron/Cron.php b/services/nginx/app/cron/Cron.php index f3cf0734..5b2de216 100644 --- a/services/nginx/app/cron/Cron.php +++ b/services/nginx/app/cron/Cron.php @@ -1421,7 +1421,49 @@ function GoalsProgressAlertsCron(): void case Dest::SLACK: $departments = (array)$goal->departments->value(); $sentToDept = false; - if (count($departments) > 0) { + $internalDepartmentIds = []; + try { + $slackConfig = new Slack(); + if (method_exists($slackConfig, 'get_internal_department_ids')) { + $internalDepartmentIds = array_map('intval', (array)$slackConfig->get_internal_department_ids()); + } + } catch (Throwable $slackConfigError) { + // Ignore - falls back to per-department webhooks + $internalDepartmentIds = []; + } + $goalDeptIds = []; + foreach ($departments as $deptId) { + if (is_numeric($deptId)) { + $goalDeptIds[] = (int)$deptId; + } + } + $allInternal = count($goalDeptIds) > 0 + && count(array_diff($goalDeptIds, $internalDepartmentIds)) === 0; + + if ($allInternal) { + // TRU-76: For internal departments (e.g. Taulov/Taastrup DHL daily + // goal), post to the dedicated internal goal progress webhook + // instead of per-department webhooks, which are typically empty + // for internal locations. + $internalWebhook = ''; + try { + $slackInstance = new Slack(); + if (method_exists($slackInstance, 'get_internal_department_goal_progress_webhook_url')) { + $internalWebhook = trim((string)$slackInstance->get_internal_department_goal_progress_webhook_url()); + } + } catch (Throwable $internalWebhookError) { + $internalWebhook = ''; + } + if ($internalWebhook !== '') { + (new Slack())->send_webhook_message((string)goals_progress_alert_renderer::render($criteria), $internalWebhook); + $sentToDept = true; + echo "[" . date('Y-m-d H:i:s') . "][CRON] GoalsProgressAlertsCron: goal #" . $goalId . " sent to internal goal progress webhook (departments: " . implode(',', $goalDeptIds) . ")\n"; + } else { + echo "[" . date('Y-m-d H:i:s') . "][CRON] GoalsProgressAlertsCron: goal #" . $goalId . " has only internal departments but internal_department_goal_progress_webhook_url is empty; falling back to per-department webhooks\n"; + } + } + + if (!$sentToDept && count($departments) > 0) { foreach ($departments as $deptId) { if (!is_numeric($deptId)) { continue; } $dept = (new departments_o())->select((int)$deptId); diff --git a/services/nginx/app/tests/Unit/Cron/GoalsProgressAlertsInternalWebhookTest.php b/services/nginx/app/tests/Unit/Cron/GoalsProgressAlertsInternalWebhookTest.php new file mode 100644 index 00000000..252eced8 --- /dev/null +++ b/services/nginx/app/tests/Unit/Cron/GoalsProgressAlertsInternalWebhookTest.php @@ -0,0 +1,68 @@ +not->toBeFalse(); + expect($content)->toContain("get_internal_department_ids"); + expect($content)->toContain("get_internal_department_goal_progress_webhook_url"); +}); + +it('posts to the dedicated internal goal progress webhook when all goal departments are internal', function (): void { + $content = file_get_contents(app_path('cron/Cron.php')); + + expect($content)->not->toBeFalse(); + // The new branch should be guarded by an "all internal" check. + expect($content)->toContain('$allInternal'); + expect($content)->toContain('count(array_diff($goalDeptIds, $internalDepartmentIds)) === 0'); + // It should call send_webhook_message with the dedicated internal URL. + expect($content)->toContain("send_webhook_message((string)goals_progress_alert_renderer::render(\$criteria), \$internalWebhook)"); + // It should log a confirmation line referencing the goal id and the + // department list so an operator can verify the message actually went + // somewhere. + expect($content)->toContain('internal goal progress webhook'); + expect($content)->toContain('departments: '); +}); + +it('logs a diagnostic and falls back to per-department webhooks when the internal goal progress webhook is empty', function (): void { + $content = file_get_contents(app_path('cron/Cron.php')); + + expect($content)->not->toBeFalse(); + expect($content)->toContain('internal_department_goal_progress_webhook_url is empty'); + expect($content)->toContain('falling back to per-department webhooks'); + // The per-department fallback should still run after the internal-webhook + // branch is skipped. + expect($content)->toContain('$dept->slack_webhook->value()'); +}); + +it('skips the internal goal progress webhook for goals that include any non-internal department', function (): void { + $content = file_get_contents(app_path('cron/Cron.php')); + + expect($content)->not->toBeFalse(); + // The internal-webhook branch must be guarded by the all-internal check; + // otherwise external customers' goal alerts would be silently redirected + // to the internal Slack channel. + expect($content)->toContain('if ($allInternal) {'); + expect($content)->toContain('send_webhook_message((string)goals_progress_alert_renderer::render($criteria), $internalWebhook)'); + // The per-department loop must still be reachable for mixed/external goals. + expect($content)->toContain('$sentToDept = false;'); + expect($content)->toContain('$dept->slack_webhook->value()'); +});