diff --git a/changedetectionio/notification_service.py b/changedetectionio/notification_service.py index 81669adb5..6ffcd62ca 100644 --- a/changedetectionio/notification_service.py +++ b/changedetectionio/notification_service.py @@ -512,7 +512,7 @@ Thanks - Your omniscient changedetection.io installation. return threshold = self.datastore.data['settings']['application'].get('filter_failure_notification_threshold_attempts') - step = step_n + 1 + step = step_n # @todo - This could be a markdown template on the disk, apprise will convert the markdown to HTML+Plaintext parts in the email, and then 'markup_text_links_to_html_links' is not needed # {{{{ }}}} because this will be Jinja2 {{ }} tokens diff --git a/changedetectionio/tests/unit/test_step_failure_notification.py b/changedetectionio/tests/unit/test_step_failure_notification.py index bd4fcd29b..2cc530a2f 100644 --- a/changedetectionio/tests/unit/test_step_failure_notification.py +++ b/changedetectionio/tests/unit/test_step_failure_notification.py @@ -65,4 +65,28 @@ def test_send_step_failure_notification_queues_item(): assert not notification_q.empty(), "Expected a notification to be queued" item = notification_q.get_nowait() assert 'notification_title' in item - assert 'position 2' in item['notification_title'] + # step_n is already 1-based (base.py increments step_n before running each step, so the + # first browser step raises with step_n=1), so the reported position must equal step_n. + assert 'position 1' in item['notification_title'] + + +def test_send_step_failure_notification_position_matches_step_number(): + """Regression for #4200: the reported browser-step position must equal the 1-based + step_n, not step_n + 1. step_n arrives already 1-based (BrowserStepsStepException is + raised with the post-increment counter, and worker.py passes e.step_n through), and the + frontend highlights nth-child(browser_steps_last_error_step) / compares === i+1, both + 1-based. An extra +1 mis-numbers the notification and highlights the wrong step.""" + from changedetectionio.notification_service import NotificationService + + for step_n in (1, 2, 5): + watch_uuid = f'test-uuid-pos-{step_n}' + notification_q = queue.Queue() + datastore, _ = _make_datastore(watch_uuid, 'post://localhost/test') + service = NotificationService(datastore=datastore, notification_q=notification_q) + + service.send_step_failure_notification(watch_uuid=watch_uuid, step_n=step_n) + + item = notification_q.get_nowait() + assert f'position {step_n} could not be run' in item['notification_title'] + assert f'position {step_n + 1}' not in item['notification_title'] + assert f'position {step_n} for the web page watch' in item['notification_body'] diff --git a/changedetectionio/worker.py b/changedetectionio/worker.py index 3813e3478..c78bf9e52 100644 --- a/changedetectionio/worker.py +++ b/changedetectionio/worker.py @@ -324,7 +324,7 @@ async def async_update_worker(worker_id, q, notification_q, app, datastore, exec if not datastore.data['watching'].get(uuid): continue - error_step = e.step_n + 1 + error_step = e.step_n from playwright._impl._errors import TimeoutError, Error # Generally enough info for TimeoutError (couldnt locate the element after default seconds)