Skip to content

Fix Notification Source edit form base class and Provider widget (#332) - #393

Open
jvanderaa wants to merge 1 commit into
developfrom
332-notificationsource-form
Open

Fix Notification Source edit form base class and Provider widget (#332)#393
jvanderaa wants to merge 1 commit into
developfrom
332-notificationsource-form

Conversation

@jvanderaa

Copy link
Copy Markdown
Collaborator

Closes #332.

Problem

NotificationSourceForm subclassed django.forms.ModelForm rather than NautobotModelForm, so none of the Nautobot form mixins applied. Two user-visible consequences:

  • Custom fields were silently omitted. NotificationSource is an OrganizationalModel, which includes CustomFieldModel — so custom fields can be defined on it — but CustomFieldModelFormMixin only arrives via NautobotModelForm. The edit form rendered a single field regardless of any custom fields configured.
  • Providers had no search. The auto-generated ModelMultipleChoiceField rendered every Provider as an <option> on each edit page load, instead of the API-backed selector used elsewhere in the app.

The viewset half of #332 was already resolved by #344; this is the remaining form half that @gsnider2195 called out in the issue ("wrong base class", "wrong widget for providers field").

Change

class NotificationSourceForm(NautobotModelForm):
    providers = DynamicModelMultipleChoiceField(queryset=Provider.objects.all(), required=False)

This matches CircuitImpactForm, CircuitMaintenanceForm, and NoteForm in the same module; both imports were already present.

required=False is required, not incidental — an explicitly declared form field bypasses fields_for_model, so the model's blank=True does not propagate and Providers would otherwise become mandatory.

Why fields stays an explicit list

Switching to NautobotModelForm activates the nb-use-fields-all pylint check, which asks for fields = "__all__". Following that here would be a bug, so the rule is suppressed with the rationale recorded inline.

name and attach_all_providers are owned by PLUGINS_CONFIG: import_notification_sources() runs on every post_migrate, re-applies attach_all_providers from config, and deletes any Notification Source whose name is absent from the config. Exposing name in the UI would let a rename silently delete the object on the next migrate. This matches the documented behavior in docs/user/app_getting_started.md — "you can only view and edit providers". providers is the only safely user-editable field.

Testing

Adds nautobot_circuit_maintenance/tests/test_forms.py, written test-first — both tests were confirmed failing against the old form before the fix ('cf_…' not found in {'providers': ModelMultipleChoiceField} and ModelMultipleChoiceField is not an instance of DynamicModelMultipleChoiceField).

It extends FormTestCases.BaseFormTestCase, which also contributes a query_params validation guard for any DynamicModelChoice field added later.

  • invoke unittest --label nautobot_circuit_maintenance.tests.test_forms → 3 tests OK (1 inherited test skips: providers declares no query_params).
  • Full suite → 296/297. The one failure is test_basics.TestVersion ('3.0.1a0' != '3.1.2a0'), which reproduces identically on a pristine develop checkout — a stale app version in the local container image, unrelated to this change.
  • invoke pylint → 10.00/10; ruff check / ruff format --check clean.

Follow-up (not in this PR)

NotificationSourceFilterSetForm (same module, wired as filterset_form_class on the same viewset) is still on BootstrapMixin, forms.ModelForm and should be a NautobotFilterForm. It also exposes only q, while NotificationSourceFilterSet supports filtering on name, providers, attach_all_providers, tags, and created — so the list view cannot filter by Provider today. That is a behavior change rather than a bug fix, so it is left out of this PR; happy to open a separate issue. It is also the last class keeping the file-level nb-incorrect-base-class suppression alive.

NotificationSourceForm subclassed django.forms.ModelForm, so the Nautobot
form mixins never applied: custom fields defined on NotificationSource were
silently omitted from the edit form, and Providers rendered as a plain
multi-select with no search.

Switch to NautobotModelForm and declare providers as a
DynamicModelMultipleChoiceField, matching the other model forms in this
module. Adds test_forms.py covering both behaviors.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  nautobot_circuit_maintenance
  forms.py
Project Total  

This report was generated by python-coverage-comment-action

@glennmatthews glennmatthews left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix looks valid as far as it goes but I think we may need to go further.

Comment on lines +184 to +186
# `name` and `attach_all_providers` are owned by PLUGINS_CONFIG and re-applied on every
# post_migrate by import_notification_sources(), which also deletes any Notification Source
# whose name is absent from the config. `providers` is the only safely user-editable field.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work for the create case that is also provided by NotificationSourceUIViewSet? Do we need to disable the create action/view entirely?

nautobot-bot pushed a commit that referenced this pull request Aug 7, 2026
…r Tool

## What's Changed
## [nautobot-app-v3.1.4 (2026-08-07)](https://github.com/nautobot/cookiecutter-nautobot-app/releases/tag/nautobot-app-v3.1.4)

### Added

- [#376](nautobot/cookiecutter-nautobot-app#376) - Added support for `--no-input` option to `invoke unittest` and `invoke tests` tasks.
- [#378](nautobot/cookiecutter-nautobot-app#378) - Added `invoke generate-test-data` task and a boilerplate `generate_<app_name>_test_data` management command to the `nautobot-app` cookie.
- [#380](nautobot/cookiecutter-nautobot-app#380) - Added previous_version to the Prepare Release workflow to allow overriding the previous_version.
- [#386](nautobot/cookiecutter-nautobot-app#386) - Added opt-in ephemeral Docker host ports for generated app development environments.
- [#388](nautobot/cookiecutter-nautobot-app#388) - Added `nautobot-app-commercial` template for creating commercial (licensed) Nautobot Apps distributed through NTC's private Artifactory repository.
- [#393](nautobot/cookiecutter-nautobot-app#393) - Added a `--missing` option to the `unittest-coverage` invoke target to show missing coverage lines.

### Changed

- [#318](nautobot/cookiecutter-nautobot-app#318) - Added a `--diff` option to the `invoke ruff` tasks.
- [#370](nautobot/cookiecutter-nautobot-app#370) - Changed the minimum version of Nautobot to 3.1.0.
- [#371](nautobot/cookiecutter-nautobot-app#371) - Added various AI agent files to the gitignore.
- [#372](nautobot/cookiecutter-nautobot-app#372) - Updated ruff target python version to 3.10.
- [#373](nautobot/cookiecutter-nautobot-app#373) - Added `target` and `recursive` options to `invoke pylint` tasks.
- [#386](nautobot/cookiecutter-nautobot-app#386) - Aligned the generated app `docs` container to serve and publish on port 8001, matching `mkdocs.yml` and Nautobot core.
- [#407](nautobot/cookiecutter-nautobot-app#407) - Changed the minimum supported Nautobot version to 3.2.0 for apps baked from the `nautobot-app-commercial` cookie, updating both the baked `pyproject.toml` constraint and the CI test matrix.

### Fixed

- [#381](nautobot/cookiecutter-nautobot-app#381) - Updated the CI pipeline so that we auto increment post-release versions when creating a release from main, develop, next, and ltm-*.
- [#383](nautobot/cookiecutter-nautobot-app#383) - Fixed the prepare release workflow bumping the version on prereleases resulting in a version number that is 2 ahead of the last release.
- [#387](nautobot/cookiecutter-nautobot-app#387) - Fixed github actions not running on pull requests when using automated releases.
- [#405](nautobot/cookiecutter-nautobot-app#405) - Changed `.gitignore` to allow committing shared Claude Code configuration, ignoring only per-user local files.

### Housekeeping

- [#385](nautobot/cookiecutter-nautobot-app#385) - Added a release workflow job to sync release notes from `ltm` branches back to `develop` via an automated pull request.
- [#389](nautobot/cookiecutter-nautobot-app#389) - Replaced the unmaintained `toml` dev dependency with stdlib `tomllib` (Python 3.11+) and `tomli` (Python 3.10 fallback).
- [#390](nautobot/cookiecutter-nautobot-app#390) - Bumped CI workflow matrix to test against Python 3.14.
- [#402](nautobot/cookiecutter-nautobot-app#402) - Fixed djlint CI failure for apps with no Django templates
- [#403](nautobot/cookiecutter-nautobot-app#403) - Changed the release workflow to define the Python and Poetry versions as workflow-level environment variables.
- [#404](nautobot/cookiecutter-nautobot-app#404) - Added a release workflow job that opens a pull request from `main` into `next` after a release is published from `main`.
- [#406](nautobot/cookiecutter-nautobot-app#406) - Excluded the templated `development/docker-compose.base.yml` and `.github/workflows/ci.yml` files from `yamllint`.
Defiantearth pushed a commit that referenced this pull request Aug 12, 2026
…r Tool (#396)

* Cookie updated targeting develop by NetworkToCode Cookie Drift Manager Tool

- [#376](nautobot/cookiecutter-nautobot-app#376) - Added support for `--no-input` option to `invoke unittest` and `invoke tests` tasks.
- [#378](nautobot/cookiecutter-nautobot-app#378) - Added `invoke generate-test-data` task and a boilerplate `generate_<app_name>_test_data` management command to the `nautobot-app` cookie.
- [#380](nautobot/cookiecutter-nautobot-app#380) - Added previous_version to the Prepare Release workflow to allow overriding the previous_version.
- [#386](nautobot/cookiecutter-nautobot-app#386) - Added opt-in ephemeral Docker host ports for generated app development environments.
- [#388](nautobot/cookiecutter-nautobot-app#388) - Added `nautobot-app-commercial` template for creating commercial (licensed) Nautobot Apps distributed through NTC's private Artifactory repository.
- [#393](nautobot/cookiecutter-nautobot-app#393) - Added a `--missing` option to the `unittest-coverage` invoke target to show missing coverage lines.

- [#318](nautobot/cookiecutter-nautobot-app#318) - Added a `--diff` option to the `invoke ruff` tasks.
- [#370](nautobot/cookiecutter-nautobot-app#370) - Changed the minimum version of Nautobot to 3.1.0.
- [#371](nautobot/cookiecutter-nautobot-app#371) - Added various AI agent files to the gitignore.
- [#372](nautobot/cookiecutter-nautobot-app#372) - Updated ruff target python version to 3.10.
- [#373](nautobot/cookiecutter-nautobot-app#373) - Added `target` and `recursive` options to `invoke pylint` tasks.
- [#386](nautobot/cookiecutter-nautobot-app#386) - Aligned the generated app `docs` container to serve and publish on port 8001, matching `mkdocs.yml` and Nautobot core.
- [#407](nautobot/cookiecutter-nautobot-app#407) - Changed the minimum supported Nautobot version to 3.2.0 for apps baked from the `nautobot-app-commercial` cookie, updating both the baked `pyproject.toml` constraint and the CI test matrix.

- [#381](nautobot/cookiecutter-nautobot-app#381) - Updated the CI pipeline so that we auto increment post-release versions when creating a release from main, develop, next, and ltm-*.
- [#383](nautobot/cookiecutter-nautobot-app#383) - Fixed the prepare release workflow bumping the version on prereleases resulting in a version number that is 2 ahead of the last release.
- [#387](nautobot/cookiecutter-nautobot-app#387) - Fixed github actions not running on pull requests when using automated releases.
- [#405](nautobot/cookiecutter-nautobot-app#405) - Changed `.gitignore` to allow committing shared Claude Code configuration, ignoring only per-user local files.

- [#385](nautobot/cookiecutter-nautobot-app#385) - Added a release workflow job to sync release notes from `ltm` branches back to `develop` via an automated pull request.
- [#389](nautobot/cookiecutter-nautobot-app#389) - Replaced the unmaintained `toml` dev dependency with stdlib `tomllib` (Python 3.11+) and `tomli` (Python 3.10 fallback).
- [#390](nautobot/cookiecutter-nautobot-app#390) - Bumped CI workflow matrix to test against Python 3.14.
- [#402](nautobot/cookiecutter-nautobot-app#402) - Fixed djlint CI failure for apps with no Django templates
- [#403](nautobot/cookiecutter-nautobot-app#403) - Changed the release workflow to define the Python and Poetry versions as workflow-level environment variables.
- [#404](nautobot/cookiecutter-nautobot-app#404) - Added a release workflow job that opens a pull request from `main` into `next` after a release is published from `main`.
- [#406](nautobot/cookiecutter-nautobot-app#406) - Excluded the templated `development/docker-compose.base.yml` and `.github/workflows/ci.yml` files from `yamllint`.

* Resolve cookiecutter drift merge conflicts

The drift manager committed unresolved conflict markers to this
branch. Resolved as follows:

- .cookiecutter.json: took the newly baked commit ref
- docs/admin/compatibility_matrix.md: kept the app's real matrix over the template placeholder
- pyproject.toml: kept the app's dependencies with the template's Nautobot >=3.1.0 floor

* Fix ruff format and lint fallout from the drift merge

- ran `invoke ruff --action format`; the template's line wrapping does not match this repo's rendered name lengths

* Regenerate poetry.lock for the updated dependencies

The drift manager changed pyproject.toml without relocking, so CI failed at
"Setup environment" with "pyproject.toml changed significantly since
poetry.lock was last generated" before any lint or test job could run.

Relocked with poetry 2.3.1 (lock-version 2.1); nautobot resolves to 3.1.1.

---------

Co-authored-by: bakebot <info@networktocode.com>
Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing views for action(s) edit

2 participants