Skip to content

fix: replace serializers.Serializer with MonitoringDeviceDetailSerial… #744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shakurah wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix: replace serializers.Serializer with MonitoringDeviceDetailSerial… #744

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b740dea
fix: replace serializers.Serializer with MonitoringDeviceDetailSerial…
shakurah Feb 24, 2026
8cb6d0d
[fix] REST API: device monitoring API mentions unused fields example …
shakurah Feb 25, 2026
dfbca66
Merge branch 'master' into fix/716-removed-unused-api-fields
shakurah Feb 26, 2026
dedd8c8
Merge branch 'openwisp:master' into fix/716-removed-unused-api-fields
shakurah Feb 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions openwisp_monitoring/device/api/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from django.utils.translation import gettext_lazy as _
from django_filters.rest_framework import DjangoFilterBackend
from pytz import UTC
from rest_framework import pagination, serializers, status
from rest_framework import pagination, status
from rest_framework.generics import (
GenericAPIView,
ListAPIView,
Expand Down Expand Up @@ -120,7 +120,7 @@ class DeviceMetricView(
)
.all()
)
serializer_class = serializers.Serializer
serializer_class = MonitoringDeviceDetailSerializer
Copy link

@coderabbitai coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

serializer_class and the explicit MonitoringDeviceDetailSerializer call on line 154 are decoupled — consider aligning them

The get() method always explicitly constructs device detail data via MonitoringDeviceDetailSerializer(self.instance).data (line 154), regardless of serializer_class. This means:

  1. If MonitoringApiViewMixin.get() internally calls self.get_serializer() (the standard DRF path), device data is now serialized twice per request — once by the mixin via get_serializer() and again on line 154.
  2. For the CSV path (return response on line 158), super().get() is returned directly. If the mixin uses get_serializer(), this response now includes MonitoringDeviceDetailSerializer fields that weren't present before (when serializers.Serializer was the serializer_class), potentially changing the CSV output structure.

If MonitoringApiViewMixin.get() does not call get_serializer() and the fix is purely for drf-yasg, the existing design is simply inconsistent — get() could be refactored to use self.get_serializer(self.instance) instead of hardcoding the class, so that both schema inference and runtime behaviour share a single source of truth:

♻️ Suggested alignment (contingent on mixin behaviour) 
     if not request.query_params.get("csv"):
         charts_data = dict(response.data)
-        device_metrics_data = MonitoringDeviceDetailSerializer(self.instance).data
+        device_metrics_data = self.get_serializer(self.instance).data
         return Response(
             {**device_metrics_data, **charts_data}, status=status.HTTP_200_OK
         )

Also applies to: 144-158

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@openwisp_monitoring/device/api/views.py` at line 123, The get() method
currently bypasses the view's serializer_class by calling
MonitoringDeviceDetailSerializer(self.instance).data directly, causing
double-serialization or schema/runtime mismatches; change get() to use the
view's standard serializer retrieval (call self.get_serializer(self.instance))
and use its .data so both drf-yasg/schema generation and runtime responses rely
on the same serializer; ensure MonitoringApiViewMixin.get() behavior is
compatible (i.e., if it already calls get_serializer(), remove the hardcoded
MonitoringDeviceDetailSerializer usage) and keep serializer_class =
MonitoringDeviceDetailSerializer as the single source of truth.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and inspect the custom schema class
echo "=== Searching for schema.py in openwisp_monitoring/device/ ==="
find openwisp_monitoring/device -name "schema.py" -type f

echo -e "\n=== Searching for MonitoringApiViewMixin definition ==="
rg -n "class MonitoringApiViewMixin" --type py -A 40

echo -e "\n=== Inspecting DeviceMetricView more closely ==="
rg -n "class DeviceMetricView" openwisp_monitoring/device/api/views.py -A 100 | head -120

echo -e "\n=== Finding where schema is imported from ==="
rg -n "from.*schema import" openwisp_monitoring/device/api/views.py

Repository: openwisp/openwisp-monitoring

Length of output: 7739

🏁 Script executed:

#!/bin/bash
echo "=== Full MonitoringApiViewMixin.get() method ==="
rg -n "def get" openwisp_monitoring/views.py -A 80 | head -100

echo -e "\n=== Full openwisp_monitoring/device/schema.py ==="
cat openwisp_monitoring/device/schema.py

Repository: openwisp/openwisp-monitoring

Length of output: 24141

🏁 Script executed:

#!/bin/bash
echo "=== Checking if there are tests for DeviceMetricView POST/GET ==="
rg -n "class.*DeviceMetric.*Test" --type py -A 5

echo -e "\n=== Checking imports in views.py to understand drf-yasg integration ==="
head -50 openwisp_monitoring/device/api/views.py | grep -E "^from|^import"

echo -e "\n=== Checking if schema is used as drf-yasg AutoSchema ==="
rg -n "from drf_yasg" openwisp_monitoring/device/ --type py
</parameter>

Repository: openwisp/openwisp-monitoring

Length of output: 1608

🏁 Script executed:

#!/bin/bash
echo "=== Checking MonitoringDeviceDetailSerializer fields ==="
rg -n "class MonitoringDeviceDetailSerializer" openwisp_monitoring/device/api/ -A 30

echo -e "\n=== Checking what data is actually returned in GET response ==="
rg -n "def get\|return Response" openwisp_monitoring/device/api/views.py | head -20

Repository: openwisp/openwisp-monitoring

Length of output: 2682

drf-yasg will incorrectly document POST and GET schemas — the plain JSON schema dict does not override serializer-based schema generation

The schema = schema assignment (line 125) is a plain JSON schema dict from openwisp_monitoring/device/schema.py, not a drf-yasg AutoSchema subclass. drf-yasg ignores plain dicts and falls back to using serializer_class for schema generation.

This causes API documentation mismatches:

  • POST request body: Documented as MonitoringDeviceDetailSerializer fields, but the actual payload is raw device-metrics JSON (matching the NetJSON schema structure).
  • GET response: Documented as MonitoringDeviceDetailSerializer only, but the actual response merges device detail data with charts data (line 156).
  • CSV response: Documented as MonitoringDeviceDetailSerializer but actually returns only chart data (line 158).

Clients using auto-generated swagger clients or reading the API docs will have incorrect schema expectations. Either implement a custom drf-yasg AutoSchema subclass that properly documents per-HTTP-method schemas, or remove serializer_class if it's not used at runtime (line 154 hardcodes the serializer independently).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@openwisp_monitoring/device/api/views.py` at line 123, The view sets
serializer_class = MonitoringDeviceDetailSerializer but assigns a plain JSON
schema dict to schema, which drf-yasg ignores and causes incorrect POST/GET/CSV
documentation; fix by creating a drf-yasg AutoSchema subclass (e.g.,
MonitoringDeviceAutoSchema) that returns the correct per-method requestBody and
responses using the existing JSON schema dict from
openwisp_monitoring/device/schema.py and the actual merged GET response (device
detail + charts) and CSV response (charts only), then set schema =
MonitoringDeviceAutoSchema() on the view; alternatively, if
MonitoringDeviceDetailSerializer is not used at runtime (the view hardcodes
serialization), remove serializer_class to avoid misleading docs.

permission_classes = [DevicePermission]
schema = schema

Expand Down
Loading