[chores:bug] Separate geo.estimated_location from config.whois

[nemesifier]

⚠️ Not suited to beginner contributors!

This is a follow up of:

• [feature] GSoC25: WHOIS Information and IP Address-Based Geolocation #1175
• [GSoC25] WHOIS Information and IP Address-Based Geolocation

The new features are working fine but introduce one fundamental issue: they violate the principle that openwisp_controller.config should only be concerned with configuration management, while openwisp_controller.geo should provide geographic capabilities as an extension.

This can introduce maintenance overhead and bugs over time as more contributors may be tempted to do the same violations and we'll end up with a total mess.

Currently, config has knowledge of geo-specific logic, and OrganizationConfigSettings mixes configuration settings with geographic settings, this needs to be fixed.

Violations Found

The exact lines may changes after the work is merged.

1. config references geo (Should be the other way around)

File: openwisp_controller/config/whois/service.py

• Lines 227-232: trigger_estimated_location_task() method in WHOISService
• Lines 304-347: _create_or_update_estimated_location() method in WHOISService
• Lines 311-312: Imports Location and DeviceLocation from geo using load_model("geo", "Location")
• Line 88: References estimated_location_enabled from org config settings
• Lines 115-141: check_estimated_location_configured() and is_estimated_location_enabled methods

File: openwisp_controller/config/whois/tasks.py

• Line 88: References estimated_location_enabled
• Line 101: Triggers estimated location task

File: openwisp_controller/config/base/whois.py

• Lines 154-163: _get_defaults_for_estimated_location() method in WHOISInfo model
• Anything needed for the estimated location feature shall be moved, we can use proxy models if needed or even simpler alternatives (another service class for estimated location)

File: openwisp_controller/config/base/multitenancy.py

• Lines 42-46: estimated_location_enabled field in AbstractOrganizationConfigSettings
• Lines 79-87: Validation logic linking estimated_location_enabled to whois_enabled

File: openwisp_controller/config/admin.py

• Line 1396: estimated_location_enabled field included in config settings inline

2. geo references config (Acceptable - dependency check only)

The following references from geo to config are acceptable because estimated location depends on WHOIS:

File: openwisp_controller/geo/base/models.py

• Lines 13, 54, 82: Imports and uses WHOISService.check_estimated_location_configured()
• This is a dependency check - estimated location requires WHOIS to be enabled

File: openwisp_controller/geo/estimated_location/handlers.py

• Lines 5, 15: Imports and checks config_app_settings.WHOIS_CONFIGURED
• This is necessary to ensure WHOIS is configured before registering notification types

This is the correct direction of dependency: geo depends on config, not vice versa.

Proposed Solution

1. Create a new model: OrganizationGeoSettings

Create a new model OrganizationGeoSettings that contains:

• estimated_location_enabled field
• Foreign key to organization (similar to OrganizationConfigSettings)
• Any other geo-specific settings

This removes geographic concerns from OrganizationConfigSettings.

2. Add OrganizationGeoSettings to admin

• Create a new inline GeoSettingsInline for OrganizationGeoSettings in organization admin
• Move estimated_location_enabled from ConfigSettingsInline to GeoSettingsInline and update related code accordingly
• Ensure the inline is displayed in the organization admin

3. Create REST API endpoint for geo settings

Add a new REST API endpoint (e.g., /api/v1/controller/organizations/{org_id}/geo-settings/) to manage geo-specific settings independently from config settings.

4. Move estimated location logic from config to geo

Move the following from config/whois/service.py to geo/estimated_location/:

• WHOISService.trigger_estimated_location_task()
• WHOISService._create_or_update_estimated_location()
• WHOISService.check_estimated_location_configured()
• WHOISService.is_estimated_location_enabled

5. Update signal handlers

Ensure geo registers its signal handlers that react to device/IP changes, rather than config triggering geo tasks directly.

6. Update migrations

Create new migrations to:

• Create the new OrganizationGeoSettings model
• Move estimated_location_enabled data from OrganizationConfigSettings to the new model
• Optionally remove the field from OrganizationConfigSettings

7. Write tests

Write basic tests for:

• Model tests: Test OrganizationGeoSettings model creation, validation, and fields
• Admin tests: Test geo settings inline in organization admin
• REST API tests: Test the new geo settings API endpoint

Migration Strategy

1. Phase 1: Preparation

   • Create the new OrganizationGeoSettings model
   • Add the new field to the new model
   • Keep the old field for backward compatibility

2. Phase 2: Data Migration

   • Copy estimated_location_enabled values to the new model
   • Update code to read from new model first, fall back to old

3. Phase 3: Cleanup

   • Remove the field from OrganizationConfigSettings
   • Update all references

Impact Assessment

Benefits

• Clear separation of concerns
• config remains focused on configuration management
• geo can be installed/uninstalled independently
• Easier to maintain and extend both modules
• Follows open/closed principle

Risks

• Breaking changes for existing installations (AFAIK we're using this feature only in dev/staging/demo so this is fine)
• Requires careful migration strategy
• May affect custom code that depends on current structure (AFAIK we're using this feature only in dev/staging/demo so this is fine)
• Test suite updates required

Files to Modify

Models

• openwisp_controller/geo/models.py - Add new OrganizationGeoSettings model
• openwisp_controller/config/base/multitenancy.py - Remove estimated_location_enabled

Admin

• openwisp_controller/geo/admin.py - Add OrganizationGeoSettings inline
• openwisp_controller/config/admin.py - Remove estimated_location_enabled from config inline

API

• openwisp_controller/geo/api/urls.py - Add geo settings API endpoint
• openwisp_controller/geo/api/views.py - Add geo settings view
• openwisp_controller/geo/api/serializers.py - Add geo settings serializer

Logic

• openwisp_controller/config/whois/service.py - Remove estimated location methods
• openwisp_controller/config/whois/tasks.py - Remove estimated location triggers
• openwisp_controller/config/base/whois.py - Remove _get_defaults_for_estimated_location
• openwisp_controller/geo/estimated_location/tasks.py - Add estimated location logic

Tests (new)

• openwisp_controller/geo/tests/test_models.py - Add tests for OrganizationGeoSettings
• openwisp_controller/geo/tests/test_admin.py - Add tests for geo settings inline
• openwisp_controller/geo/tests/test_api.py - Add tests for geo settings REST API

[Piyushkhobragade]

I'd like to take this on. Plan: move the estimated-location settings and logic from config to geo to restore the intended geo → config dependency and keep config focused on configuration management.

[nemesifier]

@Piyushkhobragade I recommend picking another issue.

[Piyushkhobragade]

@Piyushkhobragade I recommend picking another issue.

Understood, thanks. I'll look for another issue to work on.