Skip to content

Fixes: #441 - Patch ObjectSelectorView to support targeting custom objects from core custom fields#445

Open
bctiemann wants to merge 1 commit intomainfrom
441-custom-field-object-search
Open

Fixes: #441 - Patch ObjectSelectorView to support targeting custom objects from core custom fields#445
bctiemann wants to merge 1 commit intomainfrom
441-custom-field-object-search

Conversation

@bctiemann
Copy link
Copy Markdown
Contributor

Fixes: #441

Fixes the issue of core Custom Fields being unable to use the Object Selector (magnifying glass) on object fields pointing to Custom Objects (this would previously raise an ImportError).

This change patches the get_form_class and get_filterset_class methods from ObjectSelectorView upon load of the ready method in Custom Objects. The patched functions come with custom-object-aware FilterSetForm and FilterSet classes that enable the Object Selector to work.

@bctiemann
Copy link
Copy Markdown
Contributor Author

@claude /review

@claude
Copy link
Copy Markdown

claude bot commented Apr 2, 2026

Claude finished @bctiemann's task in 1m 8s —— View job


PR Review: Patch ObjectSelectorView for Custom Object Support

Overview: This PR monkey-patches ObjectSelectorView._get_form_class and _get_filterset_class in ready() to handle dynamically-generated custom object models. Without this fix, using the HTMX object selector with a custom field pointing to a custom object raises an ImportError (500 error) since there are no importable filter form/filterset classes for dynamic models.


Correctness

  • The patch logic correctly mirrors the existing CustomObjectListView.get_filterset_form() pattern at views.py:374-394. The _patched_get_form_class implementation is nearly identical to the view's method, which reduces risk of divergence bugs.
  • The filterset path reuses get_filterset_class() from filtersets.py, consistent with how CustomObjectListView and the API view handle it.
  • The app_label check (model._meta.app_label == APP_LABEL) correctly scopes the patch to only custom object models, letting all other models fall through to the original implementation.

Code Duplication

The _patched_get_form_class body is nearly identical to CustomObjectListView.get_filterset_form() (views.py:374-394). The only differences are:

  • The model comes from model parameter vs self.queryset.model
  • custom_object_type comes from model.custom_object_type vs self.custom_object_type
  • The logger.debug call is omitted in the patch (silent failure on NotImplementedError)

This is a minor issue — it would be worth extracting a shared helper (e.g., build_filterset_form_class(model, custom_object_type)) in views.py or a utils module to avoid the duplication. Fix this →

Test Coverage

  • Tests are well-structured with setUpTestData for DB-intensive setup and tearDown calling clear_model_cache().
  • test_object_selector_form_load covers the regression case (no 500 on form load).
  • test_object_selector_search covers the search path.
  • Missing negative test: There's no assertion that the patched methods still correctly delegate to the original for non-custom-object models (e.g., a built-in NetBox model). This would guard against accidentally breaking the selector for regular NetBox models.

Minor Issues

  1. __module__ value ('database.filterset_forms', __init__.py:61): This is a synthetic/fake module name. The same string is used in views.py:379, so it's an established pattern in this codebase — just worth noting it's non-standard. A more descriptive string like 'netbox_custom_objects.dynamic_forms' would be more accurate, but consistency with existing code is reasonable.

  2. Placement in __init__.py: The patch function is placed in __init__.py alongside migration-detection logic. Depending on whether ObjectSelectorView can be imported at module load vs. ready() time, this is correctly deferred inside ready(). The implementation correctly wraps the import inside the function body, so this is fine.

  3. Test _model_label uses a method, not a property: Minor style nit — _model_label() is called as a method but could be a @property. Not a bug.

  4. test_object_selector_search asserts only status 200: It would be stronger to also assert that "Alpha" appears in the response content and "Beta" does not, confirming the search actually filters results rather than just not crashing.

Summary

This is a clean, targeted fix. The approach (monkey-patching in ready()) is appropriate given that ObjectSelectorView is a core NetBox class that can't be subclassed or overridden via normal plugin extension points. The main concern is the duplicated form-building logic — a small refactor to extract a shared helper would improve long-term maintainability. The tests cover the key regression scenario.

Verdict: Approve with minor suggestions — the patch is correct and safe; the duplication and test gaps are worth addressing but not blockers.


@bctiemann bctiemann requested a review from arthanson April 2, 2026 18:38
Copy link
Copy Markdown
Contributor

@arthanson arthanson left a comment

Choose a reason for hiding this comment

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

I tink this can be made into a common routine called both here and in CustomObjectListView.get_filterset_form to be DRY.

The form-building logic in _patched_get_form_class duplicates CustomObjectListView.get_filterset_form() (views.py:374-394), with two subtle
divergences: the class name uses model.__name__ here vs model._meta.object_name in the view, and NotImplementedError is silently swallowed here vs logged at debug in the view.

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.

Object selector (magnifying glass) broken for custom object types referenced from core custom fields

2 participants