diff --git a/CHANGELOG.md b/CHANGELOG.md
index 32764a69a1..556740e783 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
This changelog format was introduced in NAV 5.4.0. Older changelogs can be
found in the [HISTORY](HISTORY) file.
+## [Unreleased]
+
+### Fixed
+
+- Allow admins to configure ports with invalid or unset native VLANs in PortAdmin ([#2477](https://github.com/Uninett/nav/issues/2477), [#2786](https://github.com/Uninett/nav/pull/2786))
+- Fix bug that caused PoE config to be completely disabled for Cisco devices where at least one port did not support PoE ([#2781](https://github.com/Uninett/nav/pull/2781))
+- Fix PortAdmin save button moving around for ports without PoE support ([#2782](https://github.com/Uninett/nav/pull/2782))
+- Fix PortAdmin bug that prevented switching PoE state back and forth without reloading entire page ([#2785](https://github.com/Uninett/nav/pull/2785))
+- Fix regression that caused maintenance tasks to be un-editable ([#2783](https://github.com/Uninett/nav/issues/2783), [#2784](https://github.com/Uninett/nav/pull/2784))
+
## [5.8.3] - 2023-12-01
### Fixed
diff --git a/python/nav/portadmin/handlers.py b/python/nav/portadmin/handlers.py
index 5a2e8816a1..7c74e559df 100644
--- a/python/nav/portadmin/handlers.py
+++ b/python/nav/portadmin/handlers.py
@@ -352,7 +352,3 @@ class POEStateNotSupportedError(ManagementError):
class XMLParseError(ManagementError):
"""Raised when failing to parse XML"""
-
-
-class POEIndexNotFoundError(ManagementError):
- """Raised when a PoE index could not be found for an interface"""
diff --git a/python/nav/portadmin/snmp/cisco.py b/python/nav/portadmin/snmp/cisco.py
index bbe1a9d18e..d2f4fa5b0c 100644
--- a/python/nav/portadmin/snmp/cisco.py
+++ b/python/nav/portadmin/snmp/cisco.py
@@ -28,7 +28,6 @@
PoeState,
POEStateNotSupportedError,
POENotSupportedError,
- POEIndexNotFoundError,
)
from nav.models import manage
@@ -351,7 +350,7 @@ def _get_poe_indexes_for_interface(
try:
poeport = manage.POEPort.objects.get(interface=interface)
except manage.POEPort.DoesNotExist:
- raise POEIndexNotFoundError(
+ raise POENotSupportedError(
"This interface does not have PoE indexes defined"
)
unit_number = poeport.poegroup.index
diff --git a/python/nav/web/maintenance/views.py b/python/nav/web/maintenance/views.py
index 7d4d653e9f..9eab84a44b 100644
--- a/python/nav/web/maintenance/views.py
+++ b/python/nav/web/maintenance/views.py
@@ -252,9 +252,7 @@ def cancel(request, task_id):
def edit(request, task_id=None, start_time=None, **_):
account = get_account(request)
quickselect = QuickSelect(service=True)
- component_trail = None
- component_keys = None
- task = None
+ component_trail = component_keys_errors = component_data = task = None
if task_id:
task = get_object_or_404(MaintenanceTask, pk=task_id)
diff --git a/python/nav/web/portadmin/utils.py b/python/nav/web/portadmin/utils.py
index 221b06e170..072054b6bd 100644
--- a/python/nav/web/portadmin/utils.py
+++ b/python/nav/web/portadmin/utils.py
@@ -15,7 +15,7 @@
#
"""Util functions for the PortAdmin"""
from __future__ import unicode_literals
-from typing import List, Sequence, Dict, Any
+from typing import List, Sequence, Dict, Any, Optional
import re
import logging
from operator import attrgetter
@@ -24,6 +24,7 @@
from nav.models import manage, profiles
from nav.django.utils import is_admin
+from nav.models.profiles import Account
from nav.portadmin.config import CONFIG
from nav.portadmin.management import ManagementFactory
from nav.portadmin.handlers import ManagementHandler
@@ -76,7 +77,7 @@ def find_and_populate_allowed_vlans(
):
"""Finds allowed vlans and indicate which interfaces can be edited"""
allowed_vlans = find_allowed_vlans_for_user_on_netbox(account, netbox, handler)
- set_editable_flag_on_interfaces(interfaces, allowed_vlans)
+ set_editable_flag_on_interfaces(interfaces, allowed_vlans, account)
return allowed_vlans
@@ -126,7 +127,9 @@ def find_allowed_vlans_for_user(account):
def set_editable_flag_on_interfaces(
- interfaces: Sequence[manage.Interface], vlans: Sequence[FantasyVlan]
+ interfaces: Sequence[manage.Interface],
+ vlans: Sequence[FantasyVlan],
+ user: Optional[Account] = None,
):
"""Sets the pseudo-attribute `iseditable` on each interface in the interfaces
list, indicating whether the PortAdmin UI should allow edits to it or not.
@@ -136,8 +139,13 @@ def set_editable_flag_on_interfaces(
an uplink, depending on how portadmin is configured.
"""
vlan_tags = {vlan.vlan for vlan in vlans}
+ allow_everything = not should_check_access_rights(account=user) if user else False
for interface in interfaces:
+ if allow_everything:
+ interface.iseditable = True
+ continue
+
vlan_is_acceptable = interface.vlan in vlan_tags
is_link = bool(interface.to_netbox)
refuse_link_edit = not CONFIG.get_link_edit() and is_link
diff --git a/python/nav/web/portadmin/views.py b/python/nav/web/portadmin/views.py
index 211910965a..e021807750 100644
--- a/python/nav/web/portadmin/views.py
+++ b/python/nav/web/portadmin/views.py
@@ -56,7 +56,6 @@
NoResponseError,
ProtocolError,
ManagementError,
- POEIndexNotFoundError,
XMLParseError,
POEStateNotSupportedError,
)
@@ -248,7 +247,6 @@ def populate_infodict(request, netbox, interfaces):
messages.error(request, str(error))
except (
- POEIndexNotFoundError,
XMLParseError,
POEStateNotSupportedError,
) as error:
diff --git a/python/nav/web/static/js/src/portadmin.js b/python/nav/web/static/js/src/portadmin.js
index 4b1224d1d7..621b0c8864 100644
--- a/python/nav/web/static/js/src/portadmin.js
+++ b/python/nav/web/static/js/src/portadmin.js
@@ -482,6 +482,9 @@ require(['libs/spin.min', 'libs/jquery-ui.min'], function (Spinner) {
if ('ifadminstatus' in data) {
updateAdminStatusDefault($row, data.ifadminstatus);
}
+ if ('poe_state' in data) {
+ updatePoeDefault($row, data.poe_state);
+ }
}
function updateIfAliasDefault($row, ifalias) {
@@ -518,6 +521,15 @@ require(['libs/spin.min', 'libs/jquery-ui.min'], function (Spinner) {
}
}
+ function updatePoeDefault($row, new_value) {
+ var old_value = $row.find('option[data-orig]').val();
+ if (old_value !== new_value) {
+ console.log('Updating PoE state default from ' + old_value + ' to ' + new_value);
+ $row.find('option[data-orig]').removeAttr('data-orig');
+ $row.find('option[value=' + new_value + ']').attr('data-orig', new_value);
+ }
+ }
+
function removeFromQueue(id) {
if (queue_data.hasOwnProperty(id)) {
delete queue_data[id];
diff --git a/python/nav/web/templates/portadmin/portlist.html b/python/nav/web/templates/portadmin/portlist.html
index e6f40e6a98..342e52033f 100644
--- a/python/nav/web/templates/portadmin/portlist.html
+++ b/python/nav/web/templates/portadmin/portlist.html
@@ -169,20 +169,19 @@
{# POE STATE #}
{% if supports_poe %}
- {% if interface.supports_poe %}
-
+ {% endif %}
- {% endif %}
{% endif %}
{# Button for saving #}
diff --git a/tests/integration/web/maintenance/views_test.py b/tests/integration/web/maintenance/views_test.py
index 8109fcaae9..1987c62c2d 100644
--- a/tests/integration/web/maintenance/views_test.py
+++ b/tests/integration/web/maintenance/views_test.py
@@ -13,8 +13,11 @@
# more details. You should have received a copy of the GNU General Public
# License along with NAV. If not, see .
#
+import datetime
+import pytest
from django.urls import reverse
+
from nav.compatibility import smart_str
from nav.models.manage import Netbox
from nav.models.msgmaint import MaintenanceTask
@@ -136,3 +139,27 @@ def test_with_end_time_before_start_time_should_fail(self, db, client, localhost
assert not MaintenanceTask.objects.filter(
description=data["description"]
).exists()
+
+
+class TestEditMaintenanceTask:
+ def test_when_existing_task_is_requested_it_should_render_with_intact_description(
+ self, db, client, localhost, empty_maintenance_task
+ ):
+ url = reverse("maintenance-edit", kwargs={"task_id": empty_maintenance_task.id})
+ response = client.get(url, follow=True)
+
+ assert response.status_code == 200
+ assert empty_maintenance_task.description in smart_str(response.content)
+
+
+@pytest.fixture
+def empty_maintenance_task(db):
+ now = datetime.datetime.now()
+ task = MaintenanceTask(
+ start_time=now,
+ end_time=now + datetime.timedelta(hours=1),
+ description="Temporary test fixture task",
+ )
+ task.save()
+ yield task
+ task.delete()
diff --git a/tests/unittests/portadmin/portadmin_poe_cisco_test.py b/tests/unittests/portadmin/portadmin_poe_cisco_test.py
index 2c48c638d0..717d0252d2 100644
--- a/tests/unittests/portadmin/portadmin_poe_cisco_test.py
+++ b/tests/unittests/portadmin/portadmin_poe_cisco_test.py
@@ -3,10 +3,7 @@
import pytest
from nav.portadmin.snmp.cisco import Cisco
-from nav.portadmin.handlers import (
- POEIndexNotFoundError,
- POEStateNotSupportedError,
-)
+from nav.portadmin.handlers import POEStateNotSupportedError
from nav.models import manage
@@ -23,24 +20,24 @@ class TestGetPoeState:
@pytest.mark.usefixtures('poeport_get_mock')
def test_should_raise_exception_if_unknown_poe_state(self, handler_cisco):
handler_cisco._query_netbox = Mock(return_value=76)
- interface = Mock(interface="interface")
+ interface = Mock(ifname="interface")
with pytest.raises(POEStateNotSupportedError):
handler_cisco.get_poe_states([interface])
@pytest.mark.usefixtures('poeport_get_mock_error')
- def test_should_raise_exception_if_interface_is_missing_poeport(
+ def test_dict_should_give_none_if_interface_does_not_have_poeport(
self, handler_cisco
):
- interface = Mock(interface="interface")
- with pytest.raises(POEIndexNotFoundError):
- handler_cisco.get_poe_states([interface])
+ interface = Mock(ifname="interface")
+ states = handler_cisco.get_poe_states([interface])
+ assert states[interface.ifname] is None
@pytest.mark.usefixtures('poeport_get_mock')
def test_dict_should_give_none_if_interface_does_not_support_poe(
self, handler_cisco
):
handler_cisco._query_netbox = Mock(return_value=None)
- interface = Mock(interface="interface")
+ interface = Mock(ifname="interface")
states = handler_cisco.get_poe_states([interface])
assert states[interface.ifname] is None
diff --git a/tests/unittests/web/portadmin/__init__.py b/tests/unittests/web/portadmin/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/unittests/web/portadmin/utils_test.py b/tests/unittests/web/portadmin/utils_test.py
new file mode 100644
index 0000000000..84ccff0824
--- /dev/null
+++ b/tests/unittests/web/portadmin/utils_test.py
@@ -0,0 +1,39 @@
+from unittest.mock import patch, Mock
+
+from nav.web.portadmin.utils import set_editable_flag_on_interfaces
+
+
+class TestSetEditableFlagOnInterfaces:
+ def test_when_user_is_admin_it_should_set_all_interfaces_to_editable(self):
+ with patch(
+ "nav.web.portadmin.utils.should_check_access_rights", return_value=False
+ ):
+ mock_admin = Mock()
+ mock_interfaces = [Mock(iseditable=False)] * 3
+ set_editable_flag_on_interfaces(mock_interfaces, [], mock_admin)
+
+ assert all(ifc.iseditable for ifc in mock_interfaces)
+
+ def test_when_user_is_not_admin_it_should_set_only_matching_interfaces_to_editable(
+ self,
+ ):
+ with patch(
+ "nav.web.portadmin.utils.should_check_access_rights", return_value=True
+ ):
+ mock_user = Mock()
+ mock_vlans = [Mock(vlan=42), Mock(vlan=69), Mock(vlan=666)]
+ editable_interface = Mock(vlan=666, iseditable=False)
+ mock_interfaces = [
+ Mock(vlan=99, iseditable=False),
+ editable_interface,
+ Mock(vlan=27, iseditable=False),
+ ]
+
+ set_editable_flag_on_interfaces(mock_interfaces, mock_vlans, mock_user)
+
+ assert editable_interface.iseditable
+ assert all(
+ not ifc.iseditable
+ for ifc in mock_interfaces
+ if ifc is not editable_interface
+ )