[fix] Detect device organization change #1091

stktyagi · 2025-07-20T11:01:57Z

Checklist

I have read the OpenWISP Contributing Guidelines.
I have manually tested the changes proposed in this pull request.
I have written new test cases for new code and/or updated existing tests for changes to existing code.
I have updated the documentation.

Reference to Existing Issue

Closes #1070.

Description of Changes

Added a new file signal_handlers.py that listens for changes to organization settings and marks related device configurations as modified when those settings change.

Added signal handler to update device config status when organizationconfig settings change and tests for the same. Fixes #1070

Added signal handler to update device config status when organization config settings change and tests for the same. Fixes #1070

Replaced direct model imports with swapper to avoid premature access to models during Django app initialization. Fixes #1070

Replaced direct model imports with swapper to avoid premature access to models during Django app initialization during tests. Fixes #1070

stktyagi · 2025-07-20T11:58:41Z

Added # noqa to prevent unused import error for now and ensure signal handlers are registered correctly, we can revisit this approach and discuss cleaner alternatives for this.

coveralls · 2025-07-20T12:00:22Z

coverage: 98.716% (-0.03%) from 98.746%
when pulling fe605ce on issues/1070-device-config-status
into 4ea68be on master.

nemesifier

This solution is not consistent with what we're using now, see:

openwisp-controller/openwisp_controller/config/base/device.py

Lines 123 to 128 in 4ea68be

    
           def _set_initial_values_for_changed_checked_fields(self): 
        
               for field in self._changed_checked_fields: 
        
                   if self._is_deferred(field): 
        
                       setattr(self, f"_initial_{field}", models.DEFERRED) 
        
                   else: 
        
                       setattr(self, f"_initial_{field}", getattr(self, field))

openwisp-controller/openwisp_controller/config/base/device.py

Lines 310 to 387 in 4ea68be

    
               def _get_initial_values_for_checked_fields(self): 
        
                   # Refresh values from database only when the checked field 
        
                   # was initially deferred, but is no longer deferred now. 
        
                   # Store the present value of such fields because they will 
        
                   # be overwritten fetching values from database 
        
                   # NOTE: Initial value of a field will only remain deferred 
        
                   # if the current value of the field is still deferred. This 
        
                   present_values = dict() 
        
                   for field in self._changed_checked_fields: 
        
                       if getattr( 
        
                           self, f"_initial_{field}" 
        
                       ) == models.DEFERRED and not self._is_deferred(field): 
        
                           present_values[field] = getattr(self, field) 
        
                   # Skip fetching values from database if all of the checked fields are 
        
                   # still deferred, or were not deferred from the begining. 
        
                   if not present_values: 
        
                       return 
        
                   self.refresh_from_db(fields=present_values.keys()) 
        
                   for field in self._changed_checked_fields: 
        
                       setattr(self, f"_initial_{field}", field) 
        
                       setattr(self, field, present_values[field]) 
        
               def _check_name_changed(self): 
        
                   if self._initial_name == models.DEFERRED: 
        
                       return 
        
                   if self._initial_name != self.name: 
        
                       device_name_changed.send( 
        
                           sender=self.__class__, 
        
                           instance=self, 
        
                       ) 
        
                       if self._has_config(): 
        
                           self.config.set_status_modified() 
        
               def _check_group_id_changed(self): 
        
                   if self._initial_group_id == models.DEFERRED: 
        
                       return 
        
                   if self._initial_group_id != self.group_id: 
        
                       self._send_device_group_changed_signal( 
        
                           self, self.group_id, self._initial_group_id 
        
                       ) 
        
               def _check_management_ip_changed(self): 
        
                   if self._initial_management_ip == models.DEFERRED: 
        
                       return 
        
                   if self.management_ip != self._initial_management_ip: 
        
                       management_ip_changed.send( 
        
                           sender=self.__class__, 
        
                           management_ip=self.management_ip, 
        
                           old_management_ip=self._initial_management_ip, 
        
                           instance=self, 
        
                       ) 
        
                   self._initial_management_ip = self.management_ip 
        
               def _check_organization_id_changed(self): 
        
                   """ 
        
                   Returns "True" if the device's organization has changed. 
        
                   """ 
        
                   return ( 
        
                       self._initial_organization_id != models.DEFERRED 
        
                       and self.organization_id != self._initial_organization_id 
        
                   ) 
        
               @classmethod 
        
               def _send_device_group_changed_signal(cls, instance, group_id, old_group_id): 
        
                   """ 
        
                   Emits ``device_group_changed`` signal. 
        
                   Called also by ``change_device_group`` admin action method. 
        
                   """ 
        
                   device_group_changed.send( 
        
                       sender=cls, 
        
                       instance=instance, 
        
                       group_id=group_id, 
        
                       old_group_id=old_group_id, 
        
                   )

It seems we already check organization_id but we don't check organization.
Please look at the existing code before writing new code.
Look for an existing test for the case in which organization_id is changed, there should be already a test for that, we could extend it to also check for organization.

Modified OrganizationConfigSettings.save() to mark related device configs as modified when the context (configuration variables) changes. This aligns with existing patterns without altering device internals. Fixes #1070

stktyagi · 2025-07-22T19:39:36Z

@nemesifier
i updated OrganizationConfigSettings.save() to mark device configs as modified when the context changes, without touching Device. This keeps the solution consistent with the current design.
i haven't extended a test for this but will do as soon as you provide a review on this fix.
Let me know if any adjustments are needed!

stktyagi · 2025-07-22T19:41:42Z

Please look at the existing code before writing new code. Look for an existing test for the case in which organization_id is changed, there should be already a test for that, we could extend it to also check for organization.

Although I do check the existing implementations for every issue, I'll make sure to be more thorough going forward and not push one shot fixes with new code.

stktyagi added 4 commits July 20, 2025 16:28

[Fix] Update device config status on org variable change #1070

90b46b1

Added signal handler to update device config status when organizationconfig settings change and tests for the same. Fixes #1070

[Fix] Update device config status on org variable change (#1070)

cf6c461

Added signal handler to update device config status when organization config settings change and tests for the same. Fixes #1070

[Fix] Added lazy-loading models in signal_handlers to fix error #1070

e5df4f7

Replaced direct model imports with swapper to avoid premature access to models during Django app initialization. Fixes #1070

[Fix] Used lazy-loading models in signal_handlers to fix error #1070

b96f09d

Replaced direct model imports with swapper to avoid premature access to models during Django app initialization during tests. Fixes #1070

nemesifier changed the title ~~Issues/1070 device config status~~ [fix] Detect device organization change Jul 21, 2025

nemesifier assigned nemesifier and stktyagi and unassigned nemesifier Jul 21, 2025

nemesifier added the bug label Jul 21, 2025

nemesifier added this to OpenWISP Priorities for next releases Jul 21, 2025

github-project-automation bot moved this to In progress in OpenWISP Priorities for next releases Jul 21, 2025

nemesifier requested changes Jul 21, 2025

View reviewed changes

[Fix] Mark device config modified on org context change #1070

fe605ce

Modified OrganizationConfigSettings.save() to mark related device configs as modified when the context (configuration variables) changes. This aligns with existing patterns without altering device internals. Fixes #1070

stktyagi requested a review from nemesifier July 23, 2025 04:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[fix] Detect device organization change #1091

[fix] Detect device organization change #1091

Uh oh!

stktyagi commented Jul 20, 2025

Uh oh!

stktyagi commented Jul 20, 2025

Uh oh!

coveralls commented Jul 20, 2025 •

edited

Loading

Uh oh!

nemesifier left a comment

Uh oh!

stktyagi commented Jul 22, 2025

Uh oh!

stktyagi commented Jul 22, 2025 •

edited by nemesifier

Loading

Uh oh!

Uh oh!

	def _set_initial_values_for_changed_checked_fields(self):
	for field in self._changed_checked_fields:
	if self._is_deferred(field):
	setattr(self, f"_initial_{field}", models.DEFERRED)
	else:
	setattr(self, f"_initial_{field}", getattr(self, field))

	def _get_initial_values_for_checked_fields(self):
	# Refresh values from database only when the checked field
	# was initially deferred, but is no longer deferred now.
	# Store the present value of such fields because they will
	# be overwritten fetching values from database
	# NOTE: Initial value of a field will only remain deferred
	# if the current value of the field is still deferred. This
	present_values = dict()
	for field in self._changed_checked_fields:
	if getattr(
	self, f"_initial_{field}"
	) == models.DEFERRED and not self._is_deferred(field):
	present_values[field] = getattr(self, field)
	# Skip fetching values from database if all of the checked fields are
	# still deferred, or were not deferred from the begining.
	if not present_values:
	return
	self.refresh_from_db(fields=present_values.keys())
	for field in self._changed_checked_fields:
	setattr(self, f"_initial_{field}", field)
	setattr(self, field, present_values[field])

	def _check_name_changed(self):
	if self._initial_name == models.DEFERRED:
	return

	if self._initial_name != self.name:
	device_name_changed.send(
	sender=self.__class__,
	instance=self,
	)

	if self._has_config():
	self.config.set_status_modified()

	def _check_group_id_changed(self):
	if self._initial_group_id == models.DEFERRED:
	return

	if self._initial_group_id != self.group_id:
	self._send_device_group_changed_signal(
	self, self.group_id, self._initial_group_id
	)

	def _check_management_ip_changed(self):
	if self._initial_management_ip == models.DEFERRED:
	return
	if self.management_ip != self._initial_management_ip:
	management_ip_changed.send(
	sender=self.__class__,
	management_ip=self.management_ip,
	old_management_ip=self._initial_management_ip,
	instance=self,
	)

	self._initial_management_ip = self.management_ip

	def _check_organization_id_changed(self):
	"""
	Returns "True" if the device's organization has changed.
	"""
	return (
	self._initial_organization_id != models.DEFERRED
	and self.organization_id != self._initial_organization_id
	)

	@classmethod
	def _send_device_group_changed_signal(cls, instance, group_id, old_group_id):
	"""
	Emits ``device_group_changed`` signal.
	Called also by ``change_device_group`` admin action method.
	"""
	device_group_changed.send(
	sender=cls,
	instance=instance,
	group_id=group_id,
	old_group_id=old_group_id,
	)

Uh oh!

[fix] Detect device organization change #1091

Are you sure you want to change the base?

[fix] Detect device organization change #1091

Uh oh!

Conversation

stktyagi commented Jul 20, 2025

Checklist

Reference to Existing Issue

Description of Changes

Uh oh!

stktyagi commented Jul 20, 2025

Uh oh!

coveralls commented Jul 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nemesifier left a comment

Choose a reason for hiding this comment

Uh oh!

stktyagi commented Jul 22, 2025

Uh oh!

stktyagi commented Jul 22, 2025 • edited by nemesifier Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

coveralls commented Jul 20, 2025 •

edited

Loading

stktyagi commented Jul 22, 2025 •

edited by nemesifier

Loading