Closes #21751: Enable toggling user notifications when executing custom scripts

[jeremystretch]

Closes: #21751

• Add a notifications ChoiceField on the Job model
• Modify Job.terminate() to send notifications conditionally
• Introduce the notifications_default Meta option for custom scripts

[jeremystretch]

@claude /review

[claude]

Claude finished @jeremystretch's task in 2m 38s —— View job

---

PR Review: Enable toggling user notifications when executing custom scripts

• Gathering context
• Reading changed files
• Analyzing code quality and correctness
• Providing review feedback

---

Overview

This PR introduces a notifications field on the Job model to allow users to control whether they receive notifications upon script completion. It supports three modes: always (default), on_failure, and never. The implementation touches the model, migration, forms, API serializers, views, and documentation — a clean end-to-end change.

---

Code Quality

The implementation is well-structured and follows existing patterns in the codebase. The logic in terminate() is correct:

# core/models/jobs.py:247-256
if self.user and self.notifications != JobNotificationChoices.NOTIFICATION_NEVER:
    if (
        self.notifications == JobNotificationChoices.NOTIFICATION_ALWAYS or
        status != JobStatusChoices.STATUS_COMPLETED
    ):
        Notification(...).save()

on_failure correctly fires for STATUS_FAILED and STATUS_ERRORED but not STATUS_COMPLETED. The user=None guard is appropriate.

Recurring job scheduling in netbox/jobs.py:145 correctly propagates notifications=job.notifications to the next scheduled run — a subtle but important detail that's handled well.

---

Minor Observations

1. Migration missing choices parameter

core/migrations/0024_job_notifications.py was written manually and omits choices=JobNotificationChoices:

field=models.CharField(default='always', max_length=30),

Auto-generated migrations include choices. This doesn't affect DB behavior (choices are model-level validation only), but makemigrations would have included it. Per CLAUDE.md, migrations should be generated via manage.py makemigrations rather than crafted manually. Consider regenerating this migration or adding choices=JobNotificationChoices for consistency. Fix this →

2. required=False on _notifications ChoiceField

extras/forms/scripts.py:38 declares the field with required=False:

_notifications = forms.ChoiceField(
    required=False,
    choices=JobNotificationChoices,
    ...
)

The choices don't include a blank option, so a browser-rendered dropdown will always have one of the three valid values selected. But required=False technically permits an empty string '' from a malformed POST, which would then pass through to Job.enqueue() as a non-None value (bypassing the None fallback guard at jobs.py:318), eventually failing full_clean(). This is low-risk given the UI context, but worth aligning with how _commit (a BooleanField with required=False) is handled. A blank choice or a stricter default would be cleaner.

3. Class-level default in ScriptInputSerializer is redundant

extras/api/serializers_/scripts.py:118-121:

notifications = serializers.ChoiceField(
    choices=JobNotificationChoices,
    required=False,
    default=JobNotificationChoices.NOTIFICATION_ALWAYS,
)

The __init__ at line 129 always overrides this default when a script context exists. The class-level default is only meaningful when no script context is provided (e.g., in some test scenarios). Since notifications_default itself falls back to NOTIFICATION_ALWAYS, this is consistent — just slightly redundant. Not a bug.

4. _notifications always appears in the "Script Execution Parameters" fieldset

In extras/scripts.py:500-502, _notifications is always included in the fieldset regardless of scheduling_enabled. This appears intentional (notifications are independent of scheduling), but it's worth confirming this is the desired UX for scripts where scheduling is disabled.

---

Tests

The three test cases in core/tests/test_models.py (always, on_failure, never) are well-written using subTest for status variants. The _make_job helper is clean. One edge case not explicitly tested: user=None with notifications=always — but since the model condition handles it correctly (if self.user and ...), this is a minor nit.

---

Documentation

The docs/customization/custom-scripts.md addition is clear and includes a helpful table of values and behaviors. ✓

---

Summary

This is a clean, well-executed implementation. The main actionable item is the migration (point 1). Points 2–4 are minor and may be acceptable as-is. The core logic, API integration, and tests are solid.

[jeremystretch]

1. This is safe to ignore.
2. Keeping required=False in place to catch some rough edges in the UI
3. Feels safer to keep the default in place on the serializer field
4. This is correct

[arthanson]

If you run it from the scripts list page:

Get error:

Traceback (most recent call last):
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.14/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.14/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.14/site-packages/django/views/generic/base.py", line 105, in view
    return self.dispatch(request, *args, **kwargs)
           ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ahanson/dev/work/netbox/netbox/netbox/views/generic/base.py", line 26, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ahanson/dev/work/netbox/netbox/utilities/views.py", line 149, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ahanson/dev/work/netbox/netbox/utilities/views.py", line 48, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.14/site-packages/django/views/generic/base.py", line 144, in dispatch
    return handler(request, *args, **kwargs)
  File "/Users/ahanson/dev/work/netbox/netbox/extras/views.py", line 1705, in post
    job = ScriptJob.enqueue(
        instance=script,
    ...<7 lines>...
        commit=form.cleaned_data.pop('_commit'),
    )
  File "/Users/ahanson/dev/work/netbox/netbox/netbox/jobs.py", line 174, in enqueue
    return Job.enqueue(cls.handle, name=name, *args, **kwargs)
           ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ahanson/dev/work/netbox/netbox/core/models/jobs.py", line 320, in enqueue
    job.full_clean()
    ~~~~~~~~~~~~~~^^
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.14/site-packages/django/db/models/base.py", line 1679, in full_clean
    raise ValidationError(errors)
django.core.exceptions.ValidationError: {'notifications': ['This field cannot be blank.']}