Merge pull request #2757 from johannaengland/bugfix/maintenance-valid…

…ation

Add more validation for posting maintenance tasks

python/nav/web/maintenance/forms.py

@@ -64,6 +64,8 @@ def clean(self):
         no_end_time = self.cleaned_data['no_end_time']
         if not no_end_time and not end_time:
             raise forms.ValidationError("End time or no end time must be specified")
+        if end_time and end_time < self.cleaned_data["start_time"]:
+            raise forms.ValidationError("End time must be after start time")
         return self.cleaned_data
 
 

python/nav/web/maintenance/utils.py

@@ -98,6 +98,7 @@ def infodict_by_state(task):
 
 def get_component_keys(post):
     remove = {}
+    errors = []
     raw_component_keys = {
         'service': post.getlist('service'),
         'netbox': post.getlist('netbox'),
@@ -125,48 +126,77 @@ def get_component_keys(post):
         for value in raw_component_keys[key]:
             if not remove or value not in remove[key]:
                 if key in PRIMARY_KEY_INTEGER:
+                    if not value.isdigit():
+                        errors.append(key + ": argument needs to be a number")
+                        continue
                     value = int(value)
                 if value not in component_keys[key]:
                     component_keys[key].append(value)
-    return component_keys
+    return component_keys, errors
 
 
 def components_for_keys(component_keys):
     component_data = {}
-    component_data['service'] = Service.objects.filter(
-        id__in=component_keys['service']
-    ).values(
-        'id',
-        'handler',
-        'netbox__id',
-        'netbox__sysname',
-        'netbox__ip',
-        'netbox__room__id',
-        'netbox__room__description',
-        'netbox__room__location__id',
-        'netbox__room__location__description',
-    )
-    component_data['netbox'] = Netbox.objects.filter(
-        id__in=component_keys['netbox']
-    ).values(
-        'id',
-        'sysname',
-        'ip',
-        'room__id',
-        'room__description',
-        'room__location__id',
-        'room__location__description',
-    )
-    component_data['room'] = Room.objects.filter(id__in=component_keys['room']).values(
-        'id', 'description', 'location__id', 'location__description'
-    )
-    component_data['location'] = Location.objects.filter(
-        id__in=component_keys['location']
-    ).values('id', 'description')
-    component_data['netboxgroup'] = NetboxGroup.objects.filter(
-        id__in=component_keys['netboxgroup']
-    ).values('id', 'description')
-    return component_data
+    component_data_errors = []
+    if component_keys['service']:
+        component_data['service'] = Service.objects.filter(
+            id__in=component_keys['service']
+        ).values(
+            'id',
+            'handler',
+            'netbox__id',
+            'netbox__sysname',
+            'netbox__ip',
+            'netbox__room__id',
+            'netbox__room__description',
+            'netbox__room__location__id',
+            'netbox__room__location__description',
+        )
+        if not component_data['service']:
+            component_data_errors.append(
+                "service: no elements with the given identifiers found"
+            )
+    if component_keys['netbox']:
+        component_data['netbox'] = Netbox.objects.filter(
+            id__in=component_keys['netbox']
+        ).values(
+            'id',
+            'sysname',
+            'ip',
+            'room__id',
+            'room__description',
+            'room__location__id',
+            'room__location__description',
+        )
+        if not component_data['netbox']:
+            component_data_errors.append(
+                "netbox: no elements with the given identifiers found"
+            )
+    if component_keys['room']:
+        component_data['room'] = Room.objects.filter(
+            id__in=component_keys['room']
+        ).values('id', 'description', 'location__id', 'location__description')
+        if not component_data['room']:
+            component_data_errors.append(
+                "room: no elements with the given identifiers found"
+            )
+    if component_keys['location']:
+        component_data['location'] = Location.objects.filter(
+            id__in=component_keys['location']
+        ).values('id', 'description')
+        if not component_data['location']:
+            component_data_errors.append(
+                "location: no elements with the given identifiers found"
+            )
+    if component_keys['netboxgroup']:
+        component_data['netboxgroup'] = NetboxGroup.objects.filter(
+            id__in=component_keys['netboxgroup']
+        ).values('id', 'description')
+        if not component_data['netboxgroup']:
+            component_data_errors.append(
+                "netboxgroup: no elements with the given identifiers found"
+            )
+    return component_data, component_data_errors
 
 
 def structure_component_data(component_data):

python/nav/web/maintenance/views.py

@@ -205,7 +205,7 @@ def view(request, task_id):
             value = int(value)
         component_keys[key].append(value)
 
-    component_data = components_for_keys(component_keys)
+    component_data, _ = components_for_keys(component_keys)
     components = structure_component_data(component_data)
     component_trail = task_component_trails(component_keys, components)
 
@@ -261,7 +261,8 @@ def edit(request, task_id=None, start_time=None, **_):
     task_form = MaintenanceTaskForm(initial=task_form_initial(task, start_time))
 
     if request.method == 'POST':
-        component_keys = get_component_keys(request.POST)
+        component_keys, component_keys_errors = get_component_keys(request.POST)
+
     elif task:
         component_keys = {
             'service': [],
@@ -275,19 +276,24 @@ def edit(request, task_id=None, start_time=None, **_):
                 value = int(value)
             component_keys[key].append(value)
     else:
-        component_keys = get_component_keys(request.GET)
+        component_keys, component_keys_errors = get_component_keys(request.GET)
 
     if component_keys:
-        component_data = components_for_keys(component_keys)
+        component_data, component_data_errors = components_for_keys(component_keys)
         components = structure_component_data(component_data)
         component_trail = task_component_trails(component_keys, components)
+        if component_data_errors:
+            new_message(request, ",".join(component_data_errors), Messages.ERROR)
+
+    if component_keys_errors:
+        new_message(request, ",".join(component_keys_errors), Messages.ERROR)
 
     if request.method == 'POST':
         if 'save' in request.POST:
             task_form = MaintenanceTaskForm(request.POST)
-            if not any(component_data.values()):
+            if component_keys and not any(component_data.values()):
                 new_message(request, "No components selected.", Messages.ERROR)
-            elif task_form.is_valid():
+            elif not component_keys_errors and task_form.is_valid():
                 start_time = task_form.cleaned_data['start_time']
                 end_time = task_form.cleaned_data['end_time']
                 no_end_time = task_form.cleaned_data['no_end_time']

tests/integration/web/maintenance/views_test.py

@@ -14,6 +14,11 @@
 # License along with NAV. If not, see <http://www.gnu.org/licenses/>.
 #
 
+from django.urls import reverse
+from nav.compatibility import smart_str
+from nav.models.manage import Netbox
+from nav.models.msgmaint import MaintenanceTask
+
 
 class TestMaintenanceCalendarView:
     def test_calendar_renders_when_no_arguments_given(self, client):
@@ -23,3 +28,111 @@ def test_calendar_renders_when_no_arguments_given(self, client):
     def test_calendar_still_renders_when_invalid_arguments_given(self, client):
         response = client.get('/maintenance/?year=invalid&month=invalid', follow=True)
         assert response.status_code == 200
+
+
+class TestAddMaintenanceTask:
+    def test_valid_data_without_end_time_should_suceed(self, db, client, localhost):
+        url = reverse('maintenance-new')
+        data = {
+            "start_time": "2023-11-21 12:40",
+            "no_end_time": "on",
+            "description": "maintenance",
+            "netbox": localhost.pk,
+            "save": "Save+task",
+        }
+
+        response = client.post(
+            url,
+            follow=True,
+            data=data,
+        )
+
+        assert response.status_code == 200
+        assert f'Saved task {data["description"]}' in smart_str(response.content)
+        assert MaintenanceTask.objects.filter(description=data["description"]).exists()
+
+    def test_valid_data_with_end_time_should_suceed(self, db, client, localhost):
+        url = reverse('maintenance-new')
+        data = {
+            "start_time": "2023-11-21 12:40",
+            "end_time": "2023-11-25 12:40",
+            "description": "maintenance",
+            "netbox": localhost.pk,
+            "save": "Save+task",
+        }
+
+        response = client.post(
+            url,
+            follow=True,
+            data=data,
+        )
+
+        assert response.status_code == 200
+        assert f'Saved task {data["description"]}' in smart_str(response.content)
+        assert MaintenanceTask.objects.filter(description=data["description"]).exists()
+
+    def test_with_non_int_netbox_key_should_fail(self, db, client):
+        url = reverse('maintenance-new')
+        data = {
+            "start_time": "2023-11-21 12:40",
+            "no_end_time": "on",
+            "description": "maintenance",
+            "netbox": "137'",
+            "save": "Save+task",
+        }
+
+        response = client.post(
+            url,
+            follow=True,
+            data=data,
+        )
+
+        assert response.status_code == 200
+        assert "netbox: argument needs to be a number" in smart_str(response.content)
+        assert not MaintenanceTask.objects.filter(
+            description=data["description"]
+        ).exists()
+
+    def test_with_non_int_netbox_key_in_url_should_fail(self, db, client):
+        url = reverse('maintenance-new') + '?netbox=foobar'
+
+        response = client.get(url, follow=True)
+
+        assert response.status_code == 200
+        assert "netbox: argument needs to be a number" in smart_str(response.content)
+
+    def test_with_non_existent_netbox_key_in_url_should_fail(self, db, client):
+        last_netbox_id = getattr(Netbox.objects.last(), "pk", 0)
+        url = (
+            reverse('maintenance-new')
+            + f'?netbox={last_netbox_id+1}&netbox={last_netbox_id+2}'
+        )
+
+        response = client.get(url, follow=True)
+
+        assert response.status_code == 200
+        assert "netbox: no elements with the given identifiers found" in smart_str(
+            response.content
+        )
+
+    def test_with_end_time_before_start_time_should_fail(self, db, client, localhost):
+        url = reverse('maintenance-new')
+        data = {
+            "start_time": "2023-11-22 14:35",
+            "end_time": "2023-11-08 14:35",
+            "description": "maintenance",
+            "netbox": localhost.pk,
+            "save": "Save+task",
+        }
+
+        response = client.post(
+            url,
+            follow=True,
+            data=data,
+        )
+
+        assert response.status_code == 200
+        assert "End time must be after start time" in smart_str(response.content)
+        assert not MaintenanceTask.objects.filter(
+            description=data["description"]
+        ).exists()