diff --git a/changelog.d/3425.changed.md b/changelog.d/3425.changed.md new file mode 100644 index 0000000000..fd2045c508 --- /dev/null +++ b/changelog.d/3425.changed.md @@ -0,0 +1 @@ +Use HTMX for selecting and modifying components for Maintenance tasks. Replaces Quickselect implementation. diff --git a/python/nav/web/maintenance/urls.py b/python/nav/web/maintenance/urls.py index ec26d62093..444380f2b0 100644 --- a/python/nav/web/maintenance/urls.py +++ b/python/nav/web/maintenance/urls.py @@ -32,6 +32,12 @@ re_path(r'^active/$', views.active, name='maintenance-active'), re_path(r'^planned/$', views.planned, name='maintenance-planned'), re_path(r'^historic/$', views.historic, name='maintenance-historic'), + re_path(r'^search/$', views.component_search, name='maintenance-component-search'), + re_path( + r'^selectcomponents/$', + views.component_select, + name='maintenance-component-select', + ), re_path(r'^new/$', views.edit, name='maintenance-new'), re_path( r'^new/(?P\d{4}-\d{2}-\d{2})/$', diff --git a/python/nav/web/maintenance/utils.py b/python/nav/web/maintenance/utils.py index 4502ab5daf..8653507a59 100644 --- a/python/nav/web/maintenance/utils.py +++ b/python/nav/web/maintenance/utils.py @@ -173,6 +173,18 @@ def get_component_keys(post): return component_keys, errors +def get_component_name(model: models.Model): + """Returns a short name for the component type based on its model class. + + Used as the input name for component keys in forms and APIs. + + Location is abbreviated to 'loc' to avoid XSS issues. + """ + if model._meta.db_table == 'location': + return 'loc' + return model._meta.db_table + + def get_components_from_keydict( component_keys: dict[str, List[Union[int, str]]], ) -> tuple[List[ComponentType], List[str]]: @@ -206,6 +218,36 @@ def get_components_from_keydict( return components, component_data_errors +def prefetch_and_group_components( + component_type: models.Model, + query_results: models.QuerySet, + group_by: Union[models.Model, None] = None, +): + """ + Prefetches the related model and groups components by the related model name. + """ + if not group_by: + return query_results + + group_by_name = group_by._meta.db_table + + if hasattr(query_results, 'prefetch_related') and hasattr( + component_type, group_by_name + ): + query_results = query_results.prefetch_related(group_by_name) + + grouped_results = {} + for component in query_results: + group_by_model = getattr(component, group_by_name) + group_name = str(group_by_model) + + if group_name not in grouped_results: + grouped_results[group_name] = [] + grouped_results[group_name].append(component) + + return [(group, components) for group, components in grouped_results.items()] + + class MaintenanceCalendar(HTMLCalendar): START = 'start' END = 'end' diff --git a/python/nav/web/maintenance/views.py b/python/nav/web/maintenance/views.py index 9be65f0778..bf9f8ab493 100644 --- a/python/nav/web/maintenance/views.py +++ b/python/nav/web/maintenance/views.py @@ -18,18 +18,21 @@ import logging import time from datetime import datetime +from typing import Optional from django.db import connection, transaction -from django.db.models import Count, Q +from django.db.models import Count, Model, Q from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils.safestring import mark_safe +from django.views.decorators.http import require_http_methods import nav.maintengine from nav.django.utils import get_account -from nav.models.manage import Netbox +from nav.models.manage import Location, Netbox, NetboxGroup, Room from nav.models.msgmaint import MaintenanceComponent, MaintenanceTask +from nav.models.service import Service from nav.web.maintenance.forms import ( MaintenanceAddSingleNetbox, MaintenanceCalendarForm, @@ -42,13 +45,14 @@ MaintenanceCalendar, component_to_trail, get_component_keys, + get_component_name, get_components, get_components_from_keydict, + prefetch_and_group_components, infodict_by_state, task_form_initial, ) from nav.web.message import Messages, new_message -from nav.web.quickselect import QuickSelect INFINITY = datetime.max @@ -242,7 +246,6 @@ def cancel(request, task_id): @transaction.atomic() def edit(request, task_id=None, start_time=None, **_): account = get_account(request) - quickselect = QuickSelect(service=True) components = task = None component_keys_errors = [] component_keys = {} @@ -342,13 +345,74 @@ def edit(request, task_id=None, start_time=None, **_): 'heading': heading, 'task_form': task_form, 'task_id': task_id, - 'quickselect': mark_safe(quickselect), 'components': component_trail, 'selected': component_keys, }, ) +@require_http_methods(["POST"]) +def component_search(request): + """HTMX endpoint for component searches from maintenance task form""" + raw_search = request.POST.get("search") + search = raw_search.strip() if raw_search else '' + if not search: + return render( + request, 'maintenance/_component-search-results.html', {'results': {}} + ) + + results = {} + searches: list[tuple[type[Model], Q, Optional[Model]]] = [ + (Location, Q(id__icontains=search), None), + (Room, Q(id__icontains=search), Location), + (Netbox, Q(sysname__icontains=search), Room), + (NetboxGroup, Q(id__icontains=search), None), + ( + Service, + Q(handler__icontains=search) | Q(netbox__sysname__icontains=search), + Netbox, + ), + ] + + for component_type, query, group_by in searches: + component_results = component_type.objects.filter(query) + grouped_results = prefetch_and_group_components( + component_type, component_results, group_by + ) + + if component_results: + component_title = get_component_name(component_type) + results[component_title] = { + 'label': component_type._meta.verbose_name.title(), + 'values': grouped_results, + 'has_grouping': group_by is not None, + } + + return render( + request, 'maintenance/_component-search-results.html', {'results': results} + ) + + +@require_http_methods(["POST"]) +def component_select(request): + """HTMX endpoint for component selection from maintenance task form""" + component_keys, component_keys_errors = get_component_keys(request.POST) + for error in component_keys_errors: + new_message(request, error, Messages.ERROR) + + components, components_errors = get_components_from_keydict(component_keys) + for error in components_errors: + new_message(request, error, Messages.ERROR) + + component_trail = [component_to_trail(c) for c in components] + + return render( + request, + 'maintenance/_selected-components-list.html', + {'components': component_trail, 'selected': component_keys}, + ) + + def add_box_to_maintenance(request): """ This view puts a Netbox on immediate, indefinite maintenance and diff --git a/python/nav/web/static/js/src/maintenance.js b/python/nav/web/static/js/src/maintenance.js index 18965b3507..0b709fcf92 100644 --- a/python/nav/web/static/js/src/maintenance.js +++ b/python/nav/web/static/js/src/maintenance.js @@ -1,15 +1,10 @@ -require(['plugins/quickselect', 'plugins/hover_highlight', "libs/jquery-ui-timepicker-addon"], function (QuickSelect, HoverHighlight) { +require(['plugins/hover_highlight', "libs/jquery-ui-timepicker-addon"], function (HoverHighlight) { var calendar = $('.calendar'); - var quickselect = $('.quickselect'); if (calendar.length) { new HoverHighlight(calendar); } - if (quickselect.length) { - new QuickSelect(quickselect); - } - $(document).ready(function(){ $('#id_no_end_time').change(function(){ toggleEndTime(this); @@ -29,6 +24,4 @@ require(['plugins/quickselect', 'plugins/hover_highlight', "libs/jquery-ui-timep $(endTime).removeAttr('disabled'); } } - - }); diff --git a/python/nav/web/templates/maintenance/_component-search-results.html b/python/nav/web/templates/maintenance/_component-search-results.html new file mode 100644 index 0000000000..4687e75a18 --- /dev/null +++ b/python/nav/web/templates/maintenance/_component-search-results.html @@ -0,0 +1,42 @@ +{% if results %} + {% for component_type, result in results.items %} +
+ + +
+ + +
+
+ {% endfor %} +{% else %} + No hits +{% endif %} diff --git a/python/nav/web/templates/maintenance/_selected-components-list.html b/python/nav/web/templates/maintenance/_selected-components-list.html new file mode 100644 index 0000000000..73fabf58db --- /dev/null +++ b/python/nav/web/templates/maintenance/_selected-components-list.html @@ -0,0 +1,52 @@ +{% load maintenance %} +{% for key, identifiers in selected.items %} + {% for id in identifiers %} + + {% endfor %} +{% endfor %} +{% if components %} +
+ + +
+ + +{% else %} +

(none)

+ +{% endif %} diff --git a/python/nav/web/templates/maintenance/new_task.html b/python/nav/web/templates/maintenance/new_task.html index 1fed9ce69c..607d9dfe7d 100644 --- a/python/nav/web/templates/maintenance/new_task.html +++ b/python/nav/web/templates/maintenance/new_task.html @@ -30,9 +30,7 @@

{{ heading }}

-
-
Details @@ -41,46 +39,37 @@

{{ heading }}

-
- Select components - {{ quickselect }} -
+
+ Select components + + + + Loading spinner Searching... + +
+
+
Selected components - {% for key, identifiers in selected.items %} - {% for id in identifiers %} - - {% endfor %} - {% endfor %} - - {% if components %} -
    - {% for trail in components %} - {% with trail|last as component %} -
  • -
    - - - {{ component|model_verbose_name }}: -
    -
    - {% include "maintenance/frag-component-trail.html" %} -
    -
    • - {% endwith %} - {% endfor %} -
- - {% else %} -

(none)

- - {% endif %} +
+ {% include 'maintenance/_selected-components-list.html' with components=components selected=selected %} +
+
{# column #}
{# row #} diff --git a/tests/integration/web/maintenance/utils_test.py b/tests/integration/web/maintenance/utils_test.py new file mode 100644 index 0000000000..2207cc8327 --- /dev/null +++ b/tests/integration/web/maintenance/utils_test.py @@ -0,0 +1,27 @@ +from nav.models.manage import Netbox, Room +from nav.web.maintenance.utils import prefetch_and_group_components + + +class TestPrefetchAndGroupComponents: + def test_should_return_list_of_tuples(self): + netbox_query = Netbox.objects.all() + results = prefetch_and_group_components(Netbox, netbox_query, Room) + assert results and all(isinstance(item, tuple) for item in results), ( + "Results should be tuples." + ) + + def test_should_group_by_room(self): + netbox = Netbox.objects.first() + netbox_query = Netbox.objects.filter(id=netbox.id) + results = prefetch_and_group_components(Netbox, netbox_query, Room) + room_name, netbox_list = results[0] + assert room_name == str(netbox.room) and netbox.id == netbox_list[0].id, ( + "Room name and Netbox ID do not match expected values." + ) + + def test_when_group_by_is_none_it_should_return_flat_list(self): + netbox_query = Netbox.objects.all() + results = prefetch_and_group_components(Netbox, netbox_query, None) + assert all(isinstance(item, Netbox) for item in results), ( + "Results should be Netbox instances." + ) diff --git a/tests/integration/web/maintenance/views_test.py b/tests/integration/web/maintenance/views_test.py index a380036c7a..3f9fb3cf32 100644 --- a/tests/integration/web/maintenance/views_test.py +++ b/tests/integration/web/maintenance/views_test.py @@ -19,7 +19,7 @@ from django.urls import reverse from django.utils.encoding import smart_str -from nav.models.manage import Netbox +from nav.models.manage import Netbox, Room, Location from nav.models.msgmaint import MaintenanceTask @@ -152,6 +152,81 @@ def test_when_existing_task_is_requested_it_should_render_with_intact_descriptio assert empty_maintenance_task.description in smart_str(response.content) +class TestSearchMaintenanceComponents: + def test_given_an_existing_room_then_component_search_should_return_results( + self, db, client, new_room + ): + url = reverse('maintenance-component-search') + response = client.post(url, {'search': new_room.id}) + assert response.status_code == 200 + assert new_room.id in smart_str(response.content) + + def test_given_a_non_existing_component_then_component_search_should_return_no_hits( + self, client + ): + url = reverse('maintenance-component-search') + response = client.post(url, {'search': 'nonexistent-component'}) + assert response.status_code == 200 + assert "No hits" in smart_str(response.content) + + def test_given_an_empty_query_then_component_search_should_return_no_hits( + self, client + ): + url = reverse('maintenance-component-search') + response = client.post(url, {'search': ''}) + assert response.status_code == 200 + assert "No hits" in smart_str(response.content) + + +class TestSelectMaintenanceComponents: + def test_given_an_existing_room_then_component_select_should_return_results( + self, db, client, new_room + ): + url = reverse('maintenance-component-select') + response = client.post( + url, + { + 'room': new_room.id, + }, + ) + assert response.status_code == 200 + assert f"name=\"room\" value=\"{new_room.id}\"" in smart_str(response.content) + + def test_should_remove_given_room_from_response(self, db, client, new_room): + url = reverse('maintenance-component-select') + response = client.post( + url, + { + 'room': new_room.id, + 'remove_room': new_room.id, + 'remove': [''], + }, + ) + assert response.status_code == 200 + assert f"name=\"room\" value=\"{new_room.id}\"" not in smart_str( + response.content + ) + + def test_when_given_all_components_to_remove_then_it_should_return_none( + self, db, client, new_room + ): + url = reverse('maintenance-component-select') + room = new_room + location_id = room.location.id + response = client.post( + url, + { + 'room': room.id, + 'remove_room': room.id, + 'location': location_id, + 'remove_location': location_id, + 'remove': [''], + }, + ) + assert response.status_code == 200 + assert "(none)" in smart_str(response.content) + + @pytest.fixture def empty_maintenance_task(db): now = datetime.datetime.now() @@ -163,3 +238,14 @@ def empty_maintenance_task(db): task.save() yield task task.delete() + + +@pytest.fixture +def new_room(db): + location = Location(id="testlocation") + location.save() + room = Room(id="123", description="Test Room", location=location) + room.save() + yield room + room.delete() + location.delete() diff --git a/tests/unittests/web/maintenance/utils_test.py b/tests/unittests/web/maintenance/utils_test.py new file mode 100644 index 0000000000..88cf902e35 --- /dev/null +++ b/tests/unittests/web/maintenance/utils_test.py @@ -0,0 +1,16 @@ +from nav.models.manage import Netbox, Room, Location +from nav.web.maintenance.utils import get_component_name + + +class TestGetComponentName: + def test_get_component_names(self): + components = [Netbox, Room, Location] + expected_names = ["netbox", "room", "loc"] + calculated_names = [get_component_name(component) for component in components] + assert calculated_names == expected_names, ( + "Component names do not match expected values." + ) + + def test_should_return_short_name_for_location(self): + name = get_component_name(Location) + assert name == "loc"