netbox-community / netbox-topology-views

A netbox plugin that draws topology views
Apache License 2.0
761 stars 64 forks source link

Fixes #547: XML Export does not work in modal window #572

Closed dreng closed 1 month ago

dreng commented 1 month ago

Fixes #547: XML Export does not work in modal window

There were two problems here:

1. The word "On" in the URL has changed to "True".

This was an easy one. I just needed to change https://github.com/netbox-community/netbox-topology-views/blob/f4b1e34826024d21e8cb29d3719c6a2232102ab1/netbox_topology_views/static_dev/js/home.js#L514 and https://github.com/netbox-community/netbox-topology-views/blob/f4b1e34826024d21e8cb29d3719c6a2232102ab1/netbox_topology_views/static_dev/js/home.js#L519

The hard-coded GET parameters are not quite elegant by the way, but I didn't touch that for now.

2. basePath is not defined in this context.

This was also easy to fix by just removing basePath in https://github.com/netbox-community/netbox-topology-views/blob/f4b1e34826024d21e8cb29d3719c6a2232102ab1/netbox_topology_views/static_dev/js/home.js#L528

But the question is why this has been added. It raises an error because basePath has never been declared or initialized. I couldn't find any occurance of "basePath" in this repository nor in NetBox' repo. Intrestingly, basePath doesn't raise an error in https://github.com/netbox-community/netbox-topology-views/blob/f4b1e34826024d21e8cb29d3719c6a2232102ab1/netbox_topology_views/static_dev/js/home.js#L227

The variable just contains an empty string here. But where does that come from?

mattieserver commented 1 month ago

Basepath is used in https://github.com/netbox-community/netbox-topology-views/blob/f4b1e34826024d21e8cb29d3719c6a2232102ab1/netbox_topology_views/templates/netbox_topology_views/index.html#L79-L82

Its set by this: https://github.com/netbox-community/netbox-topology-views/blob/f4b1e34826024d21e8cb29d3719c6a2232102ab1/netbox_topology_views/views.py#L901-L909

dreng commented 1 month ago

Thanks @mattieserver. Weird. I've searched the whole repository for this string and got no hits. Must have been some kind of typo.

So the basePath has to stay in place. I have to find out why it's not defined then and update this PR.

dreng commented 1 month ago

@mattieserver I reverted the basePath removal and added basePath to the HTMX template. That did the trick. So this is ready for a review now.