feat: geofence-based geographic targeting for programs#76
Open
jeremi wants to merge 141 commits into
Open
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.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #76 +/- ##
==========================================
+ Coverage 71.68% 72.13% +0.44%
==========================================
Files 1010 1010
Lines 60072 61250 +1178
==========================================
+ Hits 43062 44181 +1119
- Misses 17010 17069 +59
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)
Sync README.rst and index.html with the current oca-gen-addon-readme output for spp_change_request_v2, spp_drims_sl, spp_gis_indicators, spp_hazard, and spp_security (formatting-only churn).
…t area The spp_hazard severity migration (Selection -> spp.vocabulary.code M2O, severity/severity_override -> severity_id/severity_override_id) missed the spp_drims extension of spp.hazard.incident.area, whose compute still referenced the removed severity_override field. This broke registry load on any full stack (e.g. spp_drims_sl), though spp_hazard's own tests stayed green. - Replace the Selection effective_severity with a computed effective_severity_id Many2one and map CAP codes to a 1-5 severity_numeric for the choropleth. - Update the DRIMS demo generator to write severity_id/severity_override_id from CAP vocabulary codes instead of legacy numeric strings. - Add a focused test for the effective-severity compute.
- SQL injection (pylint E8103 + Semgrep): build dynamic identifiers with psycopg2.sql.Identifier in ogc_service and spp_gis_report instead of f-strings. - XXE (Semgrep): parse bundled Luzon area XML with a hardened lxml parser (resolve_entities=False, no_network=True) instead of stdlib xml.etree. - sudo on sensitive models (Semgrep CRITICAL): document why sudo is required (aggregate metrics, deletion-integrity check, API enum) and suppress with the correct rule id; these are read-only and authorized at the service boundary.
…ca-checks areas_luzon.xml and population_weights.csv are loaded at runtime (area loader / population weights), not at install. Declare them under oca_data_manual so oca-checks-odoo-module no longer reports file-not-used.
- Drop unused params (no-unused-vars errors). - Extract _matchesCondition and choropleth legend helpers to bring _matchesDomain and renderChoroplethLegends under the complexity limit. - Move inline comments to their own lines (no-inline-comments).
Mechanical formatting (ruff-format, prettier, end-of-file, mixed-line-ending) on files already modified by this PR, to satisfy the pre-commit hooks. No logic changes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…rvice ruff-format wrapped the inline-suppressed assignment across lines, leaving the nosemgrep after the match's start line so Semgrep no longer honoured it. Move the suppression to the line directly above the match, matching the other sudo sites.
The deprecated 'description' key tripped pylint-odoo's mandatory manifest-deprecated-key (C8103) check. The summary covers the module purpose; data attribution is kept as a comment.
Formatting-only (line wrapping/reflow) on files this PR modified, to satisfy the pre-commit ruff-format and prettier hooks. No logic changes.
… calls The semgrep pre-commit hook (--error) was failing on unsuppressed odoo-sudo-without-context warnings for API/system-context sudo calls (config params, job records, geofence/incident lookups). These run after router-level auth; add justified nosemgrep suppressions matching the existing pattern. No logic change.
CI's cached pre-commit env held older docutils/markdown-it-py, which render RST table columns at different widths than a fresh install, producing spurious README drift (spp_change_request_v2, spp_gis_indicators, spp_drims_sl). Pin docutils==0.23 and markdown-it-py==4.2.0 on the oca-gen-addon-readme hook so the output is reproducible across environments; this also refreshes the CI cache.
The sudo suppression comments exceeded the 120-char line length (ruff E501). Trim the inline justification; the nosemgrep directive is unchanged.
Catch up with 19.0 (45 commits behind): brings in regenerated addon READMEs (resolving pre-commit README drift seen on the PR merge ref), DRIMS UI work, spp_programs config UX, and Odoo 19 migrations. No conflicts; no overlap with the geofence CI fixes on this branch.
CI generates RST tables with different column widths than a local fresh install (an unidentified env factor; the committed text is otherwise identical). These 5 modules' fragments are unchanged vs 19.0, so regenerate the READMEs to exactly match CI's oca-gen-addon-readme output, clearing the pre-commit README drift. Cosmetic table-width only; no content change. docutils/markdown-it-py remain pinned so the output stays reproducible.
…d, bbox helpers Add tests for get_feature_count (report + error paths), get_feature_by_id / _get_report_feature_by_id (report path + not-found errors), the bbox-to-geojson and coordinate-extraction helpers across all geometry types. Layer-type cases skip when spp.area has no geo field (matching existing setup). All 353 module tests pass.
…ew actions Add tests for _calculate_quantiles, _calculate_jenks_breaks, the member-expansion constraint (valid + invalid source), next-refresh compute, action_view_data, action_view_map, and action_refresh_now. All 188 module tests pass.
Add HTTP integration tests for the geofences write router: OPTIONS discovery, gis:geofence scope enforcement (403 on POST/DELETE without scope), and a successful geofence creation (201). All 357 module tests pass.
…vice The setup probed for a field named 'polygon' (the actual field is 'geo_polygon') so spp.gis.data.layer was never created and 8 layer-type tests always skipped. Fix the field name and supply the required view_id (a gis-type ir.ui.view), so 11 data-layer tests now run, covering the data-layer code paths in LayersService. All 357 module tests pass.
Existing tests always passed load_shapes=False, leaving _load_shapes uncovered. Add tests calling it directly and via load_shapes=True; the method handles a missing geo_polygon field gracefully (spp_gis not in this module's deps).
…callback Add tests for SpatialQueryService._get_empty_statistics, _build_filter_clauses (all branches), _resolve_individuals_to_groups (empty + membership resolution), and process.job._make_batch_progress_callback (non-batch + throttled writes).
Add direct tests for the module-level cache helpers: _make_cache_key (stable + config-sensitive + unsortable-config fallback), get/cache roundtrip, invalidate, and FIFO eviction past the size limit.
Member
CI green-up + coverage pass (ACN)Worked through the failing checks, brought the branch current with CI fixes
Caught up with
|
Catch up with 19.0 (35 commits) before merge so CI validates the true final merge state. No logic changes on this branch.
The 19.0 catch-up reintroduced README.rst/index.html generated with a different docutils column width. Regenerate with the pinned doc-gen deps so they match CI's oca-gen-addon-readme output. Cosmetic table-width only; no content change.
The previous local regen used a different Python's docutils than CI, drifting 5 modules' README tables. These modules' readme fragments are unchanged vs 19.0, so restore 19.0's CI-generated README.rst/index.html — exactly what CI's hook produces, clearing the README drift.
The OSM raster fallback style object was duplicated verbatim in the GIS renderer and the geo-edit-map field widget. Extract it into osm_fallback_style.esm.js and import it in both (addresses a Gemini review note). No behavior change.
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
3 participants
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).