Skip to content

Commit b8c4d3c

Browse files
pandafynemesifier
andauthored
[chores:fix] Separated geo.estimated_location from config.whois #1224
The new WHOIS and Estimated Location features were interwined and referenced each other, leading to maintainability issues. This patch resolved this problem and introduced more separation of responsibilities and decoupling. Closes #1224 --------- Co-authored-by: Federico Capoano <f.capoano@openwisp.io>
1 parent 0d17acd commit b8c4d3c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+1313
-390
lines changed

docs/developer/extending.rst

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ Once you have created the models, add the following to your
350350
GEO_LOCATION_MODEL = "sample_geo.Location"
351351
GEO_FLOORPLAN_MODEL = "sample_geo.FloorPlan"
352352
GEO_DEVICELOCATION_MODEL = "sample_geo.DeviceLocation"
353+
GEO_ORGANIZATIONGEOSETTINGS_MODEL = "sample_geo.OrganizationGeoSettings"
353354
CONNECTION_CREDENTIALS_MODEL = "sample_connection.Credentials"
354355
CONNECTION_DEVICECONNECTION_MODEL = "sample_connection.DeviceConnection"
355356
CONNECTION_COMMAND_MODEL = "sample_connection.Command"
@@ -456,7 +457,11 @@ For example:
456457

457458
.. code-block:: python
458459
459-
from openwisp_controller.geo.admin import FloorPlanAdmin, LocationAdmin
460+
from openwisp_controller.geo.admin import (
461+
FloorPlanAdmin,
462+
LocationAdmin,
463+
GeoSettingsInline,
464+
)
460465
461466
FloorPlanAdmin.fields += ["example"] # <-- monkey patching example
462467

docs/developer/utils.rst

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,3 +379,31 @@ The signal is emitted when the peers of VPN server gets changed.
379379

380380
It is only emitted for ``Vpn`` object with **WireGuard** or **VXLAN over
381381
WireGuard** backend.
382+
383+
``whois_fetched``
384+
~~~~~~~~~~~~~~~~~
385+
386+
**Path**: ``openwisp_controller.config.signals.whois_fetched``
387+
388+
**Arguments**:
389+
390+
- ``whois``: instance of ``WHOISInfo`` that was created or updated
391+
- ``updated_fields``: list of fields updated in the lookup
392+
- ``device``: optional instance of ``Device`` related to this WHOIS lookup
393+
394+
This signal is emitted when a WHOIS lookup task successfully creates or
395+
updates a ``WHOISInfo`` record.
396+
397+
``whois_lookup_skipped``
398+
~~~~~~~~~~~~~~~~~~~~~~~~
399+
400+
**Path**: ``openwisp_controller.config.signals.whois_lookup_skipped``
401+
402+
**Arguments**:
403+
404+
- ``device``: instance of ``Device`` for which the WHOIS lookup was
405+
skipped
406+
407+
This signal is emitted when a WHOIS lookup is not triggered because the
408+
lookup conditions were not met (for example, an up-to-date WHOIS record
409+
already exists).

docs/user/rest-api.rst

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,53 @@ device is updating it's position.
801801
},
802802
}'
803803
804+
Get Organization Geographic Settings
805+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
806+
807+
.. code-block:: text
808+
809+
GET /api/v1/controller/organization/{organization_pk}/geo-settings/
810+
811+
This endpoint allows retrieving geographic settings for a specific
812+
organization.
813+
814+
Update Organization Geographic Settings
815+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
816+
817+
This endpoint allows updating geographic settings for a specific
818+
organization.
819+
820+
.. code-block:: text
821+
822+
PUT /api/v1/controller/organization/{organization_pk}/geo-settings/
823+
824+
.. code-block:: text
825+
826+
curl -X PUT \
827+
'http://127.0.0.1:8000/api/v1/controller/organization/8a85cc23-bad5-4c7e-b9f4-ffe298defb5c/geo-settings/' \
828+
-H 'authorization: Bearer <token>' \
829+
-H 'content-type: application/json' \
830+
-d '{"estimated_location_enabled": true}'
831+
832+
Partially Update Organization Geographic Settings
833+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
834+
835+
This endpoint allows partial updates of Organization geographic settings
836+
using the PATCH method. PATCH accepts a subset of fields and applies a
837+
partial update to the resource at the same endpoint path.
838+
839+
.. code-block:: text
840+
841+
PATCH /api/v1/controller/organization/{organization_pk}/geo-settings/
842+
843+
.. code-block:: text
844+
845+
curl -X PATCH \
846+
'http://127.0.0.1:8000/api/v1/controller/organization/8a85cc23-bad5-4c7e-b9f4-ffe298defb5c/geo-settings/' \
847+
-H 'authorization: Bearer <token>' \
848+
-H 'content-type: application/json' \
849+
-d '{"estimated_location_enabled": true}'
850+
804851
List Locations
805852
~~~~~~~~~~~~~~
806853

openwisp_controller/config/admin.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -694,8 +694,7 @@ def change_group(self, request, queryset):
694694
"action_checkbox_name": helpers.ACTION_CHECKBOX_NAME,
695695
"opts": self.model._meta,
696696
"changelist_url": (
697-
f"{request.resolver_match.app_name}:"
698-
f"{request.resolver_match.url_name}"
697+
f"{request.resolver_match.app_name}:{request.resolver_match.url_name}"
699698
),
700699
}
701700

@@ -1104,8 +1103,7 @@ def save_clones(view, user, queryset, organization=None):
11041103
validated_org = Organization.objects.get(pk=organization)
11051104
except (ValidationError, Organization.DoesNotExist) as e:
11061105
logger.warning(
1107-
"Detected tampering in clone template "
1108-
f"form by user {user}: {e}"
1106+
f"Detected tampering in clone template form by user {user}: {e}"
11091107
)
11101108
return
11111109
if not user.is_superuser and not user.is_manager(organization):
@@ -1393,7 +1391,7 @@ def get_fields(self, request, obj=None):
13931391
if app_settings.REGISTRATION_ENABLED:
13941392
fields += ["registration_enabled", "shared_secret"]
13951393
if app_settings.WHOIS_CONFIGURED:
1396-
fields += ["whois_enabled", "estimated_location_enabled"]
1394+
fields += ["whois_enabled"]
13971395
fields += ["context"]
13981396
return fields
13991397

openwisp_controller/config/base/multitenancy.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,6 @@ class AbstractOrganizationConfigSettings(UUIDModel):
3939
fallback=app_settings.WHOIS_ENABLED,
4040
verbose_name=_("WHOIS Enabled"),
4141
)
42-
estimated_location_enabled = FallbackBooleanChoiceField(
43-
help_text=_("Whether the estimated location feature is enabled"),
44-
fallback=app_settings.ESTIMATED_LOCATION_ENABLED,
45-
verbose_name=_("Estimated Location Enabled"),
46-
)
4742
context = JSONField(
4843
blank=True,
4944
default=dict,
@@ -77,15 +72,6 @@ def clean(self):
7772
)
7873
}
7974
)
80-
if not self.whois_enabled and self.estimated_location_enabled:
81-
raise ValidationError(
82-
{
83-
"estimated_location_enabled": _(
84-
"Estimated Location feature requires "
85-
"WHOIS Lookup feature to be enabled."
86-
)
87-
}
88-
)
8975
return super().clean()
9076

9177
def save(

openwisp_controller/config/base/whois.py

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -150,16 +150,3 @@ def _location_name(self):
150150
}
151151
# Use named placeholder for consistency
152152
return _("Estimated Location: %(ip)s") % {"ip": self.ip_address}
153-
154-
def _get_defaults_for_estimated_location(self):
155-
"""
156-
Used to get default values for creating or updating
157-
an estimated location based on the WHOIS information.
158-
"""
159-
return {
160-
"name": self._location_name,
161-
"type": "outdoor",
162-
"is_mobile": False,
163-
"geometry": self.coordinates,
164-
"address": self.formatted_address,
165-
}

openwisp_controller/config/migrations/0062_whoisinfo.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
# Generated by Django 5.2.1 on 2025-06-26 02:13
2+
# Updated on 2026-04-06
23

4+
import django.contrib.gis.db.models.fields
35
import django.utils.timezone
46
import model_utils.fields
7+
from django.contrib.gis.db.models.fields import PointField
58
from django.db import migrations, models
69

710
import openwisp_utils.fields
@@ -59,6 +62,12 @@ class Migration(migrations.Migration):
5962
max_length=100,
6063
),
6164
),
65+
(
66+
"coordinates",
67+
PointField(
68+
blank=True, help_text="Coordinates", null=True, srid=4326
69+
),
70+
),
6271
],
6372
options={
6473
"abstract": False,

openwisp_controller/config/migrations/0063_organizationconfigsettings_estimated_location_enabled_and_more.py

Lines changed: 0 additions & 35 deletions
This file was deleted.

openwisp_controller/config/settings.py

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
import logging
22

3-
from django.conf import settings
43
from django.core.exceptions import ImproperlyConfigured
54
from django.utils.translation import gettext_lazy as _
65

7-
logger = logging.getLogger(__name__)
8-
6+
from ..settings import get_setting
97

10-
def get_setting(option, default):
11-
return getattr(settings, f"OPENWISP_CONTROLLER_{option}", default)
8+
logger = logging.getLogger(__name__)
129

1310

1411
BACKENDS = get_setting(
@@ -85,9 +82,3 @@ def get_setting(option, default):
8582
raise ImproperlyConfigured(
8683
"OPENWISP_CONTROLLER_WHOIS_REFRESH_THRESHOLD_DAYS must be a positive integer"
8784
)
88-
ESTIMATED_LOCATION_ENABLED = get_setting("ESTIMATED_LOCATION_ENABLED", False)
89-
if ESTIMATED_LOCATION_ENABLED and not WHOIS_ENABLED:
90-
raise ImproperlyConfigured(
91-
"OPENWISP_CONTROLLER_WHOIS_ENABLED must be set to True before "
92-
"setting OPENWISP_CONTROLLER_ESTIMATED_LOCATION_ENABLED to True."
93-
)

openwisp_controller/config/signals.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,11 @@
6565
vpn_server_modified.__doc__ = """
6666
providing arguments: ['instance']
6767
"""
68+
whois_fetched = Signal()
69+
whois_fetched.__doc__ = """
70+
Providing arguments: ['whois', 'updated_fields', 'device']
71+
"""
72+
whois_lookup_skipped = Signal()
73+
whois_lookup_skipped.__doc__ = """
74+
Providing arguments: ['device']
75+
"""

0 commit comments

Comments
 (0)