[fix] Rebase

Author: dee077

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"]

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)

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>/revision/",
+                api_views.revision_list,
+                name="revision_list",
+            ),
+            path(
+                "controller/<str:model>/revision/<str:pk>/",
+                api_views.version_detail,
+                name="version_detail",
+            ),
+            path(
+                "controller/<str:model>/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

@@ -1,8 +1,10 @@
 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
 from django.urls.base import reverse
 from django_filters.rest_framework import DjangoFilterBackend
 from rest_framework import pagination, serializers, status
@@ -19,21 +21,20 @@
 
 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,
 )
 from .serializers import (
     DeviceDetailSerializer,
     DeviceGroupSerializer,
     DeviceListSerializer,
-    ReversionSerializer,
     TemplateSerializer,
+    VersionSerializer,
     VpnSerializer,
 )
 
@@ -53,28 +54,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 +90,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 +105,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 +134,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 +147,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 +160,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 +186,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 +303,57 @@ 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'
+class RevisionListView(ProtectedAPIMixin, ListAPIView):
+    serializer_class = VersionSerializer
+    queryset = Version.objects.select_related("revision").order_by(
+        "-revision__date_created"
     )
-    filter_backends = [DjangoFilterBackend]
-    filterset_class = ReversionFilter
+
+    def get_queryset(self):
+        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 ReversionDetailView(ProtectedAPIMixin, RetrieveAPIView):
-    serializer_class = ReversionSerializer
-    queryset = Version.objects.select_related('revision').order_by(
-        '-revision__date_created'
+class VersionDetailView(ProtectedAPIMixin, RetrieveAPIView):
+    serializer_class = VersionSerializer
+    queryset = Version.objects.select_related("revision").order_by(
+        "-revision__date_created"
     )
-    lookup_field = 'pk'
 
+    def get_queryset(self):
+        model = self.kwargs.get("model").lower()
+        return self.queryset.filter(content_type__model=model)
 
-class ReversionRestoreView(ProtectedAPIMixin, GenericAPIView):
+
+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 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}"
-            )
+    def get_queryset(self):
+        model = self.kwargs.get("model").lower()
+        return self.queryset.filter(content_type__model=model)
 
-        serializer = ReversionSerializer(version, context=self.get_serializer_context())
+    def post(self, request, *args, **kwargs):
+        qs = self.get_queryset()
+        versions = get_list_or_404(qs, revision_id=kwargs["pk"])
+        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)
 
 
@@ -341,6 +368,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()
+version_detail = VersionDetailView.as_view()
+revision_restore = RevisionRestoreView.as_view()

openwisp_controller/config/tests/test_api.py

@@ -1636,41 +1636,48 @@ 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:reversion_list")
-        response = self.client.get(path)
-        response_json = response.json()
-        version_id = response_json[0]["id"]
+        data = dict(name="change-test-device")
+        response = self.client.patch(path, data, content_type="application/json")
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(len(response_json), 1)
+        self.assertEqual(response.data["name"], "change-test-device")
 
-        with self.subTest("Test filter reversion list with model name"):
-            params = {"id": 1, "model": "Device"}
-            response = self.client.get(path, params)
+        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()), 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 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))
 
-        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)