starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

ObjectVar and MultiObjectVar should utilize DynamicModelChoiceField #3393

New Issue
Closed
opened 2025-12-29 18:28:40 +01:00 by adam · 6 comments
adam commented 2025-12-29 18:28:40 +01:00
Owner
Copy Link

Originally created by @DanSheps on GitHub (Feb 24, 2020).

Proposed Changes

ObjectVar currently uses forms.ModelChoiceField, this should be changed to utilities.forms.DynamicModelChoiceField
MultiObjectVar currently uses forms.ModelMultipleChoiceField. this should be changed to use utilities.forms.DynamicModelMultipleChoiceField.

from utilities.forms import DynamicModelChoiceField, DynamicModelMultipleChoiceField

class ObjectVar(ObjectVar):
    form_field = DynamicModelChoiceField
    
    
class MultiObjectVar(MultiObjectVar):
    form_field = DynamicModelMultipleChoiceField

Justification

The two above dynamic model fields limits the pre-population of the select fields to improve performance when using the Select2 widget on scripts.

Originally created by @DanSheps on GitHub (Feb 24, 2020). ### Proposed Changes ObjectVar currently uses forms.ModelChoiceField, this should be changed to utilities.forms.DynamicModelChoiceField MultiObjectVar currently uses forms.ModelMultipleChoiceField. this should be changed to use utilities.forms.DynamicModelMultipleChoiceField. ``` from utilities.forms import DynamicModelChoiceField, DynamicModelMultipleChoiceField class ObjectVar(ObjectVar): form_field = DynamicModelChoiceField class MultiObjectVar(MultiObjectVar): form_field = DynamicModelMultipleChoiceField ``` ### Justification The two above dynamic model fields limits the pre-population of the select fields to improve performance when using the Select2 widget on scripts.
adam added the status: acceptedtype: feature labels 2025-12-29 18:28:40 +01:00
adam closed this issue 2025-12-29 18:28:40 +01:00
adam commented 2025-12-29 18:28:41 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 24, 2020):

Introducing a new pair of variable types seems confusing and unnecessary. Why would we not simply change ObjectVar to use DynamicModelChoiceField with the Select2 widget?

@jeremystretch commented on GitHub (Feb 24, 2020): Introducing a new pair of variable types seems confusing and unnecessary. Why would we not simply change ObjectVar to use DynamicModelChoiceField with the Select2 widget?
adam commented 2025-12-29 18:28:41 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Feb 24, 2020):

I just didn't want to make any major changes that would affect any existing scripts.

I think for ObjectVar we would need to make sure it is both using a widget and specifically APISelect or APIMultipleSelect would we not? Otherwise we could get into situation where the field itself is blank. I am okay with enforcing the usage of a widget on ObjectVar and throwing a error if required too, just was trying to take the least disruptive path forward.

@DanSheps commented on GitHub (Feb 24, 2020): I just didn't want to make any major changes that would affect any existing scripts. I think for ObjectVar we would need to make sure it is both using a widget and specifically APISelect or APIMultipleSelect would we not? Otherwise we could get into situation where the field itself is blank. I am okay with enforcing the usage of a widget on ObjectVar and throwing a error if required too, just was trying to take the least disruptive path forward.
adam commented 2025-12-29 18:28:41 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Mar 4, 2020):

I think the reality is that we don't have a great solution for the problem right now. We should avoid introducing anything significantly more complex than what already exists in the interest of user-friendliness.

IMO it's not reasonable to expect a custom script author to define a specific widget or an API endpoint for a form field, particularly when the necessary information already exists. For example:

device_type = ObjectVar(
    queryset=DeviceType.objects.all(),
    widget=APISelect(api_url='/api/dcim/device-types/')
)

Specifying the widget is redundant, since we should be using APISelect by default for DynamicModelChoiceField. And specifying the API endpoint is redundant since we should be able to determine it programmatically based solely on the associated queryset.

We haven't addressed either of these points yet within the codebase, because they haven't been a problem for developers. But the custom script interface that we expose to users demands simplicity and efficiency. I think the best approach here is to extend the dynamic choice fields to render an APISelect widget by default and automatically assign the appropriate endpoint. (This has the additional benefit of removing a good amount of redundant code, as well.)

@jeremystretch commented on GitHub (Mar 4, 2020): I think the reality is that we don't have a great solution for the problem right now. We should avoid introducing anything significantly more complex than what already exists in the interest of user-friendliness. IMO it's not reasonable to expect a custom script author to define a specific widget or an API endpoint for a form field, particularly when the necessary information already exists. For example: ```python device_type = ObjectVar( queryset=DeviceType.objects.all(), widget=APISelect(api_url='/api/dcim/device-types/') ) ``` Specifying the widget is redundant, since we should be using APISelect by default for DynamicModelChoiceField. And specifying the API endpoint is redundant since we should be able to determine it programmatically based solely on the associated queryset. We haven't addressed either of these points yet within the codebase, because they haven't been a problem for _developers_. But the custom script interface that we expose to _users_ demands simplicity and efficiency. I think the best approach here is to extend the dynamic choice fields to render an APISelect widget by default and automatically assign the appropriate endpoint. (This has the additional benefit of removing a good amount of redundant code, as well.)
adam commented 2025-12-29 18:28:41 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Mar 6, 2020):

That sounds like a good approach for it. Do we currently have any way to automatically assign appropriate endpoints or do we need to build one out?

@DanSheps commented on GitHub (Mar 6, 2020): That sounds like a good approach for it. Do we currently have any way to automatically assign appropriate endpoints or do we need to build one out?
adam commented 2025-12-29 18:28:42 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Mar 6, 2020):

Something like this should work:

from django.urls import reverse

queryset=Site.objects.all()
app_label = queryset.model._meta.app_label
model_name = queryset.model._meta.model_name
reverse('{}-api:{}-list'.format(app_label, model_name))
@jeremystretch commented on GitHub (Mar 6, 2020): Something like this should work: ```python from django.urls import reverse queryset=Site.objects.all() app_label = queryset.model._meta.app_label model_name = queryset.model._meta.model_name reverse('{}-api:{}-list'.format(app_label, model_name)) ```
adam commented 2025-12-29 18:28:42 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Mar 16, 2020):

I've called out the above change in #4374. Marking this as blocked until it is implemented.

@jeremystretch commented on GitHub (Mar 16, 2020): I've called out the above change in #4374. Marking this as blocked until it is implemented.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label status: accepted type: feature
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#3393
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?