Fix incorrect filters on Circuit Maintenance list views (#377) - #391
Open
jvanderaa wants to merge 4 commits into
Open
Fix incorrect filters on Circuit Maintenance list views (#377)#391jvanderaa wants to merge 4 commits into
jvanderaa wants to merge 4 commits into
Conversation
CircuitMaintenance provider/circuit filters referenced fields that do not exist on the model; filter them through the circuitimpact -> circuit relationship (with distinct) instead. Correct the Circuit natural key lookups to use cid rather than the default name, add the missing q search filter to CircuitImpactFilterSet, and switch the impact/level filter form fields to MultipleChoiceField so choice values validate (no more 'Invalid filters were specified'). Make the remaining Note filter form fields optional. Adds regression tests covering each filter and filter-form fix. Closes #377
jvanderaa
requested review from
glennmatthews,
pke11y and
scetron
as code owners
July 20, 2026 22:09
Set shared test fixtures directly on the test class instead of returning and dereferencing a dict, and drop the redundant field_name="circuit" on CircuitImpactFilterSet.circuit (it matches the filter name).
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
Revert the shared fixtures from external cls-attribute assignment back to a returned dict accessed via self.data[...]. pylint cannot track attributes assigned inside a free helper function, so the previous form raised E1101 (no-member) in CI; dict subscripting is pylint-clean and keeps the helper DRY.
Contributor
There was a problem hiding this comment.
Was it intentional to commit these images to the PR branch? Not sure I see their value here.
| } | ||
|
|
||
|
|
||
| class CircuitMaintenanceFilterSetTestCase(TestCase): |
Contributor
There was a problem hiding this comment.
These should inherit from nautobot.apps.testing.FilterTestCases.FilterTestCase
Comment on lines
+74
to
+86
| def test_provider_filter(self): | ||
| """Filtering maintenances by the related circuit's provider (natural key) resolves and is scoped.""" | ||
| params = {"provider": [self.data["provider_1"].name]} | ||
| result = self.filterset(params, self.queryset).qs | ||
| self.assertIn(self.data["maintenance_1"], result) | ||
| self.assertNotIn(self.data["maintenance_2"], result) | ||
|
|
||
| def test_circuit_filter(self): | ||
| """Filtering maintenances by related circuit (cid natural key) resolves and is scoped.""" | ||
| params = {"circuit": [self.data["circuit_1"].cid]} | ||
| result = self.filterset(params, self.queryset).qs | ||
| self.assertIn(self.data["maintenance_1"], result) | ||
| self.assertNotIn(self.data["maintenance_2"], result) |
Contributor
There was a problem hiding this comment.
These should be covered instead (after addressing the inheritance comment above) by defining appropriate generic_filter_tests on the class, reducing the need for bespoke test code.
| queryset=Provider.objects.all(), | ||
| to_field_name="name", | ||
| label="Provider", | ||
| distinct=True, |
Contributor
There was a problem hiding this comment.
Can you confirm whether distinct=True is actually needed here and below? It does add some overhead if unneeded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: #377
What's Changed
Fixes incorrect filters on the Circuit Maintenance, Circuit Impact, and Note list views. Several filterset filters referenced model fields that don't exist (raising
FieldError), used the wrongCircuitnatural key, or were missing entirely, and two filter-form fields were free-text where the underlying filter expects choices (producing "Invalid filters were specified").Fixes
CircuitMaintenance.providerfield_name="provider"— no such field on the model →FieldErrorcircuitimpact__circuit__providerwithdistinct=TrueCircuitMaintenance.circuitfield_name="circuit"(no such field) + defaultto_field_name="name"(Circuit usescid)circuitimpact__circuit,to_field_name="cid",distinct=TrueCircuitImpact.qqfield but the filterset had noqsearch filterSearchFilter(searchescircuit__cid,maintenance__name)CircuitImpact.circuitto_field_name="name"invalid forCircuitto_field_name="cid"CircuitImpact.impact(form)CharFieldfed invalid values to a choice-based filterMultipleChoiceField(choices=CircuitImpactChoices)Note.level(form)title/commentfilter fields were also requiredMultipleChoiceField(choices=NoteLevelChoices); all Note filter fields made optionalOn the "Sort Columns" part of the issue
Investigated — no defect found. The only property-backed table columns (
circuits,providersonCircuitMaintenanceTable) are alreadyorderable=False(django-tables2 auto-detects this), and sorting the list view by them returns HTTP 200. No change was needed there.Screenshots
Circuit Maintenance — filter by provider (before: server error → after: filtered results)
Circuit Impact — Impact filter field (before: free-text → after: multi-select of valid choices)
Note — Level filter field (before: free-text → after: multi-select of valid choices)
Testing
nautobot_circuit_maintenance/tests/test_filters.pywith 10 regression tests covering each filter and filter-form fix (all failed before, pass after).test_views(195),test_api(8),test_graphql(4) all pass.rufflint + format clean; no missing migrations.