Raise an exception if the interface contains a tagged vlan but the mode is not set to tagged

[timizuoebideri1]

Closes: #2253

What's Changed

TODO

• Explanation of Change(s)
• Added change log fragment(s) (for more information see the documentation)
• Attached Screenshots, Payload Example
• Unit, Integration Tests
• Documentation Updates (when adding/changing features)
• Example Plugin Updates (when adding/changing features)
• Outline Remaining Work, Constraints from Design

[glennmatthews]

Do we need to also cover the case where an interface is initially tagged mode and has tagged_vlans, then the user changes the mode to untagged but doesn't clear the vlans? More generally, how does this impact the UI flow when creating or editing interfaces?

[gsnider2195]

Do we need to also cover the case where an interface is initially tagged mode and has tagged_vlans, then the user changes the mode to untagged but doesn't clear the vlans? More generally, how does this impact the UI flow when creating or editing interfaces?

The BaseInterface.save() method handles clearing tagged_vlans if the mode isn't tagged.

[glennmatthews]

Do we need to also cover the case where an interface is initially tagged mode and has tagged_vlans, then the user changes the mode to untagged but doesn't clear the vlans? More generally, how does this impact the UI flow when creating or editing interfaces?

The BaseInterface.save() method handles clearing tagged_vlans if the mode isn't tagged.

Isn't that the original problem described by #2253 though? It silently clears and discards tagged_vlans instead of warning the user about the invalid data.

[glennmatthews]

Based on the description of #2253 I would have expected some change to Interface.clean() and/or Interface.save(). At the very least it would seem like the logic to remove tagged vlans when changing interface mode should no longer be needed?

[gsnider2195]

Based on the description of #2253 I would have expected some change to Interface.clean() and/or Interface.save(). At the very least it would seem like the logic to remove tagged vlans when changing interface mode should no longer be needed?

I think this needs to go in the form and serializer validation since tagged_vlans is a ManyToManyField.

[timizuoebideri1]

Based on the description of #2253 I would have expected some change to Interface.clean() and/or Interface.save(). At the very least it would seem like the logic to remove tagged vlans when changing interface mode should no longer be needed?

I think this needs to go in the form and serializer validation since tagged_vlans is a ManyToManyField.

Because the UI clears values in the tagged_vlans field when the mode is changed, it would be redundant in form clean.

[glennmatthews]

Does this make the self.tagged_vlans.clear() in save() below now redundant? Should it be removed?

[phoenixswiss]

Hi

After implementing this fix, I'm unable to change an interface from mode 'tagged' with assigned vlans to mode 'access' in one api call (e.g. via pynautobot). I have to remove all the vlans from the interface in the first api call, save it and then change the mode to untagged and save it again.

The same issue occurs in the nautobot gui.
On an interface configured with mode "tagged" and vlans assigned to an interface, it's not possible to remove all vlans form the "Tagged VLANs" field and change the mode to "access" in one step. After doing so and pressing the update button, the interface edit page loads again with an invisible error message. The mode is set to "access" and the untagged vlan is also set to the expected values. If you change the mode back to "tagged" the additional "Tagged VLANs" field with the error message "Clear tagged_vlans to set mode to access" appears now. At this stage you can't modify the interface further. You have to press cancel on the interface edit page and open the interface again. Next all tagged vlans have to be removed from the interface and change must be first saved via the update button. Next you can change the interface mode to untagged and save it again. Thats not very elegant from a user perspective and doubles the amount of api calls if you have to change an interface mode from "tagged" to "untagged".

Is this an expected behaviour?

Thanks

[glennmatthews]

Doesn't sound like expected behavior to me! @timizuoebideri1 can you please look into this?