Skip to content

[fix] Guard missing skip_push_update_on_save in DeviceRegisterView #1331

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
MichaelUray wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[fix] Guard missing skip_push_update_on_save in DeviceRegisterView #1331

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion openwisp_controller/config/controller/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,13 @@ def _update_device_name(self, request, device):
normalized_name = name.replace(":", "").replace("-", "").lower()
if normalized_name != normalized_mac:
device.name = name
device.skip_push_update_on_save()
# NOTE: ``device.skip_push_update_on_save()`` is defined on
# AbstractDevice, but we guard the call defensively so that
# subclasses or older installations that may lack the method
# do not crash during factory-reset re-registration.
skip = getattr(device, "skip_push_update_on_save", None)
if callable(skip):
skip()
Comment on lines +482 to +484
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add warning log when fallback path is used

The guarded call is correct, but the missing-method case is an unusual runtime condition and is currently silent. Please log a warning in the else path so operators can detect compatibility gaps during re-registration.

Suggested patch 
             skip = getattr(device, "skip_push_update_on_save", None)
             if callable(skip):
                 skip()
+            else:
+                logger.warning(
+                    "Device %s does not implement skip_push_update_on_save(); "
+                    "continuing registration without push-skip optimization.",
+                    device.pk,
+                )

As per coding guidelines: "New code must handle errors properly: ... log unusual conditions with warning level."

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

In `@openwisp_controller/config/controller/views.py` around lines 482 - 484, The
current code calls skip = getattr(device, "skip_push_update_on_save", None) and
invokes skip() when callable but leaves the non-callable/missing case silent;
add an else branch that logs a warning via the existing logger (or import the
module logger) indicating the device is missing skip_push_update_on_save to
surface compatibility/runtime gaps during re-registration—reference the device
object and the attribute name skip_push_update_on_save in the log message so
operators can identify which device/class triggered the fallback.



class GetVpnView(SingleObjectMixin, View):
Expand Down
Loading