[feature] Added AutoRevisionMixin

Author: dee077

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/<str:model_slug>/revision/",
+                api_views.revision_list,
+                name="revision_list",
+            ),
+            path(
+                "controller/<str:model_slug>/revision/<str:pk>/",
+                api_views.revision_detail,
+                name="revision_detail",
+            ),
+            path(
+                "controller/<str:model_slug>/revision/<str:pk>/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/<str:pk>/',
-                api_views.reversion_detail,
-                name='reversion_detail',
-            ),
-            path(
-                'controller/reversion/<str:pk>/restore/',
-                api_views.reversion_restore,
-                name='reversion_restore',
-            ),
         ]
     else:
         return []

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,28 +53,30 @@ 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
     filter_backends = [DjangoFilterBackend]
     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
     filter_backends = [DjangoFilterBackend]
     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,15 +159,19 @@ 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
     filter_backends = [DjangoFilterBackend]
     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()

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)

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

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()