From 80288b6561d40ef740a5029790e1622b92bd925f Mon Sep 17 00:00:00 2001 From: dee077 Date: Fri, 21 Mar 2025 14:32:44 +0530 Subject: [PATCH 1/8] [feature] Rivision for Rest Api changes #894 Implemented three endpoints: 1. List all revisions with optional filtering by model. 2. Inspect a specific revision by its ID. 3. Restore a revision using its ID. Fixes #894 --- openwisp_controller/config/api/filters.py | 16 +++++++ openwisp_controller/config/api/serializers.py | 32 ++++++++++++++ openwisp_controller/config/api/urls.py | 15 +++++++ openwisp_controller/config/api/views.py | 44 +++++++++++++++++++ openwisp_controller/config/tests/test_api.py | 40 +++++++++++++++++ tests/openwisp2/sample_config/api/views.py | 24 ++++++++++ 6 files changed, 171 insertions(+) diff --git a/openwisp_controller/config/api/filters.py b/openwisp_controller/config/api/filters.py index 9f82a0ab7..672e0e610 100644 --- a/openwisp_controller/config/api/filters.py +++ b/openwisp_controller/config/api/filters.py @@ -4,6 +4,7 @@ from django_filters import rest_framework as filters from django_filters.rest_framework import DjangoFilterBackend from rest_framework.exceptions import ValidationError +from reversion.models import Version from swapper import load_model from openwisp_users.api.filters import OrganizationManagedFilter @@ -142,3 +143,18 @@ def __init__(self, *args, **kwargs): class Meta(BaseConfigAPIFilter.Meta): model = DeviceGroup + + +class ReversionFilter(BaseConfigAPIFilter): + model = filters.CharFilter(field_name="content_type__model", lookup_expr="iexact") + + def _set_valid_filterform_labels(self): + self.filters["model"].label = _("Model") + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._set_valid_filterform_labels() + + class Meta: + model = Version + fields = ["model"] diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index 7e67d8f5f..ec6960138 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -3,6 +3,7 @@ from django.db.models import Q from django.utils.translation import gettext_lazy as _ from rest_framework import serializers +from reversion.models import Version from swapper import load_model from openwisp_utils.api.serializers import ValidatedModelSerializer @@ -376,3 +377,34 @@ def update(self, instance, validated_data): instance = super().update(instance, validated_data) self._save_m2m_templates(instance) return instance + + +class ReversionSerializer(BaseSerializer): + user_id = serializers.SerializerMethodField() + date_created = serializers.DateTimeField( + source="revision.date_created", read_only=True + ) + comment = serializers.CharField(source="revision.comment", read_only=True) + content_type = serializers.SerializerMethodField() + + class Meta: + model = Version + fields = [ + "id", + "revision_id", + "object_id", + "content_type", + "db", + "format", + "serialized_data", + "object_repr", + "user_id", + "date_created", + "comment", + ] + + def get_user_id(self, obj): + return getattr(obj.revision, "user_id", None) + + def get_content_type(self, obj): + return getattr(obj.content_type, "model", None) diff --git a/openwisp_controller/config/api/urls.py b/openwisp_controller/config/api/urls.py index 9936ad213..2e0584838 100644 --- a/openwisp_controller/config/api/urls.py +++ b/openwisp_controller/config/api/urls.py @@ -83,6 +83,21 @@ def get_api_urls(api_views): api_download_views.download_device_config, name="download_device_config", ), + path( + 'controller/reversion/', + api_views.reversion_list, + name='reversion_list', + ), + path( + 'controller/reversion//', + api_views.reversion_detail, + name='reversion_detail', + ), + path( + 'controller/reversion//restore/', + api_views.reversion_restore, + name='reversion_restore', + ), ] else: return [] diff --git a/openwisp_controller/config/api/views.py b/openwisp_controller/config/api/views.py index db77ce15c..3b0527438 100644 --- a/openwisp_controller/config/api/views.py +++ b/openwisp_controller/config/api/views.py @@ -1,3 +1,4 @@ +import reversion from cache_memoize import cache_memoize from django.core.exceptions import ObjectDoesNotExist from django.db.models import F, Q @@ -7,11 +8,13 @@ from rest_framework import pagination, serializers, status from rest_framework.generics import ( GenericAPIView, + ListAPIView, ListCreateAPIView, RetrieveAPIView, RetrieveUpdateDestroyAPIView, ) from rest_framework.response import Response +from reversion.models import Version from swapper import load_model from openwisp_users.api.permissions import DjangoModelPermissions @@ -21,6 +24,7 @@ DeviceGroupListFilter, DeviceListFilter, DeviceListFilterBackend, + ReversionFilter, TemplateListFilter, VPNListFilter, ) @@ -28,6 +32,7 @@ DeviceDetailSerializer, DeviceGroupSerializer, DeviceListSerializer, + ReversionSerializer, TemplateSerializer, VpnSerializer, ) @@ -289,6 +294,42 @@ def certificate_delete_invalidates_cache(cls, organization_id, common_name): cls.get_device_group.invalidate(cls, org_slug, common_name) +class ReversionListView(ProtectedAPIMixin, ListAPIView): + serializer_class = ReversionSerializer + queryset = Version.objects.select_related('revision').order_by( + '-revision__date_created' + ) + filter_backends = [DjangoFilterBackend] + filterset_class = ReversionFilter + + +class ReversionDetailView(ProtectedAPIMixin, RetrieveAPIView): + serializer_class = ReversionSerializer + queryset = Version.objects.select_related('revision').order_by( + '-revision__date_created' + ) + lookup_field = 'pk' + + +class ReversionRestoreView(ProtectedAPIMixin, GenericAPIView): + serializer_class = serializers.Serializer + queryset = Version.objects.select_related('revision').order_by( + '-revision__date_created' + ) + + def post(self, request, *args, **kwargs): + version = self.get_object() + with reversion.create_revision(): + version.revert() + reversion.set_user(request.user) + reversion.set_comment( + f"Restored to previous revision: {version.revision_id}" + ) + + serializer = ReversionSerializer(version, context=self.get_serializer_context()) + return Response(serializer.data, status=status.HTTP_200_OK) + + template_list = TemplateListCreateView.as_view() template_detail = TemplateDetailView.as_view() vpn_list = VpnListCreateView.as_view() @@ -300,3 +341,6 @@ def certificate_delete_invalidates_cache(cls, organization_id, common_name): devicegroup_list = DeviceGroupListCreateView.as_view() devicegroup_detail = DeviceGroupDetailView.as_view() devicegroup_commonname = DeviceGroupCommonName.as_view() +reversion_list = ReversionListView.as_view() +reversion_detail = ReversionDetailView.as_view() +reversion_restore = ReversionRestoreView.as_view() diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 880c142d1..10b8ead05 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -1,3 +1,4 @@ +import reversion from django.contrib.auth.models import Permission from django.test import TestCase from django.test.client import BOUNDARY, MULTIPART_CONTENT, encode_multipart @@ -1634,3 +1635,42 @@ def test_device_patch_with_templates_of_same_org(self): self.assertEqual(r.status_code, 200) self.assertEqual(d1.config.templates.count(), 2) self.assertEqual(r.data["config"]["templates"], [t1.id, t2.id]) + + def test_reversion_list_and_restore_api(self): + org = self._get_org() + with reversion.create_revision(): + device = self._create_device( + organization=org, name="test", _is_deactivated=True + ) + path = reverse("config_api:device_detail", args=[device.pk]) + response = self.client.delete(path) + self.assertEqual(response.status_code, 204) + self.assertEqual(Device.objects.count(), 0) + + path = reverse("config_api:reversion_list") + response = self.client.get(path) + response_json = response.json() + version_id = response_json[0]["id"] + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response_json), 1) + + with self.subTest("Test filter reversion list with model name"): + params = {"id": 1, "model": "Device"} + response = self.client.get(path, params) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()), 1) + self.assertEqual(response.json()[0]["object_id"], str(device.pk)) + + with self.subTest("Test reversion detail"): + path = reverse("config_api:reversion_detail", args=[version_id]) + response = self.client.get(path) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["id"], version_id) + self.assertEqual(response.json()["object_id"], str(device.pk)) + + with self.subTest("Test reversion restore view"): + path = reverse("config_api:reversion_restore", args=[version_id]) + response = self.client.post(path) + self.assertEqual(response.status_code, 200) + self.assertEqual(Device.objects.count(), 1) + self.assertEqual(Device.objects.first().id, device.pk) diff --git a/tests/openwisp2/sample_config/api/views.py b/tests/openwisp2/sample_config/api/views.py index daa8e2feb..51ceb950d 100644 --- a/tests/openwisp2/sample_config/api/views.py +++ b/tests/openwisp2/sample_config/api/views.py @@ -28,6 +28,15 @@ from openwisp_controller.config.api.views import ( DeviceListCreateView as BaseDeviceListCreateView, ) +from openwisp_controller.config.api.views import ( + ReversionDetailView as BaseReversionDetailView, +) +from openwisp_controller.config.api.views import ( + ReversionListView as BaseReversionListView, +) +from openwisp_controller.config.api.views import ( + ReversionRestoreView as BaseReversionRestoreView, +) from openwisp_controller.config.api.views import ( TemplateDetailView as BaseTemplateDetailView, ) @@ -96,6 +105,18 @@ class DownloadDeviceView(BaseDownloadDeviceView): pass +class ReversionListView(BaseReversionListView): + pass + + +class ReversionDetailView(BaseReversionDetailView): + pass + + +class ReversionRestoreView(BaseReversionRestoreView): + pass + + template_list = TemplateListCreateView.as_view() template_detail = TemplateDetailView.as_view() download_template_config = DownloadTemplateconfiguration.as_view() @@ -110,3 +131,6 @@ class DownloadDeviceView(BaseDownloadDeviceView): devicegroup_list = DeviceGroupListCreateView.as_view() devicegroup_detail = DeviceGroupDetailView.as_view() devicegroup_commonname = DeviceGroupCommonName.as_view() +reversion_list = ReversionListView.as_view() +reversion_detail = ReversionDetailView.as_view() +reversion_restore = ReversionRestoreView.as_view() From 675f162acc12fc05dc666d6e85afd735655e7e25 Mon Sep 17 00:00:00 2001 From: dee077 Date: Fri, 21 Mar 2025 14:54:13 +0530 Subject: [PATCH 2/8] [fix] Fix param --- openwisp_controller/config/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 10b8ead05..997de4810 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -1655,7 +1655,7 @@ def test_reversion_list_and_restore_api(self): self.assertEqual(len(response_json), 1) with self.subTest("Test filter reversion list with model name"): - params = {"id": 1, "model": "Device"} + params = {"model": "Device"} response = self.client.get(path, params) self.assertEqual(response.status_code, 200) self.assertEqual(len(response.json()), 1) From dd5c23272d08f5d7d2c9b96a069fe34ef799b1ec Mon Sep 17 00:00:00 2001 From: dee077 Date: Sat, 12 Apr 2025 19:26:07 +0530 Subject: [PATCH 3/8] [feature] Added AutoRevisionMixin --- openwisp_controller/config/api/urls.py | 30 +++---- openwisp_controller/config/api/views.py | 94 ++++++++++++-------- openwisp_controller/config/tests/test_api.py | 42 +++++---- openwisp_controller/mixins.py | 14 +++ tests/openwisp2/sample_config/api/views.py | 18 ++-- 5 files changed, 117 insertions(+), 81 deletions(-) diff --git a/openwisp_controller/config/api/urls.py b/openwisp_controller/config/api/urls.py index 2e0584838..4cb753153 100644 --- a/openwisp_controller/config/api/urls.py +++ b/openwisp_controller/config/api/urls.py @@ -13,6 +13,21 @@ def get_api_urls(api_views): """ if getattr(settings, "OPENWISP_CONTROLLER_API", True): return [ + path( + "controller//revision/", + api_views.revision_list, + name="revision_list", + ), + path( + "controller//revision//", + api_views.revision_detail, + name="revision_detail", + ), + path( + "controller//revision//restore/", + api_views.revision_restore, + name="revision_restore", + ), path( "controller/template/", api_views.template_list, @@ -83,21 +98,6 @@ def get_api_urls(api_views): api_download_views.download_device_config, name="download_device_config", ), - path( - 'controller/reversion/', - api_views.reversion_list, - name='reversion_list', - ), - path( - 'controller/reversion//', - api_views.reversion_detail, - name='reversion_detail', - ), - path( - 'controller/reversion//restore/', - api_views.reversion_restore, - name='reversion_restore', - ), ] else: return [] diff --git a/openwisp_controller/config/api/views.py b/openwisp_controller/config/api/views.py index 3b0527438..9d8bd1bba 100644 --- a/openwisp_controller/config/api/views.py +++ b/openwisp_controller/config/api/views.py @@ -3,6 +3,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.db.models import F, Q from django.http import Http404 +from django.shortcuts import get_list_or_404 from django.urls.base import reverse from django_filters.rest_framework import DjangoFilterBackend from rest_framework import pagination, serializers, status @@ -19,12 +20,11 @@ from openwisp_users.api.permissions import DjangoModelPermissions -from ...mixins import ProtectedAPIMixin +from ...mixins import AutoRevisionMixin, ProtectedAPIMixin from .filters import ( DeviceGroupListFilter, DeviceListFilter, DeviceListFilterBackend, - ReversionFilter, TemplateListFilter, VPNListFilter, ) @@ -53,7 +53,7 @@ class ListViewPagination(pagination.PageNumberPagination): max_page_size = 100 -class TemplateListCreateView(ProtectedAPIMixin, ListCreateAPIView): +class TemplateListCreateView(ProtectedAPIMixin, AutoRevisionMixin, ListCreateAPIView): serializer_class = TemplateSerializer queryset = Template.objects.prefetch_related("tags").order_by("-created") pagination_class = ListViewPagination @@ -61,12 +61,14 @@ class TemplateListCreateView(ProtectedAPIMixin, ListCreateAPIView): filterset_class = TemplateListFilter -class TemplateDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView): +class TemplateDetailView( + ProtectedAPIMixin, AutoRevisionMixin, RetrieveUpdateDestroyAPIView +): serializer_class = TemplateSerializer queryset = Template.objects.all() -class VpnListCreateView(ProtectedAPIMixin, ListCreateAPIView): +class VpnListCreateView(ProtectedAPIMixin, AutoRevisionMixin, ListCreateAPIView): serializer_class = VpnSerializer queryset = Vpn.objects.select_related("subnet").order_by("-created") pagination_class = ListViewPagination @@ -74,7 +76,7 @@ class VpnListCreateView(ProtectedAPIMixin, ListCreateAPIView): filterset_class = VPNListFilter -class VpnDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView): +class VpnDetailView(ProtectedAPIMixin, AutoRevisionMixin, RetrieveUpdateDestroyAPIView): serializer_class = VpnSerializer queryset = Vpn.objects.all() @@ -87,7 +89,7 @@ def has_object_permission(self, request, view, obj): return perm and not obj.is_deactivated() -class DeviceListCreateView(ProtectedAPIMixin, ListCreateAPIView): +class DeviceListCreateView(ProtectedAPIMixin, AutoRevisionMixin, ListCreateAPIView): """ Templates: Templates flagged as required will be added automatically to the `config` of a device and cannot be unassigned. @@ -102,7 +104,9 @@ class DeviceListCreateView(ProtectedAPIMixin, ListCreateAPIView): filterset_class = DeviceListFilter -class DeviceDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView): +class DeviceDetailView( + ProtectedAPIMixin, AutoRevisionMixin, RetrieveUpdateDestroyAPIView +): """ Templates: Templates flagged as _required_ will be added automatically to the `config` of a device and cannot be unassigned. @@ -129,7 +133,7 @@ def get_serializer_context(self): return context -class DeviceActivateView(ProtectedAPIMixin, GenericAPIView): +class DeviceActivateView(ProtectedAPIMixin, AutoRevisionMixin, GenericAPIView): serializer_class = serializers.Serializer queryset = Device.objects.filter(_is_deactivated=True) @@ -142,7 +146,7 @@ def post(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_200_OK) -class DeviceDeactivateView(ProtectedAPIMixin, GenericAPIView): +class DeviceDeactivateView(ProtectedAPIMixin, AutoRevisionMixin, GenericAPIView): serializer_class = serializers.Serializer queryset = Device.objects.filter(_is_deactivated=False) @@ -155,7 +159,9 @@ def post(self, request, *args, **kwargs): return Response(serializer.data, status=status.HTTP_200_OK) -class DeviceGroupListCreateView(ProtectedAPIMixin, ListCreateAPIView): +class DeviceGroupListCreateView( + ProtectedAPIMixin, AutoRevisionMixin, ListCreateAPIView +): serializer_class = DeviceGroupSerializer queryset = DeviceGroup.objects.prefetch_related("templates").order_by("-created") pagination_class = ListViewPagination @@ -163,7 +169,9 @@ class DeviceGroupListCreateView(ProtectedAPIMixin, ListCreateAPIView): filterset_class = DeviceGroupListFilter -class DeviceGroupDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView): +class DeviceGroupDetailView( + ProtectedAPIMixin, AutoRevisionMixin, RetrieveUpdateDestroyAPIView +): serializer_class = DeviceGroupSerializer queryset = DeviceGroup.objects.select_related("organization").order_by("-created") @@ -177,7 +185,7 @@ def get_cached_devicegroup_args_rewrite(cls, org_slugs, common_name): return url -class DeviceGroupCommonName(ProtectedAPIMixin, RetrieveAPIView): +class DeviceGroupCommonName(ProtectedAPIMixin, AutoRevisionMixin, RetrieveAPIView): serializer_class = DeviceGroupSerializer queryset = DeviceGroup.objects.select_related("organization").order_by("-created") # Not setting lookup_field makes DRF raise error. but it is not used @@ -294,39 +302,55 @@ def certificate_delete_invalidates_cache(cls, organization_id, common_name): cls.get_device_group.invalidate(cls, org_slug, common_name) -class ReversionListView(ProtectedAPIMixin, ListAPIView): +class RevisionListView(ProtectedAPIMixin, AutoRevisionMixin, ListAPIView): serializer_class = ReversionSerializer - queryset = Version.objects.select_related('revision').order_by( - '-revision__date_created' - ) - filter_backends = [DjangoFilterBackend] - filterset_class = ReversionFilter + + def get_queryset(self): + model_slug = self.kwargs.get('model_slug').lower() + return ( + Version.objects.select_related('revision') + .filter(content_type__model=model_slug) + .order_by('-revision__date_created') + ) -class ReversionDetailView(ProtectedAPIMixin, RetrieveAPIView): +class RevisionDetailView(ProtectedAPIMixin, RetrieveAPIView): serializer_class = ReversionSerializer - queryset = Version.objects.select_related('revision').order_by( - '-revision__date_created' - ) - lookup_field = 'pk' + def get_queryset(self): + model_slug = self.kwargs.get('model_slug').lower() + return ( + Version.objects.select_related('revision') + .filter(content_type__model=model_slug) + .order_by('-revision__date_created') + ) -class ReversionRestoreView(ProtectedAPIMixin, GenericAPIView): + +class RevisionRestoreView(ProtectedAPIMixin, GenericAPIView): serializer_class = serializers.Serializer - queryset = Version.objects.select_related('revision').order_by( - '-revision__date_created' - ) + + def get_queryset(self): + model_slug = self.kwargs.get('model_slug').lower() + return ( + Version.objects.select_related('revision') + .filter(content_type__model=model_slug) + .order_by('-revision__date_created') + ) def post(self, request, *args, **kwargs): - version = self.get_object() + qs = self.get_queryset() + versions = get_list_or_404(qs, revision_id=kwargs['pk']) with reversion.create_revision(): - version.revert() + for version in versions: + version.revert() reversion.set_user(request.user) reversion.set_comment( - f"Restored to previous revision: {version.revision_id}" + f"Restored to previous revision: {self.kwargs.get('pk')}" ) - serializer = ReversionSerializer(version, context=self.get_serializer_context()) + serializer = ReversionSerializer( + versions, many=True, context=self.get_serializer_context() + ) return Response(serializer.data, status=status.HTTP_200_OK) @@ -341,6 +365,6 @@ def post(self, request, *args, **kwargs): devicegroup_list = DeviceGroupListCreateView.as_view() devicegroup_detail = DeviceGroupDetailView.as_view() devicegroup_commonname = DeviceGroupCommonName.as_view() -reversion_list = ReversionListView.as_view() -reversion_detail = ReversionDetailView.as_view() -reversion_restore = ReversionRestoreView.as_view() +revision_list = RevisionListView.as_view() +revision_detail = RevisionDetailView.as_view() +revision_restore = RevisionRestoreView.as_view() diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 997de4810..71ac49fdc 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -1636,41 +1636,39 @@ def test_device_patch_with_templates_of_same_org(self): self.assertEqual(d1.config.templates.count(), 2) self.assertEqual(r.data["config"]["templates"], [t1.id, t2.id]) - def test_reversion_list_and_restore_api(self): + def test_revision_list_and_restore_api(self): org = self._get_org() + model_slug = 'device' with reversion.create_revision(): device = self._create_device( - organization=org, name="test", _is_deactivated=True + organization=org, + name="test", ) - path = reverse("config_api:device_detail", args=[device.pk]) - response = self.client.delete(path) - self.assertEqual(response.status_code, 204) - self.assertEqual(Device.objects.count(), 0) + path = reverse('config_api:device_detail', args=[device.pk]) + data = dict(name='change-test-device') + response = self.client.patch(path, data, content_type='application/json') + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['name'], 'change-test-device') - path = reverse("config_api:reversion_list") + path = reverse("config_api:revision_list", args=[model_slug]) response = self.client.get(path) response_json = response.json() - version_id = response_json[0]["id"] + version_id = response_json[1]["id"] self.assertEqual(response.status_code, 200) - self.assertEqual(len(response_json), 1) - - with self.subTest("Test filter reversion list with model name"): - params = {"model": "Device"} - response = self.client.get(path, params) - self.assertEqual(response.status_code, 200) - self.assertEqual(len(response.json()), 1) - self.assertEqual(response.json()[0]["object_id"], str(device.pk)) + self.assertEqual(len(response_json), 2) - with self.subTest("Test reversion detail"): - path = reverse("config_api:reversion_detail", args=[version_id]) + with self.subTest("Test revision detail"): + path = reverse("config_api:revision_detail", args=[model_slug, version_id]) response = self.client.get(path) self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["id"], version_id) self.assertEqual(response.json()["object_id"], str(device.pk)) - with self.subTest("Test reversion restore view"): - path = reverse("config_api:reversion_restore", args=[version_id]) + with self.subTest("Test revision restore view"): + revision_id = response_json[1]["revision_id"] + path = reverse( + "config_api:revision_restore", args=[model_slug, revision_id] + ) response = self.client.post(path) self.assertEqual(response.status_code, 200) - self.assertEqual(Device.objects.count(), 1) - self.assertEqual(Device.objects.first().id, device.pk) + self.assertEqual(Device.objects.get(name="test").pk, device.pk) diff --git a/openwisp_controller/mixins.py b/openwisp_controller/mixins.py index db1378d92..955d2558a 100644 --- a/openwisp_controller/mixins.py +++ b/openwisp_controller/mixins.py @@ -1,3 +1,6 @@ +import reversion +from reversion.views import RevisionMixin + from openwisp_users.api.mixins import FilterByOrganizationManaged from openwisp_users.api.mixins import ProtectedAPIMixin as BaseProtectedAPIMixin from openwisp_users.api.permissions import DjangoModelPermissions, IsOrganizationManager @@ -35,3 +38,14 @@ class RelatedDeviceProtectedAPIMixin( class ProtectedAPIMixin(BaseProtectedAPIMixin, FilterByOrganizationManaged): pass + + +class AutoRevisionMixin(RevisionMixin): + def dispatch(self, request, *args, **kwargs): + if request.method in ('GET', 'HEAD', 'OPTIONS'): + return super().dispatch(request, *args, **kwargs) + with reversion.create_revision(): + response = super().dispatch(request, *args, **kwargs) + reversion.set_user(request.user) + reversion.set_comment(f'API request: {request.method} {request.path}') + return response diff --git a/tests/openwisp2/sample_config/api/views.py b/tests/openwisp2/sample_config/api/views.py index 51ceb950d..d60bf0f12 100644 --- a/tests/openwisp2/sample_config/api/views.py +++ b/tests/openwisp2/sample_config/api/views.py @@ -29,13 +29,13 @@ DeviceListCreateView as BaseDeviceListCreateView, ) from openwisp_controller.config.api.views import ( - ReversionDetailView as BaseReversionDetailView, + RevisionDetailView as BaseRevisionDetailView, ) from openwisp_controller.config.api.views import ( - ReversionListView as BaseReversionListView, + RevisionListView as BaseRevisionListView, ) from openwisp_controller.config.api.views import ( - ReversionRestoreView as BaseReversionRestoreView, + RevisionRestoreView as BaseRevisionRestoreView, ) from openwisp_controller.config.api.views import ( TemplateDetailView as BaseTemplateDetailView, @@ -105,15 +105,15 @@ class DownloadDeviceView(BaseDownloadDeviceView): pass -class ReversionListView(BaseReversionListView): +class RevisionListView(BaseRevisionListView): pass -class ReversionDetailView(BaseReversionDetailView): +class RevisionDetailView(BaseRevisionDetailView): pass -class ReversionRestoreView(BaseReversionRestoreView): +class RevisionRestoreView(BaseRevisionRestoreView): pass @@ -131,6 +131,6 @@ class ReversionRestoreView(BaseReversionRestoreView): devicegroup_list = DeviceGroupListCreateView.as_view() devicegroup_detail = DeviceGroupDetailView.as_view() devicegroup_commonname = DeviceGroupCommonName.as_view() -reversion_list = ReversionListView.as_view() -reversion_detail = ReversionDetailView.as_view() -reversion_restore = ReversionRestoreView.as_view() +revision_list = RevisionListView.as_view() +revision_detail = RevisionDetailView.as_view() +revision_restore = RevisionRestoreView.as_view() From 73d51458c3db83a4d2516be9ce58cd06356c1bb1 Mon Sep 17 00:00:00 2001 From: dee077 Date: Sat, 17 May 2025 13:58:44 +0530 Subject: [PATCH 4/8] [tests] Show failing tests --- openwisp_controller/config/api/serializers.py | 30 +++++++++---------- openwisp_controller/config/api/urls.py | 6 ++-- openwisp_controller/config/api/views.py | 6 ++-- openwisp_controller/config/tests/test_api.py | 18 +++++------ openwisp_controller/connection/api/views.py | 17 +++++++---- openwisp_controller/geo/api/views.py | 21 ++++++++++--- openwisp_controller/mixins.py | 16 ++++++---- openwisp_controller/pki/api/views.py | 16 +++++----- 8 files changed, 78 insertions(+), 52 deletions(-) diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index ec6960138..e10d2e6b6 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -382,29 +382,29 @@ def update(self, instance, validated_data): class ReversionSerializer(BaseSerializer): user_id = serializers.SerializerMethodField() date_created = serializers.DateTimeField( - source="revision.date_created", read_only=True + source='revision.date_created', read_only=True ) - comment = serializers.CharField(source="revision.comment", read_only=True) + comment = serializers.CharField(source='revision.comment', read_only=True) content_type = serializers.SerializerMethodField() class Meta: model = Version fields = [ - "id", - "revision_id", - "object_id", - "content_type", - "db", - "format", - "serialized_data", - "object_repr", - "user_id", - "date_created", - "comment", + 'id', + 'revision_id', + 'object_id', + 'content_type', + 'db', + 'format', + 'serialized_data', + 'object_repr', + 'user_id', + 'date_created', + 'comment', ] def get_user_id(self, obj): - return getattr(obj.revision, "user_id", None) + return getattr(obj.revision, 'user_id', None) def get_content_type(self, obj): - return getattr(obj.content_type, "model", None) + return getattr(obj.content_type, 'model', None) diff --git a/openwisp_controller/config/api/urls.py b/openwisp_controller/config/api/urls.py index 4cb753153..15782dbc9 100644 --- a/openwisp_controller/config/api/urls.py +++ b/openwisp_controller/config/api/urls.py @@ -14,17 +14,17 @@ def get_api_urls(api_views): if getattr(settings, "OPENWISP_CONTROLLER_API", True): return [ path( - "controller//revision/", + "controller//revision/", api_views.revision_list, name="revision_list", ), path( - "controller//revision//", + "controller//revision//", api_views.revision_detail, name="revision_detail", ), path( - "controller//revision//restore/", + "controller//revision//restore/", api_views.revision_restore, name="revision_restore", ), diff --git a/openwisp_controller/config/api/views.py b/openwisp_controller/config/api/views.py index 9d8bd1bba..f50aadbe9 100644 --- a/openwisp_controller/config/api/views.py +++ b/openwisp_controller/config/api/views.py @@ -306,7 +306,7 @@ class RevisionListView(ProtectedAPIMixin, AutoRevisionMixin, ListAPIView): serializer_class = ReversionSerializer def get_queryset(self): - model_slug = self.kwargs.get('model_slug').lower() + model_slug = self.kwargs.get('model').lower() return ( Version.objects.select_related('revision') .filter(content_type__model=model_slug) @@ -318,7 +318,7 @@ class RevisionDetailView(ProtectedAPIMixin, RetrieveAPIView): serializer_class = ReversionSerializer def get_queryset(self): - model_slug = self.kwargs.get('model_slug').lower() + model_slug = self.kwargs.get('model').lower() return ( Version.objects.select_related('revision') .filter(content_type__model=model_slug) @@ -330,7 +330,7 @@ class RevisionRestoreView(ProtectedAPIMixin, GenericAPIView): serializer_class = serializers.Serializer def get_queryset(self): - model_slug = self.kwargs.get('model_slug').lower() + model_slug = self.kwargs.get('model').lower() return ( Version.objects.select_related('revision') .filter(content_type__model=model_slug) diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 71ac49fdc..47f92f068 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -1650,25 +1650,25 @@ def test_revision_list_and_restore_api(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.data['name'], 'change-test-device') - path = reverse("config_api:revision_list", args=[model_slug]) + path = reverse('config_api:revision_list', args=[model_slug]) response = self.client.get(path) response_json = response.json() version_id = response_json[1]["id"] self.assertEqual(response.status_code, 200) self.assertEqual(len(response_json), 2) - with self.subTest("Test revision detail"): - path = reverse("config_api:revision_detail", args=[model_slug, version_id]) + with self.subTest('Test revision detail'): + path = reverse('config_api:revision_detail', args=[model_slug, version_id]) response = self.client.get(path) self.assertEqual(response.status_code, 200) - self.assertEqual(response.json()["id"], version_id) - self.assertEqual(response.json()["object_id"], str(device.pk)) + self.assertEqual(response.json()['id'], version_id) + self.assertEqual(response.json()['object_id'], str(device.pk)) - with self.subTest("Test revision restore view"): - revision_id = response_json[1]["revision_id"] + with self.subTest('Test revision restore view'): + revision_id = response_json[1]['revision_id'] path = reverse( - "config_api:revision_restore", args=[model_slug, revision_id] + 'config_api:revision_restore', args=[model_slug, revision_id] ) response = self.client.post(path) self.assertEqual(response.status_code, 200) - self.assertEqual(Device.objects.get(name="test").pk, device.pk) + self.assertEqual(Device.objects.get(name='test').pk, device.pk) diff --git a/openwisp_controller/connection/api/views.py b/openwisp_controller/connection/api/views.py index 5ebc19947..cda3d544c 100644 --- a/openwisp_controller/connection/api/views.py +++ b/openwisp_controller/connection/api/views.py @@ -14,6 +14,7 @@ from openwisp_users.api.mixins import ProtectedAPIMixin as BaseProtectedAPIMixin from ...mixins import ( + AutoRevisionMixin, ProtectedAPIMixin, RelatedDeviceModelPermission, RelatedDeviceProtectedAPIMixin, @@ -61,7 +62,7 @@ def get_serializer_context(self): return context -class CommandListCreateView(BaseCommandView, ListCreateAPIView): +class CommandListCreateView(BaseCommandView, AutoRevisionMixin, ListCreateAPIView): pagination_class = ListViewPagination def create(self, request, *args, **kwargs): @@ -81,13 +82,15 @@ def get_object(self): return obj -class CredentialListCreateView(ProtectedAPIMixin, ListCreateAPIView): +class CredentialListCreateView(ProtectedAPIMixin, AutoRevisionMixin, ListCreateAPIView): queryset = Credentials.objects.order_by("-created") serializer_class = CredentialSerializer pagination_class = ListViewPagination -class CredentialDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView): +class CredentialDetailView( + ProtectedAPIMixin, AutoRevisionMixin, RetrieveUpdateDestroyAPIView +): queryset = Credentials.objects.all() serializer_class = CredentialSerializer @@ -119,7 +122,9 @@ def get_parent_queryset(self): return Device.objects.filter(pk=self.kwargs["device_id"]) -class DeviceConnenctionListCreateView(BaseDeviceConnection, ListCreateAPIView): +class DeviceConnenctionListCreateView( + BaseDeviceConnection, AutoRevisionMixin, ListCreateAPIView +): pagination_class = ListViewPagination def get_queryset(self): @@ -131,7 +136,9 @@ def get_queryset(self): ) -class DeviceConnectionDetailView(BaseDeviceConnection, RetrieveUpdateDestroyAPIView): +class DeviceConnectionDetailView( + BaseDeviceConnection, AutoRevisionMixin, RetrieveUpdateDestroyAPIView +): def get_object(self): queryset = self.filter_queryset(self.get_queryset()) filter_kwargs = { diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index b5514cf26..b25c5f641 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -14,7 +14,11 @@ from openwisp_users.api.filters import OrganizationManagedFilter from openwisp_users.api.mixins import FilterByOrganizationManaged, FilterByParentManaged -from ...mixins import ProtectedAPIMixin, RelatedDeviceProtectedAPIMixin +from ...mixins import ( + AutoRevisionMixin, + ProtectedAPIMixin, + RelatedDeviceProtectedAPIMixin, +) from .filters import DeviceListFilter from .serializers import ( DeviceCoordinatesSerializer, @@ -57,7 +61,9 @@ class ListViewPagination(pagination.PageNumberPagination): max_page_size = 100 -class DeviceCoordinatesView(ProtectedAPIMixin, generics.RetrieveUpdateAPIView): +class DeviceCoordinatesView( + ProtectedAPIMixin, AutoRevisionMixin, generics.RetrieveUpdateAPIView +): serializer_class = DeviceCoordinatesSerializer permission_classes = (DevicePermission,) queryset = Device.objects.select_related( @@ -105,6 +111,7 @@ def create_location(self, device): class DeviceLocationView( RelatedDeviceProtectedAPIMixin, + AutoRevisionMixin, FilterByParentManaged, generics.RetrieveUpdateDestroyAPIView, ): @@ -203,7 +210,9 @@ def get_queryset(self): return qs -class FloorPlanListCreateView(ProtectedAPIMixin, generics.ListCreateAPIView): +class FloorPlanListCreateView( + ProtectedAPIMixin, AutoRevisionMixin, generics.ListCreateAPIView +): serializer_class = FloorPlanSerializer queryset = FloorPlan.objects.select_related().order_by("-created") pagination_class = ListViewPagination @@ -213,13 +222,16 @@ class FloorPlanListCreateView(ProtectedAPIMixin, generics.ListCreateAPIView): class FloorPlanDetailView( ProtectedAPIMixin, + AutoRevisionMixin, generics.RetrieveUpdateDestroyAPIView, ): serializer_class = FloorPlanSerializer queryset = FloorPlan.objects.select_related() -class LocationListCreateView(ProtectedAPIMixin, generics.ListCreateAPIView): +class LocationListCreateView( + ProtectedAPIMixin, AutoRevisionMixin, generics.ListCreateAPIView +): serializer_class = LocationSerializer queryset = Location.objects.order_by("-created") pagination_class = ListViewPagination @@ -229,6 +241,7 @@ class LocationListCreateView(ProtectedAPIMixin, generics.ListCreateAPIView): class LocationDetailView( ProtectedAPIMixin, + AutoRevisionMixin, generics.RetrieveUpdateDestroyAPIView, ): serializer_class = LocationSerializer diff --git a/openwisp_controller/mixins.py b/openwisp_controller/mixins.py index 955d2558a..80c007a0b 100644 --- a/openwisp_controller/mixins.py +++ b/openwisp_controller/mixins.py @@ -41,11 +41,15 @@ class ProtectedAPIMixin(BaseProtectedAPIMixin, FilterByOrganizationManaged): class AutoRevisionMixin(RevisionMixin): + revision_atomic = False + def dispatch(self, request, *args, **kwargs): - if request.method in ('GET', 'HEAD', 'OPTIONS'): - return super().dispatch(request, *args, **kwargs) - with reversion.create_revision(): - response = super().dispatch(request, *args, **kwargs) - reversion.set_user(request.user) - reversion.set_comment(f'API request: {request.method} {request.path}') + if request.method in ('POST', 'PUT', 'PATCH') and request.user.is_authenticated: + with reversion.create_revision(atomic=self.revision_atomic): + response = super().dispatch(request, *args, **kwargs) + reversion.set_user(request.user) + reversion.set_comment( + f'API request: {request.method} {request.get_full_path()}' + ) return response + return super().dispatch(request, *args, **kwargs) diff --git a/openwisp_controller/pki/api/views.py b/openwisp_controller/pki/api/views.py index 9d5ffdf03..e82399531 100644 --- a/openwisp_controller/pki/api/views.py +++ b/openwisp_controller/pki/api/views.py @@ -10,7 +10,7 @@ from rest_framework.response import Response from swapper import load_model -from ...mixins import ProtectedAPIMixin +from ...mixins import AutoRevisionMixin, ProtectedAPIMixin from .serializers import ( CaDetailSerializer, CaListSerializer, @@ -30,18 +30,18 @@ class ListViewPagination(pagination.PageNumberPagination): max_page_size = 100 -class CaListCreateView(ProtectedAPIMixin, ListCreateAPIView): +class CaListCreateView(ProtectedAPIMixin, AutoRevisionMixin, ListCreateAPIView): serializer_class = CaListSerializer queryset = Ca.objects.order_by("-created") pagination_class = ListViewPagination -class CaDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView): +class CaDetailView(ProtectedAPIMixin, AutoRevisionMixin, RetrieveUpdateDestroyAPIView): serializer_class = CaDetailSerializer queryset = Ca.objects.all() -class CaRenewView(ProtectedAPIMixin, GenericAPIView): +class CaRenewView(ProtectedAPIMixin, AutoRevisionMixin, GenericAPIView): serializer_class = serializers.Serializer queryset = Ca.objects.all() @@ -66,18 +66,20 @@ def retrieve(self, request, *args, **kwargs): ) -class CertListCreateView(ProtectedAPIMixin, ListCreateAPIView): +class CertListCreateView(ProtectedAPIMixin, AutoRevisionMixin, ListCreateAPIView): serializer_class = CertListSerializer queryset = Cert.objects.order_by("-created") pagination_class = ListViewPagination -class CertDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView): +class CertDetailView( + ProtectedAPIMixin, AutoRevisionMixin, RetrieveUpdateDestroyAPIView +): serializer_class = CertDetailSerializer queryset = Cert.objects.select_related("ca") -class CertRevokeRenewBaseView(ProtectedAPIMixin, GenericAPIView): +class CertRevokeRenewBaseView(ProtectedAPIMixin, AutoRevisionMixin, GenericAPIView): serializer_class = serializers.Serializer queryset = Cert.objects.select_related("ca") From 4bb281f6d093e4051e534864623543d3cd0897ec Mon Sep 17 00:00:00 2001 From: dee077 Date: Wed, 21 May 2025 23:08:13 +0530 Subject: [PATCH 5/8] [fix] Add revision_id filter to RevisionListView and renamed views and serializer for more clarity --- openwisp_controller/config/api/filters.py | 4 +- openwisp_controller/config/api/serializers.py | 12 +-- openwisp_controller/config/api/urls.py | 4 +- openwisp_controller/config/api/views.py | 73 ++++++++++--------- openwisp_controller/config/tests/test_api.py | 25 +++++-- .../connection/tests/test_api.py | 6 +- openwisp_controller/geo/tests/test_api.py | 36 ++++----- openwisp_controller/pki/tests/test_api.py | 30 ++++---- tests/openwisp2/sample_config/api/views.py | 10 +-- 9 files changed, 104 insertions(+), 96 deletions(-) diff --git a/openwisp_controller/config/api/filters.py b/openwisp_controller/config/api/filters.py index 672e0e610..378e93916 100644 --- a/openwisp_controller/config/api/filters.py +++ b/openwisp_controller/config/api/filters.py @@ -146,10 +146,10 @@ class Meta(BaseConfigAPIFilter.Meta): class ReversionFilter(BaseConfigAPIFilter): - model = filters.CharFilter(field_name="content_type__model", lookup_expr="iexact") + model = filters.CharFilter(field_name='content_type__model', lookup_expr='iexact') def _set_valid_filterform_labels(self): - self.filters["model"].label = _("Model") + self.filters['model'].label = _('Model') def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index e10d2e6b6..931081601 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -379,13 +379,13 @@ def update(self, instance, validated_data): return instance -class ReversionSerializer(BaseSerializer): - user_id = serializers.SerializerMethodField() +class VersionSerializer(BaseSerializer): + user_id = serializers.CharField(source='revision.user_id', read_only=True) date_created = serializers.DateTimeField( source='revision.date_created', read_only=True ) comment = serializers.CharField(source='revision.comment', read_only=True) - content_type = serializers.SerializerMethodField() + content_type = serializers.CharField(source='revision.content_type', read_only=True) class Meta: model = Version @@ -402,9 +402,3 @@ class Meta: 'date_created', 'comment', ] - - def get_user_id(self, obj): - return getattr(obj.revision, 'user_id', None) - - def get_content_type(self, obj): - return getattr(obj.content_type, 'model', None) diff --git a/openwisp_controller/config/api/urls.py b/openwisp_controller/config/api/urls.py index 15782dbc9..3f545bd76 100644 --- a/openwisp_controller/config/api/urls.py +++ b/openwisp_controller/config/api/urls.py @@ -20,8 +20,8 @@ def get_api_urls(api_views): ), path( "controller//revision//", - api_views.revision_detail, - name="revision_detail", + api_views.version_detail, + name="version_detail", ), path( "controller//revision//restore/", diff --git a/openwisp_controller/config/api/views.py b/openwisp_controller/config/api/views.py index f50aadbe9..a61fda2c2 100644 --- a/openwisp_controller/config/api/views.py +++ b/openwisp_controller/config/api/views.py @@ -1,6 +1,7 @@ import reversion from cache_memoize import cache_memoize from django.core.exceptions import ObjectDoesNotExist +from django.db import transaction from django.db.models import F, Q from django.http import Http404 from django.shortcuts import get_list_or_404 @@ -32,8 +33,8 @@ DeviceDetailSerializer, DeviceGroupSerializer, DeviceListSerializer, - ReversionSerializer, TemplateSerializer, + VersionSerializer, VpnSerializer, ) @@ -302,53 +303,55 @@ def certificate_delete_invalidates_cache(cls, organization_id, common_name): cls.get_device_group.invalidate(cls, org_slug, common_name) -class RevisionListView(ProtectedAPIMixin, AutoRevisionMixin, ListAPIView): - serializer_class = ReversionSerializer +class RevisionListView(ProtectedAPIMixin, ListAPIView): + serializer_class = VersionSerializer + queryset = Version.objects.select_related('revision').order_by( + '-revision__date_created' + ) def get_queryset(self): - model_slug = self.kwargs.get('model').lower() - return ( - Version.objects.select_related('revision') - .filter(content_type__model=model_slug) - .order_by('-revision__date_created') - ) - - -class RevisionDetailView(ProtectedAPIMixin, RetrieveAPIView): - serializer_class = ReversionSerializer + model = self.kwargs.get('model').lower() + queryset = self.queryset.filter(content_type__model=model) + revision_id = self.request.query_params.get('revision_id') + if revision_id: + queryset = queryset.filter(revision_id=revision_id) + return self.queryset.filter(content_type__model=model) + + +class VersionDetailView(ProtectedAPIMixin, RetrieveAPIView): + serializer_class = VersionSerializer + queryset = Version.objects.select_related('revision').order_by( + '-revision__date_created' + ) def get_queryset(self): - model_slug = self.kwargs.get('model').lower() - return ( - Version.objects.select_related('revision') - .filter(content_type__model=model_slug) - .order_by('-revision__date_created') - ) + model = self.kwargs.get('model').lower() + return self.queryset.filter(content_type__model=model) class RevisionRestoreView(ProtectedAPIMixin, GenericAPIView): serializer_class = serializers.Serializer + queryset = Version.objects.select_related('revision').order_by( + '-revision__date_created' + ) def get_queryset(self): - model_slug = self.kwargs.get('model').lower() - return ( - Version.objects.select_related('revision') - .filter(content_type__model=model_slug) - .order_by('-revision__date_created') - ) + model = self.kwargs.get('model').lower() + return self.queryset.filter(content_type__model=model) def post(self, request, *args, **kwargs): qs = self.get_queryset() versions = get_list_or_404(qs, revision_id=kwargs['pk']) - with reversion.create_revision(): - for version in versions: - version.revert() - reversion.set_user(request.user) - reversion.set_comment( - f"Restored to previous revision: {self.kwargs.get('pk')}" - ) - - serializer = ReversionSerializer( + with transaction.atomic(): + with reversion.create_revision(): + for version in versions: + version.revert() + reversion.set_user(request.user) + reversion.set_comment( + f"Restored to previous revision: {self.kwargs.get('pk')}" + ) + + serializer = VersionSerializer( versions, many=True, context=self.get_serializer_context() ) return Response(serializer.data, status=status.HTTP_200_OK) @@ -366,5 +369,5 @@ def post(self, request, *args, **kwargs): devicegroup_detail = DeviceGroupDetailView.as_view() devicegroup_commonname = DeviceGroupCommonName.as_view() revision_list = RevisionListView.as_view() -revision_detail = RevisionDetailView.as_view() +version_detail = VersionDetailView.as_view() revision_restore = RevisionRestoreView.as_view() diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 47f92f068..7bd792678 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -1650,15 +1650,24 @@ def test_revision_list_and_restore_api(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.data['name'], 'change-test-device') - path = reverse('config_api:revision_list', args=[model_slug]) - response = self.client.get(path) - response_json = response.json() - version_id = response_json[1]["id"] - self.assertEqual(response.status_code, 200) - self.assertEqual(len(response_json), 2) + with self.subTest('Test revision list'): + path = reverse('config_api:revision_list', args=[model_slug]) + response = self.client.get(path) + response_json = response.json() + version_id = response_json[1]["id"] + revision_id = response_json[1]['revision_id'] + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response_json), 2) + + with self.subTest('Test revision list filter by revision id'): + path = reverse('config_api:revision_list', args=[model_slug]) + response = self.client.get(f'{path}?revision_id={revision_id}') + response_json = response.json() + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response_json), 2) - with self.subTest('Test revision detail'): - path = reverse('config_api:revision_detail', args=[model_slug, version_id]) + with self.subTest('Test version detail'): + path = reverse('config_api:version_detail', args=[model_slug, version_id]) response = self.client.get(path) self.assertEqual(response.status_code, 200) self.assertEqual(response.json()['id'], version_id) diff --git a/openwisp_controller/connection/tests/test_api.py b/openwisp_controller/connection/tests/test_api.py index be13a7474..1a55402c5 100644 --- a/openwisp_controller/connection/tests/test_api.py +++ b/openwisp_controller/connection/tests/test_api.py @@ -494,7 +494,7 @@ def test_post_deviceconnection_list(self): "enabled": True, "failure_reason": "", } - with self.assertNumQueries(13): + with self.assertNumQueries(23): response = self.client.post(path, data, content_type="application/json") self.assertEqual(response.status_code, 201) @@ -539,7 +539,7 @@ def test_put_devceconnection_detail(self): "enabled": False, "failure_reason": "", } - with self.assertNumQueries(14): + with self.assertNumQueries(23): response = self.client.put(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) self.assertEqual( @@ -553,7 +553,7 @@ def test_patch_deviceconnectoin_detail(self): path = reverse("connection_api:deviceconnection_detail", args=(d1, dc.pk)) self.assertEqual(dc.update_strategy, app_settings.UPDATE_STRATEGIES[0][0]) data = {"update_strategy": app_settings.UPDATE_STRATEGIES[1][0]} - with self.assertNumQueries(13): + with self.assertNumQueries(22): response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) self.assertEqual( diff --git a/openwisp_controller/geo/tests/test_api.py b/openwisp_controller/geo/tests/test_api.py index b9cea2c98..3daeac8ec 100644 --- a/openwisp_controller/geo/tests/test_api.py +++ b/openwisp_controller/geo/tests/test_api.py @@ -3,6 +3,7 @@ import uuid from django.contrib.auth import get_user_model +from django.contrib.contenttypes.models import ContentType from django.contrib.gis.geos import Point from django.test import TestCase from django.test.client import BOUNDARY, MULTIPART_CONTENT, encode_multipart @@ -300,6 +301,7 @@ class TestGeoApi( def setUp(self): admin = self._create_admin() self.client.force_login(admin) + ContentType.objects.clear_cache() def _create_device_location(self, **kwargs): options = dict() @@ -494,7 +496,7 @@ def test_post_location_list(self): "address": "Via del Corso, Roma, Italia", "geometry": coords, } - with self.assertNumQueries(9): + with self.assertNumQueries(13): response = self.client.post(path, data, content_type="application/json") self.assertEqual(response.status_code, 201) @@ -525,7 +527,7 @@ def test_put_location_detail(self): "address": "Via del Corso, Roma, Italia", "geometry": coords, } - with self.assertNumQueries(6): + with self.assertNumQueries(10): response = self.client.put(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) self.assertEqual(response.data["organization"], org1.pk) @@ -536,7 +538,7 @@ def test_patch_location_detail(self): self.assertEqual(l1.name, "test-location") path = reverse("geo_api:detail_location", args=[l1.pk]) data = {"name": "change-test-location"} - with self.assertNumQueries(5): + with self.assertNumQueries(9): response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) self.assertEqual(response.data["name"], "change-test-location") @@ -566,7 +568,7 @@ def test_patch_floorplan_detail_api(self): fl = self._create_floorplan(location=l1) path = reverse("geo_api:detail_location", args=[l1.pk]) data = {"floorplan": {"floor": 13}} - with self.assertNumQueries(13): + with self.assertNumQueries(17): response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) fl.refresh_from_db() @@ -577,7 +579,7 @@ def test_change_location_type_to_outdoor_api(self): self._create_floorplan(location=l1) path = reverse("geo_api:detail_location", args=[l1.pk]) data = {"type": "outdoor"} - with self.assertNumQueries(9): + with self.assertNumQueries(13): response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) self.assertEqual(response.data["floorplan"], []) @@ -603,7 +605,7 @@ def test_create_location_with_floorplan(self): "floorplan.floor": ["23"], "floorplan.image": [fl_image], } - with self.assertNumQueries(16): + with self.assertNumQueries(20): response = self.client.post(path, data, format="multipart") self.assertEqual(response.status_code, 201) self.assertEqual(Location.objects.count(), 1) @@ -627,7 +629,7 @@ def test_create_new_floorplan_with_put_location_api(self): "floorplan.floor": "23", "floorplan.image": fl_image, } - with self.assertNumQueries(16): + with self.assertNumQueries(20): response = self.client.put( path, encode_multipart(BOUNDARY, data), content_type=MULTIPART_CONTENT ) @@ -722,7 +724,7 @@ def test_create_devicelocation_using_related_ids(self): floorplan = self._create_floorplan() location = floorplan.location url = reverse("geo_api:device_location", args=[device.id]) - with self.assertNumQueries(18): + with self.assertNumQueries(26): response = self.client.put( url, data={ @@ -760,7 +762,7 @@ def test_create_devicelocation_location_floorplan(self): "floorplan.image": self._get_simpleuploadedfile(), "indoor": ["12.342,23.541"], } - with self.assertNumQueries(32): + with self.assertNumQueries(40): response = self.client.put( url, encode_multipart(BOUNDARY, data), content_type=MULTIPART_CONTENT ) @@ -827,8 +829,8 @@ def test_create_devicelocation_only_location(self): "type": "indoor", } } - with self.assertNumQueries(21): - response = self.client.put(url, data=data, content_type="application/json") + with self.assertNumQueries(29): + response = self.client.put(url, data=data, content_type='application/json') self.assertEqual(response.status_code, 201) self.assertEqual(self.location_model.objects.count(), 1) self.assertEqual(self.object_location_model.objects.count(), 1) @@ -867,7 +869,7 @@ def test_create_devicelocation_existing_location_new_floorplan(self): "floorplan.image": self._get_simpleuploadedfile(), "indoor": ["12.342,23.541"], } - with self.assertNumQueries(26): + with self.assertNumQueries(34): response = self.client.put( url, encode_multipart(BOUNDARY, data), content_type=MULTIPART_CONTENT ) @@ -890,7 +892,7 @@ def test_update_devicelocation_change_location_outdoor_to_indoor(self): } self.assertEqual(device_location.location.type, "outdoor") self.assertEqual(device_location.floorplan, None) - with self.assertNumQueries(23): + with self.assertNumQueries(31): response = self.client.put( path, encode_multipart(BOUNDARY, data), content_type=MULTIPART_CONTENT ) @@ -909,7 +911,7 @@ def test_update_devicelocation_patch_indoor(self): "indoor": "0,0", } self.assertEqual(device_location.indoor, "-140.38620,40.369227") - with self.assertNumQueries(12): + with self.assertNumQueries(18): response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) device_location.refresh_from_db() @@ -926,8 +928,8 @@ def test_update_devicelocation_floorplan_related_id(self): data = { "floorplan": str(floor2.id), } - with self.assertNumQueries(14): - response = self.client.patch(path, data, content_type="application/json") + with self.assertNumQueries(20): + response = self.client.patch(path, data, content_type='application/json') self.assertEqual(response.status_code, 200) device_location.refresh_from_db() self.assertEqual(device_location.floorplan, floor2) @@ -940,7 +942,7 @@ def test_update_devicelocation_location_related_id(self): data = { "location": str(location2.id), } - with self.assertNumQueries(11): + with self.assertNumQueries(19): response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) device_location.refresh_from_db() diff --git a/openwisp_controller/pki/tests/test_api.py b/openwisp_controller/pki/tests/test_api.py index d2d36f1d4..05b2300b2 100644 --- a/openwisp_controller/pki/tests/test_api.py +++ b/openwisp_controller/pki/tests/test_api.py @@ -51,7 +51,7 @@ def test_ca_post_api(self): self.assertEqual(Ca.objects.count(), 0) path = reverse("pki_api:ca_list") data = self._ca_data - with self.assertNumQueries(4): + with self.assertNumQueries(7): r = self.client.post(path, data, content_type="application/json") self.assertEqual(r.status_code, 201) self.assertEqual(Ca.objects.count(), 1) @@ -61,7 +61,7 @@ def test_ca_post_with_extensions_field(self): path = reverse("pki_api:ca_list") data = self._ca_data data["extensions"] = [] - with self.assertNumQueries(4): + with self.assertNumQueries(7): r = self.client.post(path, data, content_type="application/json") self.assertEqual(r.status_code, 201) self.assertEqual(r.data["extensions"], []) @@ -77,7 +77,7 @@ def test_ca_import_post_api(self): "private_key": ca1.private_key, } expected_queries = ( - 7 if parse_version(REST_FRAMEWORK_VERSION) >= parse_version("3.15") else 6 + 10 if parse_version(REST_FRAMEWORK_VERSION) >= parse_version("3.15") else 9 ) with self.assertNumQueries(expected_queries): r = self.client.post(path, data, content_type="application/json") @@ -97,7 +97,7 @@ def test_ca_post_with_date_none_api(self): "validity_start": None, "validity_end": None, } - with self.assertNumQueries(4): + with self.assertNumQueries(7): r = self.client.post(path, data, content_type="application/json") self.assertEqual(r.status_code, 201) self.assertEqual(Ca.objects.count(), 1) @@ -124,7 +124,7 @@ def test_ca_put_api(self): path = reverse("pki_api:ca_detail", args=[ca1.pk]) org2 = self._create_org() data = {"name": "change-ca1", "organization": org2.pk, "notes": "change-notes"} - with self.assertNumQueries(8): + with self.assertNumQueries(11): r = self.client.put(path, data, content_type="application/json") self.assertEqual(r.status_code, 200) self.assertEqual(r.data["name"], "change-ca1") @@ -137,7 +137,7 @@ def test_ca_patch_api(self): data = { "name": "change-ca1", } - with self.assertNumQueries(7): + with self.assertNumQueries(10): r = self.client.patch(path, data, content_type="application/json") self.assertEqual(r.status_code, 200) self.assertEqual(r.data["name"], "change-ca1") @@ -161,7 +161,7 @@ def test_ca_post_renew_api(self): ca1 = self._create_ca(name="ca1", organization=self._get_org()) old_serial_num = ca1.serial_number path = reverse("pki_api:ca_renew", args=[ca1.pk]) - with self.assertNumQueries(5): + with self.assertNumQueries(8): r = self.client.post(path) ca1.refresh_from_db() self.assertEqual(r.status_code, 200) @@ -172,7 +172,7 @@ def test_cert_post_api(self): path = reverse("pki_api:cert_list") data = self._cert_data data["ca"] = self._create_ca().pk - with self.assertNumQueries(8): + with self.assertNumQueries(11): r = self.client.post(path, data, content_type="application/json") self.assertEqual(r.status_code, 201) self.assertEqual(Cert.objects.count(), 1) @@ -189,7 +189,7 @@ def test_import_cert_post_api(self): "private_key": ca1.private_key, } expected_queries = ( - 11 if parse_version(REST_FRAMEWORK_VERSION) >= parse_version("3.15") else 10 + 14 if parse_version(REST_FRAMEWORK_VERSION) >= parse_version("3.15") else 13 ) with self.assertNumQueries(expected_queries): r = self.client.post(path, data, content_type="application/json") @@ -205,7 +205,7 @@ def test_cert_post_with_extensions_field(self): data = self._cert_data data["ca"] = self._create_ca().pk data["extensions"] = [] - with self.assertNumQueries(8): + with self.assertNumQueries(11): r = self.client.post(path, data, content_type="application/json") self.assertEqual(r.status_code, 201) self.assertEqual(Cert.objects.count(), 1) @@ -221,7 +221,7 @@ def test_cert_post_with_date_none(self): "validity_start": None, "validity_end": None, } - with self.assertNumQueries(8): + with self.assertNumQueries(11): r = self.client.post(path, data, content_type="application/json") self.assertEqual(r.status_code, 201) self.assertEqual(Cert.objects.count(), 1) @@ -253,7 +253,7 @@ def test_cert_put_api(self): "organization": org2.pk, "notes": "new-notes", } - with self.assertNumQueries(10): + with self.assertNumQueries(13): r = self.client.put(path, data, content_type="application/json") self.assertEqual(r.status_code, 200) self.assertEqual(r.data["name"], "cert1-change") @@ -264,7 +264,7 @@ def test_cert_patch_api(self): cert1 = self._create_cert(name="cert1") path = reverse("pki_api:cert_detail", args=[cert1.pk]) data = {"name": "cert1-change"} - with self.assertNumQueries(9): + with self.assertNumQueries(12): r = self.client.patch(path, data, content_type="application/json") self.assertEqual(r.status_code, 200) self.assertEqual(r.data["name"], "cert1-change") @@ -289,7 +289,7 @@ def test_post_cert_renew_api(self): cert1 = self._create_cert(name="cert1") old_serial_num = cert1.serial_number path = reverse("pki_api:cert_renew", args=[cert1.pk]) - with self.assertNumQueries(6): + with self.assertNumQueries(9): r = self.client.post(path) self.assertEqual(r.status_code, 200) cert1.refresh_from_db() @@ -300,7 +300,7 @@ def test_post_cert_revoke_api(self): cert1 = self._create_cert(name="cert1") self.assertFalse(cert1.revoked) path = reverse("pki_api:cert_revoke", args=[cert1.pk]) - with self.assertNumQueries(5): + with self.assertNumQueries(8): r = self.client.post(path) cert1.refresh_from_db() self.assertEqual(r.status_code, 200) diff --git a/tests/openwisp2/sample_config/api/views.py b/tests/openwisp2/sample_config/api/views.py index d60bf0f12..e81ce1bbc 100644 --- a/tests/openwisp2/sample_config/api/views.py +++ b/tests/openwisp2/sample_config/api/views.py @@ -28,9 +28,6 @@ from openwisp_controller.config.api.views import ( DeviceListCreateView as BaseDeviceListCreateView, ) -from openwisp_controller.config.api.views import ( - RevisionDetailView as BaseRevisionDetailView, -) from openwisp_controller.config.api.views import ( RevisionListView as BaseRevisionListView, ) @@ -43,6 +40,9 @@ from openwisp_controller.config.api.views import ( TemplateListCreateView as BaseTemplateListCreateView, ) +from openwisp_controller.config.api.views import ( + VersionDetailView as BaseVersionDetailView, +) from openwisp_controller.config.api.views import VpnDetailView as BaseVpnDetailView from openwisp_controller.config.api.views import ( VpnListCreateView as BaseVpnListCreateView, @@ -109,7 +109,7 @@ class RevisionListView(BaseRevisionListView): pass -class RevisionDetailView(BaseRevisionDetailView): +class VersionDetailView(BaseVersionDetailView): pass @@ -132,5 +132,5 @@ class RevisionRestoreView(BaseRevisionRestoreView): devicegroup_detail = DeviceGroupDetailView.as_view() devicegroup_commonname = DeviceGroupCommonName.as_view() revision_list = RevisionListView.as_view() -revision_detail = RevisionDetailView.as_view() +version_detail = VersionDetailView.as_view() revision_restore = RevisionRestoreView.as_view() From 0aa5c4d4f67c1c0c0d8dbc923f1ef80572608be0 Mon Sep 17 00:00:00 2001 From: dee077 Date: Thu, 22 May 2025 04:02:23 +0530 Subject: [PATCH 6/8] [fix] Additional check for registered model --- openwisp_controller/mixins.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/mixins.py b/openwisp_controller/mixins.py index 80c007a0b..b034fea6f 100644 --- a/openwisp_controller/mixins.py +++ b/openwisp_controller/mixins.py @@ -44,7 +44,14 @@ class AutoRevisionMixin(RevisionMixin): revision_atomic = False def dispatch(self, request, *args, **kwargs): - if request.method in ('POST', 'PUT', 'PATCH') and request.user.is_authenticated: + qs = getattr(self, 'queryset', None) + model = getattr(qs, 'model', None) + if ( + request.method in ('POST', 'PUT', 'PATCH') + and request.user.is_authenticated + and model + and reversion.is_registered(model) + ): with reversion.create_revision(atomic=self.revision_atomic): response = super().dispatch(request, *args, **kwargs) reversion.set_user(request.user) From 7580cdccad7f16fe0e6d2faaa69ef4509e3cd5cf Mon Sep 17 00:00:00 2001 From: dee077 Date: Thu, 22 May 2025 13:56:24 +0530 Subject: [PATCH 7/8] [fix] Remove ReversionFilter class --- openwisp_controller/config/api/filters.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/openwisp_controller/config/api/filters.py b/openwisp_controller/config/api/filters.py index 378e93916..9f82a0ab7 100644 --- a/openwisp_controller/config/api/filters.py +++ b/openwisp_controller/config/api/filters.py @@ -4,7 +4,6 @@ from django_filters import rest_framework as filters from django_filters.rest_framework import DjangoFilterBackend from rest_framework.exceptions import ValidationError -from reversion.models import Version from swapper import load_model from openwisp_users.api.filters import OrganizationManagedFilter @@ -143,18 +142,3 @@ def __init__(self, *args, **kwargs): class Meta(BaseConfigAPIFilter.Meta): model = DeviceGroup - - -class ReversionFilter(BaseConfigAPIFilter): - model = filters.CharFilter(field_name='content_type__model', lookup_expr='iexact') - - def _set_valid_filterform_labels(self): - self.filters['model'].label = _('Model') - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self._set_valid_filterform_labels() - - class Meta: - model = Version - fields = ["model"] From 3afb7599e573410df0afb69db89ec2e6243ef3d2 Mon Sep 17 00:00:00 2001 From: dee077 Date: Wed, 4 Jun 2025 15:19:34 +0530 Subject: [PATCH 8/8] [fix] Resolve conflicts and increased queries based on recent changes to geo_api:device_location --- openwisp_controller/config/api/serializers.py | 30 +++++++-------- openwisp_controller/config/api/views.py | 22 +++++------ openwisp_controller/config/tests/test_api.py | 38 +++++++++---------- openwisp_controller/geo/tests/test_api.py | 20 +++++----- openwisp_controller/mixins.py | 8 ++-- 5 files changed, 59 insertions(+), 59 deletions(-) diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index 931081601..de0d252bd 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -380,25 +380,25 @@ def update(self, instance, validated_data): class VersionSerializer(BaseSerializer): - user_id = serializers.CharField(source='revision.user_id', read_only=True) + user_id = serializers.CharField(source="revision.user_id", read_only=True) date_created = serializers.DateTimeField( - source='revision.date_created', read_only=True + source="revision.date_created", read_only=True ) - comment = serializers.CharField(source='revision.comment', read_only=True) - content_type = serializers.CharField(source='revision.content_type', read_only=True) + comment = serializers.CharField(source="revision.comment", read_only=True) + content_type = serializers.CharField(source="revision.content_type", read_only=True) class Meta: model = Version fields = [ - 'id', - 'revision_id', - 'object_id', - 'content_type', - 'db', - 'format', - 'serialized_data', - 'object_repr', - 'user_id', - 'date_created', - 'comment', + "id", + "revision_id", + "object_id", + "content_type", + "db", + "format", + "serialized_data", + "object_repr", + "user_id", + "date_created", + "comment", ] diff --git a/openwisp_controller/config/api/views.py b/openwisp_controller/config/api/views.py index a61fda2c2..fa6930948 100644 --- a/openwisp_controller/config/api/views.py +++ b/openwisp_controller/config/api/views.py @@ -305,14 +305,14 @@ def certificate_delete_invalidates_cache(cls, organization_id, common_name): class RevisionListView(ProtectedAPIMixin, ListAPIView): serializer_class = VersionSerializer - queryset = Version.objects.select_related('revision').order_by( - '-revision__date_created' + queryset = Version.objects.select_related("revision").order_by( + "-revision__date_created" ) def get_queryset(self): - model = self.kwargs.get('model').lower() + model = self.kwargs.get("model").lower() queryset = self.queryset.filter(content_type__model=model) - revision_id = self.request.query_params.get('revision_id') + revision_id = self.request.query_params.get("revision_id") if revision_id: queryset = queryset.filter(revision_id=revision_id) return self.queryset.filter(content_type__model=model) @@ -320,28 +320,28 @@ def get_queryset(self): class VersionDetailView(ProtectedAPIMixin, RetrieveAPIView): serializer_class = VersionSerializer - queryset = Version.objects.select_related('revision').order_by( - '-revision__date_created' + queryset = Version.objects.select_related("revision").order_by( + "-revision__date_created" ) def get_queryset(self): - model = self.kwargs.get('model').lower() + model = self.kwargs.get("model").lower() return self.queryset.filter(content_type__model=model) class RevisionRestoreView(ProtectedAPIMixin, GenericAPIView): serializer_class = serializers.Serializer - queryset = Version.objects.select_related('revision').order_by( - '-revision__date_created' + queryset = Version.objects.select_related("revision").order_by( + "-revision__date_created" ) def get_queryset(self): - model = self.kwargs.get('model').lower() + model = self.kwargs.get("model").lower() return self.queryset.filter(content_type__model=model) def post(self, request, *args, **kwargs): qs = self.get_queryset() - versions = get_list_or_404(qs, revision_id=kwargs['pk']) + versions = get_list_or_404(qs, revision_id=kwargs["pk"]) with transaction.atomic(): with reversion.create_revision(): for version in versions: diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 7bd792678..8b1614d95 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -1638,46 +1638,46 @@ def test_device_patch_with_templates_of_same_org(self): def test_revision_list_and_restore_api(self): org = self._get_org() - model_slug = 'device' + model_slug = "device" with reversion.create_revision(): device = self._create_device( organization=org, name="test", ) - path = reverse('config_api:device_detail', args=[device.pk]) - data = dict(name='change-test-device') - response = self.client.patch(path, data, content_type='application/json') + path = reverse("config_api:device_detail", args=[device.pk]) + data = dict(name="change-test-device") + response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) - self.assertEqual(response.data['name'], 'change-test-device') + self.assertEqual(response.data["name"], "change-test-device") - with self.subTest('Test revision list'): - path = reverse('config_api:revision_list', args=[model_slug]) + with self.subTest("Test revision list"): + path = reverse("config_api:revision_list", args=[model_slug]) response = self.client.get(path) response_json = response.json() version_id = response_json[1]["id"] - revision_id = response_json[1]['revision_id'] + revision_id = response_json[1]["revision_id"] self.assertEqual(response.status_code, 200) self.assertEqual(len(response_json), 2) - with self.subTest('Test revision list filter by revision id'): - path = reverse('config_api:revision_list', args=[model_slug]) - response = self.client.get(f'{path}?revision_id={revision_id}') + with self.subTest("Test revision list filter by revision id"): + path = reverse("config_api:revision_list", args=[model_slug]) + response = self.client.get(f"{path}?revision_id={revision_id}") response_json = response.json() self.assertEqual(response.status_code, 200) self.assertEqual(len(response_json), 2) - with self.subTest('Test version detail'): - path = reverse('config_api:version_detail', args=[model_slug, version_id]) + with self.subTest("Test version detail"): + path = reverse("config_api:version_detail", args=[model_slug, version_id]) response = self.client.get(path) self.assertEqual(response.status_code, 200) - self.assertEqual(response.json()['id'], version_id) - self.assertEqual(response.json()['object_id'], str(device.pk)) + self.assertEqual(response.json()["id"], version_id) + self.assertEqual(response.json()["object_id"], str(device.pk)) - with self.subTest('Test revision restore view'): - revision_id = response_json[1]['revision_id'] + with self.subTest("Test revision restore view"): + revision_id = response_json[1]["revision_id"] path = reverse( - 'config_api:revision_restore', args=[model_slug, revision_id] + "config_api:revision_restore", args=[model_slug, revision_id] ) response = self.client.post(path) self.assertEqual(response.status_code, 200) - self.assertEqual(Device.objects.get(name='test').pk, device.pk) + self.assertEqual(Device.objects.get(name="test").pk, device.pk) diff --git a/openwisp_controller/geo/tests/test_api.py b/openwisp_controller/geo/tests/test_api.py index 3daeac8ec..1398d8720 100644 --- a/openwisp_controller/geo/tests/test_api.py +++ b/openwisp_controller/geo/tests/test_api.py @@ -724,7 +724,7 @@ def test_create_devicelocation_using_related_ids(self): floorplan = self._create_floorplan() location = floorplan.location url = reverse("geo_api:device_location", args=[device.id]) - with self.assertNumQueries(26): + with self.assertNumQueries(29): response = self.client.put( url, data={ @@ -762,7 +762,7 @@ def test_create_devicelocation_location_floorplan(self): "floorplan.image": self._get_simpleuploadedfile(), "indoor": ["12.342,23.541"], } - with self.assertNumQueries(40): + with self.assertNumQueries(43): response = self.client.put( url, encode_multipart(BOUNDARY, data), content_type=MULTIPART_CONTENT ) @@ -829,8 +829,8 @@ def test_create_devicelocation_only_location(self): "type": "indoor", } } - with self.assertNumQueries(29): - response = self.client.put(url, data=data, content_type='application/json') + with self.assertNumQueries(32): + response = self.client.put(url, data=data, content_type="application/json") self.assertEqual(response.status_code, 201) self.assertEqual(self.location_model.objects.count(), 1) self.assertEqual(self.object_location_model.objects.count(), 1) @@ -869,7 +869,7 @@ def test_create_devicelocation_existing_location_new_floorplan(self): "floorplan.image": self._get_simpleuploadedfile(), "indoor": ["12.342,23.541"], } - with self.assertNumQueries(34): + with self.assertNumQueries(37): response = self.client.put( url, encode_multipart(BOUNDARY, data), content_type=MULTIPART_CONTENT ) @@ -892,7 +892,7 @@ def test_update_devicelocation_change_location_outdoor_to_indoor(self): } self.assertEqual(device_location.location.type, "outdoor") self.assertEqual(device_location.floorplan, None) - with self.assertNumQueries(31): + with self.assertNumQueries(33): response = self.client.put( path, encode_multipart(BOUNDARY, data), content_type=MULTIPART_CONTENT ) @@ -911,7 +911,7 @@ def test_update_devicelocation_patch_indoor(self): "indoor": "0,0", } self.assertEqual(device_location.indoor, "-140.38620,40.369227") - with self.assertNumQueries(18): + with self.assertNumQueries(20): response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) device_location.refresh_from_db() @@ -928,8 +928,8 @@ def test_update_devicelocation_floorplan_related_id(self): data = { "floorplan": str(floor2.id), } - with self.assertNumQueries(20): - response = self.client.patch(path, data, content_type='application/json') + with self.assertNumQueries(22): + response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) device_location.refresh_from_db() self.assertEqual(device_location.floorplan, floor2) @@ -942,7 +942,7 @@ def test_update_devicelocation_location_related_id(self): data = { "location": str(location2.id), } - with self.assertNumQueries(19): + with self.assertNumQueries(21): response = self.client.patch(path, data, content_type="application/json") self.assertEqual(response.status_code, 200) device_location.refresh_from_db() diff --git a/openwisp_controller/mixins.py b/openwisp_controller/mixins.py index b034fea6f..d84cac9c6 100644 --- a/openwisp_controller/mixins.py +++ b/openwisp_controller/mixins.py @@ -44,10 +44,10 @@ class AutoRevisionMixin(RevisionMixin): revision_atomic = False def dispatch(self, request, *args, **kwargs): - qs = getattr(self, 'queryset', None) - model = getattr(qs, 'model', None) + qs = getattr(self, "queryset", None) + model = getattr(qs, "model", None) if ( - request.method in ('POST', 'PUT', 'PATCH') + request.method in ("POST", "PUT", "PATCH") and request.user.is_authenticated and model and reversion.is_registered(model) @@ -56,7 +56,7 @@ def dispatch(self, request, *args, **kwargs): response = super().dispatch(request, *args, **kwargs) reversion.set_user(request.user) reversion.set_comment( - f'API request: {request.method} {request.get_full_path()}' + f"API request: {request.method} {request.get_full_path()}" ) return response return super().dispatch(request, *args, **kwargs)