feat(export): Exclude hidden fields from vector data - #1705
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR adds per-field visibility states and removes excluded fields from GeoJSON, project sharing, project saves, vector exports, attribute-table exports, and PostGIS reads. The attribute table adds controls to toggle export exclusion. ChangesField exclusion
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AttributeTable
participant CoreVisibility
participant ExportHandlers
participant LayerPanel
participant PostGIS
AttributeTable->>CoreVisibility: GeoJSON and excluded field names
CoreVisibility-->>ExportHandlers: GeoJSON without excluded properties
LayerPanel->>PostGIS: excluded_fields
PostGIS-->>LayerPanel: features with filtered properties
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
🔍 Cloudflare PR preview
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/geolibre-desktop/src/components/panels/LayerPanel.tsx (1)
1557-1560: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGenerate Shapefile warnings from
egressGeojson.Line 1550 exports
egressGeojson, but Line 1559 scans the unfilteredgeojson. An excluded long field or field-name collision can produce a warning for a field that is not in the output. PassegressGeojsontoshapefileFieldWarnings.Proposed fix
- const warnings = format === "shapefile" ? shapefileFieldWarnings(geojson) : []; + const warnings = format === "shapefile" ? shapefileFieldWarnings(egressGeojson) : [];🤖 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 `@apps/geolibre-desktop/src/components/panels/LayerPanel.tsx` around lines 1557 - 1560, Update the Shapefile warning generation in the export flow to pass egressGeojson, the filtered export dataset, to shapefileFieldWarnings instead of geojson. Keep the existing format check and refresh-status behavior unchanged.
🤖 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 `@backend/geolibre_server/geolibre_server/app/postgis.py`:
- Around line 542-544: The query-building logic around read_columns must always
retain the primary-key column even when request.excluded_fields includes it, so
the feature identity remains available for the assignment around feature.id.
Keep the primary key in the selected result while omitting it from properties,
or reject attempts to exclude it before querying; add coverage for primary-key
exclusion.
- Line 581: Update the row-to-property mapping in the properties comprehension
to call zip(read_columns, row[1:], strict=True), ensuring SELECT/result-length
mismatches raise immediately instead of silently dropping values.
In `@packages/core/src/visibility.ts`:
- Around line 42-50: Update excludeHiddenFieldsFromProject to also sanitize each
layer’s metadata.embeddedGeoJSON with excludeHiddenFieldsFromGeojson before
buildEmbeddedLayers materializes embedded data, preserving unchanged metadata
when no fields are removed and marking the project changed when filtering
occurs. Add save and share coverage verifying excluded properties are absent
from embedded GeoJSON.
---
Outside diff comments:
In `@apps/geolibre-desktop/src/components/panels/LayerPanel.tsx`:
- Around line 1557-1560: Update the Shapefile warning generation in the export
flow to pass egressGeojson, the filtered export dataset, to
shapefileFieldWarnings instead of geojson. Keep the existing format check and
refresh-status behavior unchanged.
🪄 Autofix
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: 88d72d7f-20e8-4d44-83a8-3c63d29b6e0e
📒 Files selected for processing (11)
apps/geolibre-desktop/src/components/layout/TopToolbar.tsxapps/geolibre-desktop/src/components/panels/AttributeTable.tsxapps/geolibre-desktop/src/components/panels/LayerPanel.tsxapps/geolibre-desktop/src/hooks/useProjectFileActions.tsapps/geolibre-desktop/src/i18n/locales/en.jsonbackend/geolibre_server/geolibre_server/app/postgis.pybackend/geolibre_server/tests/test_postgis.pypackages/core/src/index.tspackages/core/src/types.tspackages/core/src/visibility.tspackages/processing/src/sidecar-client.ts
🔍 GitHub Pages PR preview
|
|
/claude-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -843,13 +844,18 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) { | |||
| // them. Make keeping them an explicit choice and use the same central | |||
| // redaction pass as every external egress. | |||
| let contentToSave = content; | |||
There was a problem hiding this comment.
Minor: contentToSave is now unconditionally reassigned in every branch of the if/else below (both the redactedPaths.length > 0 "keep" branch and the else branch call serializeProject(projectToEgress), and "strip" calls serializeProject(redacted.project)), so this initial let contentToSave = content; is dead — it's never read before being overwritten. Slightly misleading since it looks like a real fallback. Could simplify to declare contentToSave inside the branches, or drop the outer content variable if it's now unused elsewhere.
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
2 participants
Summary
layer.geojsondata.backend/geolibre_server.Problem
Currently, GeoLibre projects package all source fields inside the embedded GeoJSON vectors. Hiding a column merely removes it from the data grid UI but still writes it to the
.geolibre.jsonpayload, network requests (during sharing), and.geojsonfile exports, inadvertently leaking potentially sensitive metadata (names, addresses, PII, etc).Root Cause
The export pipeline and the PostGIS sync requests didn't differentiate between fields meant to be visible and fields meant to be completely excluded. The serialization steps simply pulled
layer.geojsonwholesale and SQL sync requests queried all available columns blindly.Solution
"excluded"property under the layer'sfieldVisibilitydict mapping.excludeHiddenFieldsFromProjectandexcludeHiddenFieldsFromGeojsonto@geolibre/coreto recursively strip excluded properties prior to serialization.TopToolbar,LayerPanel, anduseProjectFileActionsto route through the exclusion pipeline during sharing, exports, and saves./postgis/readin the backend sidecar to construct its SQLSELECTgeometry projection by omittingexcluded_fields.AttributeTabledropdown to exclude or include fields.Testing & Verification
npm run lint).npm run typecheck).npm run test:frontend) without dropping coverage below thresholds.test_read_drops_excluded_fieldsregression test and passed backend test suite successfully (npm run test:backend).References
Closes #1676
Summary by CodeRabbit
New Features
Bug Fixes