API: Do not write log entry for events when no changes are made (#4090)

src/pretix/api/serializers/settings.py

@@ -89,6 +89,7 @@ class SettingsSerializer(serializers.Serializer):
                         except OSError:  # pragma: no cover
                             logger.error('Deleting file %s failed.' % fname.name)
                     instance.delete(attr)
+                    self.changed_data.append(attr)
                 else:
                     # file is unchanged
                     continue

src/pretix/api/views/event.py

@@ -190,7 +190,10 @@ class EventViewSet(viewsets.ModelViewSet):
         serializer = self.get_serializer(page, many=True)
         return self.get_paginated_response(serializer.data)
 
+    @transaction.atomic()
     def perform_update(self, serializer):
+        original_data = self.get_serializer(instance=serializer.instance).data
+
         current_live_value = serializer.instance.live
         updated_live_value = serializer.validated_data.get('live', None)
         current_plugins_value = serializer.instance.get_plugins()
@@ -198,6 +201,11 @@ class EventViewSet(viewsets.ModelViewSet):
 
         super().perform_update(serializer)
 
+        if serializer.data == original_data:
+            # Performance optimization: If nothing was changed, we do not need to save or log anything.
+            # This costs us a few cycles on save, but avoids thousands of lines in our log.
+            return
+
         if updated_live_value is not None and updated_live_value != current_live_value:
             log_action = 'pretix.event.live.activated' if updated_live_value else 'pretix.event.live.deactivated'
             serializer.instance.log_action(
@@ -622,11 +630,12 @@ class EventSettingsView(views.APIView):
         s.is_valid(raise_exception=True)
         with transaction.atomic():
             s.save()
-            self.request.event.log_action(
-                'pretix.event.settings', user=self.request.user, auth=self.request.auth, data={
-                    k: v for k, v in s.validated_data.items()
-                }
-            )
+            if s.changed_data:
+                self.request.event.log_action(
+                    'pretix.event.settings', user=self.request.user, auth=self.request.auth, data={
+                        k: v for k, v in s.validated_data.items()
+                    }
+                )
         if any(p in s.changed_data for p in SETTINGS_AFFECTING_CSS):
             regenerate_css.apply_async(args=(request.event.pk,))
         s = EventSettingsSerializer(

src/tests/api/test_events.py

@@ -746,6 +746,20 @@ def test_event_update(token_client, organizer, event, item, meta_prop):
             name="Foo"
         ).exists()
 
+    # Noop does not write log
+    cnt = event.all_logentries().count()
+    resp = token_client.patch(
+        '/api/v1/organizers/{}/events/{}/'.format(organizer.slug, event.slug),
+        {
+            "date_from": "2018-12-27T10:00:00Z",
+            "date_to": "2018-12-28T10:00:00Z",
+            "currency": "DKK",
+        },
+        format='json'
+    )
+    assert resp.status_code == 200
+    assert cnt == event.all_logentries().count()
+
 
 @pytest.mark.django_db
 def test_event_test_mode(token_client, organizer, event):
@@ -1252,8 +1266,26 @@ def test_patch_event_settings(token_client, organizer, event):
         event.settings.flush()
         assert event.settings.imprint_url == 'https://example.com'
         assert not event.settings.reusable_media_active
+        assert event.all_logentries().filter(action_type="pretix.event.settings").count() == 1
         mocked.assert_not_called()
 
+        # The same settings again do not create a new log entry
+        resp = token_client.patch(
+            '/api/v1/organizers/{}/events/{}/settings/'.format(organizer.slug, event.slug),
+            {
+                'imprint_url': 'https://example.com',
+                'confirm_texts': [
+                    {
+                        'de': 'Ich bin mit den AGB einverstanden.'
+                    }
+                ],
+                'reusable_media_active': True,  # readonly, ignored
+            },
+            format='json'
+        )
+        assert resp.status_code == 200
+        assert event.all_logentries().filter(action_type="pretix.event.settings").count() == 1
+
         resp = token_client.patch(
             '/api/v1/organizers/{}/events/{}/settings/'.format(organizer.slug, event.slug),
             {
@@ -1266,6 +1298,7 @@ def test_patch_event_settings(token_client, organizer, event):
         event.settings.flush()
         mocked.assert_any_call(args=(event.pk,))
         assert event.settings.primary_color == '#ff0000'
+        assert event.all_logentries().filter(action_type="pretix.event.settings").count() == 2
 
         resp = token_client.patch(
             '/api/v1/organizers/{}/events/{}/settings/'.format(organizer.slug, event.slug),