[fix] Fixes by @coderabbitai

Author: pandafy

openwisp_controller/geo/api/views.py

@@ -5,6 +5,7 @@
 from django_filters import rest_framework as filters
 from rest_framework import generics, pagination, status
 from rest_framework.exceptions import NotFound, PermissionDenied
+from rest_framework.generics import get_object_or_404
 from rest_framework.permissions import BasePermission
 from rest_framework.request import clone_request
 from rest_framework.response import Response
@@ -352,10 +353,9 @@ class OrganizationGeoSettingsView(ProtectedAPIMixin, generics.RetrieveUpdateAPIV
 
     def get_object(self):
         org_id = self.kwargs.get("organization_pk")
-        try:
-            return self.get_queryset().get(organization_id=org_id)
-        except OrganizationGeoSettings.DoesNotExist:
-            raise Http404(_("Geo settings not found for this organization."))
+        obj = get_object_or_404(self.get_queryset(), organization_id=org_id)
+        self.check_object_permissions(self.request, obj)
+        return obj
 
 
 # add with_geo filter to device API

openwisp_controller/geo/estimated_location/service.py

@@ -89,6 +89,13 @@ def _create_or_update_estimated_location(
             device_location = DeviceLocation(content_object=self.device)
 
         current_location = device_location.location
+        # Re-check whether estimated locations are enabled for the device's
+        # organization. The check is needed here so the celery worker
+        # honors current org settings and avoids persisting estimated
+        # locations when the feature has been disabled since the task was
+        # enqueued.
+        if not self.check_estimated_location_enabled(self.device.organization_id):
+            return current_location
         if not current_location or (
             attached_devices_exists and current_location.is_estimated
         ):

openwisp_controller/geo/migrations/0006_create_geo_settings_for_existing_orgs.py

@@ -11,11 +11,15 @@ def create_geo_settings_for_existing_orgs(apps, schema_editor):
     Create OrganizationGeoSettings for all existing organizations
     that don't have one yet.
     """
-    Organization = apps.get_model("openwisp_users", "Organization")
+    Organization = get_swapped_model(apps, "openwisp_users", "Organization")
     OrganizationGeoSettings = get_swapped_model(apps, "geo", "OrganizationGeoSettings")
 
-    for org in Organization.objects.iterator():
-        OrganizationGeoSettings.objects.get_or_create(organization_id=org.pk)
+    # Use the migration's DB alias so non-default databases are respected
+    db_alias = schema_editor.connection.alias
+    for org in Organization.objects.using(db_alias).iterator():
+        OrganizationGeoSettings.objects.using(db_alias).get_or_create(
+            organization_id=org.pk
+        )
 
 
 def copy_estimated_location_enabled(apps, schema_editor):
@@ -29,17 +33,20 @@ def copy_estimated_location_enabled(apps, schema_editor):
     )
     OrganizationGeoSettings = get_swapped_model(apps, "geo", "OrganizationGeoSettings")
 
+    # Use the migration's DB alias so non-default databases are respected
+    db_alias = schema_editor.connection.alias
     # Iterate using iterator() to avoid loading all rows into memory.
-    for cfg in OrganizationConfigSettings.objects.iterator():
-        geo, _ = OrganizationGeoSettings.objects.get_or_create(
+    for cfg in OrganizationConfigSettings.objects.using(db_alias).iterator():
+        geo, _ = OrganizationGeoSettings.objects.using(db_alias).get_or_create(
             organization_id=cfg.organization_id
         )
         # Copy the value if different
         if getattr(geo, "estimated_location_enabled", None) != getattr(
             cfg, "estimated_location_enabled", None
         ):
             geo.estimated_location_enabled = cfg.estimated_location_enabled
-            geo.save(update_fields=["estimated_location_enabled"])
+            # Ensure save uses the same DB alias
+            geo.save(update_fields=["estimated_location_enabled"], using=db_alias)
 
 
 class Migration(migrations.Migration):

openwisp_controller/geo/tests/test_api.py

@@ -1529,6 +1529,8 @@ def test_organization_geo_settings_update(self):
                 )
                 self.assertEqual(response.status_code, 400)
                 self.assertIn("estimated_location_enabled", response.data)
+                org1_geo_settings.refresh_from_db()
+                self.assertEqual(org1_geo_settings.estimated_location_enabled, False)
 
         with self.subTest("Superuser can update any organization's geo settings"):
             superuser = self._get_admin()