[feat] Allow REST API to specify upgrade_options for upgrade operations#371
[feat] Allow REST API to specify upgrade_options for upgrade operations#371youhaveme9 wants to merge 9 commits into
Conversation
📝 WalkthroughWalkthroughThe changes add support for specifying and retrieving Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API View
participant Serializer
participant Model
participant Upgrader
Client->>API View: POST with upgrade_options
API View->>API View: Extract & parse upgrade_options
API View->>Serializer: Pass upgrade_options in data
Serializer->>Serializer: validate() creates temp UpgradeOperation
Serializer->>Model: Validate upgrade_options against schema
Model->>Upgrader: Check SCHEMA & validate_upgrade_options
alt Validation Success
Upgrader-->>Model: Valid
Model-->>Serializer: ✓
Serializer->>Model: create/update with upgrade_options
Model-->>Serializer: Instance saved
Serializer->>API View: Serialized data (includes upgrade_options)
API View-->>Client: 200/201 with upgrade_options
else Validation Fails
Upgrader-->>Model: ValidationError
Model-->>Serializer: Error
Serializer-->>API View: ValidationError
API View-->>Client: 400 with error details
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
openwisp_firmware_upgrader/api/views.py (1)
91-106: Silent JSON parse error may mask invalid input.When
upgrade_optionsis a malformed JSON string, the code silently falls back to an empty dict (line 103-104). This could hide user errors. Consider returning a 400 response instead.🔎 Proposed fix
if isinstance(upgrade_options, str): try: upgrade_options = json.loads(upgrade_options) except (json.JSONDecodeError, ValueError): - upgrade_options = {} + return Response( + {"upgrade_options": ["Invalid JSON format"]}, + status=status.HTTP_400_BAD_REQUEST, + ) if upgrade_options is None: upgrade_options = {}openwisp_firmware_upgrader/tests/test_api.py (1)
1883-1895: Remove unused variabledevice_fw.The static analysis hint is valid—
device_fwis assigned but never used. Consider using_or removing the assignment.🔎 Proposed fix
def test_upgrade_operation_includes_upgrade_options(self): """Test that upgrade operation includes upgrade_options in response""" - device_fw = self._create_device_firmware(upgrade=True) + self._create_device_firmware(upgrade=True) upgrade_operation = UpgradeOperation.objects.first()openwisp_firmware_upgrader/api/serializers.py (1)
164-178: Use exception chaining for better debugging.Per static analysis (B904), when re-raising an exception within an except block, use
raise ... from eto preserve the exception chain for debugging.🔎 Proposed fix
try: temp_operation.validate_upgrade_options() except ValidationError as e: - raise serializers.ValidationError({"upgrade_options": e.messages}) + raise serializers.ValidationError({"upgrade_options": e.messages}) from e return super().validate(data)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
openwisp_firmware_upgrader/api/serializers.pyopenwisp_firmware_upgrader/api/views.pyopenwisp_firmware_upgrader/base/models.pyopenwisp_firmware_upgrader/tests/test_api.pytests/openwisp2/settings.py
🧰 Additional context used
🧬 Code graph analysis (3)
openwisp_firmware_upgrader/base/models.py (1)
openwisp_firmware_upgrader/upgraders/openwrt.py (1)
validate_upgrade_options(108-126)
openwisp_firmware_upgrader/api/views.py (4)
openwisp_firmware_upgrader/models.py (1)
BatchUpgradeOperation(37-40)tests/openwisp2/sample_firmware_upgrader/models.py (1)
BatchUpgradeOperation(40-42)openwisp_firmware_upgrader/admin.py (1)
full_clean(355-366)openwisp_firmware_upgrader/base/models.py (1)
batch_upgrade(161-171)
openwisp_firmware_upgrader/api/serializers.py (4)
openwisp_firmware_upgrader/base/models.py (10)
Meta(52-53)Meta(84-88)Meta(125-130)Meta(234-238)Meta(356-358)Meta(507-510)Meta(665-666)validate_upgrade_options(55-70)save(390-399)save(781-787)openwisp_firmware_upgrader/admin.py (5)
Meta(101-103)Meta(234-236)Meta(341-343)Meta(422-423)save(368-376)openwisp_firmware_upgrader/models.py (2)
UpgradeOperation(43-46)DeviceFirmware(31-34)openwisp_firmware_upgrader/upgraders/openwrt.py (1)
validate_upgrade_options(108-126)
🪛 Ruff (0.14.10)
openwisp_firmware_upgrader/tests/test_api.py
1885-1885: Local variable device_fw is assigned to but never used
Remove assignment to unused variable device_fw
(F841)
openwisp_firmware_upgrader/api/serializers.py
177-177: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
⏰ 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.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.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.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
🔇 Additional comments (7)
openwisp_firmware_upgrader/api/views.py (1)
108-120: Validation approach is consistent with admin pattern.The use of a temporary
BatchUpgradeOperationfor validation mirrors the pattern inadmin.py(lines 354-365), ensuring consistent validation behavior across admin and API.openwisp_firmware_upgrader/base/models.py (1)
55-70: Defensive null-check improves robustness.The early return when
upgrader_classis falsy prevents potentialAttributeErrorwhen no upgrader can be determined (e.g., for builds without associated devices). Usinggetattr(..., "SCHEMA", None)is also safer than direct attribute access.openwisp_firmware_upgrader/tests/test_api.py (2)
420-446: Good test coverage for upgrade_options validation.The tests properly verify both successful acceptance of valid
upgrade_optionsand rejection of invalid/mutually exclusive options (likecandntogether).
1381-1414: Comprehensive device firmware upgrade_options tests.Tests cover both the happy path (valid options persisted and returned) and error path (invalid options rejected with 400).
openwisp_firmware_upgrader/api/serializers.py (3)
87-96: Fields explicitly listed for UpgradeOperationSerializer.The explicit field tuple with
upgrade_optionsis clean and clear.
193-214: Create and update methods correctly propagate upgrade_options.The pattern of extracting
upgrade_optionsand passing it tomodel.save()aligns with the admin form'ssave()method (admin.py lines 367-375).
180-191: Addprefetch_related("device__upgradeoperation_set")to optimize query into_representation.The
to_representationmethod currently executes an additional query per serialized instance to fetch the latestUpgradeOperation. While the detail view is the only current endpoint, the extra query can be avoided by usingprefetch_relatedin the view's queryset. If a list view is added in the future, this becomes critical.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
openwisp_firmware_upgrader/tests/test_api.py (3)
418-428: Consider testing with devices for more realistic scenario.The test creates a batch upgrade operation with
upgrade_optionson a build without any associated devices. While this tests the API acceptance and persistence, consider adding a subtest that creates an upgrade environment (similar totest_api_batch_upgrade_validates_upgrade_options) to ensureupgrade_optionswork correctly in a realistic scenario with actual devices.🔎 Suggested enhancement
def test_api_batch_upgrade_with_upgrade_options(self): """Test batch upgrade accepts upgrade_options""" - build = self._create_build() - url = reverse("upgrader:api_build_batch_upgrade", args=[build.pk]) - upgrade_options = {"c": True, "F": True} - r = self.client.post( - url, {"upgrade_options": upgrade_options}, content_type="application/json" - ) - self.assertEqual(r.status_code, 201) - batch = BatchUpgradeOperation.objects.first() - self.assertEqual(batch.upgrade_options, upgrade_options) + with self.subTest("Test with build without devices"): + build = self._create_build() + url = reverse("upgrader:api_build_batch_upgrade", args=[build.pk]) + upgrade_options = {"c": True, "F": True} + r = self.client.post( + url, {"upgrade_options": upgrade_options}, content_type="application/json" + ) + self.assertEqual(r.status_code, 201) + batch = BatchUpgradeOperation.objects.first() + self.assertEqual(batch.upgrade_options, upgrade_options) + + with self.subTest("Test with build with devices"): + env = self._create_upgrade_env() + build = env["build2"] + url = reverse("upgrader:api_build_batch_upgrade", args=[build.pk]) + upgrade_options = {"c": True, "F": True} + r = self.client.post( + url, {"upgrade_options": upgrade_options}, content_type="application/json" + ) + self.assertEqual(r.status_code, 201) + batch = BatchUpgradeOperation.objects.latest('created') + self.assertEqual(batch.upgrade_options, upgrade_options)
418-445: Consider additional test coverage for edge cases.While the existing tests cover the main functionality well, consider adding tests for the following scenarios:
Form data submission: The view code (lines 86-120 in views.py) includes logic to parse
upgrade_optionsfrom form data as a JSON string. Add a test to verify this works correctly:# Test form data with upgrade_options as JSON string r = self.client.post(url, {"upgrade_options": '{"c": true}'})Invalid JSON in form data: Test that malformed JSON in form data is handled gracefully (the code defaults to
{}on JSONDecodeError).Empty upgrade_options: Verify that an empty dict
{}is accepted and behaves correctly.Unknown option keys: Test whether unknown keys in
upgrade_optionsare validated or silently ignored.Type validation: Ensure that non-boolean values for flags (e.g.,
{"c": "true"}as string) are validated appropriately.Also applies to: 1379-1413
430-445: Consider verifying error message content.The validation tests check that
"upgrade_options"key is in the error response, but don't verify the actual error message content. Consider adding assertions to verify the error message is helpful and accurate, such as checking that it mentions the mutually exclusive options.🔎 Suggested enhancement
def test_api_batch_upgrade_validates_upgrade_options(self): """Test batch upgrade validates upgrade_options""" # Create a build with a device so upgrader_class can be determined env = self._create_upgrade_env() build = env["build2"] url = reverse("upgrader:api_build_batch_upgrade", args=[build.pk]) # -n and -c are mutually exclusive invalid_upgrade_options = {"c": True, "n": True} r = self.client.post( url, {"upgrade_options": invalid_upgrade_options}, content_type="application/json", ) self.assertEqual(r.status_code, 400) self.assertIn("upgrade_options", r.data) + # Verify error message mentions the mutually exclusive options + error_message = str(r.data["upgrade_options"]) + self.assertTrue( + "mutually exclusive" in error_message.lower() or + ("-c" in error_message and "-n" in error_message), + f"Error message should mention mutually exclusive options: {error_message}" + ) self.assertEqual(BatchUpgradeOperation.objects.count(), 0)Also applies to: 1397-1413
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
openwisp_firmware_upgrader/tests/test_api.py
🧰 Additional context used
🧬 Code graph analysis (1)
openwisp_firmware_upgrader/tests/test_api.py (4)
openwisp_firmware_upgrader/api/views.py (2)
post(87-121)get(123-134)openwisp_firmware_upgrader/models.py (2)
BatchUpgradeOperation(37-40)UpgradeOperation(43-46)openwisp_firmware_upgrader/api/serializers.py (1)
create(193-202)openwisp_firmware_upgrader/admin.py (1)
save(368-376)
⏰ 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). (12)
- GitHub Check: Python==3.13 | 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~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
🔇 Additional comments (8)
openwisp_firmware_upgrader/tests/test_api.py (8)
430-445: LGTM! Good validation test coverage.The test properly validates that mutually exclusive upgrade options (
candn) are rejected with a 400 status code, and verifies noBatchUpgradeOperationis created on validation failure.
825-836: LGTM! Proper serialization test.The test correctly verifies that
upgrade_optionsare included in the API response when retrieving aBatchUpgradeOperationdetail.
1379-1396: LGTM! Comprehensive test for device firmware upgrade options.The test properly verifies that
upgrade_optionsare accepted during device firmware updates, persisted in theUpgradeOperation, and included in the API response.
1397-1413: LGTM! Good validation coverage.The test correctly validates that mutually exclusive upgrade options are rejected with proper error handling and no side effects.
1427-1427: Verify query count adjustments in multitenancy test.Query counts have been adjusted in the multitenancy test for device firmware detail. Ensure these increases are justified by the
upgrade_optionsfeature and don't indicate permission-related query inefficiencies.Also applies to: 1460-1460, 1468-1468
1635-1650: LGTM! Good test for device upgrade operation serialization.The test properly verifies that
upgrade_optionsare included in the device upgrade operation list API response. The use ofassertGreaterbefore accessing the first result is a good defensive practice.
1881-1894: LGTM! Complete serialization coverage.The test correctly verifies that
upgrade_optionsare included in the upgrade operation detail API response, complementing the list endpoint test.
324-324: Verify that 10 queries is the appropriate expectation for this batch upgrade operation.Ensure the
upgrade_optionsfield processing does not introduce unintentional N+1 queries. Run the test with Django query logging enabled to confirm all 10 queries are necessary and properly optimized (e.g., usingselect_relatedorprefetch_relatedwhere applicable). Check that no redundant database hits occur during field validation or serialization.
| "upgrader:api_devicefirmware_detail", args=[device_fw1.device.pk] | ||
| ) | ||
| with self.assertNumQueries(9): | ||
| with self.assertNumQueries(13): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the test file and examine the specific test methods
fd test_api.py | head -1 | xargs wc -lRepository: openwisp/openwisp-firmware-upgrader
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Get the specific lines mentioned in the review to understand what changed
fd test_api.py | head -1 | xargs sed -n '1195,1210p;1245,1255p;1283,1295p;1313,1325p;1352,1365p'Repository: openwisp/openwisp-firmware-upgrader
Length of output: 3298
🏁 Script executed:
#!/bin/bash
# Search for DeviceFirmwareSerializer and its related fields
rg -n 'class DeviceFirmwareSerializer' openwisp_firmware_upgrader/api/Repository: openwisp/openwisp-firmware-upgrader
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Check for select_related/prefetch_related usage in views and serializers
rg -B 3 -A 3 'select_related|prefetch_related' openwisp_firmware_upgrader/api/views.py openwisp_firmware_upgrader/api/serializers.py | head -100Repository: openwisp/openwisp-firmware-upgrader
Length of output: 5170
🏁 Script executed:
#!/bin/bash
# Search for upgrade_options in the codebase to understand the new feature
rg -n 'upgrade_options' openwisp_firmware_upgrader/Repository: openwisp/openwisp-firmware-upgrader
Length of output: 20393
🏁 Script executed:
#!/bin/bash
# Get the full DeviceFirmwareSerializer implementation
sed -n '135,220p' openwisp_firmware_upgrader/api/serializers.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 3446
🏁 Script executed:
#!/bin/bash
# Check if there's prefetch_related in DeviceFirmwareDetailView queryset
grep -n "class DeviceFirmwareDetailView" openwisp_firmware_upgrader/api/views.py -A 10Repository: openwisp/openwisp-firmware-upgrader
Length of output: 504
🏁 Script executed:
#!/bin/bash
# Look for any prefetch setup for UpgradeOperation
rg -n "prefetch_related" openwisp_firmware_upgrader/api/ -B 2 -A 2Repository: openwisp/openwisp-firmware-upgrader
Length of output: 505
🏁 Script executed:
#!/bin/bash
# Check the test methods to understand what operations trigger queries
sed -n '1190,1210p' openwisp_firmware_upgrader/tests/test_api.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 1134
🏁 Script executed:
#!/bin/bash
# Check if there's any prefetch_related for upgradeoperation_set in the view
rg -n "prefetch_related.*upgradeoperation" openwisp_firmware_upgrader/api/Repository: openwisp/openwisp-firmware-upgrader
Length of output: 172
🏁 Script executed:
#!/bin/bash
# Check the earlier version to see what the original query count was
git log --oneline openwisp_firmware_upgrader/tests/test_api.py | head -20Repository: openwisp/openwisp-firmware-upgrader
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Look at the git diff to see what changed in the query counts
git diff HEAD~1 openwisp_firmware_upgrader/tests/test_api.py | grep -A 3 -B 3 "assertNumQueries" | head -50Repository: openwisp/openwisp-firmware-upgrader
Length of output: 265
🏁 Script executed:
#!/bin/bash
# Get the full DeviceFirmwareDetailView implementation
sed -n '281,305p' openwisp_firmware_upgrader/api/views.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 1034
🏁 Script executed:
#!/bin/bash
# Check the model to see if there are any query optimization hints
rg -n "class DeviceFirmware" openwisp_firmware_upgrader/base/models.py -A 20 | head -40Repository: openwisp/openwisp-firmware-upgrader
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Look for the actual git commit that added upgrade_options to see what changed
git log --all --oneline | grep -i "upgrade" | head -10Repository: openwisp/openwisp-firmware-upgrader
Length of output: 851
🏁 Script executed:
#!/bin/bash
# Check the commit that added upgrade_options feature
git show 86a6605 --stat | head -30Repository: openwisp/openwisp-firmware-upgrader
Length of output: 1641
🏁 Script executed:
#!/bin/bash
# Look at what changed specifically in serializers and views for upgrade_options
git show 86a6605 -- openwisp_firmware_upgrader/api/serializers.py openwisp_firmware_upgrader/api/views.py | grep -A 5 -B 5 "to_representation\|upgrade_options" | head -80Repository: openwisp/openwisp-firmware-upgrader
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Check the N+1 issue: instance.device.upgradeoperation_set in to_representation
# This is called for EVERY DeviceFirmware instance, but the view doesn't prefetch it
# Let's verify the view doesn't have prefetch_related for upgradeoperation_set
grep -A 5 "class DeviceFirmwareDetailView" openwisp_firmware_upgrader/api/views.py | grep -E "queryset|prefetch"Repository: openwisp/openwisp-firmware-upgrader
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Count how many upgrade operations would be queried per test
# The to_representation() method calls:
# instance.device.upgradeoperation_set.latest("created")
# This is an unoptimized query that happens for EVERY serialized DeviceFirmware
# Let's verify by looking at the test to see if this pattern exists
rg -n "to_representation" openwisp_firmware_upgrader/api/serializers.py -B 2 -A 15Repository: openwisp/openwisp-firmware-upgrader
Length of output: 1768
🏁 Script executed:
#!/bin/bash
# Search for any Prefetch imports or usage in serializers
rg -n "Prefetch|prefetch_related" openwisp_firmware_upgrader/api/serializers.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Check if there's a comment or TODO about optimizing the upgrade_options query
rg -n "N\+1|optimize|prefetch" openwisp_firmware_upgrader/api/ -iRepository: openwisp/openwisp-firmware-upgrader
Length of output: 172
🏁 Script executed:
#!/bin/bash
# Verify the exact behavior: does the view need to be modified to avoid N+1?
# Let's check if there are any recent changes to optimize this
git log --all --oneline -- openwisp_firmware_upgrader/api/views.py | head -5Repository: openwisp/openwisp-firmware-upgrader
Length of output: 350
🏁 Script executed:
#!/bin/bash
# Check the exact test that's failing to understand what queries are being run
sed -n '1195,1220p' openwisp_firmware_upgrader/tests/test_api.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 1483
🏁 Script executed:
#!/bin/bash
# Check if DeviceFirmwareListView exists and how it's optimized
rg -n "class DeviceFirmwareListView" openwisp_firmware_upgrader/api/views.py -A 10Repository: openwisp/openwisp-firmware-upgrader
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Look for any list endpoints that use DeviceFirmware
rg -n "DeviceFirmware" openwisp_firmware_upgrader/api/views.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 543
🏁 Script executed:
#!/bin/bash
# Check the to_representation method more carefully - is it using select_related properly?
# The issue is: instance.device.upgradeoperation_set.latest("created")
# This requires the device to be prefetched AND the upgrade operations to be prefetched
# Let's see if this pattern appears elsewhere
rg -n "upgradeoperation_set" openwisp_firmware_upgrader/Repository: openwisp/openwisp-firmware-upgrader
Length of output: 3511
🏁 Script executed:
#!/bin/bash
# Check if the view can be modified to add prefetch_related
# Look at how get_queryset is implemented
sed -n '281,320p' openwisp_firmware_upgrader/api/views.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 1551
🏁 Script executed:
#!/bin/bash
# Check lines 341 and 348 in views.py to see the other place where upgradeoperation_set is accessed
sed -n '335,355p' openwisp_firmware_upgrader/api/views.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 1073
🏁 Script executed:
#!/bin/bash
# Look at the BatchUpgradeOperationDetailView to understand the proper prefetch pattern
sed -n '160,175p' openwisp_firmware_upgrader/api/views.pyRepository: openwisp/openwisp-firmware-upgrader
Length of output: 591
🏁 Script executed:
#!/bin/bash
# Verify if there's a Prefetch import and how it's used elsewhere
rg -n "from django.db.models import.*Prefetch" openwisp_firmware_upgrader/Repository: openwisp/openwisp-firmware-upgrader
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Check if there are any comments in the PR about the N+1 issue or query optimization
git log --all --grep="upgrade_options\|N+1" --oneline | head -10Repository: openwisp/openwisp-firmware-upgrader
Length of output: 61
Fix N+1 query issue in DeviceFirmwareDetailView.
The query count increases (13, 29, 28, 30, 31) are caused by an unoptimized N+1 pattern in DeviceFirmwareSerializer.to_representation() (line 187), which calls instance.device.upgradeoperation_set.latest("created") without prefetching. This same pattern is repeated in the view's put and patch handlers (lines 341, 348).
Add prefetch_related("device__upgradeoperation_set") to DeviceFirmwareDetailView.queryset to batch the queries, similar to the pattern used in BatchUpgradeOperationDetailView (line 169).
🤖 Prompt for AI Agents
In openwisp_firmware_upgrader/tests/test_api.py around line 1202, the test
exposes an N+1 query problem originating from
DeviceFirmwareSerializer.to_representation (line 187) and repeated in
DeviceFirmwareDetailView.put/patch handlers (lines 341, 348); fix it by updating
DeviceFirmwareDetailView.queryset to include
prefetch_related("device__upgradeoperation_set") (following the
BatchUpgradeOperationDetailView pattern at line 169) so latest upgrade
operations for each device are prefetched and the repeated
instance.device.upgradeoperation_set.latest("created") calls no longer trigger
extra queries.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
Checklist
Reference to Existing Issue
Closes #366
Please open a new issue if there isn't an existing issue yet.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.