servershell: support multiple objects in history and changes

packages/serveradmin/serveradmin/serverdb/templates/serverdb/changes.html

@@ -70,7 +70,11 @@ <h3>
             <div class="form-group row input-controls">
                 <label for="object_id" class="col-sm-1 col-form-label">Object ID:</label>
                 <div class="col-md-4">
-                    <input id="object_id" name="object_id" type="text" value="{% if object_id %}{{ object_id }}{% endif %}" class="form-control form-control-sm" placeholder="466880" />
+                    {% for object_id in object_ids %}
+                        <input {% if forloop.first %}id="object_id" {% endif %}name="object_id" type="text" value="{{ object_id }}" class="form-control form-control-sm" placeholder="466880" />
+                    {% empty %}
+                        <input id="object_id" name="object_id" type="text" value="" class="form-control form-control-sm" placeholder="466880" />
+                    {% endfor %}
                 </div>
             </div>
             <div class="form-group row input-controls">

packages/serveradmin/serveradmin/serverdb/templates/serverdb/history.html

@@ -1,6 +1,6 @@
 {% extends "base.html" %}
 
-{% block title %}History for {{ name }}{% endblock %}
+{% block title %}History for {% if hostnames|length == 1 %}{{ hostnames.0 }}{% else %}{{ hostnames|length }} objects{% endif %}{% endblock %}
 
 {% block additional_styles %}
     <link rel="stylesheet" href="{{ STATIC_URL }}css/history.css"/>
@@ -10,7 +10,10 @@
 <form id="changes-form" method="get" action="{% url 'serverdb_history' %}">
     <div class="row">
         <div class="col-md-12">
-            <h3>History for {{ name }}</h3>
+            <h3>History for {% if hostnames|length == 1 %}{{ hostnames.0 }}{% else %}{{ hostnames|length }} objects{% endif %}</h3>
+            {% if hostnames|length > 1 %}
+                <p>{{ hostnames|join:", " }}</p>
+            {% endif %}
         </div>
     </div>
     <div class="row">
@@ -22,7 +25,7 @@ <h3>History for {{ name }}</h3>
             <div>
                 <ul class="attributes">
                     {% for attribute in no_history_attributes %}
-                        <li>{{ attribute.attribute_id }}</li>
+                        <li>{{ attribute }}</li>
                     {% endfor %}
                 </ul>
             </div>
@@ -34,7 +37,9 @@ <h3>History for {{ name }}</h3>
                 </div>
             </div>
             <div class="form-group row input-controls buttons">
-                <input type="hidden" id="object_id" name="object_id" value="{{ object_id }}"/>
+                {% for object_id in object_ids %}
+                    <input type="hidden" name="object_id" value="{{ object_id }}"/>
+                {% endfor %}
                 <button class="btn btn-success" type="submit">Filter</button>
             </div>
         </div>
@@ -46,6 +51,7 @@ <h3>History for {{ name }}</h3>
                 <thead>
                     <tr>
                         <th>Commit</th>
+                        <th>Hostname</th>
                         <th>Date</th>
                         <th>Time Since</th>
                         <th>App</th>
@@ -62,6 +68,7 @@ <h3>History for {{ name }}</h3>
                                 {% if not attribute_filter or attribute_filter == key %}
                                 <tr>
                                     <td><a href="{% url 'serverdb_changes' %}?commit_id={{ change.commit.id }}">{{ change.commit.id }}</a></td>
+                                    <td><a href="{% url 'servershell_inspect' %}?object_id={{ change.object_id }}">{{ change.hostname|default:change.object_id }}</a></td>
                                     <td>{{ change.commit.change_on|date:"r" }}</td>
                                     <td>{{ change.commit.change_on|timesince }}</td>
                                     <td>{{ change.commit.app|default:"Servershell" }}</td>
@@ -88,6 +95,7 @@ <h3>History for {{ name }}</h3>
                                     {% if not attribute_filter or attribute_filter == key %}
                                         <tr>
                                             <td><a href="{% url 'serverdb_changes' %}?commit_id={{ change.commit.id }}">{{ change.commit.id }}</a></td>
+                                            <td><a href="{% url 'servershell_inspect' %}?object_id={{ change.object_id }}">{{ change.hostname|default:change.object_id }}</a></td>
                                             <td>{{ change.commit.change_on|date:"r" }}</td>
                                             <td>{{ change.commit.change_on|timesince }}</td>
                                             <td>{{ change.commit.app|default:"Servershell" }}</td>
@@ -127,14 +135,15 @@ <h3>History for {{ name }}</h3>
                         {% elif change.change_type == 'delete' %}
                             <tr class="">
                                 <td><a href="{% url 'serverdb_changes' %}?commit_id={{ change.commit.id }}">{{ change.commit.id }}</a></td>
+                                <td>{{ change.hostname|default:change.object_id }}</td>
                                 <td>{{ change.commit.change_on|date:"r" }}</td>
-                                <td>{{ change.channge_commit.change_on|timesince }}</td>
+                                <td>{{ change.commit.change_on|timesince }}</td>
                                 <td>{{ change.commit.app|default:"Servershell" }}</td>
                                 <td>{{ change.commit.user }}</td>
                                 <td>
                                     <form method="post" action="{% url 'serverdb_recreate' change.commit.id %}">
                                         {% csrf_token %}
-                                        <input type="hidden" name="object_id" value="{{ change.server_id }}" />
+                                        <input type="hidden" name="object_id" value="{{ change.object_id }}" />
                                         <input class="btn btn-sm btn-danger" type="submit" value="Re-create this server" />
                                     </form>
                                 </td>
@@ -146,7 +155,7 @@ <h3>History for {{ name }}</h3>
                 </tbody>
             </table>
             {% if commit_id %}
-                <a href="{% url 'serverdb_history' %}?object_id={{ object_id }}">Show complete history</a>
+                <a href="{% url 'serverdb_history' %}?{{ object_ids_query }}">Show complete history</a>
             {% endif %}
         </div>
     </div>

packages/serveradmin/serveradmin/serverdb/tests/test_views.py

@@ -3,6 +3,7 @@
 from django.contrib.auth.models import User
 from django.test import TransactionTestCase
 
+from adminapi.filters import Any
 from serveradmin.dataset import Query
 from serveradmin.serverdb.models import Change
 
@@ -66,3 +67,104 @@ def test_recreate_fails_if_hostname_exists(self):
         self.assertEqual(200, response.status_code)
         self.assertEqual('serverdb/changes.html', response.template_name)
         self.assertEqual(302, response.redirect_chain[0][1])
+
+
+class HistoryViewTest(TransactionTestCase):
+    fixtures = ['auth_user.json', 'test_dataset.json']
+
+    def setUp(self) -> None:
+        self.client.login(username='hannah.acker', password='hannah.acker')
+
+        self.object_ids = []
+        for i in (1, 2):
+            vm = Query().new_object('vm')
+            vm['hostname'] = f'test-serverdb-history-{i}'
+            vm['intern_ip'] = ip_address(f'10.0.0.{i}')
+            vm.commit(user=User.objects.first())
+            self.object_ids.append(
+                Query({'hostname': f'test-serverdb-history-{i}'}).get()[
+                    'object_id'])
+
+        vms = Query({'object_id': Any(*self.object_ids)}, ['intern_ip'])
+        for i, vm in enumerate(vms, start=11):
+            vm['intern_ip'] = ip_address(f'10.0.0.{i}')
+        vms.commit(user=User.objects.first())
+
+    def _url(self, *object_ids, **params):
+        query = '&'.join(f'object_id={i}' for i in object_ids)
+        for key, value in params.items():
+            query += f'&{key}={value}'
+        return f'/serverdb/history?{query}'
+
+    def test_multiple_objects(self):
+        response = self.client.get(self._url(*self.object_ids))
+
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            ['test-serverdb-history-1', 'test-serverdb-history-2'],
+            response.context['hostnames'])
+        changes = list(response.context['changes'])
+        self.assertEqual(
+            set(self.object_ids), {c.object_id for c in changes})
+        self.assertEqual(
+            {'test-serverdb-history-1', 'test-serverdb-history-2'},
+            {c.hostname for c in changes})
+
+    def test_single_object(self):
+        response = self.client.get(self._url(self.object_ids[0]))
+
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            ['test-serverdb-history-1'], response.context['hostnames'])
+        self.assertEqual(
+            {self.object_ids[0]},
+            {c.object_id for c in response.context['changes']})
+
+    def test_attribute_filter(self):
+        response = self.client.get(
+            self._url(*self.object_ids, attribute_filter='intern_ip'))
+
+        changes = list(response.context['changes'])
+        self.assertTrue(changes)
+        self.assertTrue(all('intern_ip' in c.change_json for c in changes))
+        self.assertEqual(
+            set(self.object_ids), {c.object_id for c in changes})
+
+    def test_missing_object_is_skipped(self):
+        response = self.client.get(self._url(self.object_ids[0], 999999999))
+
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(
+            [self.object_ids[0]], response.context['object_ids'])
+        self.assertIn(
+            '999999999',
+            ' '.join(str(m) for m in response.context['messages']))
+
+    def test_only_missing_objects_redirects(self):
+        response = self.client.get(self._url(999999999), follow=True)
+
+        self.assertEqual('serverdb/changes.html', response.template_name)
+        self.assertEqual(302, response.redirect_chain[0][1])
+
+    def test_no_parameter(self):
+        response = self.client.get('/serverdb/history')
+        self.assertEqual(400, response.status_code)
+
+    def test_invalid_object_id(self):
+        response = self.client.get(self._url('foo'))
+        self.assertEqual(400, response.status_code)
+
+    def test_changes_multiple_objects(self):
+        object_ids = '&'.join(f'object_id={i}' for i in self.object_ids)
+        response = self.client.get(f'/serverdb/changes?{object_ids}')
+
+        self.assertEqual(200, response.status_code)
+        commit_ids = [c.id for c in response.context['commits']]
+        # Two creates and one shared update commit, without duplicates
+        self.assertEqual(3, len(commit_ids))
+        self.assertEqual(len(commit_ids), len(set(commit_ids)))
+
+    def test_changes_empty_object_id(self):
+        # The filter form submits an empty object_id if none is given
+        response = self.client.get('/serverdb/changes?object_id=')
+        self.assertEqual(200, response.status_code)

packages/serveradmin/serveradmin/serverdb/views.py

@@ -8,7 +8,7 @@
 from django.contrib.auth.decorators import login_required
 from django.core.exceptions import ValidationError
 from django.core.paginator import Paginator
-from django.db.models import Q, Subquery, OuterRef, Prefetch, CharField
+from django.db.models import Q, Subquery, OuterRef, Prefetch, CharField, QuerySet
 from django.db.models.fields.json import KeyTextTransform
 from django.db.models.functions import Coalesce, Cast
 from django.http import HttpResponseBadRequest
@@ -27,6 +27,29 @@
 from serveradmin.serverdb.query_committer import CommitError, commit_query
 
 
+def _annotate_hostname(changes: QuerySet) -> QuerySet:
+    """Annotate changes with the hostname of their object
+
+    The hostname is fetched either from the server table or from the change
+    entry where the object was deleted.
+    """
+    server_hostname = Server.objects.filter(
+        server_id=OuterRef('object_id')).values('hostname')
+    # Workaround: In previous versions of Serveradmin restoring objects with
+    # the same id was possible, so we can end up with objects being deleted
+    # multiple times. We only take the latest into account.
+    change_hostname = Change.objects.filter(
+        object_id=OuterRef('object_id'),
+        change_type=Change.Type.DELETE).values('change_json').order_by('-id')[:1]
+
+    return changes.annotate(hostname=Coalesce(
+        Subquery(server_hostname),
+        Cast(
+            KeyTextTransform('hostname', Subquery(change_hostname)),
+            output_field=CharField())
+    ))
+
+
 @login_required
 def changes(request):
     commits = ChangeCommit.objects.all().order_by('-change_on')
@@ -57,9 +80,13 @@ def changes(request):
             'change_json__object_id', flat=True))
         commits = commits.filter(change__object_id__in=object_ids)
 
-    f_object_id = request.GET.get('object_id')
-    if f_object_id:
-        commits = commits.filter(change__object_id=f_object_id)
+    try:
+        f_object_ids = [int(i) for i in request.GET.getlist('object_id') if i]
+    except ValueError:
+        return HttpResponseBadRequest('object_id must be an integer')
+    if f_object_ids:
+        commits = commits.filter(
+            change__object_id__in=f_object_ids).distinct()
 
     f_user_or_app = request.GET.get('user_or_app')
     if f_user_or_app:
@@ -69,32 +96,14 @@ def changes(request):
     f_attribute = request.GET.get('attribute')
     if f_attribute:
         # Only allow attribute filter if another filter is set (not fully index)
-        if f_hostname or f_object_id or f_user_or_app:
+        if f_hostname or f_object_ids or f_user_or_app:
             commits = commits.filter(change__change_json__has_key=f_attribute)
         else:
             f_attribute = None
 
     commits = commits.select_related('app', 'user')
-
-    # This complex statement is just here to be able to prefetch the changes'
-    # relation including the hostname fetched either from the server table
-    # or from the change entry where the object was deleted.
-    server_hostname = Server.objects.filter(
-        server_id=OuterRef('object_id')).values('hostname')
-    # Workaround: In previous versions of Serveradmin restoring objects with
-    # the same id was possible, so we can end up with objects being deleted
-    # multiple times. We only take the latest into account.
-    change_hostname = Change.objects.filter(
-        object_id=OuterRef('object_id'),
-        change_type=Change.Type.DELETE).values('change_json').order_by('-id')[:1]
     commits = commits.prefetch_related(Prefetch(
-        'change_set',
-        queryset=Change.objects.all().annotate(hostname=Coalesce(
-            Subquery(server_hostname),
-            Cast(
-                KeyTextTransform('hostname', Subquery(change_hostname)),
-                output_field=CharField())
-    ))))
+        'change_set', queryset=_annotate_hostname(Change.objects.all())))
 
     class NoCountPaginator(Paginator):
         @cached_property
@@ -113,7 +122,7 @@ def count(self):
         'from': f_from,
         'until': f_until,
         'hostname': f_hostname,
-        'object_id': f_object_id,
+        'object_ids': f_object_ids,
         'commit_id': f_commit,
         'user_or_app': f_user_or_app,
         'attribute': f_attribute,
@@ -122,25 +131,34 @@ def count(self):
 
 @login_required
 def history(request):
-    if 'object_id' in request.GET:
-        query = Q(server_id=request.GET['object_id'])
-        error_text = f'object_id {request.GET["object_id"]}'
-    elif 'hostname' in request.GET:
-        query = Q(hostname=request.GET['hostname'])
-        error_text = f'hostname {request.GET["hostname"]}'
-    else:
+    try:
+        object_ids = [int(i) for i in request.GET.getlist('object_id') if i]
+    except ValueError:
+        return HttpResponseBadRequest('object_id must be an integer')
+    hostnames = request.GET.getlist('hostname')
+    if not object_ids and not hostnames:
         return HttpResponseBadRequest(
             'object_id or hostname parameter is mandatory')
 
-    try:
-        server = Server.objects.get(query)
-    except Server.DoesNotExist:
+    servers = list(Server.objects.filter(
+        Q(server_id__in=object_ids) | Q(hostname__in=hostnames)
+    ).order_by('hostname'))
+
+    missing = [str(i) for i in object_ids
+               if i not in {s.server_id for s in servers}]
+    missing += [h for h in hostnames if h not in {s.hostname for s in servers}]
+    if not servers:
         messages.error(
             request,
-            f'Object with {error_text} does not or no longer exist.')
+            f'Object(s) {", ".join(missing)} do not or no longer exist.')
         return redirect(reverse('serverdb_changes'))
+    if missing:
+        messages.warning(
+            request,
+            f'Object(s) {", ".join(missing)} do not or no longer exist.')
 
-    obj_history = Change.objects.filter(object_id=server.server_id)
+    server_ids = [s.server_id for s in servers]
+    obj_history = Change.objects.filter(object_id__in=server_ids)
 
     commit_id = request.GET.get('commit_id')
     if commit_id:
@@ -150,24 +168,26 @@ def history(request):
     if attribute_filter:
         obj_history = obj_history.filter(change_json__has_key=attribute_filter)
 
-    obj_history = obj_history.order_by('-commit_id')
+    obj_history = obj_history.order_by('-commit_id', '-id')
     obj_history = obj_history.select_related('commit__app', 'commit__user')
+    obj_history = _annotate_hostname(obj_history)
 
     page = request.GET.get('page', 1)
     pager = Paginator(obj_history, 25)
     page_obj = pager.get_page(page)
 
     no_history_attributes = ServertypeAttribute.objects.filter(
-        servertype_id=server.servertype_id
-    ).select_related('attribute').filter(
-        attribute__history=False
-    ).only('attribute_id')
+        servertype_id__in={s.servertype_id for s in servers},
+        attribute__history=False,
+    ).order_by('attribute_id').values_list(
+        'attribute_id', flat=True).distinct()
 
     return TemplateResponse(request, 'serverdb/history.html', {
         'changes': page_obj,
         'commit_id': commit_id,
-        'object_id': server.server_id,
-        'name': server.hostname,
+        'object_ids': server_ids,
+        'object_ids_query': '&'.join(f'object_id={i}' for i in server_ids),
+        'hostnames': [s.hostname for s in servers],
         'attribute_filter': attribute_filter,
         'no_history_attributes': no_history_attributes,
     })