[feature] Estimated Location: Admin and API filters #1028

DragnEmperor · DragnEmperor · commit f934ae3b909d · 2025-09-09T21:42:40.000+05:30
Closes #1028 Signed-off-by: DragnEmperor <dragnemperor@gmail.com>
diff --git a/docs/user/estimated-location.rst b/docs/user/estimated-location.rst
@@ -70,3 +70,11 @@ includes indicators for the estimated status.
 Changes to the ``coordinates`` and ``geometry`` of the estimated location
 will set the ``is_estimated`` field to ``False`` and remove the
 "(Estimated Location)" suffix with IP from the location name.
+
+In REST API, the field will be visible in the :ref:`Device Location
+<device_location_estimated>`, :ref:`Location list
+<location_list_estimated>`, :ref:`Location Detail
+<location_detail_estimated>` and :ref:`Location list (GeoJson)
+<location_geojson_estimated>` if the feature is **enabled**. The field can
+also be used for filtering in the location list (including geojson)
+endpoints and in the :ref:`Device List <device_list_estimated_filters>`.
diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst
@@ -76,6 +76,14 @@ If :doc:`WHOIS Lookup feature <whois>` is enabled, each device in the list
 response will also include a ``whois_info`` field with related brief WHOIS
 information.
 
+.. _device_list_estimated_filters:
+
+**Estimated Location Filters**
+
+if :doc:`Estimated Location feature <estimated-location>` is enabled,
+devices can be filtered based on the estimated nature of their location
+using the ``geo_is_estimated``.
+
 **Available filters**
 
 You can filter a list of devices based on their configuration status using
@@ -544,13 +552,26 @@ of certificate's organization as show in the example below:
 
     GET /api/v1/controller/cert/{common_name}/group/?org={org1_slug},{org2_slug}
 
+.. |est_loc| replace:: Estimated Location feature
+
+.. _est_loc: estimated-location.html
+
+.. |estimated_details| replace:: If |est_loc|_ is enabled, the location
+    response will also include ``is_estimated`` status field.
+
 Get Device Location
 ~~~~~~~~~~~~~~~~~~~
 
 .. code-block:: text
 
     GET /api/v1/controller/device/{id}/location/
 
+.. _device_location_estimated:
+
+**Estimated Status**
+
+|estimated_details|
+
 .. _create_device_location:
 
 Create Device Location
@@ -787,6 +808,14 @@ List Locations
 
     GET /api/v1/controller/location/
 
+.. _location_list_estimated:
+
+**Estimated Status**
+
+|estimated_details|
+
+Locations can also be filtered using the ``is_estimated``.
+
 **Available filters**
 
 You can filter using ``organization_id`` or ``organization_slug`` to get
@@ -868,6 +897,12 @@ Get Location Details
 
     GET /api/v1/controller/location/{pk}/
 
+.. _location_detail_estimated:
+
+**Estimated Status**
+
+|estimated_details|
+
 Change Location Details
 ~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -910,6 +945,14 @@ List Locations with Devices Deployed (in GeoJSON Format)
 
     GET /api/v1/controller/location/geojson/
 
+.. _location_geojson_estimated:
+
+**Estimated Status**
+
+|estimated_details|
+
+Locations can also be filtered using the ``is_estimated``.
+
 **Available filters**
 
 You can filter using ``organization_id`` or ``organization_slug`` to get
diff --git a/docs/user/whois.rst b/docs/user/whois.rst
@@ -28,6 +28,7 @@ associated with the device's public IP address and includes:
 - CIDR block assigned to the ASN
 - Physical address registered to the ASN
 - Timezone of the ASN's registered location
+- Coordinates (Latitude and Longitude)
 
 Trigger Conditions
 ------------------
diff --git a/openwisp_controller/config/whois/service.py b/openwisp_controller/config/whois/service.py
@@ -46,10 +46,15 @@ def _get_whois_info_from_db(ip_address):
     @staticmethod
     def get_org_config_settings(org_id):
         """
-        Caches the OrganizationConfigSettings as these settings
-        are not expected to change frequently. The timeout for the cache
-        is set to the same as the checksum cache timeout for consistency
-        with DeviceChecksumView.
+        Retrieve and cache organization-specific configuration settings.
+
+        Returns a "read-only" OrganizationConfigSettings instance for the
+        given organization.
+        If no settings exist for the organization, returns an empty instance to allow
+        fallback to global defaults.
+
+        OrganizationConfigSettings are cached for performance, using the same timeout
+        as DeviceChecksumView for consistency.
         """
         OrganizationConfigSettings = load_model("config", "OrganizationConfigSettings")
         Config = load_model("config", "Config")
@@ -63,7 +68,7 @@ def get_org_config_settings(org_id):
                 )
             except OrganizationConfigSettings.DoesNotExist:
                 # If organization settings do not exist, fall back to global setting
-                return None
+                org_settings = OrganizationConfigSettings()
             cache.set(
                 cache_key,
                 org_settings,
@@ -78,23 +83,23 @@ def check_estimate_location_configured(org_id):
         if not app_settings.WHOIS_CONFIGURED:
             return False
         org_settings = WHOISService.get_org_config_settings(org_id=org_id)
-        return getattr(org_settings, "estimated_location_enabled")
+        return org_settings.estimated_location_enabled
 
     @property
     def is_whois_enabled(self):
         """
         Check if the WHOIS lookup feature is enabled.
         """
         org_settings = self.get_org_config_settings(org_id=self.device.organization.pk)
-        return getattr(org_settings, "whois_enabled", app_settings.WHOIS_ENABLED)
+        return org_settings.whois_enabled
 
     @property
     def is_estimated_location_enabled(self):
         """
         Check if the Estimated location feature is enabled.
         """
         org_settings = self.get_org_config_settings(org_id=self.device.organization.pk)
-        return getattr(org_settings, "estimated_location_enabled")
+        return org_settings.estimated_location_enabled
 
     def _need_whois_lookup(self, new_ip):
         """
diff --git a/openwisp_controller/config/whois/tests/__init__.py b/openwisp_controller/config/whois/tests/__init__.py
diff --git a/openwisp_controller/config/whois/tests/tests.py b/openwisp_controller/config/whois/tests/tests.py
@@ -15,10 +15,10 @@
 
 from openwisp_utils.tests import SeleniumTestMixin
 
-from ...tests.utils import TestAdminMixin
-from .. import settings as app_settings
-from .handlers import connect_whois_handlers
-from .tests_utils import CreateWHOISMixin, WHOISTransactionMixin
+from ....tests.utils import TestAdminMixin
+from ... import settings as app_settings
+from ..handlers import connect_whois_handlers
+from .utils import CreateWHOISMixin, WHOISTransactionMixin
 
 Device = load_model("config", "Device")
 WHOISInfo = load_model("config", "WHOISInfo")
@@ -631,6 +631,16 @@ def assert_logging_on_exception(
 class TestWHOISSelenium(CreateWHOISMixin, SeleniumTestMixin, StaticLiveServerTestCase):
     @mock.patch.object(app_settings, "WHOIS_CONFIGURED", True)
     def test_whois_device_admin(self):
+        def _assert_no_js_errors():
+            browser_logs = []
+            for log in self.get_browser_logs():
+                if self.browser == "chrome" and log["source"] != "console-api":
+                    continue
+                elif log["message"] in ["wrong event specified: touchleave"]:
+                    continue
+                browser_logs.append(log)
+            self.assertEqual(browser_logs, [])
+
         whois_obj = self._create_whois_info()
         device = self._create_device(last_ip=whois_obj.ip_address)
         self.login()
@@ -655,6 +665,7 @@ def test_whois_device_admin(self):
             self.assertIn(whois_obj.timezone, additional_text[1].text)
             self.assertIn(whois_obj.formatted_address, additional_text[2].text)
             self.assertIn(whois_obj.cidr, additional_text[3].text)
+            _assert_no_js_errors()
 
         with mock.patch.object(app_settings, "WHOIS_CONFIGURED", False):
             with self.subTest(
@@ -664,6 +675,7 @@ def test_whois_device_admin(self):
                 self.open(reverse("admin:config_device_change", args=[device.pk]))
                 self.wait_for_invisibility(By.CSS_SELECTOR, "table.whois-table")
                 self.wait_for_invisibility(By.CSS_SELECTOR, "details.whois")
+                _assert_no_js_errors()
 
         with self.subTest(
             "WHOIS details not visible in device admin when WHOIS is disabled"
@@ -674,6 +686,7 @@ def test_whois_device_admin(self):
             self.open(reverse("admin:config_device_change", args=[device.pk]))
             self.wait_for_invisibility(By.CSS_SELECTOR, "table.whois-table")
             self.wait_for_invisibility(By.CSS_SELECTOR, "details.whois")
+            _assert_no_js_errors()
 
         with self.subTest(
             "WHOIS details not visible in device admin when WHOIS Info does not exist"
@@ -685,3 +698,4 @@ def test_whois_device_admin(self):
             self.open(reverse("admin:config_device_change", args=[device.pk]))
             self.wait_for_invisibility(By.CSS_SELECTOR, "table.whois-table")
             self.wait_for_invisibility(By.CSS_SELECTOR, "details.whois")
+            _assert_no_js_errors()
diff --git a/openwisp_controller/config/whois/tests/utils.py b/openwisp_controller/config/whois/tests/utils.py
@@ -4,7 +4,7 @@
 from django.urls import reverse
 from swapper import load_model
 
-from ..tests.utils import CreateConfigMixin
+from ...tests.utils import CreateConfigMixin
 
 Device = load_model("config", "Device")
 WHOISInfo = load_model("config", "WHOISInfo")
diff --git a/openwisp_controller/geo/admin.py b/openwisp_controller/geo/admin.py
@@ -12,6 +12,7 @@
 )
 from swapper import load_model
 
+from openwisp_controller.config import settings as config_app_settings
 from openwisp_controller.config.whois.service import WHOISService
 from openwisp_users.multitenancy import MultitenantOrgFilter
 
@@ -146,16 +147,39 @@ class DeviceLocationFilter(admin.SimpleListFilter):
     title = _("has geographic position set?")
     parameter_name = "with_geo"
 
+    def __init__(self, request, params, model, model_admin):
+        super().__init__(request, params, model, model_admin)
+        if config_app_settings.WHOIS_CONFIGURED:
+            self.title = _("geographic position")
+
     def lookups(self, request, model_admin):
+        if config_app_settings.WHOIS_CONFIGURED:
+            return (
+                ("outdoor", _("Outdoor")),
+                ("indoor", _("Indoor")),
+                ("estimated", _("Estimated")),
+                ("false", _("No Location")),
+            )
         return (
             ("true", _("Yes")),
             ("false", _("No")),
         )
 
     def queryset(self, request, queryset):
-        if self.value():
-            return queryset.filter(devicelocation__isnull=self.value() == "false")
-        return queryset
+        value = self.value()
+        if not value:
+            return queryset
+        if config_app_settings.WHOIS_CONFIGURED:
+            if value == "estimated":
+                return queryset.filter(devicelocation__location__is_estimated=True)
+            elif value in ("indoor", "outdoor"):
+                # estimated locations are outdoor by default
+                # so we need to exclude them from the result
+                return queryset.filter(
+                    devicelocation__location__type=value,
+                    devicelocation__location__is_estimated=False,
+                )
+        return queryset.filter(devicelocation__isnull=self.value() == "false")
 
 
 # Prepend DeviceLocationInline to config.DeviceAdminExportable
diff --git a/openwisp_controller/geo/api/filters.py b/openwisp_controller/geo/api/filters.py
@@ -1,6 +1,7 @@
 from django.utils.translation import gettext_lazy as _
 from django_filters import rest_framework as filters
 
+from openwisp_controller.config import settings as config_app_settings
 from openwisp_controller.config.api.filters import (
     DeviceListFilter as BaseDeviceListFilter,
 )
@@ -21,6 +22,20 @@ def filter_devicelocation(self, queryset, name, value):
         # Returns list of device that have devicelocation objects
         return queryset.exclude(devicelocation__isnull=value)
 
+    def filter_is_estimated(self, queryset, name, value):
+        return queryset.filter(devicelocation__location__is_estimated=value)
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if config_app_settings.WHOIS_CONFIGURED:
+            self.filters["geo_is_estimated"] = filters.BooleanFilter(
+                field_name="devicelocation__location__is_estimated",
+                method=self.filter_is_estimated,
+            )
+            self.filters["geo_is_estimated"].label = _(
+                "Is geographic location estimated?"
+            )
+
     class Meta:
         model = BaseDeviceListFilter.Meta.model
         fields = BaseDeviceListFilter.Meta.fields[:]
diff --git a/openwisp_controller/geo/api/serializers.py b/openwisp_controller/geo/api/serializers.py
@@ -11,6 +11,10 @@
 from openwisp_utils.api.serializers import ValidatedModelSerializer
 
 from ...serializers import BaseSerializer
+from ..estimated_location.mixins import (
+    EstimatedLocationGeoJsonSerializer,
+    EstimatedLocationMixin,
+)
 
 Device = load_model("config", "Device")
 Location = load_model("geo", "Location")
@@ -31,7 +35,9 @@ class Meta:
         fields = "__all__"
 
 
-class GeoJsonLocationSerializer(gis_serializers.GeoFeatureModelSerializer):
+class GeoJsonLocationSerializer(
+    EstimatedLocationGeoJsonSerializer, gis_serializers.GeoFeatureModelSerializer
+):
     device_count = IntegerField()
 
     class Meta:
@@ -126,7 +132,7 @@ class Meta:
         read_only_fields = ("name",)
 
 
-class LocationSerializer(BaseSerializer):
+class LocationSerializer(EstimatedLocationMixin, BaseSerializer):
     floorplan = FloorPlanLocationSerializer(required=False, allow_null=True)
 
     class Meta:
@@ -225,7 +231,9 @@ def update(self, instance, validated_data):
         return super().update(instance, validated_data)
 
 
-class NestedtLocationSerializer(gis_serializers.GeoFeatureModelSerializer):
+class NestedtLocationSerializer(
+    EstimatedLocationGeoJsonSerializer, gis_serializers.GeoFeatureModelSerializer
+):
     class Meta:
         model = Location
         geo_field = "geometry"
diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py
@@ -10,6 +10,7 @@
 from rest_framework_gis.pagination import GeoJsonPagination
 from swapper import load_model
 
+from openwisp_controller.config import settings as config_app_settings
 from openwisp_controller.config.api.views import DeviceListCreateView
 from openwisp_users.api.filters import OrganizationManagedFilter
 from openwisp_users.api.mixins import FilterByOrganizationManaged, FilterByParentManaged
@@ -45,6 +46,13 @@ class Meta(OrganizationManagedFilter.Meta):
         model = Location
         fields = OrganizationManagedFilter.Meta.fields + ["is_mobile", "type"]
 
+    def __init__(self, data=None, queryset=None, *, request=None, prefix=None):
+        super().__init__(data, queryset, request=request, prefix=prefix)
+        if config_app_settings.WHOIS_CONFIGURED:
+            self.filters["is_estimated"] = filters.BooleanFilter(
+                field_name="is_estimated"
+            )
+
 
 class FloorPlanOrganizationFilter(OrganizationManagedFilter):
     class Meta(OrganizationManagedFilter.Meta):
diff --git a/openwisp_controller/geo/estimated_location/mixins.py b/openwisp_controller/geo/estimated_location/mixins.py
diff --git a/openwisp_controller/geo/estimated_location/tests/__init__.py b/openwisp_controller/geo/estimated_location/tests/__init__.py
diff --git a/openwisp_controller/geo/estimated_location/tests/tests.py b/openwisp_controller/geo/estimated_location/tests/tests.py
diff --git a/openwisp_controller/geo/estimated_location/tests/utils.py b/openwisp_controller/geo/estimated_location/tests/utils.py
