feat(geolens): edit and save vector data back to a GeoLens dataset - #1430
Conversation
A GeoLens dataset added as GeoJSON is already an ordinary editable GeoLibre
layer — the GeoEditor can redraw it in place and the attribute table can retype
it — but nothing carried those edits back to the server. GeoLens does expose
per-feature CRUD (`POST /api/datasets/{id}/features/`, and `PUT`/`PATCH`/
`DELETE` on `.../features/{gid}`), so the round trip only needed a client.
The plugin now tracks a baseline for every dataset it loads, shows what changed
in an Edits section, and writes the difference back:
- Identity is the integer row id GeoLens already returns as the GeoJSON `id`,
so it survives the store, the attribute table, and the GeoEditor's
tag-and-restore round trip. Features it never held are inserts; baseline ids
no current feature claims are deletes.
- Attribute changes go out as PUT (full replacement) because GeoLens does not
document whether PATCH merges the properties map, and a merge would silently
keep an attribute the user cleared. A geometry-only move PATCHes.
- There is no bulk/transaction endpoint, so a save is one request per changed
feature, issued sequentially with progress. A rejected write is reported and
the rest of the plan continues; the baseline then advances only for the
writes that landed, so a partial save leaves exactly the failures pending
rather than absorbing them.
- Row ids assigned to inserted features are stamped back onto the layer, so a
second save updates those rows instead of inserting duplicates.
- Saving is offered only when the server's `enable_dataset_editing` flag is on
(read from the public `/api/settings/feature-flags/`) and the connection
carries an API key — the write endpoints reject anonymous requests. The panel
says which of the two is missing instead of showing a dead button.
- A restored project has no in-memory baseline, so the first save reads the
dataset back from GeoLens and diffs against what it holds now. Reload
discards local changes the same way.
Verified in the browser against a live GeoLens (datasets.geolibre.app, editing
flag off → Save correctly disabled, counts still track a real attribute-table
edit) and against a local stand-in with editing enabled: a GeoEditor-drawn
feature POSTs, an attribute edit PUTs, and the row settles back to no pending
changes.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGeoLens dataset editing now supports capability detection, local baseline diffing, authenticated per-feature writes, partial-save handling, and an Edits panel. Geometry editing preserves original feature attributes across Geoman sessions, and the attribute table displays feature, filter, and selection counts. ChangesGeoLens editing workflow
Attribute table status
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GeoLensPlugin
participant AppStore
participant GeoLensAPI
participant EditsPanel
GeoLensPlugin->>GeoLensAPI: Fetch editing capability
GeoLensAPI-->>GeoLensPlugin: Return datasetEditing flag
GeoLensPlugin->>AppStore: Store editable GeoJSON and baseline
EditsPanel->>GeoLensPlugin: Request save
GeoLensPlugin->>GeoLensAPI: Apply feature edit plan
GeoLensAPI-->>GeoLensPlugin: Return write results
GeoLensPlugin->>AppStore: Stamp IDs and retain failed changes
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugins/src/plugins/maplibre-geolens.ts`:
- Around line 656-665: Add a pruning helper near editSessions and
pendingCountsCache that removes entries whose layer IDs are absent from
useAppStore.getState().layers. Invoke it in the layers-changed branch of the
store subscription before renderEdits(), so removed layers release both baseline
and pending-count bookkeeping while existing live-layer behavior remains
unchanged.
- Around line 843-858: Update reloadLayerFeatures to check
pendingCountsFor(layer) before fetching and replacing geojson; when unsaved
changes exist, require user confirmation and abort if it is declined, while
preserving the immediate reload behavior for clean layers. Ensure the baseline
is recaptured only after a confirmed successful reload.
- Around line 1270-1273: Update the capability-read flow in connect() around
capabilitiesFor and state.editingEnabled to capture the current state.generation
before awaiting, then apply the result and call renderEdits() only if that
generation is still current. Preserve the existing disconnected/client guard and
ensure superseded connections cannot update editingEnabled for the newer server.
- Around line 805-837: Derive the post-save baseline in the save completion
logic from the originally diffed collection (layer.geojson), applying only the
successful gid stamping, rather than from the current store collection in
patched. Preserve current-store edits for rendering and continue restoring
failed updates and deletes from baseline so edits made during the save remain
pending for the next diff.
In `@tests/geolens-api.test.ts`:
- Around line 719-738: Add a test alongside “PATCHes a geometry-only change and
PUTs a full replacement” covering updateFeature with mode "patch", geometry
null, and changed properties. Assert it issues PATCH and sends a body containing
only the properties, exercising the properties-only branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b95321ed-25cb-4e22-8696-82c1ec9a75f2
📒 Files selected for processing (7)
README.mddocs/roadmap.mdpackages/plugins/src/index.tspackages/plugins/src/plugins/geolens-api.tspackages/plugins/src/plugins/maplibre-geolens.tstests/geolens-api.test.tstests/geolens-editing.test.ts
Trying the plugin previously meant already knowing a GeoLens URL. The panel now
offers the two public deployments — datasets.geolibre.app and
demo.getgeolens.com — above the URL field. Picking one fills the field and
connects (that is the whole intent of the choice; leaving Connect to a second
click would only add a step), then resets to the placeholder, because the URL
field stays the source of truth and the user can edit it afterwards.
Also makes a blocked request legible. demo.getgeolens.com serves its catalog to
curl but sends no `Access-Control-Allow-Origin` at all, so no browser can reach
it — where datasets.geolibre.app allowlists the requesting origin and works.
`fetch` reports that as a bare TypeError ("Failed to fetch") with no detail by
design, which read as "GeoLibre is broken" rather than "this server does not
allow browser access". Every failure this module raises itself is a plain
Error, so the constructor cleanly separates the two, and a transport failure
now names the host and says the server refused a cross-origin request.
Verified in the browser, light and dark: the dropdown lists both entries, the
GeoLibre catalog loads (22 datasets), and the demo entry shows the CORS message
instead of a bare fetch error.
|
Added a Sample server dropdown to the panel (commit 4b94b70), listing both public deployments above the URL field:
Picking one fills the URL field and connects, then resets to the placeholder — the field stays the source of truth, so a stuck selection would soon be a lie once the user edits it. One thing to flag
It serves the catalog happily to Rather than drop the entry, this makes the failure legible.
If the demo deployment later sends Verified in the browser in light and dark themes: dropdown lists both, GeoLibre datasets loads (22 datasets), demo shows the CORS message. Tests cover the sample list being unique, labelled, and already normalized (a trailing slash or non-canonical URL would connect to a different string than the one shown). |
🔍 PR preview
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/plugins/src/plugins/maplibre-geolens.ts (1)
1391-1403: 🩺 Stability & Availability | 🟠 MajorPrune edit-session state when layers are removed.
This subscription observes removed layers but does not delete their entries from
editSessionsorpendingCountsCache. Repeatedly adding and removing GeoLens layers retains full baselines until plugin deactivation; prune IDs absent fromstore.layersbefore rendering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/src/plugins/maplibre-geolens.ts` around lines 1391 - 1403, Update the useAppStore subscription around lastLayersRef to prune editSessions and pendingCountsCache entries whose layer IDs are absent from store.layers before calling renderEdits. Preserve the existing layer-reference change guard and busyLayerIds behavior, using the store layer IDs as the authoritative active set.
♻️ Duplicate comments (1)
packages/plugins/src/plugins/maplibre-geolens.ts (1)
1344-1347: 🩺 Stability & Availability | 🟠 MajorGuard superseded connections before applying capability state.
A stale
connect()can continue afterrunSearch()is superseded, clear the newerstate.clientat Line 1342, and then run this capability check against the wrong host. Capture a generation for the connection and return before all post-search UI/client mutations when it no longer matches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/src/plugins/maplibre-geolens.ts` around lines 1344 - 1347, Update the connection flow around capabilitiesFor and renderEdits to capture the current connection generation before the asynchronous capability check, then verify it still matches after the await and return immediately when superseded. Ensure stale connect() work cannot apply editingEnabled or any subsequent client/UI mutations after runSearch() has replaced or cleared state.client.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugins/src/plugins/maplibre-geolens.ts`:
- Around line 161-163: Update blockedError and the corresponding error wording
in the related transport-failure handling to avoid asserting that every failure
is a CORS refusal. Describe that the browser or network could not reach the
host, and conditionally suggest checking CORS if the host is otherwise
reachable.
- Around line 1361-1368: Update the sampleSelect change handler and the manual
base-URL change flow around connect so changing origins clears the existing
apiKeyInput value, preventing credentials from being reused across hosts.
Preserve the selected base URL behavior and ensure both sample selection and
user-edited URL changes apply the same credential reset rule.
---
Outside diff comments:
In `@packages/plugins/src/plugins/maplibre-geolens.ts`:
- Around line 1391-1403: Update the useAppStore subscription around
lastLayersRef to prune editSessions and pendingCountsCache entries whose layer
IDs are absent from store.layers before calling renderEdits. Preserve the
existing layer-reference change guard and busyLayerIds behavior, using the store
layer IDs as the authoritative active set.
---
Duplicate comments:
In `@packages/plugins/src/plugins/maplibre-geolens.ts`:
- Around line 1344-1347: Update the connection flow around capabilitiesFor and
renderEdits to capture the current connection generation before the asynchronous
capability check, then verify it still matches after the await and return
immediately when superseded. Ensure stale connect() work cannot apply
editingEnabled or any subsequent client/UI mutations after runSearch() has
replaced or cleared state.client.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 67c7b438-a7fb-4236-a05b-88da00483378
📒 Files selected for processing (3)
packages/plugins/src/index.tspackages/plugins/src/plugins/maplibre-geolens.tstests/geolens-editing.test.ts
…server Connecting to a different server left the previous one's dataset cards on screen, so a failed connect showed an error above what looked like that server's catalog. Those cards are worse than stale: their Add buttons build source paths from the old base URL, and the panel has already dropped the client, so they describe a server it is no longer pointed at. Connect now empties the list before querying the new server, so switching clears immediately rather than after the response arrives, and any failed request empties it too — after a request that never landed the panel does not know what the server holds, and leaving earlier results up presents them as the answer to a query that never ran. Verified in the browser: connect to datasets.geolibre.app (22 datasets), switch to the demo server that no browser can reach (0 datasets, just the CORS message), switch back (22 datasets).
|
Fixed the stale-catalog bug (commit 09c32dd): switching servers kept the previous server's dataset cards on screen, so a failed connect showed an error above what read as that server's catalog. Those cards were worse than stale — their Add buttons build source paths from the old base URL, and Two changes:
Verified in the browser by counting rendered cards across a switch:
|
…ry edit
Geoman claims eleven property names as its own "shape properties" — id, shape,
center, width, height, xSemiAxis, ySemiAxis, angle, text, disableEdit, group.
On import it reads them from a feature's plain attributes, and on export
`parseExtraProperties` deletes both the plain and the prefixed form and
re-emits the value as `__gm_<name>`. So a layer with a `height` or `id` column
— building footprints, anything carrying a source id — came back from a pure
geometry edit with those columns *renamed* to `__gm_height` / `__gm_id`, plus a
`__gm_shape` column that was never in the data. The attribute table showed the
renamed columns; the original ones were gone from the layer.
The session only ever edits geometry, so the attributes it started with are the
attributes it must end with. `startLayerGeometryEdit` now snapshots them (from
the already-tagged collection, before Geoman sees it) and the write-back
restores them by feature tag, dropping Geoman's `__gm_*` bookkeeping. A feature
drawn during the session has no snapshot, so it keeps its own properties minus
that bookkeeping, which would otherwise appear as columns in the layer.
Found via the GeoLens plugin, where it also made every feature look edited: an
edit session on the 540-feature Las Vegas Buildings demo dataset marked all 540
changed and would have PUT `{}` over every row's attributes — the geometry was
byte-identical, only the renamed columns differed.
Also stops GeoLens counting a dropped null-valued attribute as a change: a
GeoLens row exposes every column, so an empty feature loads as
`{"id": null, "height": null}` and returns from the editor as `{}`. Absent and
null both leave the column NULL, so folding them together drops writes that
could not change anything, while a real value that disappears still registers.
Verified against the live dataset: a session with no edits went from 540 writes
to none ("No local changes", Save disabled) with the attribute table keeping its
id/height columns, and a single-feature attribute edit now issues exactly one
PUT (HTTP 200, "Saved 1 change to GeoLens"). The demo row used for that check
was restored to its original values.
…a status bar The table had no readout of how much data it was showing: a layer's size was only visible in the Layers panel, and the size of a selection nowhere at all. A status bar under the table now reports the layer's feature count and how many features are selected. The count of rows currently shown appears only when a search or the Selected view is narrowing the table — showing it always would print the same number twice in the ordinary case and teach the eye to skip it. It sits outside the scroll area so it stays put while scrolling, and is hidden when the panel is collapsed or the layer has no attributes. Counts come from the same collections the table renders (`attributeRows`, `filtered`, `selectedFeatureIds`), so they cannot drift from what is on screen. Strings are plural-aware (`_one`/`_other`) in en.json; the other catalogs fall back to English until translated. Covered by a hermetic e2e spec over the existing smoke fixture: totals, the "shown" count appearing only while filtered, and selection.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugins/src/plugins/maplibre-geo-editor.ts`:
- Around line 697-700: Update the edit setup around captureEditedProperties and
tagFeatureKeys so original properties are captured before tagging mutates null
properties. Associate those pre-tag property snapshots with the generated tags,
then ensure reconciliation restores null when the original GeoJSON properties
were null. Add a regression test covering the null → tag → reconcile flow and
confirming geometry-only saves preserve null properties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 825ccd49-a57e-4b84-826a-cdcdda3dc05f
📒 Files selected for processing (5)
packages/plugins/src/plugins/geo-editor-geometry.tspackages/plugins/src/plugins/geolens-api.tspackages/plugins/src/plugins/maplibre-geo-editor.tstests/geo-editor-geometry.test.tstests/geolens-api.test.ts
Root cause of the 540 writes — it was not the diff, it was attribute corruptionReproduced on the Las Vegas Buildings demo dataset (540 features) with writes intercepted so nothing touched the server: an Edit geometry session with zero edits marked all 540 features changed and issued 540 Comparing an intercepted payload against the server row showed the geometry was byte-identical. The difference was the attributes:
The attribute table told the rest of the story: after the session the layer's columns were Geoman claims eleven property names as its own shape properties — That also meant this PR's save would have Fixes (commit c027dc1)
Verified against the live dataset
New unit tests pin the round trip ( One loose end worth a look separately: an earlier attempt that typed |
Attribute table status bar (commit 3777250)Added a status bar under the attribute table reporting the layer's feature count and the selection:
The "shown" count appears only while something is filtering — always showing it would print the same number twice in the common case and train the eye to skip the bar. It sits outside the scroll area so it stays put while scrolling, and is hidden when the panel is collapsed or the layer has no attributes. Counts derive from the same collections the table renders ( Covered by a hermetic e2e spec ( |
- saveLayerEdits: baseline the collection that was actually diffed and written,
not a fresh read of the store. An edit made while the save was in flight was
being snapshotted as if the server already held it, so it would read as
unchanged forever and could never be saved. Covered by a test that fails
against the previous logic.
- Prune editSessions/pendingCountsCache for layers that have left the store.
Each entry holds a full copy of its dataset, so adding and removing GeoLens
layers retained one dataset per discarded layer until the plugin deactivated.
- Reload now confirms before discarding unsaved changes, naming the counts it
would throw away. A clean layer still reloads on a single click.
- Clear the API key when the target server changes origin (sample-server pick
or a retyped host), so a key issued by a private deployment is not carried to
a different one. Manual edits check on `change`, not per keystroke, so fixing
a path within one host leaves the key alone.
- Reword the transport-failure message: a bare fetch TypeError also covers DNS,
offline and TLS failures, so it no longer asserts the server refused CORS —
it says the request never completed and names CORS as one possibility.
- captureEditedProperties reads the pre-tag collection, so a feature whose
properties were null stays null through tag → edit → reconcile instead of
becoming {}. Regression test added for the null round trip.
- Test the properties-only PATCH branch (mode "patch" with no geometry), both
in updateFeature and in the diffFeatures fallback that selects it.
Add GeoJSON took the first N features of a dataset, which on a large catalog layer is an arbitrary slice with no relationship to what the user is looking at. It now asks the server for the features inside the current map view (OGC `bbox`), so the feature limit caps what is loaded *from that area* instead. Verified against the Las Vegas Buildings demo dataset: zoomed into a few blocks, the load returned 49 of 534 features — the ones on screen. - Settings gains "Only load features in the current map view", persisted like the feature limit, and on by default: a catalog dataset is usually far larger than the area being looked at. A view that spans the world, wraps the antimeridian, or has no map sends no bbox at all rather than a wrong half. - A view-filtered layer is named "<dataset> (current view)", because the Layers panel is where someone will later wonder why the layer is a subset. - The load terms (limit and extent) are recorded on the layer, and the baseline and Reload now re-read on those terms rather than the panel's current settings. Without this a view-filtered layer would diff against the whole dataset and the next save would DELETE every feature outside the view. The same recording fixes the pre-existing case of changing the limit between load and save. - A save that would delete features now asks first, naming the count. Deletions are the only part of a plan that can appear without the user touching a feature — one the editor silently failed to load is simply absent afterwards, which diffs identically to a deliberate delete — and the one part that cannot be undone.
Adding a dataset as vector tiles, editing it as GeoJSON and saving left the
tiles showing pre-edit data at every zoom the user had already looked at, while
zooms visited for the first time came back correct — removing and re-adding the
layer fixed it. Three things were wrong, in order of depth:
- `syncVectorTileLayer` created its source once and never touched it again, so
a changed tile template never reached MapLibre. It now pushes the new
endpoint into the live source with setTiles/setUrl, and only when it actually
changed (a needless reload blanks the layer for a frame). This also repairs
the GeoLens token refresh, which patched the store every few minutes and, as
it turns out, never affected the map at all — a layer left open past its
token's lifetime would have started 404ing.
- Saving edits did not tell the tiles anything had changed. A successful save
now re-points every GeoLens vector-tile layer showing that dataset, which
drops MapLibre's cached tiles and re-requests them.
- Re-minting a token is not enough to do that: GeoLens returns the *same*
signature and expiry for the rest of its time bucket, so the URL would be
unchanged and both MapLibre and the HTTP cache would answer from cache. The
refresh stamps a `_v` parameter instead. Signature validation ignores unknown
parameters — verified against the live server: the same tile returns
byte-identical content (5414 bytes, HTTP 200) with and without it.
Verified end to end on the Las Vegas Buildings demo dataset: with the map at
zoom 15 and those tiles already rendered, saving one edit re-requested them
immediately, and every request after further navigation carried the new version.
Also fixes the attribute-table type inference that this exposed. A cell holding
null carried no type, so typing into one stored a string — which made a number
column mixed, and a PostGIS-backed dataset rejected the write outright ("a
value is incompatible with a column's type or constraints"). The type now comes
from the column across the layer, falling back to the raw string only when the
column is empty everywhere.
Moving to a different area meant removing the GeoJSON layer and adding it again. Each Edits row now has a "Load this view" button that replaces the layer's features with the ones in the current view, in place. Unsaved work is never discarded silently. With pending changes the button offers to save them first; a save that did not fully succeed aborts the reload, so the features those writes failed on are still there to retry. Declining the save asks separately about discarding, so save, discard and cancel are all reachable from the one button — the existing Reload keeps its narrower meaning of "discard and re-read the same extent". The refresh records the new extent and limit on the layer and re-baselines from what it loaded, so the features that just left the view are not mistaken for deletions on the next save. An auto-generated layer name is kept accurate (gaining or losing the "(current view)" suffix); a name the user changed is left alone, which is why the dataset title is now recorded in the layer metadata.
A GeoLens dataset added as Add GeoJSON was already an ordinary editable GeoLibre layer (the GeoEditor's in-place Edit geometry accepts it, the attribute table retypes it), but nothing carried those edits back. GeoLens does expose per-feature CRUD —
POST /api/datasets/{id}/features/andPUT/PATCH/DELETEon…/features/{gid}— so only the client side was missing.What it does
The plugin keeps a baseline for each dataset it loads, shows the pending changes in a new Edits section, and writes the difference back.
id. It survives the store, the attribute table, and the GeoEditor's tag-and-restore round trip, so nothing extra is stamped intoproperties(no stray column in the attribute table or exports).POST. The assigned row id is written back onto the layer, so a second save updates that row instead of inserting a duplicate.PUT(full replacement): GeoLens does not document whetherPATCHmerges the properties map, and a merge would silently keep an attribute the user cleared. A geometry-only movePATCHes.enable_dataset_editingflag is on (read from the public/api/settings/feature-flags/) and the connection carries an API key — the write endpoints reject anonymous requests. The panel names whichever is missing instead of showing a dead button.Editor-internal properties (
__geolibre_fid, Geoman's__gm_*) are stripped before anything reaches the server.Verification
Unit + integration tests (
tests/geolens-api.test.ts,tests/geolens-editing.test.ts) cover the diff, each verb, error-detail surfacing, abort, gid stamping, and the partial-failure re-baselining.Driven in a real browser two ways:
datasets.geolibre.app— editing flag is off there, so Save is correctly disabled with the explanation; the pending counts still track a real edit made through the app's own attribute table (1 changed).POSTs, an attribute editPUTs, the status readsSaved 1 change to GeoLens., and the row settles back to No local changes with Save disabled.Full frontend suite, coverage floors, and
npm run buildpass.Notes
Add vector tiles) remain non-editable by design; the GeoJSON path is the editing surface. Edits are not reflected in a signed MVT layer until GeoLens re-tiles.setGeoLensLabels, so wiring GeoLens into the i18n catalogs stays a separate change.Summary by CodeRabbit
@ishiland GeoLibre now supports editing features from GeoLens and saving edited features back to GeoLens.
GeoLens-edit-demo.mp4