From e7016097740117be55c45debfc640e08c2d12c82 Mon Sep 17 00:00:00 2001 From: Nitheesh T Ganesh Date: Tue, 7 Jul 2026 18:05:48 -0600 Subject: [PATCH 1/6] implement mandatory timeout for geospatial processing and add user feedback for processing failures. --- .../src/submit_api/services/geo/processor.py | 67 ++++++++++++++++++- .../services/geo/validation_rules.py | 7 +- .../src/submit_api/services/geo/validator.py | 3 +- .../tests/unit/services/test_geo_validator.py | 23 ++++++- .../GenericDocumentUploadSection.tsx | 14 +++- .../components/App/Map/MapPreviewModal.tsx | 11 ++- .../AddDocumentActionButton.tsx | 7 ++ .../App/SubmissionItem/DocumentsTable/Row.tsx | 7 ++ submit-web/src/utils/constants.ts | 8 +++ 9 files changed, 138 insertions(+), 9 deletions(-) diff --git a/submit-api/src/submit_api/services/geo/processor.py b/submit-api/src/submit_api/services/geo/processor.py index 7f1f64349..c278a158a 100644 --- a/submit-api/src/submit_api/services/geo/processor.py +++ b/submit-api/src/submit_api/services/geo/processor.py @@ -23,12 +23,14 @@ import json import logging import math +import multiprocessing import os import tempfile import threading import zipfile import glob from datetime import datetime, timedelta, timezone +from queue import Empty from typing import Any, Dict import pandas as pd import geopandas as gpd @@ -273,6 +275,69 @@ def process_geo_file(local_path: str) -> Dict[str, Any]: GEO_MAX_CONCURRENT_JOBS = int(os.environ.get("GEO_MAX_CONCURRENT_JOBS", 3)) _GEO_SEMAPHORE = threading.Semaphore(GEO_MAX_CONCURRENT_JOBS) +# Hard wall-clock limit for converting a single file. A malformed or overly large +# file can make geopandas spin indefinitely, leaving the upload stuck in +# 'processing' forever. When exceeded, the worker process is forcibly killed and +# the upload is marked as failed. Configurable via GEO_PROCESSING_TIMEOUT_SECONDS. +GEO_PROCESSING_TIMEOUT_SECONDS = int(os.environ.get("GEO_PROCESSING_TIMEOUT_SECONDS", 60)) + + +def _process_geo_file_worker(local_path: str, result_queue: "multiprocessing.Queue") -> None: + """Child-process entry point: run process_geo_file and push the outcome onto the queue. + + Runs in a separate process so a runaway conversion can be terminated by the + parent. The result dict (GeoJSON bytes + metadata) is picklable; exceptions + are stringified because arbitrary exception types may not be. + """ + try: + result_queue.put(("ok", process_geo_file(local_path))) + except Exception as exc: # pylint: disable=broad-except # noqa: B902 + result_queue.put(("error", str(exc))) + + +def process_geo_file_with_timeout( + local_path: str, timeout_seconds: int = GEO_PROCESSING_TIMEOUT_SECONDS +) -> Dict[str, Any]: + """Run process_geo_file in a child process, enforcing a hard wall-clock timeout. + + Raises: + TimeoutError: if processing exceeds ``timeout_seconds`` (the child is killed). + RuntimeError: if the child process fails or dies without producing a result. + """ + ctx = multiprocessing.get_context() + result_queue: "multiprocessing.Queue" = ctx.Queue() + proc = ctx.Process( + target=_process_geo_file_worker, + args=(local_path, result_queue), + daemon=True, + ) + proc.start() + + try: + # Block until the child publishes a result or the timeout elapses. Reading + # from the queue before join() avoids a deadlock when the result is large. + status, payload = result_queue.get(timeout=timeout_seconds) + except Empty as exc: + logger.warning( + "Geospatial processing exceeded %ss for %s; terminating worker process.", + timeout_seconds, local_path, + ) + proc.terminate() + proc.join(timeout=5) + if proc.is_alive(): + proc.kill() + raise TimeoutError( + f"Processing timed out after {timeout_seconds} seconds and was stopped." + ) from exc + finally: + result_queue.close() + + proc.join(timeout=5) + + if status == "error": + raise RuntimeError(payload) + return payload + class GeoService: """Service class for geospatial upload management.""" @@ -377,7 +442,7 @@ def _process_upload_in_background(cls, app, upload_id: int) -> None: if not cls._validate_or_fail(upload, local_path): return - result = process_geo_file(local_path) + result = process_geo_file_with_timeout(local_path) cls._upload_processed_tiers(upload, result) metadata = result["metadata"] diff --git a/submit-api/src/submit_api/services/geo/validation_rules.py b/submit-api/src/submit_api/services/geo/validation_rules.py index c66daa84d..16d700ce3 100644 --- a/submit-api/src/submit_api/services/geo/validation_rules.py +++ b/submit-api/src/submit_api/services/geo/validation_rules.py @@ -52,7 +52,12 @@ } # Fields whose value may be null/empty — all others are required. -NULLABLE_FIELDS: set[str] = {"sensitive_data"} +NULLABLE_FIELDS: set[str] = {"sensitive_data", "footprint"} + +# Fields that are NOT required to be present as a column in the shapefile. +# A missing column for one of these fields does not fail validation; footprint +# is optional and files without it must still pass. +OPTIONAL_FIELDS: set[str] = {"footprint"} # Set to True to fold both the feature value and the allowed set to uppercase # before comparing. False = exact case match. diff --git a/submit-api/src/submit_api/services/geo/validator.py b/submit-api/src/submit_api/services/geo/validator.py index 037f4d405..0e891e74f 100644 --- a/submit-api/src/submit_api/services/geo/validator.py +++ b/submit-api/src/submit_api/services/geo/validator.py @@ -40,6 +40,7 @@ FIELD_MAP, MAX_ERRORS, NULLABLE_FIELDS, + OPTIONAL_FIELDS, VALIDATE_VALUES, ) @@ -120,7 +121,7 @@ def validate_shapefile(shp_path: str) -> _Result: "message": f"Required column '{dbf_col}' is absent from the shapefile schema.", } for logical, dbf_col in FIELD_MAP.items() - if dbf_col not in schema_props + if logical not in OPTIONAL_FIELDS and dbf_col not in schema_props ] if missing: return False, missing, { diff --git a/submit-api/tests/unit/services/test_geo_validator.py b/submit-api/tests/unit/services/test_geo_validator.py index 639111497..1bb0a8c67 100644 --- a/submit-api/tests/unit/services/test_geo_validator.py +++ b/submit-api/tests/unit/services/test_geo_validator.py @@ -116,7 +116,7 @@ def test_missing_required_column_fails_fast(): @patch("submit_api.services.geo.validator.VALIDATE_VALUES", True) def test_missing_required_value_fails(): """A null/empty value in a non-nullable field produces a missing_value error.""" - props = {**VALID_PROPERTIES, _DBF["footprint"]: None} + props = {**VALID_PROPERTIES, _DBF["category"]: None} collection = _FakeCollection( schema_props=VALID_SCHEMA, features=[_feature(props)], @@ -126,10 +126,27 @@ def test_missing_required_value_fails(): assert is_valid is False assert any( - e["error_type"] == "missing_value" and e["dbf_column"] == _DBF["footprint"] + e["error_type"] == "missing_value" and e["dbf_column"] == _DBF["category"] for e in errors ) - assert "footprint" in summary["fields_with_errors"] + assert "category" in summary["fields_with_errors"] + + +def test_missing_footprint_column_passes(): + """footprint is optional: a file lacking the Footprint column still validates.""" + schema_without_footprint = { + k: v for k, v in VALID_SCHEMA.items() if k != _DBF["footprint"] + } + collection = _FakeCollection( + schema_props=schema_without_footprint, + features=[_feature(VALID_PROPERTIES)], + ) + with patch("submit_api.services.geo.validator.fiona.open", return_value=collection): + is_valid, errors, summary = validate_shapefile("dummy.shp") + + assert is_valid is True + assert errors == [] + assert summary["total_errors"] == 0 @patch("submit_api.services.geo.validator.VALIDATE_VALUES", False) diff --git a/submit-web/src/components/App/DocumentUpload/GenericDocumentUploadSection.tsx b/submit-web/src/components/App/DocumentUpload/GenericDocumentUploadSection.tsx index ecf1ec08b..ae62cf6a6 100644 --- a/submit-web/src/components/App/DocumentUpload/GenericDocumentUploadSection.tsx +++ b/submit-web/src/components/App/DocumentUpload/GenericDocumentUploadSection.tsx @@ -18,7 +18,10 @@ import { BarBlueTitle } from "@/components/Shared/Text/BarTitle"; import { getSubmissionFolderName } from "@/components/Shared/Table/utils"; import { DEFAULT_ACCEPTED_FILE_TYPES, + DEFAULT_MAX_FILE_SIZE_MB, EXTENSION_TO_MIME_TYPE_MAP, + GEO_MAX_FILE_SIZE_BYTES, + GEO_MAX_FILE_SIZE_MB, } from "@/utils/constants"; import { Accept } from "react-dropzone"; @@ -146,6 +149,14 @@ export const GenericDocumentUploadSection: React.FC< (document) => document.folder === section.folder, ); + const isGeoSpatialSection = section.name === "geospatial"; + const sectionMaxSize = isGeoSpatialSection + ? GEO_MAX_FILE_SIZE_BYTES + : undefined; + const sectionMaxSizeMb = isGeoSpatialSection + ? GEO_MAX_FILE_SIZE_MB + : DEFAULT_MAX_FILE_SIZE_MB; + return ( @@ -215,6 +226,7 @@ export const GenericDocumentUploadSection: React.FC< } maxFiles={section.maxFiles} maxFilesErrorMessage={section.maxFilesErrorMessage} + maxSize={sectionMaxSize} accept={fileUploadAccept} /> Accepted file types: {acceptedFileTypes.join(", ")}. Max file - size: 500 MB. + size: {sectionMaxSizeMb} MB. diff --git a/submit-web/src/components/App/Map/MapPreviewModal.tsx b/submit-web/src/components/App/Map/MapPreviewModal.tsx index 780f8447e..3e6deb64b 100644 --- a/submit-web/src/components/App/Map/MapPreviewModal.tsx +++ b/submit-web/src/components/App/Map/MapPreviewModal.tsx @@ -156,6 +156,9 @@ export const MapPreviewModal: React.FC = ({ }, [metaData, geoJson]); const isProcessing = status === "processing" || (!uploadId && open); + const isTimeout = + status === "failed" && + (errorMessage?.toLowerCase().includes("timed out") ?? false); const handleReject = async () => { setIsRejecting(true); @@ -348,7 +351,9 @@ export const MapPreviewModal: React.FC = ({ {status === "validation_failed" ? "Attribute validation failed" - : "Preview not available for this file"} + : isTimeout + ? "Geospatial processing failed" + : "Preview not available for this file"} = ({ > {status === "validation_failed" ? "This file does not meet the required attribute standards. Please correct the issue and re-upload." - : "Geometry must be valid; no NULL geometry or self-intersecting polygons"} + : isTimeout + ? "This file took too long to process and was automatically stopped. It may be too large or complex to process." + : "Geometry must be valid; no NULL geometry or self-intersecting polygons"} {errorMessage && ( GEO_MAX_FILE_SIZE_BYTES) { + notify.error( + `Geospatial files must be ${GEO_MAX_FILE_SIZE_MB} MB or smaller.`, + ); + return; + } try { setIsAddingDocument(true); setIsPendingUpload(true); diff --git a/submit-web/src/components/App/SubmissionItem/DocumentsTable/Row.tsx b/submit-web/src/components/App/SubmissionItem/DocumentsTable/Row.tsx index d53c81f43..3c5551f7a 100644 --- a/submit-web/src/components/App/SubmissionItem/DocumentsTable/Row.tsx +++ b/submit-web/src/components/App/SubmissionItem/DocumentsTable/Row.tsx @@ -26,6 +26,7 @@ import { QUERY_KEY } from "@/hooks/api/constants"; import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; import DocumentsSubTable from "@/components/App/Submission/ItemsTable/DocumentsSubTable"; import { useGetGeoUploads } from "@/hooks/api/useGeo"; +import { GEO_MAX_FILE_SIZE_BYTES, GEO_MAX_FILE_SIZE_MB } from "@/utils/constants"; type DocumentRowProps = Readonly<{ documentSubmission: Submission; @@ -139,6 +140,12 @@ export default function Row({ notify.error("Only .zip and .shp files are allowed for geospatial data."); return; } + if (isGeoSpatial && fileToUpload.size > GEO_MAX_FILE_SIZE_BYTES) { + notify.error( + `Geospatial files must be ${GEO_MAX_FILE_SIZE_MB} MB or smaller.`, + ); + return; + } try { setIsReplacingDocument(true); setIsPendingUpload(true); diff --git a/submit-web/src/utils/constants.ts b/submit-web/src/utils/constants.ts index 10ca17f4e..620193dde 100644 --- a/submit-web/src/utils/constants.ts +++ b/submit-web/src/utils/constants.ts @@ -18,6 +18,14 @@ export const DEFAULT_ACCEPTED_FILE_TYPES = [ "xlsx", ]; +// Default maximum upload size for documents (in MB). +export const DEFAULT_MAX_FILE_SIZE_MB = 500; + +// Geospatial files (.shp/.zip) are capped lower because processing large +// files is expensive and can time out. This limit applies to geospatial only. +export const GEO_MAX_FILE_SIZE_MB = 20; +export const GEO_MAX_FILE_SIZE_BYTES = GEO_MAX_FILE_SIZE_MB * 1024 * 1024; + export const EXTENSION_TO_MIME_TYPE_MAP: Record = { pdf: ["application/pdf"], doc: ["application/msword"], From ef6f4bdf6e2b0eb0af1d717731c5e3750830b5ca Mon Sep 17 00:00:00 2001 From: Nitheesh T Ganesh Date: Wed, 8 Jul 2026 11:27:04 -0600 Subject: [PATCH 2/6] implement geospatial document approval workflow and added gates in the submission page --- ...f6a7b8c9_add_is_approved_to_geo_uploads.py | 30 +++++++ .../src/submit_api/models/geo_data_upload.py | 6 +- submit-api/src/submit_api/resources/geo.py | 20 +++++ .../src/submit_api/services/geo/processor.py | 41 ++++++++++ .../submit_api/services/package_service.py | 6 ++ .../services/submission/__init__.py | 15 ++++ .../App/DocumentUpload/DocumentTableRow.tsx | 50 ++++++++---- .../components/App/Map/MapPreviewModal.tsx | 35 ++++++-- .../App/Submission/DocumentRow/index.tsx | 59 +++++++++----- .../App/SubmissionItem/DocumentsTable/Row.tsx | 27 +++++-- .../GeoSpatialProponentView.tsx | 65 +++++++++++++-- .../GeoSpatialUpdateForm/index.tsx | 81 +++++++++++++++++-- .../components/Shared/FileUpload/index.tsx | 3 + .../components/Shared/GeoApprovedBadge.tsx | 44 ++++++++++ .../_submissionLayout/index.tsx | 25 ++++-- 15 files changed, 432 insertions(+), 75 deletions(-) create mode 100644 submit-api/migrations/versions/d4e5f6a7b8c9_add_is_approved_to_geo_uploads.py create mode 100644 submit-web/src/components/Shared/GeoApprovedBadge.tsx diff --git a/submit-api/migrations/versions/d4e5f6a7b8c9_add_is_approved_to_geo_uploads.py b/submit-api/migrations/versions/d4e5f6a7b8c9_add_is_approved_to_geo_uploads.py new file mode 100644 index 000000000..c0414ad4b --- /dev/null +++ b/submit-api/migrations/versions/d4e5f6a7b8c9_add_is_approved_to_geo_uploads.py @@ -0,0 +1,30 @@ +"""Add is_approved column to geo_data_uploads + +Revision ID: d4e5f6a7b8c9 +Revises: 6319d2b3933e +Create Date: 2026-07-07 00:00:00.000000 + +Adds a boolean flag recording whether the proponent explicitly approved the +geospatial file in the preview modal. Existing rows default to False so that +previously uploaded (un-reviewed) files must be re-reviewed before the item +can be completed. +""" +from alembic import op +import sqlalchemy as sa + + +revision = 'd4e5f6a7b8c9' +down_revision = '6319d2b3933e' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + 'geo_data_uploads', + sa.Column('is_approved', sa.Boolean(), nullable=False, server_default=sa.false()), + ) + + +def downgrade(): + op.drop_column('geo_data_uploads', 'is_approved') diff --git a/submit-api/src/submit_api/models/geo_data_upload.py b/submit-api/src/submit_api/models/geo_data_upload.py index dfafcb46c..68cf5dbb7 100644 --- a/submit-api/src/submit_api/models/geo_data_upload.py +++ b/submit-api/src/submit_api/models/geo_data_upload.py @@ -19,7 +19,7 @@ from datetime import datetime, UTC -from sqlalchemy import Column, DateTime, Float, Integer, JSON, String, Text +from sqlalchemy import Boolean, Column, DateTime, Float, Integer, JSON, String, Text from .base_model import BaseModel @@ -37,6 +37,7 @@ class GeoDataUpload(BaseModel): # pylint: disable=too-many-instance-attributes preview_s3_key = Column(String(512), nullable=True) standard_s3_key = Column(String(512), nullable=True) status = Column(String(20), nullable=False, default='processing') + is_approved = Column(Boolean, nullable=False, default=False) error_message = Column(Text, nullable=True) feature_count = Column(Integer, nullable=True) geometry_type = Column(String(50), nullable=True) @@ -62,6 +63,7 @@ def __init__( # pylint: disable=too-many-arguments preview_s3_key: str | None = None, standard_s3_key: str | None = None, status: str = 'processing', + is_approved: bool = False, error_message: str | None = None, feature_count: int | None = None, geometry_type: str | None = None, @@ -78,6 +80,7 @@ def __init__( # pylint: disable=too-many-arguments self.preview_s3_key = preview_s3_key self.standard_s3_key = standard_s3_key self.status = status + self.is_approved = is_approved self.error_message = error_message self.feature_count = feature_count self.geometry_type = geometry_type @@ -96,6 +99,7 @@ def to_dict(self): 'preview_s3_key': self.preview_s3_key, 'standard_s3_key': self.standard_s3_key, 'status': self.status, + 'is_approved': self.is_approved, 'error_message': self.error_message, 'feature_count': self.feature_count, 'geometry_type': self.geometry_type, diff --git a/submit-api/src/submit_api/resources/geo.py b/submit-api/src/submit_api/resources/geo.py index ea54e85c2..a2a72e1b2 100644 --- a/submit-api/src/submit_api/resources/geo.py +++ b/submit-api/src/submit_api/resources/geo.py @@ -144,6 +144,26 @@ def get(upload_id): return result, HTTPStatus.OK +@cors_preflight("POST, OPTIONS") +@API.route("//approve", methods=["POST", "OPTIONS"]) +class GeoUploadApprove(Resource): + """Resource for approving a processed upload.""" + + @staticmethod + @cross_origin(origins=allowedorigins()) + @auth.require + def post(upload_id): + """Mark a ready upload as approved by the proponent.""" + try: + upload = GeoService.approve_upload(upload_id) + except LookupError as exc: + return {"error": str(exc)}, HTTPStatus.NOT_FOUND + except ValueError as exc: + return {"error": str(exc)}, HTTPStatus.BAD_REQUEST + + return {"id": upload.id, "status": upload.status, "is_approved": upload.is_approved}, HTTPStatus.OK + + @cors_preflight("POST, OPTIONS") @API.route("//retry", methods=["POST", "OPTIONS"]) class GeoUploadRetry(Resource): diff --git a/submit-api/src/submit_api/services/geo/processor.py b/submit-api/src/submit_api/services/geo/processor.py index c278a158a..b44a4f2c7 100644 --- a/submit-api/src/submit_api/services/geo/processor.py +++ b/submit-api/src/submit_api/services/geo/processor.py @@ -579,6 +579,46 @@ def get_presigned_read_url(cls, upload_id: int, tier: str = "preview") -> dict: "crs_original": upload.crs_original, } + @classmethod + def approve_upload(cls, upload_id: int) -> GeoDataUpload: + """Mark a processed upload as approved by the proponent. + + Raises: + LookupError: if the upload does not exist. + ValueError: if the upload is not in a 'ready' state. + """ + upload = cls.get_upload(upload_id) + if not upload: + raise LookupError(f"GeoDataUpload {upload_id} not found.") + if upload.status != "ready": + raise ValueError( + f"Only a ready upload can be approved (status: {upload.status})." + ) + + upload.is_approved = True + db.session.commit() + return upload + + @classmethod + def has_unapproved_uploads(cls, item_id: int) -> bool: + """Return True if the item has any geospatial upload not yet approved. + + Covers uploads still processing, failed, or ready-but-unapproved. Used to + block completing a submission item until every file has been reviewed. + """ + uploads = cls.list_uploads(item_id=item_id) + return any(not upload.is_approved for upload in uploads) + + @classmethod + def has_unapproved_uploads_for_package(cls, package_id: int) -> bool: + """Return True if any geospatial upload in the package is not yet approved. + + Used to block submitting a package to the EAO until every geospatial file + across all of its items has been reviewed and approved. + """ + uploads = cls.list_uploads(package_id=package_id) + return any(not upload.is_approved for upload in uploads) + @classmethod def retry_upload(cls, app, upload_id: int) -> GeoDataUpload: """Reset a failed upload and re-trigger processing. @@ -594,6 +634,7 @@ def retry_upload(cls, app, upload_id: int) -> GeoDataUpload: raise ValueError("Only failed uploads can be retried.") upload.status = "processing" + upload.is_approved = False upload.error_message = None upload.validation_errors = None db.session.commit() diff --git a/submit-api/src/submit_api/services/package_service.py b/submit-api/src/submit_api/services/package_service.py index 5f5c9994e..0628fdf34 100644 --- a/submit-api/src/submit_api/services/package_service.py +++ b/submit-api/src/submit_api/services/package_service.py @@ -32,6 +32,7 @@ from submit_api.models.user import UserType from submit_api.services import authorization from submit_api.services.activity_log_service import ActivityLogService +from submit_api.services.geo import GeoService from submit_api.utils.constants import ( MANAGEMENT_PLAN_SUBMISSION_CONFIRMATION_EMAIL_TEMPLATE, MANAGEMENT_PLAN_UPDATE_REQUEST_CREATED_EMAIL_TEMPLATE, MANAGEMENT_PLAN_SUBMISSION_NOTIFY_STAFF_EMAIL_TEMPLATE, MANAGEMENT_PLAN_RESUBMISSION_REQUEST_EMAIL_TEMPLATE) @@ -645,6 +646,11 @@ def submit_package(cls, package_id): permissions=[ProponentPermissionsEnum.SUBMIT_PACKAGE.value], account_project_ids=[package.account_project_id] ) + if GeoService.has_unapproved_uploads_for_package(package_id): + raise BadRequestError( + "All geospatial files must be reviewed and approved before " + "submitting the package to the EAO." + ) if package.submitted_on: submitted_package: PackageModel = cls._resubmit_package(package, session) else: diff --git a/submit-api/src/submit_api/services/submission/__init__.py b/submit-api/src/submit_api/services/submission/__init__.py index 13f6c67e1..510c55486 100644 --- a/submit-api/src/submit_api/services/submission/__init__.py +++ b/submit-api/src/submit_api/services/submission/__init__.py @@ -231,9 +231,24 @@ def update_submission_item_status(cls, item_id, status, session): # Validate required document submissions before marking as COMPLETED if status == ItemStatus.COMPLETED.value: cls._validate_required_document_submission(item_id) + cls._validate_geospatial_uploads_approved(item_id) ItemService.update_submission_item_status(item_id, status, session) + @classmethod + def _validate_geospatial_uploads_approved(cls, item_id): + """Block completion while any geospatial upload for the item is unapproved. + + Prevents bypassing the map-preview approve/reject step by navigating away + (back/refresh) while a file is still processing. Items without geospatial + uploads are unaffected. + """ + if GeoService.has_unapproved_uploads(item_id): + raise BadRequestError( + "Cannot mark item as COMPLETED. All geospatial files must be " + "reviewed and approved before submitting." + ) + @classmethod def _validate_required_document_submission(cls, item_id): """Validate that required document items have at least one document submission.""" diff --git a/submit-web/src/components/App/DocumentUpload/DocumentTableRow.tsx b/submit-web/src/components/App/DocumentUpload/DocumentTableRow.tsx index 260643606..afb8b2ef2 100644 --- a/submit-web/src/components/App/DocumentUpload/DocumentTableRow.tsx +++ b/submit-web/src/components/App/DocumentUpload/DocumentTableRow.tsx @@ -1,5 +1,6 @@ import React, { useState } from "react"; import { + Box, styled, TableCell, TableRow, @@ -18,6 +19,7 @@ import { useFormContext } from "react-hook-form"; import { useParams } from "@tanstack/react-router"; import { getObjectFromS3 } from "@/components/Shared/Table/utils"; import { DocumentLink } from "@/components/Shared/DocumentLink"; +import { GeoApprovedBadge } from "@/components/Shared/GeoApprovedBadge"; import { useGetGeoUploads } from "@/hooks/api/useGeo"; export const StyledHeadTableCell = styled(TableCell, { @@ -177,28 +179,42 @@ export default function DocumentTableRow({ error={error} > - - onDocumentClick(documentItem) - : downloadDocument - } - loading={pendingGetObject} + + onDocumentClick(documentItem) + : downloadDocument + } + loading={pendingGetObject} + /> + + - + {submitted_by} {version} diff --git a/submit-web/src/components/App/Map/MapPreviewModal.tsx b/submit-web/src/components/App/Map/MapPreviewModal.tsx index 3e6deb64b..66a0f66b5 100644 --- a/submit-web/src/components/App/Map/MapPreviewModal.tsx +++ b/submit-web/src/components/App/Map/MapPreviewModal.tsx @@ -38,8 +38,10 @@ interface MapPreviewModalProps { fileSizeKb?: number; status?: string; errorMessage?: string; + isApproved?: boolean; onApprove: () => void; onReject: () => Promise; + onClose?: () => void; } export const MapPreviewModal: React.FC = ({ @@ -49,8 +51,10 @@ export const MapPreviewModal: React.FC = ({ fileSizeKb, status, errorMessage, + isApproved, onApprove, onReject, + onClose, }) => { const mapRef = useRef(null); const [loading, setLoading] = useState(false); @@ -160,6 +164,13 @@ export const MapPreviewModal: React.FC = ({ status === "failed" && (errorMessage?.toLowerCase().includes("timed out") ?? false); + const [displayApproved, setDisplayApproved] = useState(false); + useEffect(() => { + if (open) { + setDisplayApproved(Boolean(isApproved)); + } + }, [open, isApproved]); + const handleReject = async () => { setIsRejecting(true); try { @@ -567,14 +578,22 @@ export const MapPreviewModal: React.FC = ({ > Reject - + {displayApproved ? ( + + ) : ( + + )} ); diff --git a/submit-web/src/components/App/Submission/DocumentRow/index.tsx b/submit-web/src/components/App/Submission/DocumentRow/index.tsx index df4ae124a..bb7db6e6f 100644 --- a/submit-web/src/components/App/Submission/DocumentRow/index.tsx +++ b/submit-web/src/components/App/Submission/DocumentRow/index.tsx @@ -16,6 +16,7 @@ import { ActionButton } from "./ActionButton"; import PermissionsGate from "@/components/Shared/PermissionGate"; import { SubmissionPackage, PackageType } from "@/models/Package"; import { DocumentLink } from "@/components/Shared/DocumentLink"; +import { GeoApprovedBadge } from "@/components/Shared/GeoApprovedBadge"; import ActionSplitButton, { SplitButtonAction, } from "@/components/Shared/ActionSplitButton/ActionSplitButton"; @@ -148,32 +149,46 @@ export default function DocumentRow({ ]} > - - {staff ? ( - - - - ) : ( - - )} - + + {staff ? ( + + + + ) : ( + + )} + + + {submitted_by || ""} diff --git a/submit-web/src/components/App/SubmissionItem/DocumentsTable/Row.tsx b/submit-web/src/components/App/SubmissionItem/DocumentsTable/Row.tsx index 3c5551f7a..602920baf 100644 --- a/submit-web/src/components/App/SubmissionItem/DocumentsTable/Row.tsx +++ b/submit-web/src/components/App/SubmissionItem/DocumentsTable/Row.tsx @@ -1,4 +1,4 @@ -import { TableRow, IconButton, Typography } from "@mui/material"; +import { Box, TableRow, IconButton, Typography } from "@mui/material"; import { StatusChip } from "@/components/Shared/StatusChip"; import { Submission, @@ -18,6 +18,7 @@ import { deleteDocument, saveObject } from "@/hooks/api/useObjectStorage"; import { FileUploadButton } from "@/components/Shared/FileUploadButton"; import { isAxiosError } from "axios"; import { DocumentLink } from "@/components/Shared/DocumentLink"; +import { GeoApprovedBadge } from "@/components/Shared/GeoApprovedBadge"; import { LoadingButton } from "@/components/Shared/LoadingButton"; import { BCDesignTokens } from "epic.theme"; import { useFileStore } from "@/store/fileStore"; @@ -210,11 +211,25 @@ export default function Row({ <> - + + + + {submitted_by || ""} diff --git a/submit-web/src/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialProponentView.tsx b/submit-web/src/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialProponentView.tsx index 268b93760..b88d86062 100644 --- a/submit-web/src/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialProponentView.tsx +++ b/submit-web/src/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialProponentView.tsx @@ -8,7 +8,7 @@ import { GenericDocumentUploadSection, UploadSectionConfig, } from "@/components/App/DocumentUpload/GenericDocumentUploadSection"; -import { useCallback, useMemo, useState, lazy, Suspense } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState, lazy, Suspense } from "react"; import Form from "@/components/Shared/Forms/common"; import { FormProvider, useForm } from "react-hook-form"; import { Submission } from "@/models/Submission"; @@ -32,7 +32,7 @@ import { useGetSubmissionPackage } from "@/hooks/api/usePackages"; import { notify } from "@/components/Shared/Snackbar/snackbarStore"; import { isAxiosError } from "axios"; import SubmissionActionButtons from "@/components/App/SubmissionItem/SubmissionActionButtons"; -import { useGetGeoUploads, GeoUpload } from "@/hooks/api/useGeo"; +import { useGetGeoUploads, useApproveGeoUpload, GeoUpload } from "@/hooks/api/useGeo"; import { useFileStore } from "@/store/fileStore"; // Lazy-load the map modal so maplibre-gl is not downloaded until first use const MapPreviewModal = lazy(() => @@ -88,6 +88,7 @@ export const GeoSpatialProponentView = () => { const { mutateAsync: deleteSubmissionAsync } = useDeleteSubmission({ submissionItemId: Number(submissionItemId), }); + const { mutateAsync: approveGeoUpload } = useApproveGeoUpload(); const { removeFile } = useFileStore(); const documentSubmissions = submissionItem?.submissions?.filter( @@ -181,9 +182,20 @@ export const GeoSpatialProponentView = () => { [previewDocument], ); - const handleApprove = useCallback(() => { + const handleApprove = useCallback(async () => { + // Persist approval server-side so navigating away (back/refresh) cannot + // bypass the review step — the item can't be completed until every upload + // is approved. + if (previewUpload) { + try { + await approveGeoUpload(previewUpload.id); + } catch { + notify.error("Failed to approve geospatial file"); + return; + } + } openNextInQueue(reviewQueue); - }, [reviewQueue, openNextInQueue]); + }, [previewUpload, reviewQueue, openNextInQueue, approveGeoUpload]); const handleReject = useCallback(async () => { if (!previewDocument) return; @@ -220,8 +232,47 @@ export const GeoSpatialProponentView = () => { openNextInQueue, ]); - // Save & Exit is blocked while the review modal is open or files are queued - const isReviewPending = previewDocument !== null || reviewQueue.length > 0; + // On return to the page (e.g. after a back/refresh during processing), re-open + // the review modal for any upload the proponent never approved. Runs once per + // mount; new uploads within the session are handled by onUploadComplete. + const didSeedReview = useRef(false); + useEffect(() => { + if (didSeedReview.current) return; + if (!uploads || !documentSubmissions) return; + + const unapproved = uploads.filter((u) => !u.is_approved); + if (unapproved.length === 0) { + didSeedReview.current = true; + return; + } + + const docs = unapproved + .map((upload) => + documentSubmissions.find( + (s) => s.submitted_document?.url === upload.raw_s3_key, + ), + ) + .filter((d): d is Submission => Boolean(d)); + + // Wait until the matching document submissions are available in cache. + if (docs.length < unapproved.length) return; + + didSeedReview.current = true; + const [first, ...rest] = docs; + setPreviewDocument(first); + setReviewQueue(rest); + }, [uploads, documentSubmissions]); + + // Save & Exit is blocked while a review is open/queued, or any upload for this + // item is still unapproved (the server enforces the same rule on submit). + const hasUnapprovedUploads = useMemo( + () => (uploads ?? []).some((u) => !u.is_approved), + [uploads], + ); + const isReviewPending = + previewDocument !== null || + reviewQueue.length > 0 || + hasUnapprovedUploads; const documentUploadSections: UploadSectionConfig[] = useMemo( () => [ @@ -283,8 +334,10 @@ export const GeoSpatialProponentView = () => { (previewDocument !== null ? "processing" : undefined) } errorMessage={previewUpload?.error_message} + isApproved={previewUpload?.is_approved} onApprove={handleApprove} onReject={handleReject} + onClose={() => openNextInQueue(reviewQueue)} /> diff --git a/submit-web/src/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialUpdateForm/index.tsx b/submit-web/src/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialUpdateForm/index.tsx index 1440016fa..08e9c955f 100644 --- a/submit-web/src/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialUpdateForm/index.tsx +++ b/submit-web/src/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialUpdateForm/index.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState, lazy, Suspense } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState, lazy, Suspense } from "react"; import { Box, Button, Typography, Grid } from "@mui/material"; import { SubmissionFormContainer } from "@/components/App/SubmissionItem/SubmissionFormContainer"; import { useNavigate, useParams } from "@tanstack/react-router"; @@ -6,8 +6,9 @@ import { deleteDocument, S3_FOLDER } from "@/hooks/api/useObjectStorage"; import DocumentsTable from "@/components/App/SubmissionItem/DocumentsTable"; import { UnfinishedUploadsCheck } from "@/components/Shared/UnfinishedUploadsCheck"; import { BCDesignTokens } from "epic.theme"; -import { useGetGeoUploads, GeoUpload } from "@/hooks/api/useGeo"; -import { Submission } from "@/models/Submission"; +import { useGetGeoUploads, useApproveGeoUpload, GeoUpload } from "@/hooks/api/useGeo"; +import { Submission, SUBMISSION_TYPE } from "@/models/Submission"; +import { SubmissionItem } from "@/models/SubmissionItem"; import { useQueryClient } from "@tanstack/react-query"; import { QUERY_KEY } from "@/hooks/api/constants"; import { notify } from "@/components/Shared/Snackbar/snackbarStore"; @@ -31,6 +32,7 @@ export const GeoSpatialUpdateForm = () => { const queryClient = useQueryClient(); const [isPendingUpload, setIsPendingUpload] = useState(false); const [previewDocument, setPreviewDocument] = useState(null); + const [reviewQueue, setReviewQueue] = useState([]); const { data: geoUploads } = useGetGeoUploads({ itemId: Number(submissionItemId), @@ -38,6 +40,23 @@ export const GeoSpatialUpdateForm = () => { }); const uploads = geoUploads as unknown as GeoUpload[]; + const { mutateAsync: approveGeoUpload } = useApproveGeoUpload(); + + const submissionItem = queryClient.getQueryData([ + QUERY_KEY.SUBMISSION_ITEM, + Number(submissionItemId), + ]); + const documentSubmissions = submissionItem?.submissions?.filter( + (submission) => submission.type === SUBMISSION_TYPE.DOCUMENT, + ); + + // Advances to the next file awaiting review + const openNextInQueue = useCallback((queue: Submission[]) => { + const [next, ...remaining] = queue; + setReviewQueue(remaining); + setPreviewDocument(next ?? null); + }, []); + // Derive previewUpload reactively so the modal updates as polling resolves status const previewUpload = useMemo(() => { if (!previewDocument || !uploads) return null; @@ -78,11 +97,24 @@ export const GeoSpatialUpdateForm = () => { // Called by AddDocumentActionButton once a geo file finishes uploading const onUploadComplete = (submission: Submission) => { - setPreviewDocument(submission); + if (previewDocument !== null) { + setReviewQueue((prev) => [...prev, submission]); + } else { + setPreviewDocument(submission); + } }; - const handleApprove = () => { - setPreviewDocument(null); + const handleApprove = async () => { + // Persist approval so back/refresh cannot bypass the review step. + if (previewUpload) { + try { + await approveGeoUpload(previewUpload.id); + } catch { + notify.error("Failed to approve geospatial file."); + return; + } + } + openNextInQueue(reviewQueue); }; const handleReject = async () => { @@ -99,11 +131,42 @@ export const GeoSpatialUpdateForm = () => { } catch { notify.error("Failed to remove geospatial file."); } finally { - setPreviewDocument(null); + openNextInQueue(reviewQueue); } }; - const isBlockedFromExit = isPendingUpload || previewDocument !== null; + // Re-open review for any upload left unapproved after a back/refresh. Runs + // once per mount; in-session uploads are handled by onUploadComplete. + const didSeedReview = useRef(false); + useEffect(() => { + if (didSeedReview.current) return; + if (!uploads || !documentSubmissions) return; + + const unapproved = uploads.filter((u) => !u.is_approved); + if (unapproved.length === 0) { + didSeedReview.current = true; + return; + } + + const docs = unapproved + .map((upload) => + documentSubmissions.find( + (s) => s.submitted_document?.url === upload.raw_s3_key, + ), + ) + .filter((d): d is Submission => Boolean(d)); + + if (docs.length < unapproved.length) return; + + didSeedReview.current = true; + const [first, ...rest] = docs; + setPreviewDocument(first); + setReviewQueue(rest); + }, [uploads, documentSubmissions]); + + const hasUnapprovedUploads = (uploads ?? []).some((u) => !u.is_approved); + const isBlockedFromExit = + isPendingUpload || previewDocument !== null || hasUnapprovedUploads; return ( @@ -147,8 +210,10 @@ export const GeoSpatialUpdateForm = () => { (previewDocument !== null ? "processing" : undefined) } errorMessage={previewUpload?.error_message} + isApproved={previewUpload?.is_approved} onApprove={handleApprove} onReject={handleReject} + onClose={() => openNextInQueue(reviewQueue)} /> diff --git a/submit-web/src/components/Shared/FileUpload/index.tsx b/submit-web/src/components/Shared/FileUpload/index.tsx index e403b3b55..34923849a 100644 --- a/submit-web/src/components/Shared/FileUpload/index.tsx +++ b/submit-web/src/components/Shared/FileUpload/index.tsx @@ -9,6 +9,7 @@ export type FileUploadProps = { accept?: Accept; onDrop: (acceptedFiles: File[]) => void; error?: boolean; + maxSize?: number; maxFiles?: number; maxFilesErrorMessage?: string; currentFileCount?: number; @@ -25,6 +26,7 @@ export const FileUpload = ({ }, error = false, onDrop, + maxSize, maxFiles, maxFilesErrorMessage, currentFileCount @@ -35,6 +37,7 @@ export const FileUpload = ({ accept={accept} onDrop={onDrop} error={error} + maxSize={maxSize} maxFiles={maxFiles} maxFilesErrorMessage={maxFilesErrorMessage} currentFileCount={currentFileCount} diff --git a/submit-web/src/components/Shared/GeoApprovedBadge.tsx b/submit-web/src/components/Shared/GeoApprovedBadge.tsx new file mode 100644 index 000000000..07a8ca0e9 --- /dev/null +++ b/submit-web/src/components/Shared/GeoApprovedBadge.tsx @@ -0,0 +1,44 @@ +import CheckCircleOutlinedIcon from "@mui/icons-material/CheckCircleOutlined"; +import { Tooltip } from "@mui/material"; +import { S3_FOLDER } from "@/hooks/api/useObjectStorage"; +import { useGetGeoUploads, GeoUpload } from "@/hooks/api/useGeo"; + +type GeoApprovedBadgeProps = Readonly<{ + itemId?: number; + url?: string; + folder?: string; +}>; + +/** + * Small green check badge shown next to a geospatial file's name once the + * proponent has approved it. Renders nothing for non-geospatial files or files + * that are not yet approved. Fetches geo uploads by item; React Query dedupes + * the request across rows of the same item. + */ +export const GeoApprovedBadge = ({ itemId, url, folder }: GeoApprovedBadgeProps) => { + const isGeoSpatial = folder === S3_FOLDER.GEOSPATIAL.value; + + const { data: geoUploads } = useGetGeoUploads( + { itemId }, + { enabled: isGeoSpatial && Boolean(itemId) }, + ); + + if (!isGeoSpatial) return null; + + const upload = (geoUploads as GeoUpload[] | undefined)?.find( + (u) => u.raw_s3_key === url, + ); + if (!upload?.is_approved) return null; + + return ( + + + + ); +}; + +export default GeoApprovedBadge; diff --git a/submit-web/src/routes/proponent/_proponentLayout/projects/$projectId/_projectLayout/submission-packages/$submissionPackageId/_submissionLayout/index.tsx b/submit-web/src/routes/proponent/_proponentLayout/projects/$projectId/_projectLayout/submission-packages/$submissionPackageId/_submissionLayout/index.tsx index 02b3c59e9..e0a319cc6 100644 --- a/submit-web/src/routes/proponent/_proponentLayout/projects/$projectId/_projectLayout/submission-packages/$submissionPackageId/_submissionLayout/index.tsx +++ b/submit-web/src/routes/proponent/_proponentLayout/projects/$projectId/_projectLayout/submission-packages/$submissionPackageId/_submissionLayout/index.tsx @@ -268,6 +268,17 @@ export default function SubmissionPage() { return; } + const hasUnapprovedGeoFiles = (geoUploads as GeoUpload[] | undefined)?.some( + (u: GeoUpload) => !u.is_approved, + ); + if (hasUnapprovedGeoFiles) { + setIsValidating(true); + notify.warning( + "One or more geospatial files have not been approved. Please open each geospatial file, review the preview, and approve it before submitting.", + ); + return; + } + // Check for unaddressed update request sections const unaddressed = getUnaddressedUpdateRequestSections( submissionPackage, @@ -313,7 +324,7 @@ export default function SubmissionPage() { (updateRequest) => (updateRequest.status === UPDATE_REQUEST_STATUS.OPEN.value || updateRequest.status === - UPDATE_REQUEST_STATUS.PENDING_REVIEW.value) && + UPDATE_REQUEST_STATUS.PENDING_REVIEW.value) && updateRequest.active, ); @@ -436,12 +447,12 @@ export default function SubmissionPage() { const isWithdrawDisabled = useMemo(() => { // Disable if versioning is not enabled if (!submissionPackage?.type?.versioning_enabled) return true; - + if (!submissionPackage?.submitted_on) return true; - + // Disable if already withdrawn if (isPackageWithdrawn) return true; - + // Disable if in terminal states if ( submissionPackage.status.includes(PACKAGE_STATUS.APPROVED.value) || @@ -450,12 +461,12 @@ export default function SubmissionPage() { ) { return true; } - + // Enable if in submitted or acknowledged status const isInWithdrawableStatus = submissionPackage.status.includes(PACKAGE_STATUS.SUBMITTED.value) || submissionPackage.status.includes(PACKAGE_STATUS.ACKNOWLEDGED.value); - + return !isInWithdrawableStatus; }, [submissionPackage, isPackageWithdrawn]); @@ -598,7 +609,7 @@ export default function SubmissionPage() { condition={ isValidating && submissionPackage.type.name === - SubmissionPackageType.ADDITIONAL_INFORMATION && + SubmissionPackageType.ADDITIONAL_INFORMATION && !hasDocuments } > From 76734185e66f2d51f081a01717786aed9d4c71a8 Mon Sep 17 00:00:00 2001 From: Nitheesh T Ganesh Date: Wed, 8 Jul 2026 11:27:23 -0600 Subject: [PATCH 3/6] geo --- submit-web/src/hooks/api/useGeo.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/submit-web/src/hooks/api/useGeo.ts b/submit-web/src/hooks/api/useGeo.ts index 9acd53d64..bafdbb460 100644 --- a/submit-web/src/hooks/api/useGeo.ts +++ b/submit-web/src/hooks/api/useGeo.ts @@ -18,6 +18,7 @@ export interface GeoUpload { file_type: string; file_size_kb: number; status: string; + is_approved: boolean; feature_count?: number; geometry_type?: string; crs_original?: string; @@ -94,6 +95,30 @@ const retryGeoUpload = (uploadId: number) => { }); }; +/** + * Hook to approve a processed geospatial upload. + */ +export const useApproveGeoUpload = (options?: Options) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (uploadId: number) => approveGeoUpload(uploadId), + onSuccess: (data) => { + queryClient.invalidateQueries({ queryKey: [QUERY_KEY.GEO_UPLOADS] }); + if (options?.onSuccess) { + options.onSuccess(data); + } + }, + ...options, + }); +}; + +const approveGeoUpload = (uploadId: number) => { + return submitRequest({ + url: `/geo/uploads/${uploadId}/approve`, + method: "post", + }); +}; + /** * Hook to create a new geospatial upload and trigger processing. */ From bae31e23525a4691175c0f2efdf66b10297f9be0 Mon Sep 17 00:00:00 2001 From: Nitheesh T Ganesh Date: Thu, 9 Jul 2026 11:39:04 -0600 Subject: [PATCH 4/6] refactor: improve geo-spatial validation error messaging and update UI to display detailed validation results. --- ...f6a7b8c9_add_is_approved_to_geo_uploads.py | 4 +- .../src/submit_api/services/geo/processor.py | 52 ++++- .../services/geo/validation_rules.py | 7 + .../src/submit_api/services/geo/validator.py | 86 +++++++- .../tests/unit/services/test_geo_validator.py | 75 ++++++- .../App/DocumentUpload/DocumentTable.tsx | 6 +- .../App/DocumentUpload/DocumentTableRow.tsx | 35 ---- .../App/DocumentUpload/PendingDocumentRow.tsx | 7 - .../components/App/Map/MapPreviewModal.tsx | 188 ++++++++++++++---- .../App/Submission/DocumentRow/index.tsx | 1 + .../Submission/ItemsTable/ItemsTableHead.tsx | 2 +- .../App/SubmissionItem/DocumentsTable/Row.tsx | 43 +--- .../SubmissionItem/DocumentsTable/index.tsx | 5 +- .../GeoSpatialGuidelines.tsx | 5 +- .../GeoSpatialProponentView.tsx | 1 + .../GeoSpatialStaffView.tsx | 1 + 16 files changed, 368 insertions(+), 150 deletions(-) diff --git a/submit-api/migrations/versions/d4e5f6a7b8c9_add_is_approved_to_geo_uploads.py b/submit-api/migrations/versions/d4e5f6a7b8c9_add_is_approved_to_geo_uploads.py index c0414ad4b..31f96b23f 100644 --- a/submit-api/migrations/versions/d4e5f6a7b8c9_add_is_approved_to_geo_uploads.py +++ b/submit-api/migrations/versions/d4e5f6a7b8c9_add_is_approved_to_geo_uploads.py @@ -1,7 +1,7 @@ """Add is_approved column to geo_data_uploads Revision ID: d4e5f6a7b8c9 -Revises: 6319d2b3933e +Revises: 8bc9fafd24f1 Create Date: 2026-07-07 00:00:00.000000 Adds a boolean flag recording whether the proponent explicitly approved the @@ -14,7 +14,7 @@ revision = 'd4e5f6a7b8c9' -down_revision = '6319d2b3933e' +down_revision = '8bc9fafd24f1' branch_labels = None depends_on = None diff --git a/submit-api/src/submit_api/services/geo/processor.py b/submit-api/src/submit_api/services/geo/processor.py index df3f51f5f..8ded5880d 100644 --- a/submit-api/src/submit_api/services/geo/processor.py +++ b/submit-api/src/submit_api/services/geo/processor.py @@ -344,8 +344,8 @@ class GeoService: # -- Background worker --------------------------------------------------- - @staticmethod - def _validate_or_fail(upload: GeoDataUpload, local_path: str) -> bool: + @classmethod + def _validate_or_fail(cls, upload: GeoDataUpload, local_path: str) -> bool: """Run attribute validation, persisting a failure state if invalid. Returns True if the upload passed validation (or validation is skipped), @@ -366,21 +366,51 @@ def _validate_or_fail(upload: GeoDataUpload, local_path: str) -> bool: ) upload.status = "validation_failed" upload.validation_errors = errors + upload.error_message = cls._build_validation_message(errors, summary) + db.session.commit() + return False + + @staticmethod + def _build_validation_message(errors: list[dict], summary: Dict[str, Any]) -> str: + """Compose a human-readable summary covering each distinct failure type. + + The structured ``validation_errors`` list carries the per-feature detail; + this string is the at-a-glance headline shown to the proponent. + """ + def _count(*types: str) -> int: + return sum(1 for e in errors if e.get("error_type") in types) + missing_cols = [ e["dbf_column"] for e in errors if e.get("error_type") == "missing_column" ] + null_geom = _count("null_geometry") + self_int = _count("self_intersection") + invalid_geom = _count("invalid_geometry") + attr_issues = _count("missing_value", "invalid_code") + + parts: list[str] = [] if missing_cols: - upload.error_message = ( - "The shapefile is missing required column(s): " - f"{', '.join(missing_cols)}." + parts.append(f"missing required column(s): {', '.join(missing_cols)}") + if null_geom: + parts.append(f"{null_geom} feature(s) with missing or empty geometry") + if self_int: + parts.append(f"{self_int} self-intersecting polygon(s)") + if invalid_geom: + parts.append(f"{invalid_geom} feature(s) with invalid geometry") + if attr_issues: + parts.append( + f"{attr_issues} attribute value issue(s) in: " + f"{', '.join(summary['fields_with_errors']) or 'N/A'}" ) - else: - upload.error_message = ( - f"Attribute validation failed with {summary['total_errors']} issue(s) in: " - f"{', '.join(summary['fields_with_errors']) or 'N/A'}." + + if not parts: + parts.append( + f"{summary['total_errors']} issue(s) in: " + f"{', '.join(summary['fields_with_errors']) or 'N/A'}" ) - db.session.commit() - return False + + prefix = "Capped at first " + str(len(errors)) + " issue(s). " if summary.get("capped") else "" + return prefix + "Validation failed: " + "; ".join(parts) + "." @staticmethod def _upload_processed_tiers(upload: GeoDataUpload, result: Dict[str, Any]) -> None: diff --git a/submit-api/src/submit_api/services/geo/validation_rules.py b/submit-api/src/submit_api/services/geo/validation_rules.py index 16d700ce3..a6e3ecef2 100644 --- a/submit-api/src/submit_api/services/geo/validation_rules.py +++ b/submit-api/src/submit_api/services/geo/validation_rules.py @@ -42,6 +42,13 @@ # back on without rework once the spec is confirmed. VALIDATE_VALUES: bool = False +# Master switch for per-feature geometry validation (null/empty geometry and +# invalid topology such as self-intersecting polygons). Independent of +# VALIDATE_VALUES because geometry integrity does not depend on the attribute +# allowed-value spec. Set False to fall back to the old behaviour of silently +# dropping bad geometries during processing. +VALIDATE_GEOMETRY: bool = True + ALLOWED_VALUES: dict[str, set[str]] = { "category": {"PN", "EAC", "PD"}, "sub_category": {"Pre EA", "EE", "RD", "PP", "ADR", "EAR", "P", "C", "O", "CM", "D", "A", "N"}, diff --git a/submit-api/src/submit_api/services/geo/validator.py b/submit-api/src/submit_api/services/geo/validator.py index 0e891e74f..6ecc836ae 100644 --- a/submit-api/src/submit_api/services/geo/validator.py +++ b/submit-api/src/submit_api/services/geo/validator.py @@ -20,7 +20,8 @@ Both return (is_valid: bool, errors: list[dict], summary: dict). Error dict keys: feature_index, field, dbf_column, error_type, value, message. -Error types: missing_column | missing_value | invalid_code | no_shapefile. +Error types: missing_column | missing_value | invalid_code | no_shapefile | + null_geometry | self_intersection | invalid_geometry. Fiona is used for streaming so the entire feature set is never held in memory. """ @@ -33,6 +34,8 @@ import zipfile import fiona +from shapely.geometry import shape +from shapely.validation import explain_validity from submit_api.services.geo.validation_rules import ( ALLOWED_VALUES, @@ -41,6 +44,7 @@ MAX_ERRORS, NULLABLE_FIELDS, OPTIONAL_FIELDS, + VALIDATE_GEOMETRY, VALIDATE_VALUES, ) @@ -98,6 +102,67 @@ def _validate_feature(idx: int, props: dict) -> _ErrorList: return feature_errors +def _validate_geometry(idx: int, geom: dict | None) -> _ErrorList: + """Validate a single feature's geometry. + + Detects three distinct failures and tags each with its own ``error_type``: + - ``null_geometry`` — geometry is absent or empty. + - ``self_intersection`` — invalid topology caused by a self-intersection. + - ``invalid_geometry`` — any other invalid topology (ring order, etc.). + + Returns a list of error dicts (empty if the geometry is valid). + """ + if geom is None: + return [{ + "feature_index": idx, + "field": "geometry", + "dbf_column": None, + "error_type": "null_geometry", + "value": None, + "message": f"Feature {idx}: geometry is missing (null).", + }] + + try: + shp = shape(geom) + except (TypeError, ValueError, AttributeError) as exc: + return [{ + "feature_index": idx, + "field": "geometry", + "dbf_column": None, + "error_type": "invalid_geometry", + "value": None, + "message": f"Feature {idx}: geometry could not be parsed ({exc}).", + }] + + if shp.is_empty: + return [{ + "feature_index": idx, + "field": "geometry", + "dbf_column": None, + "error_type": "null_geometry", + "value": None, + "message": f"Feature {idx}: geometry is empty.", + }] + + if not shp.is_valid: + reason = explain_validity(shp) + is_self_intersection = "self-intersection" in reason.lower() + return [{ + "feature_index": idx, + "field": "geometry", + "dbf_column": None, + "error_type": "self_intersection" if is_self_intersection else "invalid_geometry", + "value": None, + "message": ( + f"Feature {idx}: polygon is self-intersecting ({reason})." + if is_self_intersection + else f"Feature {idx}: geometry is invalid ({reason})." + ), + }] + + return [] + + def validate_shapefile(shp_path: str) -> _Result: """Validate a single shapefile's attributes against FIELD_MAP rules. @@ -130,9 +195,11 @@ def validate_shapefile(shp_path: str) -> _Result: "fields_with_errors": [e["field"] for e in missing], } - # Value/nullability checks are gated until the official allowed-value - # spec is confirmed; until then a file with all required columns passes. - if not VALIDATE_VALUES: + # Row-level checks are split in two: attribute value/nullability checks + # are gated behind VALIDATE_VALUES (spec not yet confirmed), while + # geometry checks are gated behind VALIDATE_GEOMETRY. When neither is on, + # a file with all required columns passes without scanning any rows. + if not VALIDATE_VALUES and not VALIDATE_GEOMETRY: return True, [], {"total_errors": 0, "capped": False, "fields_with_errors": []} # --- Row-level validation (streaming) -------------------------------- @@ -143,9 +210,16 @@ def validate_shapefile(shp_path: str) -> _Result: capped = True break - feature_errors = _validate_feature(idx, feature.get("properties", {}) or {}) + feature_errors: _ErrorList = [] + if VALIDATE_GEOMETRY: + feature_errors.extend(_validate_geometry(idx, feature.get("geometry"))) + if VALIDATE_VALUES: + feature_errors.extend( + _validate_feature(idx, feature.get("properties", {}) or {}) + ) + errors.extend(feature_errors) - fields_with_errors.update(e["field"] for e in feature_errors) + fields_with_errors.update(e["field"] for e in feature_errors if e["field"]) return ( len(errors) == 0, diff --git a/submit-api/tests/unit/services/test_geo_validator.py b/submit-api/tests/unit/services/test_geo_validator.py index 1bb0a8c67..9d385d18b 100644 --- a/submit-api/tests/unit/services/test_geo_validator.py +++ b/submit-api/tests/unit/services/test_geo_validator.py @@ -70,8 +70,19 @@ def __iter__(self): return iter(self._features) -def _feature(props: dict) -> dict: - return {"type": "Feature", "geometry": None, "properties": props} +# A minimal valid geometry so attribute-focused tests are not tripped up by the +# geometry validator (which is on by default). +_VALID_GEOMETRY = {"type": "Point", "coordinates": [0.0, 0.0]} + +# A classic self-intersecting "bowtie" polygon. +_SELF_INTERSECTING_GEOMETRY = { + "type": "Polygon", + "coordinates": [[[0.0, 0.0], [1.0, 1.0], [1.0, 0.0], [0.0, 1.0], [0.0, 0.0]]], +} + + +def _feature(props: dict, geometry: dict | None = _VALID_GEOMETRY) -> dict: + return {"type": "Feature", "geometry": geometry, "properties": props} # --------------------------------------------------------------------------- @@ -250,3 +261,63 @@ def test_whitespace_stripped_from_valid_value(): assert is_valid is True assert errors == [] + + +# --------------------------------------------------------------------------- +# Geometry validation +# --------------------------------------------------------------------------- + +def test_null_geometry_fails(): + """A feature with no geometry produces a null_geometry error.""" + collection = _FakeCollection( + schema_props=VALID_SCHEMA, + features=[_feature(VALID_PROPERTIES, geometry=None)], + ) + with patch("submit_api.services.geo.validator.fiona.open", return_value=collection): + is_valid, errors, summary = validate_shapefile("dummy.shp") + + assert is_valid is False + assert any(e["error_type"] == "null_geometry" for e in errors) + assert "geometry" in summary["fields_with_errors"] + + +def test_empty_geometry_fails(): + """A feature with an empty geometry produces a null_geometry error.""" + empty_geom = {"type": "Polygon", "coordinates": []} + collection = _FakeCollection( + schema_props=VALID_SCHEMA, + features=[_feature(VALID_PROPERTIES, geometry=empty_geom)], + ) + with patch("submit_api.services.geo.validator.fiona.open", return_value=collection): + is_valid, errors, _ = validate_shapefile("dummy.shp") + + assert is_valid is False + assert any(e["error_type"] == "null_geometry" for e in errors) + + +def test_self_intersecting_polygon_fails(): + """A self-intersecting polygon produces a self_intersection error.""" + collection = _FakeCollection( + schema_props=VALID_SCHEMA, + features=[_feature(VALID_PROPERTIES, geometry=_SELF_INTERSECTING_GEOMETRY)], + ) + with patch("submit_api.services.geo.validator.fiona.open", return_value=collection): + is_valid, errors, summary = validate_shapefile("dummy.shp") + + assert is_valid is False + assert any(e["error_type"] == "self_intersection" for e in errors) + assert "geometry" in summary["fields_with_errors"] + + +@patch("submit_api.services.geo.validator.VALIDATE_GEOMETRY", False) +def test_geometry_validation_disabled_skips_geometry_checks(): + """With VALIDATE_GEOMETRY off, a null geometry no longer fails the file.""" + collection = _FakeCollection( + schema_props=VALID_SCHEMA, + features=[_feature(VALID_PROPERTIES, geometry=None)], + ) + with patch("submit_api.services.geo.validator.fiona.open", return_value=collection): + is_valid, errors, _ = validate_shapefile("dummy.shp") + + assert is_valid is True + assert errors == [] diff --git a/submit-web/src/components/App/DocumentUpload/DocumentTable.tsx b/submit-web/src/components/App/DocumentUpload/DocumentTable.tsx index 5dd225cf4..5f52e8d5b 100644 --- a/submit-web/src/components/App/DocumentUpload/DocumentTable.tsx +++ b/submit-web/src/components/App/DocumentUpload/DocumentTable.tsx @@ -66,15 +66,12 @@ export default function DocumentTable({ Uploaded by Version - {isGeoSpatial && ( - Status - )} Actions - + ))} diff --git a/submit-web/src/components/App/DocumentUpload/DocumentTableRow.tsx b/submit-web/src/components/App/DocumentUpload/DocumentTableRow.tsx index afb8b2ef2..4269595fe 100644 --- a/submit-web/src/components/App/DocumentUpload/DocumentTableRow.tsx +++ b/submit-web/src/components/App/DocumentUpload/DocumentTableRow.tsx @@ -7,7 +7,6 @@ import { TableRowProps, Typography, } from "@mui/material"; -import { StatusChip } from "@/components/Shared/StatusChip"; import { BCDesignTokens } from "epic.theme"; import { deleteDocument } from "@/hooks/api/useObjectStorage"; import { notify } from "@/components/Shared/Snackbar/snackbarStore"; @@ -16,11 +15,9 @@ import { LoadingButton } from "@/components/Shared/LoadingButton"; import { useDeleteSubmission } from "@/hooks/api/useSubmissions"; import { useFileStore } from "@/store/fileStore"; import { useFormContext } from "react-hook-form"; -import { useParams } from "@tanstack/react-router"; import { getObjectFromS3 } from "@/components/Shared/Table/utils"; import { DocumentLink } from "@/components/Shared/DocumentLink"; import { GeoApprovedBadge } from "@/components/Shared/GeoApprovedBadge"; -import { useGetGeoUploads } from "@/hooks/api/useGeo"; export const StyledHeadTableCell = styled(TableCell, { shouldForwardProp: (prop) => prop !== "error", @@ -100,14 +97,12 @@ type DocumentTableRowProps = Readonly<{ error?: boolean; formFieldName?: string; folder?: string; - isGeoSpatial?: boolean; onDocumentClick?: (documentItem: Submission) => void; }>; export default function DocumentTableRow({ documentItem, error = false, formFieldName, - isGeoSpatial, onDocumentClick, }: DocumentTableRowProps) { const { submitted_by, version, submitted_document } = documentItem; @@ -115,15 +110,6 @@ export default function DocumentTableRow({ const [isRemovingDocument, setIsRemovingDocument] = useState(false); const { setValue, trigger, getValues } = useFormContext(); // Get form context directly const { removeFile } = useFileStore(); - const { submissionId: subItemId } = useParams({ - from: "/proponent/_proponentLayout/projects/$projectId/_projectLayout/submission-packages/$submissionPackageId/_submissionLayout/submissions/$submissionId", - }); - const { data: geoUploads } = useGetGeoUploads({ itemId: Number(subItemId) }); - - const geoUpload = (geoUploads as any[])?.find( - (u: any) => u.raw_s3_key === submitted_document?.url, - ); - const { mutateAsync: deleteSubmission } = useDeleteSubmission({ submissionItemId: documentItem.item_id, }); @@ -218,27 +204,6 @@ export default function DocumentTableRow({ {submitted_by} {version} - {isGeoSpatial && ( - - {geoUpload ? ( - - ) : ( - - N/A - - )} - - )} - {isGeoSpatial && ( - - - Uploading... - - - )} ); diff --git a/submit-web/src/components/App/Map/MapPreviewModal.tsx b/submit-web/src/components/App/Map/MapPreviewModal.tsx index eb2fd8134..aface1af3 100644 --- a/submit-web/src/components/App/Map/MapPreviewModal.tsx +++ b/submit-web/src/components/App/Map/MapPreviewModal.tsx @@ -7,7 +7,11 @@ import Map, { } from "react-map-gl/maplibre"; import "maplibre-gl/dist/maplibre-gl.css"; import { getJsonObject } from "@/hooks/api/useObjectStorage"; -import { useGetGeoUploadUrl, GeoUploadUrlResponse } from "@/hooks/api/useGeo"; +import { + useGetGeoUploadUrl, + GeoUploadUrlResponse, + GeoValidationError, +} from "@/hooks/api/useGeo"; import { LoadingButton } from "@/components/Shared/LoadingButton"; import { Alert, @@ -27,9 +31,11 @@ import { } from "@mui/material"; import SatelliteIcon from "@mui/icons-material/Satellite"; import MapIcon from "@mui/icons-material/Map"; +import HighlightOffIcon from "@mui/icons-material/HighlightOff"; import type { GeoJSON } from "geojson"; import { Submission } from "@/models/Submission"; import { BCDesignTokens } from "epic.theme"; +import { GeoSpatialGuidelines } from "@/components/App/SubmissionItem/GeoSpatialInformation/GeoSpatialGuidelines"; interface MapPreviewModalProps { open: boolean; @@ -38,6 +44,7 @@ interface MapPreviewModalProps { fileSizeKb?: number; status?: string; errorMessage?: string; + validationErrors?: GeoValidationError[]; isApproved?: boolean; onApprove?: () => void; onReject?: () => Promise; @@ -52,6 +59,7 @@ export const MapPreviewModal: React.FC = ({ fileSizeKb, status, errorMessage, + validationErrors, isApproved, onApprove, onReject, @@ -162,10 +170,35 @@ export const MapPreviewModal: React.FC = ({ }, [metaData, geoJson]); const isProcessing = status === "processing" || (!uploadId && open); + const isValidationFailure = status === "validation_failed"; + const isFailure = status === "failed" || isValidationFailure; const isTimeout = status === "failed" && (errorMessage?.toLowerCase().includes("timed out") ?? false); + // Columns the shapefile is missing. Prefer the structured validation errors, + // but fall back to parsing the human-readable error_message (e.g. "Validation + // failed: missing required column(s): Category, Subcategor, ...") so the + // bulleted list still renders when only the message string is available. + const structuredMissingColumns = (validationErrors ?? []) + .filter((e) => e.error_type === "missing_column") + .map((e) => e.dbf_column ?? e.field) + .filter((c): c is string => Boolean(c)); + + const parseMissingColumns = (message?: string): string[] => { + const match = message?.match(/missing required column\(s\):\s*([^;.]+)/i); + if (!match) return []; + return match[1] + .split(",") + .map((c) => c.trim()) + .filter(Boolean); + }; + + const missingColumns = + structuredMissingColumns.length > 0 + ? structuredMissingColumns + : parseMissingColumns(errorMessage); + const [displayApproved, setDisplayApproved] = useState(false); useEffect(() => { if (open) { @@ -186,7 +219,7 @@ export const MapPreviewModal: React.FC = ({ return ( = ({ {/* Alert Banner */} - {!previewOnly && ( + {!isApproved && !isFailure && ( Please verify this geospatial file @@ -347,48 +380,121 @@ export const MapPreviewModal: React.FC = ({ {/* Dynamic Map Container */} - {(status === "failed" || status === "validation_failed") ? ( + {isFailure ? ( - - {status === "validation_failed" - ? "Attribute validation failed" - : isTimeout - ? "Geospatial processing failed" - : "Preview not available for this file"} - - - {status === "validation_failed" - ? "This file does not meet the required attribute standards. Please correct the issue and re-upload." - : isTimeout - ? "This file took too long to process and was automatically stopped. It may be too large or complex to process." - : "Geometry must be valid; no NULL geometry or self-intersecting polygons"} - - {errorMessage && ( - - {errorMessage} - + + {isValidationFailure ? ( + <> + + Attribute validation failed + + {missingColumns.length > 0 ? ( + <> + + We couldn't process this file because the shapefile + is missing the following required column(s): + + + {missingColumns.map((column) => ( + + {column} + + ))} + + + ) : ( + + {errorMessage || + "This file does not meet the required attribute standards."} + + )} + + + ) : isTimeout ? ( + <> + + We apologize. The file upload timed out. + + + Please try uploading the file again. + + + ) : ( + <> + + Geospatial processing failed + + + {errorMessage || + "We couldn't process this file. The geometry must be valid, with no NULL or self-intersecting geometry."} + + + )} ) : ( @@ -574,8 +680,20 @@ export const MapPreviewModal: React.FC = ({ - {previewOnly ? ( + {isFailure && !previewOnly ? ( + // Failed upload/processing: the file is unusable, so the only action is + // to close, which fully removes the file when a removal handler exists. + + Close + + ) : previewOnly ? (