Skip to content

Commit 5cb9df5

Browse files
fix: address remaining gemini-code-assist findings
- eligibility_manager: guard the queued import against archived/deleted programs (with regression test); drop the NOT-IN anti-pattern from the tier-2 domain (tier1 | tier2 already dedups); recordset subtraction for already-enrolled exclusion; iterate ids when building membership values. - gis_edit_map widget: only re-create the MapLibre map when the geometry value actually changed (prevents zoom/pan loss and WebGL context churn on unrelated OWL patches).
1 parent 5163802 commit 5cb9df5

3 files changed

Lines changed: 36 additions & 4 deletions

File tree

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,20 @@ export class FieldGisEditMap extends Component {
5151
defaultCenter: default_center,
5252
});
5353

54+
this._lastValue = this.props.record.data[this.props.name] || false;
5455
this.renderMap();
5556
this.addDrawInteraction();
5657
});
5758

5859
onPatched(() => {
60+
// Re-creating the MapLibre map on every OWL patch loses zoom/pan
61+
// state and can exhaust WebGL contexts; only re-render when the
62+
// geometry value actually changed.
63+
const value = this.props.record.data[this.props.name] || false;
64+
if (value === this._lastValue) {
65+
return;
66+
}
67+
this._lastValue = value;
5968
this.defaultZoom = this.map.getZoom();
6069
this.renderMap();
6170
this.addDrawInteraction();

spp_program_geofence/models/eligibility_manager.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,11 @@ def _find_eligible_registrants(self, membership=None):
151151
area_domain += [("area_type_id", "=", self.fallback_area_type_id.id)]
152152
matching_areas = self.env["spp.area"].search(area_domain)
153153
if matching_areas:
154+
# No "id not in tier1.ids" filter: with many tier-1 matches it
155+
# generates a heavy NOT IN clause, and the tier1 | tier2 union
156+
# below already deduplicates.
154157
tier2_domain = base_domain + [
155158
("area_id", "in", matching_areas.ids),
156-
("id", "not in", tier1.ids),
157159
]
158160
tier2 = self.env["res.partner"].search(tier2_domain)
159161
return tier1 | tier2
@@ -185,8 +187,7 @@ def import_eligible_registrants(self, state="draft"):
185187
new_beneficiaries = self._find_eligible_registrants()
186188

187189
# Exclude already-enrolled beneficiaries (match base manager's exclusion source)
188-
existing_partner_ids = set(self.program_id.get_beneficiaries().mapped("partner_id").ids)
189-
new_beneficiaries = new_beneficiaries.filtered(lambda r: r.id not in existing_partner_ids)
190+
new_beneficiaries -= self.program_id.get_beneficiaries().mapped("partner_id")
190191

191192
ben_count = len(new_beneficiaries)
192193
if ben_count < self.ASYNC_IMPORT_THRESHOLD:
@@ -222,9 +223,19 @@ def mark_import_as_done(self):
222223
self.program_id.message_post(body=_("Import finished."))
223224

224225
def _import_registrants(self, new_beneficiaries, state="draft", do_count=False):
226+
program = self.program_id
227+
# This runs from the job queue: the program may have been archived or
228+
# deleted between enqueue and execution (a Many2one still dereferences
229+
# an archived record as truthy).
230+
if not program.exists() or not program.active:
231+
_logger.warning(
232+
"spp_program_geofence: program for manager %s is missing or archived; skipping import",
233+
self.id,
234+
)
235+
return
225236
_logger.info("spp_program_geofence: Importing %s beneficiaries", len(new_beneficiaries))
226237
self.env["spp.program.membership"].create(
227-
[{"program_id": self.program_id.id, "partner_id": b.id, "state": state} for b in new_beneficiaries]
238+
[{"program_id": program.id, "partner_id": pid, "state": state} for pid in new_beneficiaries.ids]
228239
)
229240

230241
if do_count:

spp_program_geofence/tests/test_geofence_eligibility.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,3 +786,15 @@ def mock_delayable(self_inner, **kwargs):
786786
channels,
787787
f"Expected 'statistics_refresh' channel for on_done job, got: {channels}",
788788
)
789+
790+
def test_import_skips_archived_program(self):
791+
"""The queued import must not create memberships for an archived program."""
792+
registrant = self.env["res.partner"].create({"name": "Archived Prog Registrant", "is_registrant": True})
793+
Membership = self.env["spp.program.membership"]
794+
before = Membership.with_context(active_test=False).search_count([("program_id", "=", self.program.id)])
795+
self.program.active = False
796+
with self.assertLogs("odoo.addons.spp_program_geofence.models.eligibility_manager", level="WARNING"):
797+
self.manager._import_registrants(registrant)
798+
self.program.active = True
799+
after = Membership.with_context(active_test=False).search_count([("program_id", "=", self.program.id)])
800+
self.assertEqual(before, after, "no memberships may be created for an archived program")

0 commit comments

Comments
 (0)