Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Edit ip address - change device assignment, doesnt clear interface assignment. causes confusion #10757

Closed
ITJamie opened this issue Oct 26, 2022 · 15 comments · Fixed by #12452
Closed

Edit ip address - change device assignment, doesnt clear interface assignment. causes confusion #10757

ITJamie opened this issue Oct 26, 2022 · 15 comments · Fixed by #12452
Assignees
@DanSheps
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@ITJamie
Copy link
Contributor

ITJamie commented Oct 26, 2022 •
edited
Loading

NetBox version

v3.3.5

Python version

3.10

Steps to Reproduce

  1. goto an ip address with an existing assignment, eg https://demo.netbox.dev/ipam/ip-addresses/186/
  2. edit the ip address, change the vm or device assignment, eg from vm1 to vm2.
  3. Notice that interface field below still says eth0
    (its not obvious but that eth0 field is still set to eth0 on vm1, not eth0 on vm2 which is selected above)
  4. user saves form, result is accepted. however interface assignment is still set to vm1 - eth0.

Expected Behavior

I would expect that at step 2 once the device or vm relationship was changed that the interfaces field would empty out

OR

that during the save process data validation would fail and tell the user that the device / virtual machine assignment + interface are not part of the same device.

OR

maybe the interface assignment (eth0) should also have the vm or device name listed after it? making it obvious that the field above didnt change anything and is just a filter?

Observed Behavior

assignment does not change in netbox. user gets no warning that there was a conflict or no change made
@ITJamie ITJamie added the type: bug A confirmed report of unexpected behavior in the application label Oct 26, 2022
@a084ed22
Copy link

From experience, the device/virtual machine dropdowns are meant to be only filters for the respective interface field. Only the interface field matters in terms of association with the IP, therefore just changing the virtual machine won't result in any change to the association.

@kkthxbye-code kkthxbye-code added the status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation label Oct 27, 2022
@kkthxbye-code
Copy link
Contributor

This is a common issue with fields that filter other fields. The obvious solution would be to clear dependent fields after updating the parent. Not sure if there are any usability issues to consider though.

Code for the API select fields can be found here if anyone wants to give it a shot:

https://github.com/netbox-community/netbox/blob/develop/netbox/project-static/src/select/api/apiSelect.ts

@ITJamie
Copy link
Contributor Author

ITJamie commented Nov 7, 2022

Another (quicker) option would be to also show the name of the attached device/vm in brackets after the interface name.

Eg right now it just shows "eth0" but it could show "eth0 (device1)" in the interface name field. It would at least make it slightly more obvious that the data didnt update when you change the field above

@PieterL75
Copy link
Contributor

PieterL75 commented Nov 15, 2022 •
edited
Loading

something like this

apiSelect.ts
add on line 604:

    // Remove the existing select option, as the parent changed, and the current selected options is not relevant to the new parent
    this.resetOptions()

This will remove the data of a child selection if the parent has changed value.

I have a branch ready with this change
https://github.com/PieterL75/netbox/tree/issue10757_clearchilselectlist
If the idea is ok, then I'll create a PR form it

@ZPrimed ZPrimed mentioned this issue Nov 16, 2022
Closed
@ITJamie
Copy link
Contributor Author

ITJamie commented Nov 17, 2022

My worry about doing it for every parent child field is that in some cases the current logic is actually useful.

Eg if your editing an interfaces vlan tags. I have several interfaces where there are vlans in multiple different vlan groups that need to be tagged on one interface.

So with the change above if i changed the vlan group to find another vlan i need to tag on that interface, it would clear out the existing tagged vlans.

@PieterL75
Copy link
Contributor

PieterL75 commented Nov 17, 2022 •
edited
Loading

interesting one.. so it should only clear on singlevalue dropdowns and not al the time..

and still then.. if you selected an untagged vlan from groupA and added some tagged, then got to groupB, the untagged vlan is cleared too..

Looks like this will need to be an option to either always-clear-except; or never-clear-except

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Jan 17, 2023
@DanSheps DanSheps added status: blocked Another issue or external requirement is preventing implementation and removed pending closure Requires immediate attention to avoid being closed for inactivity status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jan 20, 2023
@PieterL75
Copy link
Contributor

maybe, the element that is displayed in the list needs to show more information.
Currently you only see the value of the listiems (ex eth1, eth1, eth2, eth2, ...)

What if the list is dependent on another list, then show that too.
VM1/Eth1
VM2/Eth2
VM1/Eth1
VM2/Eth2.

Then the current logic can remain, but the confusion on what the relation of the Eth1 is to the parent is more clear

@jeremystretch
Copy link
Member

@DanSheps why did you mark this as blocked?

@DanSheps
Copy link
Member

DanSheps commented May 2, 2023

No idea, might have been by accident

@DanSheps
Copy link
Member

DanSheps commented May 2, 2023

There is an identical one however that would be fixed by moving to the new filtering logic. I am willing to take both of those

@jeremystretch
Copy link
Member

What is the other issue? Can we consolidate them?

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: blocked Another issue or external requirement is preventing implementation labels May 2, 2023
@DanSheps
Copy link
Member

DanSheps commented May 2, 2023 •
edited
Loading

Pretty sure you "just" closed it out: #11477

If we move to the new filtering mechanism, it would remove the "device assignment" all together, and make it clear that you can only assign an ip address to a interface.

@ITJamie
Copy link
Contributor Author

ITJamie commented May 2, 2023

the new filtering mechanism should solve this right? https://twitter.com/jstretch85/status/1631677080252424192?s=20

@DanSheps
Copy link
Member

DanSheps commented May 2, 2023 •
edited
Loading

Yeah, it should solve it.

@DanSheps DanSheps self-assigned this May 3, 2023
@DanSheps DanSheps added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels May 3, 2023
DanSheps added a commit that referenced this issue May 3, 2023
@DanSheps
Fixes: #10757 - Change interface assignment to use new selector. Perf…
dd7fda1 
…orm the same change to the NAT assignment as well.
@DanSheps DanSheps mentioned this issue May 3, 2023
Merged
jeremystretch pushed a commit that referenced this issue May 4, 2023
@DanSheps
Fixes: #10757 - Change IP interface assignment to use new selector (#…
46914d9 
…12452)

* Fixes: #10757 - Change interface assignment to use new selector.  Perform the same change to the NAT assignment as well.

* Remove nat_vrf from form and remove query_params that are not required anymore
jeremystretch added a commit that referenced this issue May 4, 2023
@jeremystretch
Changelog for #10757, #11652, #11791, #11190, #12363, #12464
3eba65b
@jeremystretch jeremystretch mentioned this issue May 5, 2023
Merged
@mgob mgob mentioned this issue May 19, 2023
Closed
@ITJamie ITJamie mentioned this issue May 29, 2023
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@DanSheps DanSheps

Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes: #10757 - Change IP interface assignment to use new selector netbox-community/netbox
6 participants
@kkthxbye-code @ITJamie @DanSheps @jeremystretch @PieterL75 @a084ed22