From 557d85f57d9b960cc1dad77aa2236d4eb8559016 Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Thu, 25 Sep 2025 16:26:00 -0400 Subject: [PATCH 1/9] Optimizing polling --- .ds.baseline | 4 +- .gitignore | 1 + app/assets/javascripts/job-status-polling.js | 186 ++++++++++++++++++ app/assets/javascripts/socketio.js | 132 ------------- app/config.py | 4 +- app/main/views/jobs.py | 46 ++++- app/main/views/organizations.py | 11 +- app/main/views/templates.py | 12 +- app/templates/views/jobs/job.html | 5 +- app/utils/login.py | 2 +- gulpfile.js | 2 +- .../views/test_job_notification_updates.py | 136 +++++++++++++ tests/app/main/views/test_jobs.py | 140 +++++++++++++ tests/app/main/views/test_template_folders.py | 4 +- tests/app/test_navigation.py | 1 + 15 files changed, 523 insertions(+), 163 deletions(-) create mode 100644 app/assets/javascripts/job-status-polling.js delete mode 100644 app/assets/javascripts/socketio.js create mode 100644 tests/app/main/views/test_job_notification_updates.py diff --git a/.ds.baseline b/.ds.baseline index 5afd27402..78a916432 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -161,7 +161,7 @@ "filename": "app/config.py", "hashed_secret": "577a4c667e4af8682ca431857214b3a920883efc", "is_verified": false, - "line_number": 124, + "line_number": 122, "is_secret": false } ], @@ -634,5 +634,5 @@ } ] }, - "generated_at": "2025-09-24T22:03:22Z" + "generated_at": "2025-09-25T20:25:57Z" } diff --git a/.gitignore b/.gitignore index 09973207f..ba86027ad 100644 --- a/.gitignore +++ b/.gitignore @@ -85,6 +85,7 @@ coverage/ coverage.xml test_results.xml *,cover +.hypothesis/ # Translations *.mo diff --git a/app/assets/javascripts/job-status-polling.js b/app/assets/javascripts/job-status-polling.js new file mode 100644 index 000000000..47858a040 --- /dev/null +++ b/app/assets/javascripts/job-status-polling.js @@ -0,0 +1,186 @@ +document.addEventListener('DOMContentLoaded', function () { + // Verify we are on the job page + const isJobPage = window.location.pathname.includes('/jobs/'); + if (!isJobPage) return; + + // Check if polling elements exist + const hasPollingElements = document.querySelector('[data-key="counts"]'); + if (!hasPollingElements) return; + + // 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; + + 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; + return uuidRegex.test(id); + } + + // Validate both IDs are UUIDs to prevent path injection attacks + if (!isValidUuid(serviceId) || !isValidUuid(jobId)) { + console.warn('Invalid service or job ID format detected'); + return; + } + + const DEFAULT_INTERVAL_MS = 10000; + const MIN_INTERVAL_MS = 1000; + const MAX_INTERVAL_MS = 30000; + + let pollInterval; + let currentInterval = DEFAULT_INTERVAL_MS; + let isPolling = false; + let lastProcessedCount = 0; + + function calculateBackoff(responseTime) { + return Math.min( + MAX_INTERVAL_MS, + Math.max( + MIN_INTERVAL_MS, + Math.floor((250 * Math.sqrt(responseTime)) - 1000) + ) + ); + } + + async function updateNotifications() { + const notificationsUrl = `/services/${serviceId}/jobs/${jobId}.json`; + + try { + const response = await fetch(notificationsUrl); + if (!response.ok) { + throw new Error(`Failed to fetch notifications: ${response.status}`); + } + + const data = await response.json(); + + // Update notifications container if it exists + const notificationsContainer = document.querySelector('[data-key="notifications"]'); + if (notificationsContainer && data.notifications) { + notificationsContainer.innerHTML = data.notifications; + } + } catch (error) { + console.warn('Failed to update notifications:', error.message); + } + } + + async function updateAllJobSections(retryCount = 0) { + if (isPolling || document.hidden) { + return; + } + + isPolling = true; + + const pollStatusUrl = `/services/${serviceId}/jobs/${jobId}/status.json`; + + try { + const response = await fetch(pollStatusUrl); + + if (!response.ok) { + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + } + + const data = await response.json(); + + 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'); + + if (countElements.length >= 4) { + if (data.total_count !== undefined) { + countElements[0].textContent = data.total_count.toLocaleString(); + } + + if (data.pending_count !== undefined) { + countElements[1].textContent = data.pending_count.toLocaleString(); + } + + if (data.sent_count !== undefined) { + countElements[2].textContent = data.sent_count.toLocaleString(); + } + + if (data.failed_count !== undefined) { + countElements[3].textContent = data.failed_count.toLocaleString(); + } + } + } + + currentInterval = calculateBackoff(DEFAULT_INTERVAL_MS); + + // Calculate how many messages have been processed + const processedCount = (data.sent_count || 0) + (data.failed_count || 0); + + // 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) { + // Update notifications for first 50 messages to show early results + await updateNotifications(); + lastProcessedCount = processedCount; + } + + if (data.finished === true) { + await updateNotifications(); + stopPolling(); + } + + } catch (error) { + if (retryCount < 3) { + console.debug(`Job polling retry ${retryCount}`, error.message); + isPolling = false; + + const retryDelay = Math.pow(2, retryCount) * 1000; + setTimeout(() => { + updateAllJobSections(retryCount + 1); + }, retryDelay); + return; + } + + console.warn('Job polling failed after 3 retries:', { + error: error.message, + url: pollStatusUrl, + jobId: jobId, + timestamp: new Date().toISOString() + }); + currentInterval = Math.min(currentInterval * 2, MAX_INTERVAL_MS); + } finally { + isPolling = false; + } + } + + function startPolling() { + updateAllJobSections(); + + function scheduleNext() { + if (pollInterval) clearTimeout(pollInterval); + pollInterval = setTimeout(() => { + updateAllJobSections(); + scheduleNext(); + }, currentInterval); + } + + scheduleNext(); + } + + function stopPolling() { + if (pollInterval) { + clearTimeout(pollInterval); + pollInterval = null; + } + } + + document.addEventListener('visibilitychange', () => { + if (document.hidden) { + stopPolling(); + } else { + startPolling(); + } + }); + + window.addEventListener('beforeunload', stopPolling); + + startPolling(); +}); diff --git a/app/assets/javascripts/socketio.js b/app/assets/javascripts/socketio.js deleted file mode 100644 index 77d6a1028..000000000 --- a/app/assets/javascripts/socketio.js +++ /dev/null @@ -1,132 +0,0 @@ -document.addEventListener('DOMContentLoaded', function () { - const isJobPage = window.location.pathname.includes('/jobs/'); - if (!isJobPage) return; - - const jobEl = document.querySelector('[data-job-id]'); - const jobId = jobEl?.dataset?.jobId; - const featureEnabled = jobEl?.dataset?.feature === 'true'; - const apiHost = jobEl?.dataset?.host; - - if (!jobId || !featureEnabled) return; - - const DEFAULT_INTERVAL_MS = 10000; - const MIN_INTERVAL_MS = 1000; - const MAX_INTERVAL_MS = 30000; - - let pollInterval; - let currentInterval = DEFAULT_INTERVAL_MS; - let isPolling = false; - - function calculateBackoff(responseTime) { - return Math.min( - MAX_INTERVAL_MS, - Math.max( - MIN_INTERVAL_MS, - Math.floor((250 * Math.sqrt(responseTime)) - 1000) - ) - ); - } - - async function updateAllJobSections(retryCount = 0) { - if (isPolling || document.hidden) { - return; - } - - isPolling = true; - const startTime = Date.now(); - - const resourceEl = document.querySelector('[data-socket-update="status"]'); - const url = resourceEl?.dataset?.resource; - - if (!url) { - isPolling = false; - return; - } - - try { - const response = await fetch(url); - if (!response.ok) { - throw new Error(`HTTP ${response.status}: ${response.statusText}`); - } - const data = await response.json(); - - const sections = { - status: document.querySelector('[data-socket-update="status"]'), - counts: document.querySelector('[data-socket-update="counts"]'), - notifications: document.querySelector('[data-socket-update="notifications"]'), - }; - - if (data.status && sections.status) { - sections.status.innerHTML = data.status; - } - if (data.counts && sections.counts) { - sections.counts.innerHTML = data.counts; - } - if (data.notifications && sections.notifications) { - sections.notifications.innerHTML = data.notifications; - } - - const responseTime = Date.now() - startTime; - currentInterval = calculateBackoff(responseTime); - - if (data.finished === true) { - stopPolling(); - } - - } catch (error) { - if (retryCount < 3) { - console.debug(`Job polling retry ${retryCount}`, error.message); - isPolling = false; - - const retryDelay = Math.pow(2, retryCount) * 1000; - setTimeout(() => { - updateAllJobSections(retryCount + 1); - }, retryDelay); - return; - } - - console.warn('Job polling failed after 3 retries:', { - error: error.message, - url: url, - jobId: jobId, - timestamp: new Date().toISOString() - }); - currentInterval = Math.min(currentInterval * 2, MAX_INTERVAL_MS); - } finally { - isPolling = false; - } - } - - function startPolling() { - updateAllJobSections(); - - function scheduleNext() { - if (pollInterval) clearTimeout(pollInterval); - pollInterval = setTimeout(() => { - updateAllJobSections(); - scheduleNext(); - }, currentInterval); - } - - scheduleNext(); - } - - function stopPolling() { - if (pollInterval) { - clearTimeout(pollInterval); - pollInterval = null; - } - } - - document.addEventListener('visibilitychange', () => { - if (document.hidden) { - stopPolling(); - } else { - startPolling(); - } - }); - - window.addEventListener('beforeunload', stopPolling); - - startPolling(); -}); diff --git a/app/config.py b/app/config.py index 556d455e5..6fc83946f 100644 --- a/app/config.py +++ b/app/config.py @@ -90,9 +90,7 @@ class Config(object): ], } - # TODO FIX!!! - # FEATURE_SOCKET_ENABLED = getenv("FEATURE_SOCKET_ENABLED", "true") == "true" - FEATURE_SOCKET_ENABLED = False + FEATURE_SOCKET_ENABLED = getenv("FEATURE_SOCKET_ENABLED", "true") == "true" def _s3_credentials_from_env(bucket_prefix): diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 263f04e56..da562dc3a 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +import json import os from functools import partial @@ -67,12 +68,6 @@ def view_job(service_id, job_id): FEATURE_SOCKET_ENABLED=current_app.config["FEATURE_SOCKET_ENABLED"], job=job, status=request.args.get("status", ""), - updates_url=url_for( - ".view_job_updates", - service_id=service_id, - job_id=job.id, - status=request.args.get("status", ""), - ), partials=get_job_partials(job), ) @@ -112,11 +107,48 @@ def cancel_job(service_id, job_id): return redirect(url_for("main.service_dashboard", service_id=service_id)) +@main.route("/services//jobs//status.json") +@user_has_permissions() +def view_job_status_poll(service_id, job_id): + """ + Poll status endpoint that only queries jobs table. + Returns minimal data needed for polling. + """ + import time + + start_time = time.time() + + job = Job.from_id(job_id, service_id=service_id) + + processed_count = job.notifications_delivered + job.notifications_failed + total_count = job.notification_count + + 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, + } + + 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}" + ) + + return jsonify(response_data) + + @main.route("/services//jobs/.json") @user_has_permissions() def view_job_updates(service_id, job_id): job = Job.from_id(job_id, service_id=service_id) - return jsonify(**get_job_partials(job)) diff --git a/app/main/views/organizations.py b/app/main/views/organizations.py index 5ed38db1d..3c1e447ff 100644 --- a/app/main/views/organizations.py +++ b/app/main/views/organizations.py @@ -92,7 +92,11 @@ def organization_dashboard(org_id): def download_organization_usage_report(org_id): selected_year_input = request.args.get("selected_year") # Validate selected_year to prevent header injection - if selected_year_input and selected_year_input.isdigit() and len(selected_year_input) == 4: + if ( + selected_year_input + and selected_year_input.isdigit() + and len(selected_year_input) == 4 + ): selected_year = selected_year_input else: selected_year = str(datetime.now().year) @@ -128,8 +132,9 @@ def download_organization_usage_report(org_id): # Sanitize organization name for filename to prevent header injection import re - safe_org_name = re.sub(r'[^\w\s-]', '', current_organization.name).strip() - safe_org_name = re.sub(r'[-\s]+', '-', safe_org_name) + + safe_org_name = re.sub(r"[^\w\s-]", "", current_organization.name).strip() + safe_org_name = re.sub(r"[-\s]+", "-", safe_org_name) return ( Spreadsheet.from_rows(org_usage_data).as_csv_data, diff --git a/app/main/views/templates.py b/app/main/views/templates.py index bd5895315..518dd1207 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -198,16 +198,14 @@ def process_folder_management_form(form, current_folder_id): # Use request.full_path which includes query string but not host # This avoids host header injection while preserving all parameters # Hardened redirect: only allow relative URLs, and strip any backslashes - target = request.full_path.replace('\\', '') + target = request.full_path.replace("\\", "") parts = urlparse(target) - if not parts.scheme and not parts.netloc and target.startswith('/'): + if not parts.scheme and not parts.netloc and target.startswith("/"): return redirect(target) # Fallback to main template list for this service - return redirect(url_for( - '.choose_template', - service_id=current_service.id, - template_type='all' - )) + return redirect( + url_for(".choose_template", service_id=current_service.id, template_type="all") + ) def get_template_nav_label(value): diff --git a/app/templates/views/jobs/job.html b/app/templates/views/jobs/job.html index 38ff39162..755f6b3bc 100644 --- a/app/templates/views/jobs/job.html +++ b/app/templates/views/jobs/job.html @@ -14,10 +14,9 @@ {% if not job.finished_processing %}

This page refreshes automatically to show the latest message activity delivery rates, details, and reports.
You can watch it in progress or check back later.

{% endif %} -
+
{% if not job.finished_processing and FEATURE_SOCKET_ENABLED %}
{% if not job.processing_finished and FEATURE_SOCKET_ENABLED %}
{ paths.src + 'javascripts/activityChart.js', paths.src + 'javascripts/sidenav.js', paths.src + 'javascripts/validation.js', - paths.src + 'javascripts/socketio.js', + paths.src + 'javascripts/job-status-polling.js', paths.src + 'javascripts/scrollPosition.js', ]) .pipe(plugins.prettyerror()) diff --git a/tests/app/main/views/test_job_notification_updates.py b/tests/app/main/views/test_job_notification_updates.py new file mode 100644 index 000000000..df5028fd8 --- /dev/null +++ b/tests/app/main/views/test_job_notification_updates.py @@ -0,0 +1,136 @@ +""" +Tests for job notification update logic during polling. + +These tests verify the poll status endpoint behavior and document +the JavaScript notification refresh logic: +1. Notifications update for first 50 messages +2. Notifications stop updating after 50 messages (to prevent performance issues) +3. Notifications always update when job finishes +""" + +import json + +import pytest + +from tests import job_json, user_json + + +@pytest.mark.parametrize( + "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)"), + (45, 15, 40, False, False, "60 messages processed (>50 threshold)"), + (450, 50, 0, True, True, "500 messages but job finished (always updates)"), + ], +) +def test_poll_status_notification_update_logic( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_data_retention, + mocker, + fake_uuid, + delivered, + failed, + pending, + finished, + js_should_update_notifications, + reason, +): + """ + Test poll status endpoint for various scenarios. + + The JavaScript updates notifications when: + processedCount ≤ 50 AND job not finished + job is finished (regardless of count) + """ + total = delivered + failed + pending + job_status = "finished" if finished else "sending" + + 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=job_status, + notification_count=total, + notifications_requested=total, + ), + "statistics": [ + {"status": "delivered", "count": delivered}, + {"status": "failed", "count": failed}, + {"status": "pending", "count": pending}, + ], + } + } + + response = client_request.get_response( + "main.view_job_status_poll", + service_id=service_one["id"], + job_id=fake_uuid, + ) + + assert response.status_code == 200 + data = json.loads(response.get_data(as_text=True)) + + # Verify the response + assert data["sent_count"] == delivered + assert data["failed_count"] == failed + assert data["pending_count"] == pending + assert data["total_count"] == total + assert data["finished"] is finished + + processed_count = delivered + failed + + if js_should_update_notifications: + assert (processed_count <= 50 and not finished) or finished, \ + f"JS updates notifications: {reason}" + else: + assert processed_count > 50 and not finished, \ + f"JS skips notification update: {reason}" + + +def test_poll_status_provides_required_fields( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_data_retention, + mocker, + fake_uuid, +): + """Verify poll status endpoint returns all fields needed for notification update logic.""" + 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=25, + notifications_requested=25, + ), + "statistics": [ + {"status": "delivered", "count": 15}, + {"status": "failed", "count": 5}, + {"status": "pending", "count": 5}, + ], + } + } + + response = client_request.get_response( + "main.view_job_status_poll", + service_id=service_one["id"], + job_id=fake_uuid, + ) + + data = json.loads(response.get_data(as_text=True)) + + 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)) + assert response_size < 200, f"Response too large: {response_size} bytes" diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index dd04ec348..8dc508444 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -499,3 +499,143 @@ def test_should_show_message_note( 'Messages are sent immediately to the cell phone carrier, but will remain in "pending" status until we hear ' "back from the carrier they have received it and attempted deliver. More information on delivery status." ) + + +def test_poll_status_endpoint( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_data_retention, + mocker, + 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}, + ], + } + } + + response = client_request.get_response( + "main.view_job_status_poll", + service_id=service_one["id"], + job_id=fake_uuid, + ) + + assert response.status_code == 200 + data = json.loads(response.get_data(as_text=True)) + + expected_keys = { + "sent_count", + "failed_count", + "pending_count", + "total_count", + "finished", + } + assert set(data.keys()) == expected_keys + + assert data["sent_count"] == 90 + assert data["failed_count"] == 10 + assert data["pending_count"] == 0 + assert data["total_count"] == 100 + assert data["finished"] is True + + +def test_poll_status_with_zero_notifications( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_data_retention, + mocker, + 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": [], + } + } + + response = client_request.get_response( + "main.view_job_status_poll", + service_id=service_one["id"], + job_id=fake_uuid, + ) + + assert response.status_code == 200 + data = json.loads(response.get_data(as_text=True)) + + assert data["total_count"] == 0 + assert ( + data["finished"] is True + ) + + +def test_poll_status_endpoint_does_not_query_notifications_table( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_data_retention, + mocker, + 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_get_notifications = mocker.patch( + "app.notification_api_client.get_notifications_for_service" + ) + + response = client_request.get_response( + "main.view_job_status_poll", + service_id=service_one["id"], + job_id=fake_uuid, + ) + + assert response.status_code == 200 + + # Verify no notifications were fetched + mock_get_notifications.assert_not_called() + + data = json.loads(response.get_data(as_text=True)) + assert data["total_count"] == 500 + assert data["sent_count"] == 300 diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 19ad5f935..e72bc7ff6 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -1533,9 +1533,7 @@ def test_should_be_able_to_move_to_new_folder( ], }, _expected_status=302, - _expected_redirect=url_for( - "main.choose_template", service_id=SERVICE_ONE_ID - ), + _expected_redirect=url_for("main.choose_template", service_id=SERVICE_ONE_ID), ) mock_create_template_folder.assert_called_once_with( diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 4e9ef54c0..7d00769b9 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -242,6 +242,7 @@ EXCLUDED_ENDPOINTS = tuple( "verify_email", "view_job", "view_job_csv", + "view_job_status_poll", "view_job_updates", "view_jobs", "view_notification", From 4f92b1908428ab80c58c59649631d70e9dead1fa Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Thu, 25 Sep 2025 16:59:00 -0400 Subject: [PATCH 2/9] Fixed formatting issue --- .../main/views/test_job_notification_updates.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/app/main/views/test_job_notification_updates.py b/tests/app/main/views/test_job_notification_updates.py index df5028fd8..a06d9f03a 100644 --- a/tests/app/main/views/test_job_notification_updates.py +++ b/tests/app/main/views/test_job_notification_updates.py @@ -16,7 +16,7 @@ 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)"), @@ -86,11 +86,16 @@ def test_poll_status_notification_update_logic( processed_count = delivered + failed if js_should_update_notifications: - assert (processed_count <= 50 and not finished) or finished, \ - f"JS updates notifications: {reason}" + # JavaScript would call: await updateNotifications() + if finished: + assert finished, f"JS updates notifications: {reason}" + else: + assert processed_count <= 50, f"JS updates notifications: {reason}" + assert not finished, f"JS updates notifications: {reason}" else: - assert processed_count > 50 and not finished, \ - f"JS skips notification update: {reason}" + # JavaScript would NOT update notifications + assert processed_count > 50, f"JS skips notification update: {reason}" + assert not finished, f"JS skips notification update: {reason}" def test_poll_status_provides_required_fields( From e552d95702f79ab2d7444ebb11abd2105712569d Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Fri, 26 Sep 2025 14:02:11 -0400 Subject: [PATCH 3/9] Pointed new endpoint to new backend polling endpoint --- app/__init__.py | 2 +- app/assets/javascripts/job-status-polling.js | 30 +++++--- app/main/views/jobs.py | 39 ++++++---- app/notify_client/job_api_client.py | 3 + .../views/test_job_notification_updates.py | 17 ++++- tests/app/main/views/test_jobs.py | 75 +++++++------------ 6 files changed, 89 insertions(+), 77 deletions(-) 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 From 7f270879d6ad6261337ee48a0835e5f659d7e4e0 Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Fri, 26 Sep 2025 14:36:50 -0400 Subject: [PATCH 4/9] Missed a unit test update --- .../views/test_job_notification_updates.py | 49 ++++++------------- 1 file changed, 14 insertions(+), 35 deletions(-) diff --git a/tests/app/main/views/test_job_notification_updates.py b/tests/app/main/views/test_job_notification_updates.py index a06d9f03a..fc7621928 100644 --- a/tests/app/main/views/test_job_notification_updates.py +++ b/tests/app/main/views/test_job_notification_updates.py @@ -46,25 +46,14 @@ def test_poll_status_notification_update_logic( job is finished (regardless of count) """ total = delivered + failed + pending - job_status = "finished" if finished else "sending" - 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=job_status, - notification_count=total, - notifications_requested=total, - ), - "statistics": [ - {"status": "delivered", "count": delivered}, - {"status": "failed", "count": failed}, - {"status": "pending", "count": pending}, - ], - } + mock_job_status = mocker.patch("app.job_api_client.get_job_status") + mock_job_status.return_value = { + "sent_count": delivered, + "failed_count": failed, + "pending_count": pending, + "total_count": total, + "processing_finished": finished, } response = client_request.get_response( @@ -107,23 +96,13 @@ def test_poll_status_provides_required_fields( fake_uuid, ): """Verify poll status endpoint returns all fields needed for notification update logic.""" - 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=25, - notifications_requested=25, - ), - "statistics": [ - {"status": "delivered", "count": 15}, - {"status": "failed", "count": 5}, - {"status": "pending", "count": 5}, - ], - } + mock_job_status = mocker.patch("app.job_api_client.get_job_status") + mock_job_status.return_value = { + "sent_count": 15, + "failed_count": 5, + "pending_count": 5, + "total_count": 25, + "processing_finished": False, } response = client_request.get_response( From 80065c8200af84043888ad1532599de7379c6364 Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Fri, 26 Sep 2025 14:40:31 -0400 Subject: [PATCH 5/9] Fixed flake issue --- tests/app/main/views/test_job_notification_updates.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/app/main/views/test_job_notification_updates.py b/tests/app/main/views/test_job_notification_updates.py index fc7621928..a07f4d07a 100644 --- a/tests/app/main/views/test_job_notification_updates.py +++ b/tests/app/main/views/test_job_notification_updates.py @@ -12,8 +12,6 @@ import json import pytest -from tests import job_json, user_json - @pytest.mark.parametrize( ("delivered", "failed", "pending", "finished", "js_should_update_notifications", "reason"), From 5b4a1820a94d022bb80ddd678c15691bebb248a1 Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Fri, 26 Sep 2025 16:21:50 -0400 Subject: [PATCH 6/9] Removed polling logic from status page and added button instead --- app/templates/views/jobs/job.html | 8 +++++++- gulpfile.js | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/templates/views/jobs/job.html b/app/templates/views/jobs/job.html index 755f6b3bc..c11908764 100644 --- a/app/templates/views/jobs/job.html +++ b/app/templates/views/jobs/job.html @@ -12,7 +12,13 @@ {{ page_header("Message status") }} {% if not job.finished_processing %} -

This page refreshes automatically to show the latest message activity delivery rates, details, and reports.
You can watch it in progress or check back later.

+

This page no longer refreshes automatically. Use the refresh button below to check the latest message status.

+
+ +
+

{% endif %}
{% if not job.finished_processing and FEATURE_SOCKET_ENABLED %} diff --git a/gulpfile.js b/gulpfile.js index 220627fe6..832967a07 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -79,7 +79,7 @@ const javascripts = () => { paths.src + 'javascripts/activityChart.js', paths.src + 'javascripts/sidenav.js', paths.src + 'javascripts/validation.js', - paths.src + 'javascripts/job-status-polling.js', + // paths.src + 'javascripts/job-status-polling.js', // Disabled for manual refresh mode paths.src + 'javascripts/scrollPosition.js', ]) .pipe(plugins.prettyerror()) @@ -105,6 +105,7 @@ const copySetTimezone = () => { return src(paths.src + 'js/setTimezone.js').pipe(dest(paths.dist + 'js/')); }; + // Task to copy images const copyImages = () => { return src(paths.src + 'images/**/*', { encoding: false }).pipe( From ef8914fbcd43e86ba5c1f70fa37f29886081f94f Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Fri, 26 Sep 2025 16:26:48 -0400 Subject: [PATCH 7/9] Disabled new endpoint as well --- app/main/views/jobs.py | 80 ++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 79f043c16..68d6b3d96 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -113,46 +113,50 @@ def cancel_job(service_id, job_id): @user_has_permissions() def view_job_status_poll(service_id, job_id): """ - Poll status endpoint using new lightweight cached API endpoint. - Returns minimal data needed for polling. + DISABLED: Lightweight polling endpoint no longer supported. + Use manual page refresh instead. """ - start_time = time.time() + current_app.logger.info(f"Disabled lightweight polling endpoint accessed for job {job_id[:8]} - returning 410 Gone") + abort(410, "Lightweight polling endpoint disabled. Please refresh the page manually.") - # Use new lightweight status endpoint - try: - status_data = job_api_client.get_job_status(service_id, job_id) - - # 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") - - 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}/{response_data['total_count']}" - ) - - return jsonify(response_data) + # Original polling code - commented out for manual refresh mode + # start_time = time.time() + # + # # Use new lightweight status endpoint + # try: + # status_data = job_api_client.get_job_status(service_id, job_id) + # + # # 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") + # + # 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}/{response_data['total_count']}" + # ) + # + # return jsonify(response_data) @main.route("/services//jobs/.json") From 96c9bc3e5e8057b40c3c38e8185d31e46b6120f5 Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Fri, 26 Sep 2025 16:54:35 -0400 Subject: [PATCH 8/9] More linting and changed default var to false for polling --- app/config.py | 2 +- app/main/views/jobs.py | 3 --- tests/app/main/views/test_jobs.py | 44 +++++++++---------------------- 3 files changed, 13 insertions(+), 36 deletions(-) diff --git a/app/config.py b/app/config.py index 6fc83946f..68b780290 100644 --- a/app/config.py +++ b/app/config.py @@ -90,7 +90,7 @@ class Config(object): ], } - FEATURE_SOCKET_ENABLED = getenv("FEATURE_SOCKET_ENABLED", "true") == "true" + FEATURE_SOCKET_ENABLED = getenv("FEATURE_SOCKET_ENABLED", "false") == "true" def _s3_credentials_from_env(bucket_prefix): diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 68d6b3d96..e58651f8e 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -1,7 +1,5 @@ # -*- coding: utf-8 -*- -import json import os -import time from functools import partial from flask import ( @@ -22,7 +20,6 @@ from markupsafe import Markup from app import ( current_service, format_datetime_table, - job_api_client, notification_api_client, service_api_client, ) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 3f1db6f5f..97b123f79 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -358,7 +358,11 @@ def test_should_show_scheduled_job( assert page.select("main p a")[0]["href"] == url_for( "main.message_status", ) - assert page.select_one("main button[type=submit]").text.strip() == "Cancel sending" + # Test that both buttons are present + buttons = page.select("main button[type=submit]") + button_texts = [b.text.strip() for b in buttons] + assert "Refresh Status" in button_texts + assert "Cancel sending" in button_texts def test_should_cancel_job( @@ -523,25 +527,10 @@ def test_poll_status_endpoint( "main.view_job_status_poll", service_id=service_one["id"], job_id=fake_uuid, + _expected_status=410, ) - assert response.status_code == 200 - data = json.loads(response.get_data(as_text=True)) - - expected_keys = { - "sent_count", - "failed_count", - "pending_count", - "total_count", - "finished", - } - assert set(data.keys()) == expected_keys - - assert data["sent_count"] == 90 - assert data["failed_count"] == 10 - assert data["pending_count"] == 0 - assert data["total_count"] == 100 - assert data["finished"] is True + assert response.status_code == 410 def test_poll_status_with_zero_notifications( @@ -566,13 +555,10 @@ def test_poll_status_with_zero_notifications( "main.view_job_status_poll", service_id=service_one["id"], job_id=fake_uuid, + _expected_status=410, ) - assert response.status_code == 200 - data = json.loads(response.get_data(as_text=True)) - - assert data["total_count"] == 0 - assert data["finished"] is True + assert response.status_code == 410 def test_poll_status_endpoint_does_not_query_notifications_table( @@ -601,16 +587,10 @@ def test_poll_status_endpoint_does_not_query_notifications_table( "main.view_job_status_poll", service_id=service_one["id"], job_id=fake_uuid, + _expected_status=410, ) - assert response.status_code == 200 + assert response.status_code == 410 - # Verify no notifications were fetched + # Verify no notifications were fetched (since endpoint is disabled) mock_get_notifications.assert_not_called() - - 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 From 50ed613ebffb4e8c8c56ee9bf2779382a93a93fe Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Fri, 26 Sep 2025 17:42:01 -0400 Subject: [PATCH 9/9] Test updates --- .../views/test_job_notification_updates.py | 48 +++++-------------- 1 file changed, 11 insertions(+), 37 deletions(-) diff --git a/tests/app/main/views/test_job_notification_updates.py b/tests/app/main/views/test_job_notification_updates.py index a07f4d07a..d3839b699 100644 --- a/tests/app/main/views/test_job_notification_updates.py +++ b/tests/app/main/views/test_job_notification_updates.py @@ -1,15 +1,11 @@ """ -Tests for job notification update logic during polling. +Tests for disabled job polling endpoint. -These tests verify the poll status endpoint behavior and document -the JavaScript notification refresh logic: -1. Notifications update for first 50 messages -2. Notifications stop updating after 50 messages (to prevent performance issues) -3. Notifications always update when job finishes +These tests verify that the poll status endpoint is properly disabled +and returns 410 Gone status. The JavaScript notification refresh logic +is no longer used as polling has been replaced with manual refresh. """ -import json - import pytest @@ -58,31 +54,14 @@ def test_poll_status_notification_update_logic( "main.view_job_status_poll", service_id=service_one["id"], job_id=fake_uuid, + _expected_status=410, ) - assert response.status_code == 200 - data = json.loads(response.get_data(as_text=True)) + assert response.status_code == 410 + # Endpoint is disabled, so no data to verify - # Verify the response - assert data["sent_count"] == delivered - assert data["failed_count"] == failed - assert data["pending_count"] == pending - assert data["total_count"] == total - assert data["finished"] is finished - - processed_count = delivered + failed - - if js_should_update_notifications: - # JavaScript would call: await updateNotifications() - if finished: - assert finished, f"JS updates notifications: {reason}" - else: - assert processed_count <= 50, f"JS updates notifications: {reason}" - assert not finished, f"JS updates notifications: {reason}" - else: - # JavaScript would NOT update notifications - assert processed_count > 50, f"JS skips notification update: {reason}" - assert not finished, f"JS skips notification update: {reason}" + # Since the polling endpoint is disabled, the JavaScript logic being tested + # is no longer relevant. The test now verifies the endpoint is disabled. def test_poll_status_provides_required_fields( @@ -107,12 +86,7 @@ def test_poll_status_provides_required_fields( "main.view_job_status_poll", service_id=service_one["id"], job_id=fake_uuid, + _expected_status=410, ) - data = json.loads(response.get_data(as_text=True)) - - 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)) - assert response_size < 200, f"Response too large: {response_size} bytes" + assert response.status_code == 410