[fix] Corrected initial field value assignment in AbstractDevice class#1301
[fix] Corrected initial field value assignment in AbstractDevice class#1301
Conversation
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
github-project-automation
bot
moved this to To do (general)
in OpenWISP Contributor's Board
pandafy
moved this from To do (general)
to To do (Python & Django)
in OpenWISP Contributor's Board
![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
🤖 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_controller/config/base/device.py`:
- Around line 341-343: The loop currently iterates over
self._changed_checked_fields but then indexes present_values[field], which
raises KeyError because present_values only contains deferred fields that were
refreshed; change the loop to iterate over present_values.keys() (or
present_values) so only refreshed fields are processed, and keep the existing
setattr calls that set _initial_{field} from getattr(self, field) and then
assign setattr(self, field, present_values[field]).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70fe1648-0ef1-4a3d-a2d6-bc0498e23d9a
📒 Files selected for processing (1)
openwisp_controller/config/base/device.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~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.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.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}
📄 CodeRabbit inference engine (Custom checks)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}: Flag potential security vulnerabilities in code
Avoid unnecessary comments or docstrings for code that is already clear
Code formatting is compact and readable. Do not add excessive blank lines, especially inside function or method bodies
Flag unused or redundant code
Ensure variables, functions, classes, and files have descriptive and consistent names
New code must handle errors properly: log errors that cannot be resolved by the user with error level, log unusual conditions with warning level, log important background actions with info level, and provide user-facing messages for errors that the user can solve autonomously
Files:
openwisp_controller/config/base/device.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sql}
📄 CodeRabbit inference engine (Custom checks)
Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries
Files:
openwisp_controller/config/base/device.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sh,bash,sql}
📄 CodeRabbit inference engine (Custom checks)
Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable
Files:
openwisp_controller/config/base/device.py
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_controller/config/base/device.py
🧠 Learnings (4)
📚 Learning: 2026-01-15T14:06:53.460Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/base/models.py:63-93
Timestamp: 2026-01-15T14:06:53.460Z
Learning: In openwisp_controller/geo/base/models.py, the BaseLocation.save() method protects the is_estimated field from manual changes when the estimated location feature is disabled by reverting it to its initial database value using the elif branch. This is intentional behavior to ensure the field can only be modified when the feature is properly configured.
Applied to files:
openwisp_controller/config/base/device.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/base/device.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/base/device.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/base/device.py
github-project-automation
bot
moved this from To do (Python & Django)
to In progress
in OpenWISP Contributor's Board
pandafy
force-pushed
the
fix-device-get-deferred-values
branch
from
320571b to
4145eed
Compare
When a Device instance was loaded with deferred fields (e.g. using QuerySet.only()), `_check_changed_fields()` incorrectly populated the `_initial_<field>` attributes. Instead of storing the original value retrieved from the database, it stored the field name itself. This happened because the code iterated over `_changed_checked_fields` and assigned the field name rather than the actual value coming from `present_values`. The logic has been updated to iterate over `present_values.items()`, store the current database value in `_initial_<field>`, and then restore the updated value on the instance. This ensures change detection works correctly even when fields were initially deferred. Changelog (Bugfix): Fixed incorrect initialization of `_initial_<field>` values when a Device instance is loaded with deferred fields, which could break change detection logic.
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_controller/config/tests/test_device.py`:
- Around line 544-557: The test test_deferred_fields_populated_correctly should
also assert that the instance retains the user-updated value after calling
Device._check_changed_fields(); after invoking device._check_changed_fields(),
add an assertion that device.last_ip == "172.16.1.1" (while still asserting
getattr(device, "_initial_last_ip") == "172.16.0.1"), referencing the test
function name and the Device._check_changed_fields and _initial_last_ip
attributes to locate where to add the extra assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9f9cfc99-0109-4550-94a2-1c527064a1bf
📒 Files selected for processing (2)
openwisp_controller/config/base/device.pyopenwisp_controller/config/tests/test_device.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.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}
📄 CodeRabbit inference engine (Custom checks)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}: Flag potential security vulnerabilities in code
Avoid unnecessary comments or docstrings for code that is already clear
Code formatting is compact and readable. Do not add excessive blank lines, especially inside function or method bodies
Flag unused or redundant code
Ensure variables, functions, classes, and files have descriptive and consistent names
New code must handle errors properly: log errors that cannot be resolved by the user with error level, log unusual conditions with warning level, log important background actions with info level, and provide user-facing messages for errors that the user can solve autonomously
Files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sql}
📄 CodeRabbit inference engine (Custom checks)
Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries
Files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sh,bash,sql}
📄 CodeRabbit inference engine (Custom checks)
Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable
Files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.py
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.py
🧠 Learnings (5)
📚 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_device.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_device.pyopenwisp_controller/config/base/device.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.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_device.pyopenwisp_controller/config/base/device.py
📚 Learning: 2026-01-15T14:06:53.460Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/base/models.py:63-93
Timestamp: 2026-01-15T14:06:53.460Z
Learning: In openwisp_controller/geo/base/models.py, the BaseLocation.save() method protects the is_estimated field from manual changes when the estimated location feature is disabled by reverting it to its initial database value using the elif branch. This is intentional behavior to ensure the field can only be modified when the feature is properly configured.
Applied to files:
openwisp_controller/config/base/device.py
🧬 Code graph analysis (1)
openwisp_controller/config/tests/test_device.py (4)
openwisp_controller/config/tests/utils.py (1)
_create_device(31-44)openwisp_controller/config/models.py (1)
Device(16-23)tests/openwisp2/sample_config/models.py (1)
Device(26-32)openwisp_controller/config/base/device.py (1)
_check_changed_fields(309-315)
🔇 Additional comments (1)
openwisp_controller/config/base/device.py (1)
342-344: Guarded update correctly avoids deferred-field lookup errors.Line 342 now safely restricts updates to keys present in
present_values, and Line 343 correctly stores the refreshed DB value in_initial_<field>before restoring the in-memory value.
| def test_deferred_fields_populated_correctly(self): | ||
| device = self._create_device( | ||
| name="deferred-test", | ||
| last_ip="172.16.0.1", | ||
| management_ip="10.0.0.1", | ||
| ) | ||
| # Load the instance with deferred fields omitted | ||
| device = Device.objects.only("id").get(pk=device.pk) | ||
| device.last_ip = "172.16.1.1" | ||
| # Populate initial values for checked fields | ||
| device._check_changed_fields() | ||
| # Ensure `_initial_<field>` contains the actual value, not the field name | ||
| self.assertEqual(getattr(device, "_initial_last_ip"), "172.16.0.1") | ||
| self.assertNotEqual(getattr(device, "_initial_last_ip"), "last_ip") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Strengthen the regression test with a post-check value assertion.
This test verifies _initial_last_ip, but it should also assert that last_ip still equals the user-updated value after Line 554.
✅ Suggested test addition
# Ensure `_initial_<field>` contains the actual value, not the field name
self.assertEqual(getattr(device, "_initial_last_ip"), "172.16.0.1")
self.assertNotEqual(getattr(device, "_initial_last_ip"), "last_ip")
+ self.assertEqual(device.last_ip, "172.16.1.1")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_controller/config/tests/test_device.py` around lines 544 - 557, The
test test_deferred_fields_populated_correctly should also assert that the
instance retains the user-updated value after calling
Device._check_changed_fields(); after invoking device._check_changed_fields(),
add an assertion that device.last_ip == "172.16.1.1" (while still asserting
getattr(device, "_initial_last_ip") == "172.16.0.1"), referencing the test
function name and the Device._check_changed_fields and _initial_last_ip
attributes to locate where to add the extra assertion.
4145eed to
4ea7ace
Compare
|
@nemesifier this will needed to be backported to 1.2 branch. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
openwisp_controller/config/tests/test_device.py (1)
554-557:⚠️ Potential issue | 🟡 MinorAdd a post-check assertion for the current field value.
After Line 554, assert that
device.last_ipis still"172.16.1.1"so the test verifies both sides of the contract (initial DB snapshot + preserved in-memory update).Suggested test addition
device._check_changed_fields() # Ensure `_initial_<field>` contains the actual value, not the field name self.assertEqual(getattr(device, "_initial_last_ip"), "172.16.0.1") self.assertNotEqual(getattr(device, "_initial_last_ip"), "last_ip") + self.assertEqual(device.last_ip, "172.16.1.1")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openwisp_controller/config/tests/test_device.py` around lines 554 - 557, Add a post-check assertion to verify the current in-memory field value is preserved: after calling device._check_changed_fields() and after asserting getattr(device, "_initial_last_ip") equals "172.16.0.1", add an assertion that device.last_ip == "172.16.1.1" so the test validates both the stored initial snapshot (_initial_last_ip) and the preserved in-memory update (last_ip).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@openwisp_controller/config/tests/test_device.py`:
- Around line 554-557: Add a post-check assertion to verify the current
in-memory field value is preserved: after calling device._check_changed_fields()
and after asserting getattr(device, "_initial_last_ip") equals "172.16.0.1", add
an assertion that device.last_ip == "172.16.1.1" so the test validates both the
stored initial snapshot (_initial_last_ip) and the preserved in-memory update
(last_ip).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2cb91baf-6ac8-4750-b141-52407df11385
📒 Files selected for processing (2)
openwisp_controller/config/base/device.pyopenwisp_controller/config/tests/test_device.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.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | 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.11 | django~=5.1.0
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}
📄 CodeRabbit inference engine (Custom checks)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}: Flag potential security vulnerabilities in code
Avoid unnecessary comments or docstrings for code that is already clear
Code formatting is compact and readable. Do not add excessive blank lines, especially inside function or method bodies
Flag unused or redundant code
Ensure variables, functions, classes, and files have descriptive and consistent names
New code must handle errors properly: log errors that cannot be resolved by the user with error level, log unusual conditions with warning level, log important background actions with info level, and provide user-facing messages for errors that the user can solve autonomously
Files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sql}
📄 CodeRabbit inference engine (Custom checks)
Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries
Files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sh,bash,sql}
📄 CodeRabbit inference engine (Custom checks)
Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable
Files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.py
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.py
🧠 Learnings (5)
📚 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_device.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_device.pyopenwisp_controller/config/base/device.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/tests/test_device.pyopenwisp_controller/config/base/device.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_device.pyopenwisp_controller/config/base/device.py
📚 Learning: 2026-01-15T14:06:53.460Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/base/models.py:63-93
Timestamp: 2026-01-15T14:06:53.460Z
Learning: In openwisp_controller/geo/base/models.py, the BaseLocation.save() method protects the is_estimated field from manual changes when the estimated location feature is disabled by reverting it to its initial database value using the elif branch. This is intentional behavior to ensure the field can only be modified when the feature is properly configured.
Applied to files:
openwisp_controller/config/base/device.py
🧬 Code graph analysis (1)
openwisp_controller/config/tests/test_device.py (3)
openwisp_controller/config/tests/utils.py (1)
_create_device(31-44)openwisp_controller/connection/tests/utils.py (1)
_create_device(51-56)openwisp_controller/config/base/device.py (1)
_check_changed_fields(309-315)
🔇 Additional comments (1)
openwisp_controller/config/base/device.py (1)
341-343: Deferred-field restoration fix looks correct.This correctly scopes restoration to
present_values, captures refreshed DB values into_initial_<field>, and restores the in-memory value without the previous key-mismatch risk.
Test Failure in
|
Test Failure in Device FieldHello @pandafy, The CI failed due to an Fix: The test is likely attempting to verify the population of deferred fields. The attribute name |
2 participants
Checklist
Description of Changes
When a Device instance was loaded with deferred fields (e.g. using
QuerySet.only()),
_check_changed_fields()incorrectly populated the_initial_<field>attributes. Instead of storing the original valueretrieved from the database, it stored the field name itself.
This happened because the code iterated over
_changed_checked_fieldsand assigned the field name rather than the actual value coming from
present_values.The logic has been updated to iterate over
present_values.items(),store the current database value in
_initial_<field>, and thenrestore the updated value on the instance.
This ensures change detection works correctly even when fields were
initially deferred.
Changelog (Bugfix):
Fixed incorrect initialization of
_initial_<field>values when aDevice instance is loaded with deferred fields, which could break
change detection logic.