search

netbox-community / netbox

The premier source of truth powering network automation. Open source under Apache 2. Public demo: https://demo.netbox.dev
http://netboxlabs.com/oss/netbox/
Apache License 2.0
15.44k stars 2.52k forks source link

NoReverseMatch exception for plugins with models without *_list views #16825

Open jsenecal opened 2 weeks ago

jsenecal commented 2 weeks ago

Deployment Type

Self-hosted

NetBox Version

v4.0.6

Python Version

3.10

Steps to Reproduce

A bit complex, let me know if you really want me to go through a thorough plugin example

  1. Create a plugin with a model related to Core models.
  2. Create a set of views and urls for that model, without creating any _list views.
  3. Register that plugin in your netbox instance
  4. Create and install required migrations

Expected Behavior

Related core model detail view should work as expected regardless of the missing _list view in the plugin.

Observed Behavior

Bug Introduced by https://github.com/netbox-community/netbox/commit/5353f837108ee5db11537bc95dde3ce9bc4a042c

I have models with no *_list views related to core models in my plugin and they get picked up by get_related_models in GetRelatedModelsMixin which raises a NoReverseMatch exception in netbox/templates/inc/panels/related_objects.html

Technically get_related_models has been designed with the idea that one can set the omit method parameter to an iterable to avoid including these but there does not seem to have API which is exposed to plugin authors for Core models.

jeremystretch commented 2 weeks ago

@jsenecal would you like to tackle this?

jsenecal commented 2 weeks ago

Sure, I can give it a go. You can assign this to me.

I will propose a solution here first before submitting a PR.

jeremystretch commented 2 weeks ago

It may be sufficient to just catch exceptions on view name resolution when determining related models.

alehaa commented 2 weeks ago

I think catching the exception should be enough. Maybe we can remove the omit parameter then, as the models are listed explicitly to avoid that exception. Sorry for the inconvenience.

jsenecal commented 2 weeks ago

I think catching the exception should be enough. Maybe we can remove the omit parameter then, as the models are listed explicitly to avoid that exception. Sorry for the inconvenience.

The exception is generated when Django renders the template. We may need to resolve the view names earlier to see if they exist prior to passing these into the context.

jsenecal commented 2 weeks ago

So I've designed two fixes to this:

  1. Use the validated_viewname filter instead of the viewname one: https://github.com/jsenecal/netbox/compare/develop...related_objects-template-fix

    Pros: Simple, effective Cons: Not really reusable for other dynamic functionalities in the future

  2. Modify get_related_models to try to resolve urls at this point and omit the relationship if it cannot be resolved. https://github.com/jsenecal/netbox/compare/develop...related_objects-utilities-fix

    Pros: More targeted fix, allows for reuse of code Cons: Moves a bit more code around

In both cases omit could be removed everywhere but it does provide some performance improvements for netbox native models.

@jeremystretch let me know which approach you prefer and I'll prepare a proper PR.

jsenecal commented 1 week ago

@jeremystretch any chance you could look into my proposed fixes?