fix(spp_gis): fix map widget bugs, SQL parameterization, and OSM fallback

- 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

Author: jeremi

spp_gis/operators.py

@@ -263,9 +263,12 @@ def create_from_geojson(self, geojson_dict, srid):
 
         Used for complex geometry types (MultiPolygon, GeometryCollection)
         that cannot be easily constructed from coordinates.
+
+        Returns a SQL object with the GeoJSON string as a bound parameter
+        to avoid SQL injection via string interpolation.
         """
-        geojson_str = json.dumps(geojson_dict).replace("'", "''")
-        return self.st_setsrid(f"ST_GeomFromGeoJSON('{geojson_str}')", srid)
+        geojson_str = json.dumps(geojson_dict)
+        return SQL("ST_SetSRID(ST_GeomFromGeoJSON(%s), %s)", geojson_str, srid)
 
     def validate_coordinates_for_point(self, coordinates):
         """
@@ -505,7 +508,8 @@ def domain_query(self, operator, value):
         if layer_type in ("multipolygon", "geometrycollection"):
             # Complex types use ST_GeomFromGeoJSON directly
             geom = self.create_from_geojson(geojson_val, self.field.srid)
-            return SQL(f"{self.POSTGIS_SPATIAL_RELATION[operation]}({geom}, {self.qualified_field_name})")
+            postgis_fn = self.POSTGIS_SPATIAL_RELATION[operation]
+            return SQL("%s(%s, %s)", SQL(postgis_fn), geom, SQL(self.qualified_field_name))
 
         coordinates = geojson_val["coordinates"]
         return SQL(self.get_postgis_query(operation, coordinates, distance=distance, layer_type=layer_type))

spp_gis/static/src/js/views/gis/gis_renderer/gis_renderer.esm.js

@@ -91,7 +91,9 @@ export class GisRenderer extends Component {
         });
 
         onMounted(() => {
-            maptilersdk.config.apiKey = this.mapTilerKey;
+            if (this.mapTilerKey) {
+                maptilersdk.config.apiKey = this.mapTilerKey;
+            }
             this.setupSourceAndLayer();
 
             this.renderMap();
@@ -106,14 +108,11 @@ export class GisRenderer extends Component {
     async getMapTilerKey() {
         try {
             const response = await this.rpc("/get_maptiler_api_key");
-            this.mapTilerKey = response.mapTilerKey;
             if (response.mapTilerKey) {
                 this.mapTilerKey = response.mapTilerKey;
-            } else {
-                console.log("Error: Api Key not found.");
             }
         } catch (error) {
-            console.error("Error fetching environment variable:", error);
+            console.warn("Could not fetch MapTiler API key:", error);
         }
     }
 
@@ -459,11 +458,38 @@ export class GisRenderer extends Component {
 
         this.addMouseInteraction();
 
-        const gc = new maptilersdkMaptilerGeocoder.GeocodingControl({});
-        this.map.addControl(gc, "top-left");
+        if (this.mapTilerKey) {
+            const gc = new maptilersdkMaptilerGeocoder.GeocodingControl({});
+            this.map.addControl(gc, "top-left");
+        }
     }
 
     getMapStyle(layer) {
+        if (!this.mapTilerKey) {
+            // 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:
+                            '&copy; <a href="https://www.openstreetmap.org/copyright">OpenStreetMap</a> contributors',
+                    },
+                },
+                layers: [
+                    {
+                        id: "osm-tiles",
+                        type: "raster",
+                        source: "osm",
+                        minzoom: 0,
+                        maxzoom: 19,
+                    },
+                ],
+            };
+        }
+
         let mapStyle = maptilersdk.MapStyle.STREETS;
 
         if (layer) {

spp_gis/static/src/js/widgets/gis_edit_map/field_gis_edit_map.esm.js

@@ -132,6 +132,10 @@ export class FieldGisEditMap extends Component {
             this.defaultZoom = 10;
         }
 
+        if (this.map) {
+            this.map.remove();
+        }
+
         this.map = new maptilersdk.Map({
             container: this.id,
             style: this._getMapStyle(),
@@ -227,7 +231,9 @@ export class FieldGisEditMap extends Component {
     }
 
     removeSourceAndLayer(source) {
-        this.map.removeLayer(source);
+        this.map.removeLayer(`${source}-polygon-layerid`);
+        this.map.removeLayer(`${source}-point-layerid`);
+        this.map.removeLayer(`${source}-linestring-layerid`);
         this.map.removeSource(source);
     }
 
@@ -241,13 +247,16 @@ export class FieldGisEditMap extends Component {
         const self = this;
 
         function updateArea(e) {
-            console.log(e);
             var data = self.draw.getAll();
             self.props.record.update({
                 [self.props.name]: JSON.stringify(data.features[0].geometry),
             });
         }
 
+        if (this.draw) {
+            this.map.removeControl(this.draw);
+        }
+
         this.draw = new MapboxDraw({
             displayControlsDefault: false,
             controls: {
@@ -274,17 +283,6 @@ export class FieldGisEditMap extends Component {
 
         this.map.on("draw.create", updateArea);
         this.map.on("draw.update", updateArea);
-
-        const url = `/spp_gis/static/src/images/laos_farm.png`;
-
-        this.map.on("click", `${this.sourceId}-polygon-layerid`, (e) => {
-            new maptilersdk.Popup()
-                .setLngLat(e.lngLat)
-                .setHTML(
-                    `<img src="${url}" height="200" width="300" alt="Placeholder Image">`
-                )
-                .addTo(this.map);
-        });
     }
 
     addDrawInteractionStyle() {
@@ -398,7 +396,6 @@ export class FieldGisEditMap extends Component {
         const customMode = {};
         const self = this;
         customMode.onTrash = function (state) {
-            console.log(state);
             self.props.record.update({[self.props.name]: null});
         };
 

spp_program_geofence/models/eligibility_manager.py

@@ -99,7 +99,7 @@ def _prepare_eligible_domain(self, membership=None):
         """
         domain = []
         if membership is not None:
-            ids = membership.mapped("partner_id.id")
+            ids = membership.partner_id.ids
             domain += [("id", "in", ids)]
 
         # Exclude disabled registrants (disabled is a Datetime field)
@@ -159,39 +159,38 @@ def _find_eligible_registrants(self, membership=None):
         return tier1
 
     def enroll_eligible_registrants(self, program_memberships):
-        for rec in self:
-            eligible = rec._find_eligible_registrants(program_memberships)
-            return self.env["spp.program.membership"].search(
-                [
-                    ("partner_id", "in", eligible.ids),
-                    ("program_id", "=", rec.program_id.id),
-                ]
-            )
+        self.ensure_one()
+        eligible = self._find_eligible_registrants(program_memberships)
+        return self.env["spp.program.membership"].search(
+            [
+                ("partner_id", "in", eligible.ids),
+                ("program_id", "=", self.program_id.id),
+            ]
+        )
 
     def verify_cycle_eligibility(self, cycle, membership):
-        for rec in self:
-            eligible = rec._find_eligible_registrants(membership)
-            return self.env["spp.cycle.membership"].search(
-                [
-                    ("partner_id", "in", eligible.ids),
-                    ("cycle_id", "=", cycle.id),
-                ]
-            )
+        self.ensure_one()
+        eligible = self._find_eligible_registrants(membership)
+        return self.env["spp.cycle.membership"].search(
+            [
+                ("partner_id", "in", eligible.ids),
+                ("cycle_id", "=", cycle.id),
+            ]
+        )
 
     def import_eligible_registrants(self, state="draft"):
-        ben_count = 0
-        for rec in self:
-            new_beneficiaries = rec._find_eligible_registrants()
-
-            # Exclude already-enrolled beneficiaries
-            beneficiary_ids = rec.program_id.get_beneficiaries().mapped("partner_id")
-            new_beneficiaries = new_beneficiaries - beneficiary_ids
-
-            ben_count = len(new_beneficiaries)
-            if ben_count < 1000:
-                rec._import_registrants(new_beneficiaries, state=state, do_count=True)
-            else:
-                rec._import_registrants_async(new_beneficiaries, state=state)
+        self.ensure_one()
+        new_beneficiaries = self._find_eligible_registrants()
+
+        # Exclude already-enrolled beneficiaries
+        existing_partner_ids = set(self.program_id.program_membership_ids.partner_id.ids)
+        new_beneficiaries = new_beneficiaries.filtered(lambda r: r.id not in existing_partner_ids)
+
+        ben_count = len(new_beneficiaries)
+        if ben_count < 1000:
+            self._import_registrants(new_beneficiaries, state=state, do_count=True)
+        else:
+            self._import_registrants_async(new_beneficiaries, state=state)
         return ben_count
 
     def _import_registrants_async(self, new_beneficiaries, state="draft"):
@@ -221,9 +220,7 @@ def mark_import_as_done(self):
 
     def _import_registrants(self, new_beneficiaries, state="draft", do_count=False):
         _logger.info("spp_program_geofence: Importing %s beneficiaries", len(new_beneficiaries))
-        beneficiaries_val = [
-            Command.create({"partner_id": b.id, "state": state}) for b in new_beneficiaries
-        ]
+        beneficiaries_val = [Command.create({"partner_id": b.id, "state": state}) for b in new_beneficiaries]
         self.program_id.update({"program_membership_ids": beneficiaries_val})
 
         if do_count:
@@ -236,9 +233,10 @@ def action_preview_eligible(self):
             eligible = self._find_eligible_registrants()
             self.preview_count = len(eligible)
             self.preview_error = False
-        except Exception as e:
+        except Exception:
+            _logger.exception("Geofence eligibility preview failed for manager %s", self.id)
             self.preview_count = 0
-            self.preview_error = str(e)
+            self.preview_error = "Preview failed. Check the server logs for details."
         return {
             "type": "ir.actions.client",
             "tag": "display_notification",

spp_program_geofence/tests/test_geofence_eligibility.py

@@ -212,7 +212,7 @@ def setUpClass(cls):
             {
                 "name": "Geofence Test Program",
                 "target_type": "individual",
-                "geofence_ids": [(6, 0, [cls.geofence.id])],
+                "geofence_ids": [Command.set([cls.geofence.id])],
             }
         )
 
@@ -319,22 +319,22 @@ def test_hybrid_no_duplicates(self):
 
     def test_multiple_geofences(self):
         """Registrant in second geofence is eligible."""
-        self.program.geofence_ids = [(6, 0, [self.geofence.id, self.geofence2.id])]
+        self.program.geofence_ids = [Command.set([self.geofence.id, self.geofence2.id])]
         eligible = self.manager._find_eligible_registrants()
         self.assertIn(self.reg_in_geofence2, eligible)
         self.assertIn(self.reg_inside, eligible)
         # Restore
-        self.program.geofence_ids = [(6, 0, [self.geofence.id])]
+        self.program.geofence_ids = [Command.set([self.geofence.id])]
 
     # --- No geofences ---
 
     def test_no_geofences_empty_result(self):
         """Program with no geofences returns no eligible registrants."""
-        self.program.geofence_ids = [(5, 0, 0)]
+        self.program.geofence_ids = [Command.clear()]
         eligible = self.manager._find_eligible_registrants()
         self.assertEqual(len(eligible), 0)
         # Restore
-        self.program.geofence_ids = [(6, 0, [self.geofence.id])]
+        self.program.geofence_ids = [Command.set([self.geofence.id])]
 
     # --- Enrollment pipeline ---
 
@@ -365,7 +365,7 @@ def test_import_eligible_registrants(self):
             {
                 "name": "Import Test Program",
                 "target_type": "individual",
-                "geofence_ids": [(6, 0, [self.geofence.id])],
+                "geofence_ids": [Command.set([self.geofence.id])],
             }
         )
         manager2 = self.env["spp.program.membership.manager.geofence"].create(
@@ -386,7 +386,7 @@ def test_import_excludes_already_enrolled(self):
             {
                 "name": "Dedup Test Program",
                 "target_type": "individual",
-                "geofence_ids": [(6, 0, [self.geofence.id])],
+                "geofence_ids": [Command.set([self.geofence.id])],
             }
         )
         manager3 = self.env["spp.program.membership.manager.geofence"].create(
@@ -431,25 +431,25 @@ def test_target_type_group(self):
 
     def test_multipolygon_geofence(self):
         """Program with multiple non-overlapping geofences uses MultiPolygon via unary_union."""
-        self.program.geofence_ids = [(6, 0, [self.geofence.id, self.geofence2.id])]
+        self.program.geofence_ids = [Command.set([self.geofence.id, self.geofence2.id])]
         eligible = self.manager._find_eligible_registrants()
         # Both registrants from different geofences should be eligible
         self.assertIn(self.reg_inside, eligible)
         self.assertIn(self.reg_in_geofence2, eligible)
         # Outside registrant should not be
         self.assertNotIn(self.reg_outside, eligible)
         # Restore
-        self.program.geofence_ids = [(6, 0, [self.geofence.id])]
+        self.program.geofence_ids = [Command.set([self.geofence.id])]
 
     # --- Program geofence field ---
 
     def test_program_geofence_count(self):
         """geofence_count is computed correctly."""
         self.assertEqual(self.program.geofence_count, 1)
-        self.program.geofence_ids = [(6, 0, [self.geofence.id, self.geofence2.id])]
+        self.program.geofence_ids = [Command.set([self.geofence.id, self.geofence2.id])]
         self.assertEqual(self.program.geofence_count, 2)
         # Restore
-        self.program.geofence_ids = [(6, 0, [self.geofence.id])]
+        self.program.geofence_ids = [Command.set([self.geofence.id])]
 
 
 @tagged("post_install", "-at_install")
@@ -497,7 +497,7 @@ def setUpClass(cls):
             {
                 "name": "Officer Test Program",
                 "target_type": "individual",
-                "geofence_ids": [(6, 0, [cls.geofence.id])],
+                "geofence_ids": [Command.set([cls.geofence.id])],
             }
         )