feat(spp_hazard): CAP vocabulary severity and alert ingestion, with upgrade migration (re-land from #76)#274
feat(spp_hazard): CAP vocabulary severity and alert ingestion, with upgrade migration (re-land from #76)#274gonzalesedwin1123 wants to merge 2 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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 migrates hazard incident severity tracking from a hardcoded 1-5 Selection to a CAP v1.2 vocabulary code, introducing new CAP-aligned fields (urgency, certainty, message type, and event) and robust alert ingestion methods that automatically create or update incidents and geofences from GeoJSON alerts. It also includes a post-migration script to backfill legacy severity values and updates related tests, views, and demo generators across dependent modules. The review feedback highlights three important improvements: making the _parse_datetime_string helper robust against already-parsed datetime objects, wrapping the PostGIS intersection query in a savepoint to prevent transaction aborts on database errors, and restricting the migration's column existence check to the active schema to avoid false positives.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _parse_datetime_string(value): | ||
| """Parse a datetime string in either ISO 8601 or Odoo format. | ||
|
|
||
| Handles both '2026-04-01T00:00:00Z' (ISO 8601) and | ||
| '2026-04-01 00:00:00' (Odoo Datetime format). | ||
|
|
||
| Returns: | ||
| datetime object | ||
| """ | ||
| # Replace 'Z' with '+00:00' for fromisoformat compatibility | ||
| normalized = value.replace("Z", "+00:00") | ||
| dt = datetime.fromisoformat(normalized) | ||
| # Odoo requires naive (UTC) datetimes | ||
| if dt.tzinfo is not None: | ||
| dt = dt.astimezone(UTC).replace(tzinfo=None) | ||
| return dt |
There was a problem hiding this comment.
The helper function _parse_datetime_string assumes that value is always a string. However, in Odoo, datetime fields or values passed programmatically (e.g., in properties or context) can sometimes already be datetime objects. Calling .replace("Z", "+00:00") on a datetime object will raise a TypeError.\n\nTo make this helper robust and prevent potential runtime crashes, check if value is already a datetime instance before attempting string manipulation.
| def _parse_datetime_string(value): | |
| """Parse a datetime string in either ISO 8601 or Odoo format. | |
| Handles both '2026-04-01T00:00:00Z' (ISO 8601) and | |
| '2026-04-01 00:00:00' (Odoo Datetime format). | |
| Returns: | |
| datetime object | |
| """ | |
| # Replace 'Z' with '+00:00' for fromisoformat compatibility | |
| normalized = value.replace("Z", "+00:00") | |
| dt = datetime.fromisoformat(normalized) | |
| # Odoo requires naive (UTC) datetimes | |
| if dt.tzinfo is not None: | |
| dt = dt.astimezone(UTC).replace(tzinfo=None) | |
| return dt | |
| def _parse_datetime_string(value): | |
| """Parse a datetime string in either ISO 8601 or Odoo format. | |
| Handles both '2026-04-01T00:00:00Z' (ISO 8601) and | |
| '2026-04-01 00:00:00' (Odoo Datetime format). Also handles datetime objects. | |
| Returns: | |
| datetime object | |
| """ | |
| if isinstance(value, datetime): | |
| dt = value | |
| else: | |
| # Replace 'Z' with '+00:00' for fromisoformat compatibility | |
| normalized = value.replace("Z", "+00:00") | |
| dt = datetime.fromisoformat(normalized) | |
| # Odoo requires naive (UTC) datetimes | |
| if dt.tzinfo is not None: | |
| dt = dt.astimezone(UTC).replace(tzinfo=None) | |
| return dt |
| self.env.cr.execute( | ||
| """ | ||
| SELECT id FROM spp_area | ||
| WHERE geo_polygon IS NOT NULL | ||
| AND ST_Intersects( | ||
| geo_polygon::geometry, | ||
| ST_SetSRID(ST_GeomFromGeoJSON(%s), 4326) | ||
| ) | ||
| """, | ||
| (geojson_str,), | ||
| ) | ||
| area_ids = [row[0] for row in self.env.cr.fetchall()] | ||
| if area_ids: | ||
| self.write({"area_ids": [Command.set(area_ids)]}) | ||
| else: | ||
| _logger.info( | ||
| "No intersecting areas found for incident %s", | ||
| self.code, | ||
| ) | ||
| except psycopg2.Error as e: | ||
| _logger.warning( | ||
| "Failed to link areas from geometry for incident %s: %s", | ||
| self.code, | ||
| e, | ||
| ) | ||
|
|
There was a problem hiding this comment.
In PostgreSQL/psycopg2, any error occurring during a query execution (such as an invalid GeoJSON format passed to ST_GeomFromGeoJSON) will abort the current transaction. Simply catching psycopg2.Error and logging a warning is insufficient because the transaction remains in an aborted state, causing all subsequent database operations in the same request to fail with InFailedSqlTransaction.\n\nTo safely handle potential database errors during geometry intersection and allow the transaction to continue, wrap the database execution inside a savepoint using with self.env.cr.savepoint():.
try:
with self.env.cr.savepoint():
self.env.cr.execute(
"""
SELECT id FROM spp_area
WHERE geo_polygon IS NOT NULL
AND ST_Intersects(
geo_polygon::geometry,
ST_SetSRID(ST_GeomFromGeoJSON(%s), 4326)
)
""",
(geojson_str,),
)
area_ids = [row[0] for row in self.env.cr.fetchall()]
if area_ids:
self.write({"area_ids": [Command.set(area_ids)]})
else:
_logger.info(
"No intersecting areas found for incident %s",
self.code,
)
except psycopg2.Error as e:
_logger.warning(
"Failed to link areas from geometry for incident %s: %s",
self.code,
e,
)| def _column_exists(cr, table, column): | ||
| cr.execute( | ||
| """ | ||
| SELECT 1 | ||
| FROM information_schema.columns | ||
| WHERE table_name = %s AND column_name = %s | ||
| """, | ||
| (table, column), | ||
| ) | ||
| return bool(cr.fetchone()) |
There was a problem hiding this comment.
When querying information_schema.columns, filtering only by table_name and column_name can lead to false positives if the same table name exists in other schemas (such as pg_catalog or other custom schemas in the database).\n\nTo ensure the check is isolated to the active schema, add AND table_schema = current_schema() to the query.
| def _column_exists(cr, table, column): | |
| cr.execute( | |
| """ | |
| SELECT 1 | |
| FROM information_schema.columns | |
| WHERE table_name = %s AND column_name = %s | |
| """, | |
| (table, column), | |
| ) | |
| return bool(cr.fetchone()) | |
| def _column_exists(cr, table, column): | |
| cr.execute( | |
| """ | |
| SELECT 1 | |
| FROM information_schema.columns | |
| WHERE table_name = %s AND column_name = %s | |
| AND table_schema = current_schema() | |
| """, | |
| (table, column), | |
| ) | |
| return bool(cr.fetchone()) |
| cr.execute( | ||
| sql.SQL( | ||
| """ | ||
| SELECT DISTINCT t.{legacy} | ||
| FROM {table} t | ||
| WHERE t.{legacy} IS NOT NULL | ||
| AND t.{new} IS NULL | ||
| """ | ||
| ).format(**ids) | ||
| ) |
…ion (from #76) Re-lands the spp_hazard severity->vocabulary change from reverted PR #76, together with the dependent adaptations in spp_drims, spp_drims_sl_demo and spp_hazard_programs, plus the two fixes the original PR lacked: - migrations/19.0.2.1.0/post-migration.py backfills severity_id / severity_override_id from the legacy 1-5 Selection columns for databases upgrading from v19.0.2.0.x (release Biliran ships the old schema). Mapping: 1->minor, 2->moderate, 3->severe, 4->severe, 5->extreme. - hazard_demo.xml incident-area records now use severity_override_id vocabulary refs; the stale severity_override field broke demo installs. Includes migration regression tests (mapping, area override, idempotency, unmapped values, fresh-install no-op).
…enerate READMEs Semgrep flagged f-string SQL in the severity migration; identifiers were constants but composed SQL via psycopg2.sql removes the pattern entirely. READMEs regenerated from the updated HISTORY fragments (in-scope modules only).
gonzalesedwin1123
force-pushed
the
reland/hazard-cap
branch
from
3beb190 to
8a74908
Compare
2 participants
Re-lands the spp_hazard portion of reverted PR #76 (revert: #271), together with the dependent adaptations and the two fixes whose absence triggered the revert discussion.
Summary
spp.hazard.incident.severity(1-5 Selection) becomesseverity_id, aspp.vocabulary.codeMany2one on the CAP v1.2 severity vocabulary;severity_overrideon incident areas becomesseverity_override_idlikewise.create_incident_from_alert(creates hazard-zone geofences, auto-links intersecting areas); incidentuuid.spp_drims(effective_severity_id, numeric choropleth severity),spp_drims_sl_demo(scenario severity resolution),spp_hazard_programs(program view).Added vs the original PR
spp_hazard 19.0.2.1.0): backfillsseverity_id/severity_override_idfrom the legacy Selection columns for databases upgrading from v19.0.2.0.x (release Biliran ships the old schema). Label-faithful mapping: 1→minor, 2→moderate, 3→severe (CAP defines severe as 'Significant threat'), 4→severe, 5→extreme. Idempotent; never overwrites values; unmapped values logged and left empty; no-op on fresh installs. Regression tests included.severity_overridefield, breaking fresh installs with demo data; they now useseverity_override_idvocabulary refs.Verification
./spp t spp_hazard: 87 passed, 0 failed (includes 5 new migration tests)./spp t spp_drims: 250/0,spp_drims_sl_demo: 8/0,spp_hazard_programs: 24/0