fix: Skip unsupported KML NetworkLinks instead of failing the whole import#53
Merged
Merged
Conversation
…mport A KMZ exported from Google Earth that mixes embedded placemark geometry with `<NetworkLink>` elements (references to remotely hosted layers) failed to import entirely, surfacing only a generic map data error. `SpatialFeatures::Importers::KML#kml_document` raised `ImportError` the moment it encountered any `NetworkLink`, so the BC Hydro Facilities IVMP KMZ reported by Michelle from Ktunaxa — 133 importable points and polygons alongside two NetworkLinks — produced zero features. Instead of raising `ImportError` when a `NetworkLink` is present, the importer now removes the NetworkLink nodes and records a non-fatal warning naming the skipped layers before importing the embedded geometry. Warnings flow through a new `warnings` accumulator on `Importers::Base`, are aggregated in `update_features!`, and are persisted to `spatial_processing_status_cache` (readable via `feature_update_warnings`) so they survive job completion, since successful `Delayed::Job`s are deleted and cannot be read back. A file containing only NetworkLinks still fails, but now via an `EmptyImportError` whose message names the skipped layers rather than a bare "no features found". Each warning is prefixed with its importer's `source_identifier` (e.g. `archive.zip/layer.kml`), so a multi-file or multi-source import makes clear which file the warning belongs to. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6c55854 to
f4272be
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A KMZ exported from Google Earth that mixes embedded placemark geometry with
<NetworkLink>elements (links to remotely hosted layers) failed to import entirely, surfacing only a generic "Unable to import features due to a map data error."Importers::KML#kml_documentraisedImportErroron the firstNetworkLinkit found.Reported by Michelle from Ktunaxa: her
BC Hydro Facilities IVMP.kmzcontains 133 importable points + polygons plus 2 NetworkLinks (Substations,Mobile Radio Repeaters), and imported nothing.Change
Importers::KMLnow strips NetworkLink nodes and records a non-fatal warning naming the skipped layers, then imports the embedded geometry.warningsaccumulator onImporters::Base; aggregated inupdate_features!and persisted tospatial_processing_status_cache(read viafeature_update_warnings) so they survive job completion (successfulDelayed::Jobs are deleted).source_identifier(e.g.archive.zip/layer.kml), so a multi-file / multi-source import identifies which file the warning belongs to.EmptyImportErrorwhose message explains which layers were skipped.This preserves the original 2023 intent (fail loud when a file has nothing importable) while no longer discarding valid geometry.
Validation
update_features!coverage, a hybrid fixture, and a multi-file attribution test).connect_enginepins~> 3.9.0; the NetworkLink code path is identical on master.)Downstream (toward Stolo/SC)
connect_enginesurfaces these via a companion PR (specific reason on failure + warning notice on partial import). On a Referral / ReferralSubmission the warning renders as an in-page notice viafeature_update_notifications, since those models import the footprint directly (include UploadedFileMapLayer). To reach deployments, CE'sspatial_featurespin needs to allow3.10.2, then CE + SC bumps follow.🤖 Generated with Claude Code