diff --git a/app/__init__.py b/app/__init__.py index 41c73b6f5..a56ce9a6b 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -199,7 +199,7 @@ def create_app(application): current_app.logger.info( f"FEATURE_SOCKET_ENABLED value in __init__.py coming \ - from config is {application.config.get('FEATURED_SOCKET_ENABLED')} and \ + from config is {application.config.get('FEATURE_SOCKET_ENABLED')} and \ the ending value is {feature_socket_enabled}" ) return dict( diff --git a/app/assets/javascripts/job-status-polling.js b/app/assets/javascripts/job-status-polling.js index 47858a040..2074576de 100644 --- a/app/assets/javascripts/job-status-polling.js +++ b/app/assets/javascripts/job-status-polling.js @@ -9,14 +9,20 @@ document.addEventListener('DOMContentLoaded', function () { // Extract job info from URL path: /services/{serviceId}/jobs/{jobId} const pathParts = window.location.pathname.split('/'); - if (pathParts.length < 5 || pathParts[1] !== 'services' || pathParts[3] !== 'jobs') return; + if ( + pathParts.length < 5 || + pathParts[1] !== 'services' || + pathParts[3] !== 'jobs' + ) + return; const serviceId = pathParts[2]; const jobId = pathParts[4]; // Validate service and job IDs to prevent path injection function isValidUuid(id) { - const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + const uuidRegex = + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; return uuidRegex.test(id); } @@ -40,8 +46,8 @@ document.addEventListener('DOMContentLoaded', function () { MAX_INTERVAL_MS, Math.max( MIN_INTERVAL_MS, - Math.floor((250 * Math.sqrt(responseTime)) - 1000) - ) + Math.floor(250 * Math.sqrt(responseTime) - 1000), + ), ); } @@ -57,7 +63,9 @@ document.addEventListener('DOMContentLoaded', function () { const data = await response.json(); // Update notifications container if it exists - const notificationsContainer = document.querySelector('[data-key="notifications"]'); + const notificationsContainer = document.querySelector( + '[data-key="notifications"]', + ); if (notificationsContainer && data.notifications) { notificationsContainer.innerHTML = data.notifications; } @@ -87,7 +95,8 @@ document.addEventListener('DOMContentLoaded', function () { const countsContainer = document.querySelector('[data-key="counts"]'); if (countsContainer) { // Get all big-number elements in order: total, pending, delivered, failed - const countElements = countsContainer.querySelectorAll('.big-number-number'); + const countElements = + countsContainer.querySelectorAll('.big-number-number'); if (countElements.length >= 4) { if (data.total_count !== undefined) { @@ -116,7 +125,11 @@ document.addEventListener('DOMContentLoaded', function () { // Update notifications conditionally: // 1. If we have new messages and still under 50 total // 2. Always when job is finished - if (processedCount > lastProcessedCount && processedCount <= 50 && !data.finished) { + if ( + processedCount > lastProcessedCount && + processedCount <= 50 && + !data.finished + ) { // Update notifications for first 50 messages to show early results await updateNotifications(); lastProcessedCount = processedCount; @@ -126,7 +139,6 @@ document.addEventListener('DOMContentLoaded', function () { await updateNotifications(); stopPolling(); } - } catch (error) { if (retryCount < 3) { console.debug(`Job polling retry ${retryCount}`, error.message); @@ -143,7 +155,7 @@ document.addEventListener('DOMContentLoaded', function () { error: error.message, url: pollStatusUrl, jobId: jobId, - timestamp: new Date().toISOString() + timestamp: new Date().toISOString(), }); currentInterval = Math.min(currentInterval * 2, MAX_INTERVAL_MS); } finally { diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index da562dc3a..6f736d702 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import json import os +import time from functools import partial from flask import ( @@ -21,6 +22,7 @@ from markupsafe import Markup from app import ( current_service, format_datetime_table, + job_api_client, notification_api_client, service_api_client, ) @@ -111,35 +113,42 @@ def cancel_job(service_id, job_id): @user_has_permissions() def view_job_status_poll(service_id, job_id): """ - Poll status endpoint that only queries jobs table. + Poll status endpoint using new lightweight cached API endpoint. Returns minimal data needed for polling. """ - import time - start_time = time.time() - job = Job.from_id(job_id, service_id=service_id) + # Use new lightweight status endpoint + try: + status_data = job_api_client.get_job_status(service_id, job_id) - processed_count = job.notifications_delivered + job.notifications_failed - total_count = job.notification_count + # Validate the response has expected fields + required_fields = ["sent_count", "failed_count", "pending_count", "total_count", "processing_finished"] + if all(key in status_data for key in required_fields): + response_data = { + "sent_count": status_data["sent_count"], + "failed_count": status_data["failed_count"], + "pending_count": status_data["pending_count"], + "total_count": status_data["total_count"], + "finished": status_data["processing_finished"], + } + processed_count = status_data["sent_count"] + status_data["failed_count"] + else: + current_app.logger.error(f"Status endpoint returned invalid response for job {job_id[:8]}: {status_data}") + abort(500, "Invalid status response from API") - response_data = { - "sent_count": job.notifications_delivered, - "failed_count": job.notifications_failed, - "pending_count": job.notifications_sending, - "total_count": total_count, - "finished": job.finished_processing, - } + except Exception as e: + current_app.logger.error(f"Status endpoint failed for job {job_id[:8]}: {e}") + abort(500, "Status endpoint unavailable") response_time_ms = round((time.time() - start_time) * 1000, 2) response_json = json.dumps(response_data) response_size_bytes = len(response_json.encode("utf-8")) - current_app.logger.info( f"Poll status request - job_id={job_id[:8]} " f"response_size={response_size_bytes}b " f"response_time={response_time_ms}ms " - f"progress={processed_count}/{total_count}" + f"progress={processed_count}/{response_data['total_count']}" ) return jsonify(response_data) diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index c9a02a1f2..ac473d370 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -32,6 +32,9 @@ class JobApiClient(NotifyAdminAPIClient): return job + def get_job_status(self, service_id, job_id): + return self.get(url=f"/service/{service_id}/job/{job_id}/status") + def get_jobs( self, service_id, diff --git a/tests/app/main/views/test_job_notification_updates.py b/tests/app/main/views/test_job_notification_updates.py index a06d9f03a..1b2e0120a 100644 --- a/tests/app/main/views/test_job_notification_updates.py +++ b/tests/app/main/views/test_job_notification_updates.py @@ -16,7 +16,14 @@ from tests import job_json, user_json @pytest.mark.parametrize( - ("delivered", "failed", "pending", "finished", "js_should_update_notifications", "reason"), + ( + "delivered", + "failed", + "pending", + "finished", + "js_should_update_notifications", + "reason", + ), [ (20, 10, 70, False, True, "30 messages processed (≤50 threshold)"), (40, 10, 50, False, True, "50 messages processed (exactly at threshold)"), @@ -134,7 +141,13 @@ def test_poll_status_provides_required_fields( data = json.loads(response.get_data(as_text=True)) - required_fields = {"sent_count", "failed_count", "finished", "pending_count", "total_count"} + required_fields = { + "sent_count", + "failed_count", + "finished", + "pending_count", + "total_count", + } assert set(data.keys()) == required_fields response_size = len(response.get_data(as_text=True)) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 8dc508444..3f1db6f5f 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -510,23 +510,13 @@ def test_poll_status_endpoint( fake_uuid, ): """Test that the poll status endpoint returns only required data without notifications""" - mock_job = mocker.patch("app.job_api_client.get_job") - mock_job.return_value = { - "data": { - **job_json( - service_one["id"], - created_by=user_json(), - job_id=fake_uuid, - job_status="finished", - notification_count=100, - notifications_requested=100, - ), - "statistics": [ - {"status": "delivered", "count": 90}, - {"status": "failed", "count": 10}, - {"status": "pending", "count": 0}, - ], - } + mock_job_status = mocker.patch("app.job_api_client.get_job_status") + mock_job_status.return_value = { + "sent_count": 90, + "failed_count": 10, + "pending_count": 0, + "total_count": 100, + "processing_finished": True, } response = client_request.get_response( @@ -563,19 +553,13 @@ def test_poll_status_with_zero_notifications( fake_uuid, ): """Test poll status endpoint handles edge case of no notifications""" - mock_job = mocker.patch("app.job_api_client.get_job") - mock_job.return_value = { - "data": { - **job_json( - service_one["id"], - created_by=user_json(), - job_id=fake_uuid, - job_status="pending", - notification_count=0, - notifications_requested=0, - ), - "statistics": [], - } + mock_job_status = mocker.patch("app.job_api_client.get_job_status") + mock_job_status.return_value = { + "sent_count": 0, + "failed_count": 0, + "pending_count": 0, + "total_count": 0, + "processing_finished": True, } response = client_request.get_response( @@ -588,9 +572,7 @@ def test_poll_status_with_zero_notifications( data = json.loads(response.get_data(as_text=True)) assert data["total_count"] == 0 - assert ( - data["finished"] is True - ) + assert data["finished"] is True def test_poll_status_endpoint_does_not_query_notifications_table( @@ -602,23 +584,13 @@ def test_poll_status_endpoint_does_not_query_notifications_table( fake_uuid, ): """Critical regression test: ensure poll status endpoint never queries notifications""" - mock_job = mocker.patch("app.job_api_client.get_job") - mock_job.return_value = { - "data": { - **job_json( - service_one["id"], - created_by=user_json(), - job_id=fake_uuid, - job_status="sending", - notification_count=500, - notifications_requested=500, - ), - "statistics": [ - {"status": "delivered", "count": 300}, - {"status": "failed", "count": 50}, - {"status": "pending", "count": 150}, - ], - } + mock_job_status = mocker.patch("app.job_api_client.get_job_status") + mock_job_status.return_value = { + "sent_count": 300, + "failed_count": 50, + "pending_count": 150, + "total_count": 500, + "processing_finished": False, } mock_get_notifications = mocker.patch( @@ -639,3 +611,6 @@ def test_poll_status_endpoint_does_not_query_notifications_table( data = json.loads(response.get_data(as_text=True)) assert data["total_count"] == 500 assert data["sent_count"] == 300 + assert data["failed_count"] == 50 + assert data["pending_count"] == 150 + assert data["finished"] is False