[1.2] Backport fixes Jan 2026 #1201
Conversation
📝 WalkthroughWalkthroughThis pull request introduces defensive programming guards in the admin layer and extends permission filtering in the API layer. Changes to Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2026-01-12T22:27:48.342ZApplied to files:
📚 Learning: 2026-01-15T15:05:49.557ZApplied to files:
📚 Learning: 2026-01-15T15:07:17.354ZApplied to files:
🧬 Code graph analysis (1)openwisp_controller/config/tests/test_admin.py (2)
🪛 Ruff (0.14.13)openwisp_controller/geo/tests/test_api.py1048-1048: Possible hardcoded password assigned to argument: "password" (S106) ⏰ 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)
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openwisp_controller/config/admin.py (1)
351-363: Handle invalid organization IDs to avoid 500s
Organization.objects.get(pk=org_id)can raiseValidationError/DoesNotExiston malformed or tampered input, which will bubble up before the device form can surface errors. Consider catching these and returningNone(same as missing org).🔧 Suggested fix
- org = Organization.objects.get(pk=org_id) + try: + org = Organization.objects.get(pk=org_id) + except (Organization.DoesNotExist, ValidationError): + return
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
openwisp_controller/config/admin.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/geo/api/views.pyopenwisp_controller/geo/tests/test_api.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-12T22:27:48.342Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: tests/openwisp2/sample_config/migrations/0008_whoisinfo_organizationconfigsettings_whois_enabled.py:18-67
Timestamp: 2026-01-12T22:27:48.342Z
Learning: In tests/openwisp2/sample_config/models.py and corresponding test migrations, the WHOISInfo model intentionally includes an additional "details" field not present in the base AbstractWHOISInfo model. This is a testing pattern to verify that swappable models (CONFIG_WHOISINFO_MODEL) can be extended with custom fields without errors.
Applied to files:
openwisp_controller/config/tests/test_admin.py
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/tests/test_admin.pyopenwisp_controller/geo/api/views.pyopenwisp_controller/geo/tests/test_api.pyopenwisp_controller/config/admin.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/config/tests/test_admin.pyopenwisp_controller/geo/api/views.pyopenwisp_controller/geo/tests/test_api.pyopenwisp_controller/config/admin.py
🧬 Code graph analysis (1)
openwisp_controller/config/tests/test_admin.py (2)
openwisp_controller/config/tests/utils.py (1)
_create_template(77-87)openwisp_controller/config/models.py (1)
Device(15-22)
🪛 Ruff (0.14.13)
openwisp_controller/geo/tests/test_api.py
1048-1048: Possible hardcoded password assigned to argument: "password"
(S106)
⏰ 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.12 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
🔇 Additional comments (5)
openwisp_controller/geo/api/views.py (1)
116-119: LGTM: organization-aware lookup hook
This aligns with org-based filtering in the view mixins.openwisp_controller/config/admin.py (1)
371-380: Good defensive early-return
Skipping template validation when a temporary device can’t be built avoids spurious errors in the inline flow.openwisp_controller/config/tests/test_admin.py (1)
2282-2311: Nice regression coverage for empty device form
This test hits the exact admin inline edge case and validates the non-creation behavior.openwisp_controller/geo/tests/test_api.py (2)
12-12: Import addition is fine
No concerns with the addedstatusimport for test assertions.
1041-1056: Solid cross-organization access regression test
Ensures managers can’t access devices outside their org, matching the intended permission behavior.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Backporting these: