feat: geofence-based geographic targeting for programs#76
Draft
feat: geofence-based geographic targeting for programs#76
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust geofence-based geographic targeting system for programs, allowing for precise and flexible definition of eligibility zones. It significantly enhances the underlying GIS infrastructure by adding support for complex geometric types and improving the reliability and user experience of map widgets. These changes collectively empower programs with more advanced spatial management tools and ensure a more secure and resilient mapping environment. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a major new feature for geofence-based geographic targeting, including a new spp_program_geofence module and extensive tests. It also enhances the spp_gis module by adding support for MultiPolygon and GeometryCollection, and crucially fixes a potential SQL injection vulnerability by using parameterized queries. The map widgets are improved by making the MapTiler API key optional with an OpenStreetMap fallback, and several pre-existing bugs related to WebGL context leaks and control stacking are fixed. The changes are well-structured and of high quality. I have a few suggestions to improve maintainability and robustness.
Comment on lines
249
to
254
| function updateArea(e) { | ||
| console.log(e); | ||
| var data = self.draw.getAll(); | ||
| self.props.record.update({ | ||
| [self.props.name]: JSON.stringify(data.features[0].geometry), | ||
| }); | ||
| } |
There was a problem hiding this comment.
The updateArea function assumes that self.draw.getAll().features will always contain at least one feature. If it's empty for any reason, data.features[0] will be undefined and accessing .geometry will cause a runtime error. It's safer to add a check.
Additionally, the draw.create and draw.update events pass the affected features in the event object e, which is more direct and efficient to use than calling getAll().
function updateArea(e) {
const features = e.features;
if (features && features.length > 0) {
self.props.record.update({
[self.props.name]: JSON.stringify(features[0].geometry),
});
}
}
Comment on lines
+190
to
+193
| if ben_count < 1000: | ||
| self._import_registrants(new_beneficiaries, state=state, do_count=True) | ||
| else: | ||
| self._import_registrants_async(new_beneficiaries, state=state) |
There was a problem hiding this comment.
The threshold 1000 for switching to asynchronous import, and the chunk size 10000 used in _import_registrants_async (line 203), are magic numbers. It would be better to define them as constants at the class or module level, for example ASYNC_IMPORT_THRESHOLD = 1000 and IMPORT_CHUNK_SIZE = 10000. This improves readability and makes these values easier to find and adjust.
Comment on lines
+89
to
+115
| _getMapStyle() { | ||
| if (this.mapTilerKey) { | ||
| return maptilersdk.MapStyle.STREETS; | ||
| } | ||
| // Fallback: OSM raster tiles (no API key required) | ||
| return { | ||
| version: 8, | ||
| sources: { | ||
| osm: { | ||
| type: "raster", | ||
| tiles: ["https://tile.openstreetmap.org/{z}/{x}/{y}.png"], | ||
| tileSize: 256, | ||
| attribution: | ||
| '© <a href="https://www.openstreetmap.org/copyright">OpenStreetMap</a> contributors', | ||
| }, | ||
| }, | ||
| layers: [ | ||
| { | ||
| id: "osm-tiles", | ||
| type: "raster", | ||
| source: "osm", | ||
| minzoom: 0, | ||
| maxzoom: 19, | ||
| }, | ||
| ], | ||
| }; | ||
| } |
There was a problem hiding this comment.
The code to generate the fallback OpenStreetMap map style object is duplicated from spp_gis/static/src/js/views/gis/gis_renderer/gis_renderer.esm.js. To improve maintainability and avoid inconsistencies, this object could be defined as a constant or returned by a utility function in a shared file, and then imported in both field_gis_edit_map.esm.js and gis_renderer.esm.js.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #76 +/- ##
==========================================
+ Coverage 70.39% 70.41% +0.01%
==========================================
Files 674 691 +17
Lines 37005 37266 +261
==========================================
+ Hits 26051 26240 +189
- Misses 10954 11026 +72
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Member
Author
|
Addressed Gemini review feedback in commit
Kept one medium suggestion as follow-up (optional): deduplicate identical OSM fallback style object between |
…operators
The Operator class only supported Point, LineString, and Polygon types
in domain queries. When shapely's unary_union creates a MultiPolygon
from non-overlapping polygons, the operator validation silently rejected
it, returning SQL("FALSE") and matching zero registrants.
Add ST_GeomFromGeoJSON path for complex geometry types that cannot be
easily constructed from coordinates.
New module that adds geofence-based geographic targeting to programs: - Program-level geofence_ids field on Overview tab - Geofence eligibility manager with hybrid two-tier spatial queries (GPS coordinates + administrative area fallback) - Preview button showing matched registrant count - Composable with other eligibility managers via AND logic
…ibility - Use gis_intersects instead of gis_within for Tier 1 spatial query; gis_within generates ST_Within(value, field) which is backwards for point-in-polygon checks, while gis_intersects is symmetric - Use disabled=None instead of disabled=False in domain (Datetime field) - Use fields.Datetime.now() for disabled test data (not Boolean True) - Use group_ids with Command.link() for Odoo 19 compatibility in tests
- Escape single quotes in create_from_geojson to prevent SQL injection - Make preview_count/preview_error regular fields instead of computed; spatial queries now only run when the Preview button is clicked - Use elif instead of two independent if statements for target_type - Simplify _import_registrants loop to list comprehension
…ent UI - Add fallback_area_type_id field to restrict Tier 2 area fallback to a specific administrative level (e.g. District), preventing overly broad matches from large provinces or regions - Add geofence list/form/search views with menu under Area top-level, so users can browse and manage geofences independently - Allow inline geofence creation from the program form - Add 3 tests for area type filter behavior
When no MapTiler API key is configured, the map widget now falls back to OpenStreetMap raster tiles instead of failing silently. This makes the GIS features work out of the box without requiring a third-party API key. Users who want vector tiles can still configure a MapTiler key.
…back - Fix WebGL context leak: destroy previous map before creating new one in renderMap() to prevent accumulating WebGL contexts on onPatched - Fix draw control stacking: remove previous MapboxDraw control before adding a new one in addDrawInteraction() - Fix removeSourceAndLayer: remove all three layer IDs (polygon, point, linestring) instead of the source ID which doesn't match any layer - Remove console.log debug statements from updateArea and onTrash - Remove hardcoded laos_farm.png placeholder popup on polygon click - Fix SQL injection in create_from_geojson: use SQL() with bound parameters instead of manual string escaping - Apply OSM raster tile fallback to gis_renderer (matching edit widget) - Guard GeocodingControl behind API key check in renderer - Fix early-return-in-loop in eligibility manager methods with ensure_one() - Log exceptions in preview instead of silently swallowing them - Use efficient set lookup for beneficiary exclusion - Use Command.set()/Command.clear() instead of tuple syntax in tests
The default system parameter value "YOUR_MAPTILER_API_KEY_HERE" was being returned as a valid key, causing 403 errors from MapTiler instead of falling back to OSM tiles.
renderMap() was overriding the OSM fallback style with a MapTiler style reference when defaultRaster was set, even without an API key. Guard the raster style override behind mapTilerKey check.
…nu order The GeoPolygonField edit widget requires a GIS view (ir.ui.view with type=gis) with data and raster layers to render the map. Without it, opening a geofence form raised "No GIS view defined". Also moved the Geofences menu item to sequence 200 so it appears last in the Area menu.
When renderMap() destroys the old map, the MapboxDraw control's internal map reference becomes null. Later, addDrawInteraction() tried to removeControl(this.draw) from the new map, but the draw control called this.map.off() on its now-null internal reference. Fix: set this.draw = null before map.remove() so addDrawInteraction skips the removeControl call for stale controls.
Move the Geographic Scope card from Overview to Configuration tab, matching the card-based UI pattern. Add geofence_ids field to the program creation wizard so geofences can be set during initial setup.
Existing geometry was added as a static map source/layer, making it non-interactive: shapes couldn't be clicked, selected, or edited. Now geometry is loaded into the MapboxDraw control via draw.add(), enabling click-to-select, vertex editing, and trash deletion. Also handles draw.delete event to clear the field value.
The tag_ids field on spp.gis.geofence was pointing to spp.vocabulary, which is a generic vocabulary model containing all tag categories (Country, Currency, etc.). Replace with a dedicated spp.gis.geofence.tag model so the tags dropdown only shows geofence-specific tags.
- Remove dead methods from field_gis_edit_map (onLoadMap, addSourceAndLayer,
addSource, addLayer, removeSourceAndLayer) no longer called after draw
refactor
- Fix event listener stacking in addDrawInteraction: store handler refs
and remove previous listeners before adding new ones, preventing
duplicate record.update() calls when onUIChange() is called
- Fix disabled registrant filter: use ("disabled", "=", False) for
consistency with DefaultEligibilityManager
jeremi
force-pushed
the
feature/program-geofence
branch
from
46c90b6 to
8e77302
Compare
Add foundational components for OGC API - Processes (Part 1: Core): - spp.gis.process.job model for async job tracking with job_worker - Pydantic schemas for process list, description, execution, status - ProcessRegistry service with dynamic indicator enum from spp.indicator - Cron job for cleanup of stale/expired process jobs - ACL entries for the new model
- processes.py: GET /gis/ogc/processes, GET /gis/ogc/processes/{id},
POST /gis/ogc/processes/{id}/execution with sync/async support
- jobs.py: GET /gis/ogc/jobs, GET /gis/ogc/jobs/{id},
GET /gis/ogc/jobs/{id}/results, DELETE /gis/ogc/jobs/{id}
- Wire routers into FastAPI endpoint registry
- Update OGC conformance to declare Processes classes
- Add processes link to OGC landing page
- Refactor statistics endpoint to delegate to ProcessRegistry
Tests cover: - ProcessRegistry: list, describe, unknown ID, indicator enum, x-openspp-statistics - Pydantic schemas: ProcessSummary, ExecuteRequest, StatusInfo, JobList - spp.gis.process.job model: create, dismiss, stale cleanup, cron - Input validation: single/batch geometry, bare arrays, limits, proximity - HTTP integration: process list/describe, execution, async flow, job scoping, dismiss, conformance, landing page links
- Remove 'icon' field from test category creation (spp.metric.category does not have an icon field; Odoo 19 raises ValueError on unknown fields) - Fix Odoo False-vs-None for empty Text fields: use `message or None` in _build_status_info to prevent Pydantic ValidationError when job.message is False instead of None - Apply fix in both processes.py and jobs.py routers
- Restrict ACL: base.group_user gets read-only, base.group_system gets full CRUD (routers use sudo() for all mutations) - Add UNIQUE constraint on job_id field - Sanitize exception messages in job records to prevent leaking internals - Extract shared helpers (_helpers.py): build_status_info, check_gis_scope, get_base_url used by both processes and jobs routers - Extract process execution logic (process_execution.py): run_spatial_statistics, run_proximity_statistics used by both sync (router) and async (model) paths - Add safe int parsing for ir.config_parameter values with fallback defaults - Use SPATIAL_STATISTICS/PROXIMITY_STATISTICS constants instead of string literals
…OGC Processes Address consumer feedback from the QGIS plugin team: - Always include geometries_failed in batch summary responses (#1) - Always populate computed_at with current UTC timestamp, even for empty results (#2) - Add Retry-After: 5 header to async 201 responses and in-progress job status (#3/#8) - Track batch progress per-geometry via on_progress callback in job execution (#4) - Add x-openspp-batch-limit: 100 extension to spatial-statistics process description (#5)
When random_groups_count > 500 and queue_job is available, the group creation is dispatched as a background job that commits in chunks of 500. This prevents transaction timeouts and OOM for large volumes (25K+ registrants), and provides progress visibility through the job queue. Area assignment and coordinate generation run after all chunks complete.
Legend items now include min_value/max_value so labels display as "Very Low (0 - 50)" instead of just "Very Low". Applied to: - UI legend (area_ext.py + gis_renderer.esm.js) - GIS API GeoJSON per-feature bucket (gis_report.py _to_geojson) - GIS API single-feature endpoint (layers_service.py)
Individual-level CEL expressions (e.g. senior_citizens_60) returned 0 results because the spatial query always compiled them with the registry_groups profile and matched against groups. Now reads the expression's context_type to select the correct profile, and for individual-context expressions, resolves matched individuals to their group IDs via spp_group_membership.
Age distribution: was 60% ages 3-22, 40% ages 60-85 (no working-age adults, 100% of households matched has_children/has_elderly). Now: 15% infant (0-4), 30% child (5-17), 40% adult (18-59), 15% elderly. Performance: area assignment and coordinate generation now use executemany and bulk UPDATE+JOIN instead of per-record ORM writes, cutting time from minutes to seconds at 25K+ registrants.
Add "volume" demo mode with 7K groups, geodata enabled, and non-GIS features (GRM, cases, change requests) disabled for faster generation. Map misluzon CLI profile to volume mode so that `spp start --demo misluzon --generate --wipe -y` produces the full 25K+ registrant dataset automatically.
_wait_for_module_installed was passing the full comma-separated module string (e.g. "spp_mis_demo_v2,spp_demo_phl_luzon") to _get_module_state, which queries for a single module name. This caused an infinite wait loop after successful installation. Now checks the last module in the list, which is installed last by Odoo's dependency resolver.
queue_job dispatch via with_delay() doesn't work in Odoo shell context (no HTTP request, transaction may not commit). Detect UI vs CLI by checking odoo.http.request, and run _run_volume_generation_job inline with chunked commits when in CLI mode. Also add explicit cr.commit() in CLI generator script to ensure all data is persisted.
7000 groups takes ~15 minutes to generate inline. The previous 600s timeout was insufficient, killing the process at ~4500 groups.
Replace per-record create() calls with batch create() in chunks of 100 groups. Key changes: - Cache gender IDs upfront (2 lookups instead of ~28K) - Build val dicts directly instead of calling helper methods - Batch-create groups, then batch-create all their individuals - Batch-create memberships per flush cycle - ~3-5x faster due to Odoo batching SQL INSERTs and triggers The old _create_random_individual method is preserved for tests and non-volume use cases.
Age 0-1 infants could get future birthdates (e.g., 2026-11 when today is 2026-03), which violates the registration_date >= birthdate constraint on res.partner. This was the silent failure causing volume generation to abort.
Add ODOO_DB_MAXCONN env var (default 128) to entrypoint and odoo.conf template. Mount custom postgresql.conf with configurable slow query logging via PG_LOG_MIN_DURATION.
Compute per-dimension percentages (disagg_{dim}_{val}_pct) alongside
raw counts in GeoJSON feature properties. Include percentage_suffix
in disaggregation metadata for client rendering.
Append lower-upper range to each threshold label in generated QML (e.g. "Low (0.0 - 10.0)") so QGIS legends show the value boundaries. Fix falsy-check on min_value/max_value to allow zero thresholds.
…bution Add _populate_area_metadata to compute area_sqkm from geometry and set population from weights CSV with rollup to parent areas. Guarantee a minimum floor of groups per municipality to avoid empty map areas. Switch demo report defaults to per_population normalization and auto_quartile thresholds. Increase random_groups_count to 10000.
Fix three issues in geofence eligibility manager: - Import group from job_worker instead of uninstalled queue_job - Create memberships directly to avoid serialization failures from concurrent One2many writes to the same program record - Use correct context key (queue_job__no_delay) for test no-delay mode Add area fallback fields and geofence eligibility manager creation to the program creation wizard. Add wizard test and move geographic scope to its own tab in the wizard form.
Clients (e.g. QGIS plugin) already compute percentages from raw counts; no need to duplicate that server-side.
…y scheme The security scheme was declared as generic HTTPBearer, which didn't tell API consumers that tokens come from the OAuth2 client credentials flow at /oauth/token. Switched to FastAPI's OAuth2 class with clientCredentials flow so the OpenAPI spec correctly advertises the token endpoint and auth mechanism.
…OGC API The geofence, proximity, and spatial_query routers were fully duplicated by OGC API - Features and OGC API - Processes endpoints. The QGIS plugin has already migrated to the OGC endpoints. Removed: /gis/geofences, /gis/query/proximity, /gis/query/statistics Kept: /gis/export/geopackage and /gis/statistics (unique, no OGC equivalent)
Schema fixes (geojson.py, ogc.py, processes.py): - GeoJSON geometry type constrained to RFC 7946 Literal values - Feature: add id, typed geometry, optional links - FeatureCollection: replace non-standard metadata/styling with OGC timeStamp, numberMatched, numberReturned, links - Add geofence-specific input models with extra="ignore" for QGIS - CollectionInfo: add optional storageCrs - ProcessSummary/ProcessList/ProcessDescription: add links - ExecuteRequest.response: constrain to Literal["raw", "document"] Router wiring (ogc_features.py, processes.py, jobs.py): - POST/PUT geofences: switch from await request.json() to Pydantic Body() with CreateGeofenceInput/ReplaceGeofenceInput - GET items/item: add responses with GeoJSON schema docs - POST execution: add responses with oneOf result schemas - GET job results: add responses with oneOf result schemas Cleanup: - Remove query.py from schemas/__init__ (only used by tests)
Add realistic examples via json_schema_extra to all key models: - GeoJSONGeometry, GeoJSONFeature, GeoJSONFeatureCollection - CreateGeofenceInput, ReplaceGeofenceInput - ExecuteRequest (single, batch, and proximity variants) - StatusInfo, SingleStatisticsResult, BatchStatisticsResult, ProximityResult Examples are based on actual QGIS plugin usage patterns and test fixtures to help API consumers understand request/response shapes.
Add examples to LandingPage, Conformance, CollectionInfo (3 types: report, geofences, data layer), Collections, ProcessSummary, ProcessDescription, and ProcessList. Examples reflect actual service output shapes and QGIS plugin usage patterns.
model_json_schema() produces $defs with local $ref paths that break
when inlined into OpenAPI responses. Switch to FastAPI's native
{"model": ...} in the responses dict, which properly registers
schemas in components/schemas with valid $ref pointers.
Replace hardcoded severity Selection with vocabulary-backed Many2one fields using spp.vocabulary.code (CAP v1.2 standard). This enables future mapping to other alerting protocols. Changes: - Add uuid field (auto-generated UUID4, indexed) - Refactor severity to severity_id (Many2one to CAP severity vocabulary) - Add cap_urgency_id, cap_certainty_id, cap_msg_type_id (vocabulary-backed) - Add cap_event, effective, expires, source, source_alert_id fields - Make category_id and start_date optional on model (required in form XML) - Auto-populate start_date/end_date from effective/expires on create - Create CAP vocabulary data (severity, urgency, certainty, msg_type) - Add "Alert Details" tab to form view - Update search view with new filter/grouping options - Update demo data to use vocabulary code references
Implement reusable model methods for creating/updating incidents from external alert systems (EWS), independent of the API layer: - create_from_alert(): create incident + hazard_zone geofence + area linking - update_from_alert(): update properties/geometry, handle cancellations - _link_areas_from_geometry(): PostGIS ST_Intersects area auto-linking - to_geojson(): GeoJSON Feature representation with CAP-aligned properties - _get_alert_geometry(): resolve geometry from linked hazard_zone geofence Includes comprehensive tests for all methods.
Implement OGC API Features Part 4 for hazard incidents, enabling EWS
systems to push alerts via standard GeoJSON endpoints.
API surface:
- POST /collections/incidents/items: create incident from alert
- PUT /collections/incidents/items/{uuid}: update incident
- GET /collections/incidents/items: list with filters
- GET /collections/incidents/items/{uuid}: single incident
Features:
- Duplicate detection: 409 Conflict with Location header on duplicate
source_alert_id
- Named query filters: event, severity, status (incidents); incident_code
(geofences); datetime (both)
- OGC datetime parameter parsing (instant, interval, open-ended)
- gis:incident scope for write access
- Pydantic schemas for input validation (CreateIncidentInput,
ReplaceIncidentInput)
- Collection-specific POST/PUT routes for proper OpenAPI schema generation
Includes comprehensive service-level tests for all operations.
…ences - Use datetime.fromisoformat() instead of fields.Datetime.from_string() to handle ISO 8601 format with T separator and Z timezone suffix - Update spp_hazard_programs view to use severity_id (vocabulary-backed)
Critical: - Generate fallback code when source_alert_id absent (prevents 500) - Convert ISO 8601 datetimes to naive UTC for Odoo ORM compatibility - Add missing base.group_user ACL for spp.hazard.impact - Fix emergency ribbon placement (inside sheet, not before) - Add .sudo() to VocabCode lookup in severity filter Important: - Narrow _link_areas_from_geometry exception to psycopg2.Error - Replace deprecated name_get with _compute_display_name (Odoo 19) - Merge cancel close into single ORM write (avoid double chatter) - Add no_create/no_open options to all vocabulary Many2one fields - Add group-by for cap_msg_type_id in search view - Add active=True to bbox geofence search for consistency - Add effective!=False guard in temporal filtering - Add scope enforcement tests for incident endpoints - Fix shared mutable state in TestUpdateFromAlert (setUp per test) - Assert geometry actually changed in test_update_with_new_geometry Minor: - Move Command import to top-level - Improve field labels (Effective From, Expires At) - Add empty state message to Impacts tab - Add optional attributes to list view columns
- Add typed input schemas for spatial/proximity statistics processes - Type ExecuteRequest.inputs as union instead of bare dict - Add OpenAPI examples to GeoJSON, OGC, and statistics schemas - Fix relative imports in processes.py - Add model_rebuild() calls for forward reference resolution
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spp_program_geofencemodule for geofence-based program targeting and eligibility management.spp_gisspatial operators to supportMultiPolygonandGeometryCollection.Follow-up Fixes From Review
MultiPolygon/GeometryCollection,(geojson, distance)now appliesST_Buffer(...)correctly instead of ignoring the distance operand.spp_program_geofenceforspp.gis.geofenceandspp.gis.geofence.tagfor Programs Viewer/Validator roles.groupsrestriction on the Geofences menu to avoid exposing menu items to users lacking read access.Notable Functional Additions
spp_program_geofencegeofence_idsandgeofence_count.spp.program.membership.manager.geofence) with:spp_gisST_GeomFromGeoJSON.YOUR_MAPTILER_API_KEY_HEREtreated as unconfigured).Test Plan
MultiPolygon/GeometryCollectionSQL generation.python3 -m py_compile spp_gis/operators.py spp_gis/tests/test_geo_fields.py./spp t ...is environment-blocked here (tomllibmissing in local Python, and fallback script depends onshuf).