diff --git a/python/nav/web/maintenance/forms.py b/python/nav/web/maintenance/forms.py index 2ef1961ad9..9bfe3a2c7e 100644 --- a/python/nav/web/maintenance/forms.py +++ b/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 diff --git a/python/nav/web/maintenance/utils.py b/python/nav/web/maintenance/utils.py index bdfb951ad1..c512df60f8 100644 --- a/python/nav/web/maintenance/utils.py +++ b/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): diff --git a/python/nav/web/maintenance/views.py b/python/nav/web/maintenance/views.py index fe0751ed48..7d4d653e9f 100644 --- a/python/nav/web/maintenance/views.py +++ b/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'] diff --git a/tests/integration/web/maintenance/views_test.py b/tests/integration/web/maintenance/views_test.py index 78b1dc91ea..8109fcaae9 100644 --- a/tests/integration/web/maintenance/views_test.py +++ b/tests/integration/web/maintenance/views_test.py @@ -14,6 +14,11 @@ # License along with NAV. If not, see . # +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()