[fix] Add dashboard API filters to browsable API docs #715#751
[fix] Add dashboard API filters to browsable API docs #715#751sargunn20 wants to merge 1 commit intoopenwisp:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds a resilient OpenAPI/Swagger shim and documents DashboardTimeseriesView GET query parameters using Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cbb97c0 to
cfb8707
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_monitoring/monitoring/api/views.py`:
- Around line 3-4: Unconditional drf_yasg imports cause import errors when the
package is not installed; either add drf-yasg to runtime dependencies or guard
the imports and provide fallbacks: wrap the imports of openapi and
swagger_auto_schema in a try/except ImportError and, on ImportError, define
no-op stand-ins (e.g., a dummy swagger_auto_schema decorator and a minimal
openapi namespace) so code using `@swagger_auto_schema` (used on the view
function/class where the decorator is applied) continues to import in
environments without drf_yasg.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5a49c671-8e5d-4f2e-9439-02b0e6983f45
📒 Files selected for processing (2)
openwisp_monitoring/monitoring/api/views.pyopenwisp_monitoring/monitoring/tests/test_api.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-02-21T01:03:37.822Z
Learnt from: nemesifier
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/tests/test_selenium.py:827-859
Timestamp: 2026-02-21T01:03:37.822Z
Learning: In Selenium tests (e.g., in openwisp_monitoring/tests/test_selenium.py and similar test files), when testing JS animations on dashboards or elements driven by JavaScript (such as real-time location updates), insert a short sleep (e.g., sleep(0.3)) before WebDriverWait assertions to allow animations to complete and reduce flakiness. Note: use this as a targeted workaround and prefer explicit waits or animation-end checks where possible to avoid relying on fixed delays.
Applied to files:
openwisp_monitoring/monitoring/tests/test_api.py
📚 Learning: 2026-02-21T18:44:28.852Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/device/api/views.py:263-281
Timestamp: 2026-02-21T18:44:28.852Z
Learning: In openwisp-monitoring, MonitoringIndoorCoordinatesList inherits organization scoping from the parent IndoorCoordinatesList (from openwisp-controller), which uses FilterByParentManaged mixin and filters by location_id in get_queryset(). The child class only overrides the queryset attribute to add monitoring-specific select_related fields; this pattern is safe as long as get_queryset() from the parent is not bypassed. During reviews, verify that MonitoringIndoorCoordinatesList continues to rely on the parent's get_queryset() and that any added select_related fields in the child do not alter the parent's filtering logic.
Applied to files:
openwisp_monitoring/monitoring/tests/test_api.pyopenwisp_monitoring/monitoring/api/views.py
📚 Learning: 2026-02-25T18:42:08.825Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/tests/test_selenium.py:309-317
Timestamp: 2026-02-25T18:42:08.825Z
Learning: In Selenium tests that use ChannelsLiveServerTestCase (e.g., openwisp_monitoring/tests/test_selenium.py), override settings to configure CHANNEL_LAYERS with channels_redis.core.RedisChannelLayer instead of InMemoryChannelLayer. This is required because the live server runs in a separate process from the test process, and InMemoryChannelLayer is per-process only and cannot handle cross-process WebSocket broadcasting needed for real-time location update tests.
Applied to files:
openwisp_monitoring/monitoring/tests/test_api.py
🔇 Additional comments (2)
openwisp_monitoring/monitoring/api/views.py (1)
186-243: Good schema coverage for dashboard filters.The manual parameter set matches the dashboard query filters and closes the discoverability gap in browsable/Swagger docs.
openwisp_monitoring/monitoring/tests/test_api.py (1)
728-760: Regression test is focused and useful.This test cleanly enforces the exact documented filter set for the dashboard schema.
|
Just noting that the two CI failures (Python 3.12/3.13 with Django 5.2) appear to be pre-existing issues in the repository and are unrelated to these changes, identical to the failures in #716. |
|
There is an unresolved comment made by code rabbit, please address it or reply to it saying why you dont agree to it |
Added @swagger_auto_schema to DashboardTimeseriesView.get() to expose query parameters (organization_slug, location_id, floorplan_id, time, start, end, timezone, csv) in the Swagger/browsable API documentation. Since DashboardTimeseriesView extends APIView and manually reads query parameters from request.query_params, drf-yasg had no way to discover them. The @swagger_auto_schema decorator explicitly declares these parameters so they appear in the API docs. Also added a regression test to verify the parameters are properly declared. Closes openwisp#715
cfb8707 to
0029558
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_monitoring/monitoring/api/views.py`:
- Around line 19-23: The fallback swagger_auto_schema decorator currently
returns the original function without metadata; update the swagger_auto_schema
in monitoring/api/views.py so the inner _decorator attaches the decorator kwargs
(or a representative object) as func._swagger_auto_schema before returning func
(i.e., set func._swagger_auto_schema = kwargs or similar) so tests like
test_dashboard_api_filters_in_schema can introspect the stored parameters from
DashboardTimeseriesView.get._swagger_auto_schema.
In `@openwisp_monitoring/monitoring/tests/test_api.py`:
- Around line 728-759: The test test_dashboard_api_filters_in_schema fails when
drf_yasg is not installed because manual_parameters may contain None from the
shim; update the test function test_dashboard_api_filters_in_schema to import
drf_yasg in a try/except and call self.skipTest("drf_yasg is not installed")
when ImportError is raised so the rest of the assertions against
DashboardTimeseriesView.get and its "_swagger_auto_schema" decorator only run
when drf_yasg is available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: baba5074-16a5-4a59-b89d-151ecfdadecf
📒 Files selected for processing (3)
openwisp_monitoring/monitoring/api/views.pyopenwisp_monitoring/monitoring/tests/test_api.pyopenwisp_monitoring/tests/test_selenium.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-02-21T01:03:37.822Z
Learnt from: nemesifier
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/tests/test_selenium.py:827-859
Timestamp: 2026-02-21T01:03:37.822Z
Learning: In Selenium tests (e.g., in openwisp_monitoring/tests/test_selenium.py and similar test files), when testing JS animations on dashboards or elements driven by JavaScript (such as real-time location updates), insert a short sleep (e.g., sleep(0.3)) before WebDriverWait assertions to allow animations to complete and reduce flakiness. Note: use this as a targeted workaround and prefer explicit waits or animation-end checks where possible to avoid relying on fixed delays.
Applied to files:
openwisp_monitoring/monitoring/tests/test_api.pyopenwisp_monitoring/tests/test_selenium.py
📚 Learning: 2026-02-21T18:44:28.852Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/device/api/views.py:263-281
Timestamp: 2026-02-21T18:44:28.852Z
Learning: In openwisp-monitoring, MonitoringIndoorCoordinatesList inherits organization scoping from the parent IndoorCoordinatesList (from openwisp-controller), which uses FilterByParentManaged mixin and filters by location_id in get_queryset(). The child class only overrides the queryset attribute to add monitoring-specific select_related fields; this pattern is safe as long as get_queryset() from the parent is not bypassed. During reviews, verify that MonitoringIndoorCoordinatesList continues to rely on the parent's get_queryset() and that any added select_related fields in the child do not alter the parent's filtering logic.
Applied to files:
openwisp_monitoring/monitoring/tests/test_api.pyopenwisp_monitoring/tests/test_selenium.pyopenwisp_monitoring/monitoring/api/views.py
📚 Learning: 2026-02-25T18:42:08.825Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/tests/test_selenium.py:309-317
Timestamp: 2026-02-25T18:42:08.825Z
Learning: In Selenium tests that use ChannelsLiveServerTestCase (e.g., openwisp_monitoring/tests/test_selenium.py), override settings to configure CHANNEL_LAYERS with channels_redis.core.RedisChannelLayer instead of InMemoryChannelLayer. This is required because the live server runs in a separate process from the test process, and InMemoryChannelLayer is per-process only and cannot handle cross-process WebSocket broadcasting needed for real-time location update tests.
Applied to files:
openwisp_monitoring/monitoring/tests/test_api.pyopenwisp_monitoring/tests/test_selenium.py
🔇 Additional comments (2)
openwisp_monitoring/tests/test_selenium.py (1)
763-769: LGTM!The changes are purely cosmetic - reflowing multi-line JavaScript string literals without altering the executed code. No functional impact.
Also applies to: 780-786, 836-844, 870-878, 903-911
openwisp_monitoring/monitoring/api/views.py (1)
207-265: LGTM!The
@swagger_auto_schemadecorator correctly documents all eight query parameters used byDashboardTimeseriesViewand its parentMonitoringApiViewMixin. The parameter descriptions are accurate and align with the implementation.
|
|
@atif09 Thanks for the catch! Just wanted to note that I've addressed the CodeRabbit comment. The |
|
@sargunn20 why did you close this? |
Added @swagger_auto_schema to DashboardTimeseriesView.get() to expose
query parameters (organization_slug, location_id, floorplan_id, time,
start, end, timezone, csv) in the Swagger/browsable API documentation.
Since DashboardTimeseriesView extends APIView and manually reads query
parameters from request.query_params, drf-yasg had no way to discover them.
Checklist
Reference to Existing Issue
Closes #715
Description of Changes
@swagger_auto_schemadecorator toDashboardTimeseriesView.get()inmonitoring/api/views.pywith all 8 query parameters explicitly declaredtest_dashboard_api_filters_in_schemato verify the parameters are properly exposed