Skip to content

feat: import QGIS project files - #1563

Merged
giswqs merged 19 commits into
mainfrom
feat/issue-1562-qgis-project-import
Jul 31, 2026
Merged

feat: import QGIS project files#1563
giswqs merged 19 commits into
mainfrom
feat/issue-1562-qgis-project-import

Conversation

@giswqs

@giswqs giswqs commented Jul 30, 2026

Copy link
Copy Markdown
Member

Summary

  • add an Import menu with an Import QGIS Project action
  • convert .qgs and .qgz projects into GeoLibre projects
  • preserve supported vector layers, order, groups, visibility, opacity, common single-symbol styling, labels, and map extent
  • resolve relative file paths and report unsupported providers or browser-only filesystem limitations without aborting the import

Verification

  • npm run build -w geolibre-desktop
  • npm run test:frontend (4,407 passed, 1 skipped)
  • pre-commit run --files ...
  • Playwright browser verification with .qgs and .qgz fixtures in light and dark themes

Fixes #1562

Summary by CodeRabbit

  • New Features
    • Added a Project menu action for importing QGIS .qgs and .qgz projects.
    • Preserves supported layer styles, visibility, opacity, labels, ordering, groups, and map views.
    • Supports compatible local, remote GeoJSON, and raster layers.
    • Displays completion results with layer-specific warnings and localized explanations.
    • Provides validation and clear error messages for invalid or unsupported projects.
    • Raster layers can be added without automatically zooming to their extent.

Copilot AI review requested due to automatic review settings July 30, 2026 03:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds QGS and QGZ project import support. The importer restores supported project metadata, views, layers, groups, paths, styles, and labels. Desktop wiring adds file selection, import actions, localized warnings, raster authorization, and validation tests.

Changes

QGIS project import

Layer / File(s) Summary
QGIS project parsing
apps/geolibre-desktop/src/lib/qgis-project-import.ts
Parses QGS and QGZ input. Converts supported layers, metadata, groups, views, sources, styles, and labels. Returns categorized warnings.
Desktop import flow
apps/geolibre-desktop/src/lib/tauri-io.ts, apps/geolibre-desktop/src/hooks/useProjectFileActions.ts, apps/geolibre-desktop/src-tauri/src/lib.rs, packages/plugins/src/plugins/maplibre-raster.ts
Adds QGIS file selection and import handling. Materializes remote GeoJSON layers, authorizes project rasters, loads imported projects as dirty, exposes warning state, and supports optional raster zoom behavior.
Import menu and result UI
apps/geolibre-desktop/src/components/layout/..., apps/geolibre-desktop/src/lib/ui-profile.ts, apps/geolibre-desktop/src/i18n/locales/en.json
Adds the profile-gated import action, callback wiring, localized messages, and warning dialog.
Importer validation
tests/qgis-project-import.test.ts, package.json
Adds QGS and QGZ fixtures and tests for metadata, layers, styles, views, paths, archives, and warnings. Adds linkedom for test support.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ProjectMenu
  participant FilePicker
  participant Importer
  participant ProjectState
  participant ResultDialog
  User->>ProjectMenu: Select Import QGIS Project
  ProjectMenu->>FilePicker: Open .qgs or .qgz picker
  FilePicker-->>ProjectMenu: Return file data and path
  ProjectMenu->>Importer: Parse and materialize project layers
  Importer-->>ProjectMenu: Return GeoLibre project and warnings
  ProjectMenu->>ProjectState: Load project as dirty
  ProjectState-->>ResultDialog: Provide import warnings
  ResultDialog-->>User: Show completion status and warning details
Loading

Possibly related PRs

  • opengeos/GeoLibre#1472: Related QGIS raster import, local raster authorization, and raster restoration changes.

Suggested reviewers: craun718

Poem

A rabbit imports maps with care,
QGS and QGZ arrive there.
Layers align and groups nest tight,
Warnings list each missing sight.
“Done!” says the bunny, ears held high.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: importing QGIS project files.
Linked Issues check ✅ Passed The changes implement the linked issue's import action, .qgs/.qgz support, supported-layer restoration, path resolution, and actionable warnings.
Out of Scope Changes check ✅ Passed All changes support QGIS project import, including raster authorization, UI integration, dependency support, and focused tests.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-1562-qgis-project-import

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

🔍 Cloudflare PR preview

Item Value
Site https://131f0658.geolibre-preview.pages.dev
Demo app https://131f0658.geolibre-preview.pages.dev/demo/
Commit 517084c

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

🔍 GitHub Pages PR preview

Item Value
Site Deploy failed. See the job log.
Demo app Unavailable
Commit 517084c

Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/hooks/useProjectFileActions.ts
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • numberText() (qgis-project-import.ts:347-349) returns 0 instead of NaN for a missing extent element, since Number("") is 0. A project with no saved mapcanvas > extent therefore passes the Number.isFinite guard in parseMapView and opens zoomed into "null island" [0,0] at ~zoom 20, instead of falling back to the documented default view. High confidence.
  • HTTP(S)-sourced vector layers (e.g. an OGR datasource pointing at a URL) pass isSupportedVectorLayer and get imported as type: "geojson" layers with no data, but nothing ever fetches them — restoreLocalFileLayers only reloads local absolute paths, and the browser-only warning loop in useProjectFileActions.ts:164-172 explicitly skips isHttpUrl sourcePaths. Result: a permanently empty, unwarned layer on both desktop and browser. Medium confidence.
  • UNC network paths (\\server\share\...) are treated as "absolute" by isAbsolutePath and kept verbatim as sourcePath, but isAbsoluteLocalPath in tauri-io.ts deliberately rejects UNC paths (NTLM-capture risk), so on desktop these layers never reload and never get any warning either (the warning loop that would catch this only runs off-desktop). Medium confidence.

Security

  • No issues found. Datasource/path handling defers to existing guarded readers (isAbsoluteLocalPath, hasPathTraversal, isRestorableVectorPath) before ever touching disk, and the XML is parsed with DOMParser (no eval/external entity resolution). User-supplied warning reason values come from a fixed union, not raw XML, so the i18n interpolation isn't an injection vector.

Performance

  • No notable issues; the parser runs once per import on a single small XML document.

Quality

  • No test file was added for this new 375-line XML-parsing module (qgis-project-import.ts), unlike sibling import features (tests/qml-import.test.ts, tests/sld-import.test.ts). Given the module's many edge cases (CRS handling, path resolution, nested groups, delimited-text sources), this is a real coverage gap. Medium-high confidence.
  • Nested layer-tree-group elements (a group inside a group) aren't represented as their own LayerGroup; layerGroupAssignments's querySelectorAll folds a nested subgroup's layers into their top-level ancestor group instead. Silent one-level-deep flattening, not an outright bug but at odds with the PR's "preserve... groups" claim. Low-medium confidence.
  • toWgs84 only converts EPSG:3857; any other projected CRS is passed through as if already WGS84 degrees, silently producing a nonsensical initial view rather than falling back to the default. Possibly an accepted "first version" limitation. Low confidence.

CLAUDE.md

  • No violations found in the changed files (i18n uses t() with new en.json keys, RTL-safe classes used, no MapLibre state mutated directly outside the store).

Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

All four inline comments posted successfully.

Code review

Bugs

  • qgisVectorSource mishandles Windows file:// datasource URIs: new URL(source).pathname yields a spurious leading slash before the drive letter (e.g. /C:/data/points.csv), which then passes the isAbsolutePath check unchanged and produces a malformed sourcePath. Since QGIS's delimitedtext provider (explicitly supported here) always encodes its datasource as a file:// URL, Windows users importing CSV/TSV point layers are likely to hit this. Confidence: medium.

Quality

  • parseLayerStyle hardcodes a mm→px conversion (× 3.78) for outline_width/line_width and size without checking QGIS's outline_width_unit/line_width_unit, so symbols authored in pixel or point units get inflated stroke widths/marker sizes. Confidence: medium.
  • The same function grabs the first renderer-v2 symbols symbol layer regardless of renderer type, so categorized/graduated/rule-based renderers silently produce a misleading single style (the first category's color) instead of falling back to defaults or warning the user that categorization was dropped. Confidence: medium.
  • Minor/grouped: (1) the directory computation in qgisVectorSource is a no-op when projectPath has no separator (the case for browser-picked files, which only expose a bare filename), producing a nonsensical joined sourcePath — low practical impact since those layers are already flagged unusable in-browser, but the stored path is still misleading. (2) GDAL's /vsizip/… / zip://… virtual-path syntax (QGIS's standard way to reference a shapefile inside a zip) isn't unwrapped, so such layers pass through as unresolvable absolute paths rather than a clear "unsupported" warning. Confidence: low.

CLAUDE.md / Security / Performance

  • No CLAUDE.md-guideline violations found: i18n strings go through t()/en.json, no ml-/left- physical Tailwind utilities were introduced, no node_modules edits, and linkedom is scoped to a devDependency used only by the new test file.
  • No injection or leaked-secret issues; XML parsing uses the browser's native DOMParser (no external-entity resolution) and file access stays within the app's existing local-file trust model. Worth noting only as a general (pre-existing, not introduced by this PR) consideration: an untrusted .qgs/.qgz file can reference arbitrary local paths via datasource, which the importer will read on the desktop without surfacing the resolved path to the user — consistent with how other project-file flows already behave, so not flagged as a regression.
  • No obvious performance problems; group/layer parsing is O(n²)-ish for very large projects (many querySelectorAll calls in loops) but reasonable for a first version of this feature.

The core conversion logic (extent/CRS handling, group flattening into single-level display names, visibility, opacity, layer ordering, and the reload path integration via localFileReloadable) all checked out correctly against the test fixtures and the rest of the codebase's local-file-restore machinery.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@tests/qgis-project-import.test.ts`:
- Around line 20-39: Extend the QGIS project fixture generation and coverage
around importQgisProject to include supported raster map layers alongside the
existing vector layers. Add a raster conversion path that restores each raster’s
source, visible state, opacity, beforeId, and groupId, while retaining the
“non-vector” warning for unsupported layer types; update the test to assert all
five restored raster 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: dab1e8bc-91af-4cc9-a196-5c2de56627ee

📥 Commits

Reviewing files that changed from the base of the PR and between 2856ef8 and 2329d72.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (10)
  • apps/geolibre-desktop/src/components/layout/TopToolbar.tsx
  • apps/geolibre-desktop/src/components/layout/toolbar/ProjectFileDialogs.tsx
  • apps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsx
  • apps/geolibre-desktop/src/hooks/useProjectFileActions.ts
  • apps/geolibre-desktop/src/i18n/locales/en.json
  • apps/geolibre-desktop/src/lib/qgis-project-import.ts
  • apps/geolibre-desktop/src/lib/tauri-io.ts
  • apps/geolibre-desktop/src/lib/ui-profile.ts
  • package.json
  • tests/qgis-project-import.test.ts

Comment thread tests/qgis-project-import.test.ts
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts
@github-actions

Copy link
Copy Markdown
Contributor

All four inline comments posted. Now the final summary.

Code review

Security

  • apps/geolibre-desktop/src/lib/qgis-project-import.ts:246-259qgisVectorSource passes through absolute datasource paths from the parsed .qgs/.qgz verbatim, with no containment to the project's own directory. On Tauri desktop, the resulting sourcePath is auto-read from disk by restoreLocalFileLayers on the next project load, with no user confirmation of which file is being read. A shared/downloaded QGIS project can point at any local file (matching a supported vector extension) elsewhere on the victim's machine and have it loaded and rendered as a layer. High confidence on the mechanics.

Bugs

  • apps/geolibre-desktop/src/lib/qgis-project-import.ts:257 — the directory-stripping regex is a no-op when projectPath has no //\ (e.g. the browser file-picker fallback returns a bare filename), leaving directory equal to the full path instead of "", which corrupts relative-path joins for datasource paths without a .. to cancel the error out. High confidence on the logic bug, medium on real-world impact (browser build only, and the resulting sourcePath is never re-read there anyway).

Quality

  • apps/geolibre-desktop/src/lib/qgis-project-import.ts:307parseLayerStyle always reads the first symbol layer regardless of renderer type, so categorized/graduated/rule-based QGIS renderers get silently collapsed to one arbitrary color with no warning that styling fidelity was lost. Medium confidence.
  • apps/geolibre-desktop/src/lib/qgis-project-import.ts:30-48SUPPORTED_VECTOR_EXTENSIONS duplicates VECTOR_FILE_DIALOG_EXTENSIONS (tauri-io.ts) without importing it or a cross-reference comment, unlike this repo's established convention for mirrored constants (e.g. the // SYNC: comment on the list it duplicates). Medium confidence — not a bug today, but a drift risk.

No findings under Performance or CLAUDE.md beyond the maintainability point above; the new i18n keys, menu wiring, ui-profile.ts tier entry, and test coverage all looked consistent with existing patterns.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@apps/geolibre-desktop/src/lib/qgis-project-import.ts`:
- Around line 44-63: Update the remote-layer processing in
handleImportQgisProject to use bounded concurrency instead of launching every
request through Promise.all at once. Add a per-request timeout or cancellation
deadline to fetcher calls, and ensure timed-out requests follow the existing
catch path that records failedLayerIds and the "remote-file" warning while
allowing other layers to import.

In `@tests/qgis-project-import.test.ts`:
- Around line 143-168: Extend the test around materializeQgisRemoteLayers to
place successful and failed remote layers in the same layerGroups entry, then
assert the failed layer is removed while the group and successful layer remain.
Also cover a group containing only failed layers and assert that unused group
cleanup removes it if that behavior is part of the contract.
🪄 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: 900a55af-287c-4c83-87c1-00ccf2e4268e

📥 Commits

Reviewing files that changed from the base of the PR and between 2329d72 and 5b01d23.

📒 Files selected for processing (3)
  • apps/geolibre-desktop/src/hooks/useProjectFileActions.ts
  • apps/geolibre-desktop/src/lib/qgis-project-import.ts
  • tests/qgis-project-import.test.ts

Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread tests/qgis-project-import.test.ts
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • None found beyond the item noted under Security — the parsing/resolution logic (extent/CRS handling, group nesting, relative-path joining, VSI/file:// stripping) is well-guarded and matches its test coverage. Confidence: medium (based on tracing the logic, not running it).

Security

  • allow_raster_asset (apps/geolibre-desktop/src-tauri/src/lib.rs:406-431) accepts a raster path whenever some previously-selected .qgs/.qgz project is already in fs_scope, without ever re-parsing that project to confirm the raster is actually one of its datasources, and without requiring the raster to live under the project's own directory. Combined with qgisSourcePath (qgis-project-import.ts:328) passing absolute/../-resolved datasource paths through unchanged, a crafted .qgs/.qgz — or even a compromised caller passing arbitrary arguments to this IPC command — can get any .tif/.tiff file on the local filesystem authorized into the asset-protocol scope merely because the user opened an unrelated project earlier in the session. This defeats the "file was not selected or dropped by the user" invariant the surrounding code otherwise enforces. Confidence: medium-high. Posted inline with a suggested mitigation (bind the raster to the project's own directory tree).

Performance

  • Imported rasters are loaded and added to the map sequentially (for (const raster of imported.rasters) { await …; await …; } in useProjectFileActions.ts) rather than in parallel — likely fine for typical projects with a handful of rasters, but worth noting for projects with many. Confidence: low.
  • materializeQgisRemoteLayers fetches remote GeoJSON with no size cap or timeout, unlike the size-mirroring conventions documented elsewhere in this repo for remote vector fetches; a very large or slow remote file could stall/balloon memory. Confidence: low.

Quality

  • unsupportedReason mislabels unsupported rasters (remote GDAL sources, or local ones with an extension outside the supported set like .jp2/.vrt) as "non-vector", producing a slightly misleading "Only file-based vector layers are supported" message for something that actually is a raster. Posted inline. Confidence: low.
  • i18n and RTL conventions are followed correctly (new strings via t(), logical me-2 spacing in the new menu item), and the new test file appropriately keeps linkedom scoped to devDependencies.

CLAUDE.md

  • No violations found: translation strings added to en.json as source of truth, menu wiring follows the existing ui-profile.ts/ProjectMenu.tsx catalog pattern, RTL logical utility classes used correctly, and packages/plugins/src/plugins/maplibre-raster.ts's new zoomTo option is a minimal, backward-compatible addition (defaults preserved).

Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts
Comment thread apps/geolibre-desktop/src-tauri/src/lib.rs
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts
@github-actions

Copy link
Copy Markdown
Contributor

Code review

Security

  • High confidenceqgisSourcePath/normalizeJoinedPath (apps/geolibre-desktop/src/lib/qgis-project-import.ts:328-362) apply no containment check to datasource paths from an imported .qgs/.qgz. An absolute datasource, or a relative one with enough ../ segments, resolves to any file on disk with a supported extension. The Rust-side is_safe_absolute_path/hasPathTraversal guards don't catch it because the path is already fully resolved (no literal ..) by the time it reaches them, and the existing restoreLocalFileLayers() pipeline then reads and renders it automatically with no further confirmation.
  • High confidenceallow_raster_asset's new qgis_project_path bypass (apps/geolibre-desktop/src-tauri/src/lib.rs:417-422) exposes any .tif/.tiff absolute path to the asset protocol as long as some previously-selected QGIS project path is passed alongside it, without verifying that path is actually related to that project. This weakens what was previously a strict per-file selection/drop consent gate, and is invocable by any script with invoke access (e.g. a rogue plugin), not just the QGIS import flow.

Bugs

  • Medium confidence — Trailing ?query strings in a datasource are stripped for extension detection (sourceExtension) but not from the stored sourcePath itself unless the value happens to start with file://. A non-file://-prefixed datasource with query params (e.g. QGIS delimitedtext's relative file:./x.csv?delimiter=... form) could be accepted as supported but end up with a broken sourcePath that later fails to reload. (apps/geolibre-desktop/src/lib/qgis-project-import.ts:328-346)

Quality

  • Low confidenceqgisColor validates only the RGB components for finiteness; a malformed alpha component yields opacity: NaN.
  • Low confidence — HTTP-hosted/unsupported raster providers report the "non-vector" warning reason, which is misleading (they are rasters, just unsupported ones).

Performance / CLAUDE.md

  • No issues found. MAX_VECTOR_PMTILES_ZOOM/similar drift-tracked mirrors aren't touched by this PR, and the new i18n strings are correctly added under toolbar.item/toolbar.error in en.json.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/hooks/useProjectFileActions.ts (1)

188-240: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Prevent overlapping QGIS imports.

handleImportQgisProject has several awaited operations before it updates the store. If two imports overlap, the older import can finish last and load its project or add rasters into the newer project.

Serialize imports with a ref-based guard, or use an import generation token before every project-state mutation.

Proposed guard
+  const isImportingQgisRef = useRef(false);
+
   const handleImportQgisProject = async () => {
+    if (isImportingQgisRef.current) return;
+    isImportingQgisRef.current = true;
     const result = await openQgisProjectFile();
-    if (!result) return;
+    if (!result) {
+      isImportingQgisRef.current = false;
+      return;
+    }
     try {
       // import work
     } catch (error) {
       // existing error handling
+    } finally {
+      isImportingQgisRef.current = false;
     }
   };
🤖 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/hooks/useProjectFileActions.ts` around lines 188 -
240, Prevent concurrent executions of handleImportQgisProject by adding a
ref-based in-progress guard around the full import flow. Return immediately when
an import is already active, set the guard before the first awaited operation,
and clear it in a finally block so failures or cancellations do not leave
imports permanently blocked; ensure project loading, raster additions, and store
updates occur only for the active import.
🤖 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.

Outside diff comments:
In `@apps/geolibre-desktop/src/hooks/useProjectFileActions.ts`:
- Around line 188-240: Prevent concurrent executions of handleImportQgisProject
by adding a ref-based in-progress guard around the full import flow. Return
immediately when an import is already active, set the guard before the first
awaited operation, and clear it in a finally block so failures or cancellations
do not leave imports permanently blocked; ensure project loading, raster
additions, and store updates occur only for the active import.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3fb9d896-6881-4359-af52-6639f5283d0a

📥 Commits

Reviewing files that changed from the base of the PR and between 0635120 and 21e9258.

📒 Files selected for processing (5)
  • apps/geolibre-desktop/src-tauri/src/lib.rs
  • apps/geolibre-desktop/src/components/layout/toolbar/ProjectFileDialogs.tsx
  • apps/geolibre-desktop/src/hooks/useProjectFileActions.ts
  • apps/geolibre-desktop/src/i18n/locales/en.json
  • apps/geolibre-desktop/src/lib/tauri-io.ts
💤 Files with no reviewable changes (1)
  • apps/geolibre-desktop/src/i18n/locales/en.json

Comment thread packages/plugins/src/plugins/maplibre-raster.ts Outdated
Comment thread apps/geolibre-desktop/src-tauri/src/lib.rs
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/qgis-project-import.ts
@github-actions

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • packages/plugins/src/plugins/maplibre-raster.ts:376-377 — the new waitForTauriWasmLayerRegistration/warmTauriWasmEngine calls run at the end of every addRasterToMap(), not just after a QGIS import or project/basemap load (the scenario the surrounding comment justifies). On Tauri with the cog-tiler-wasm engine this round-trips setEngine() for every single raster add — including plain drag-and-drop — which appears to toggle the rendering engine for all currently-loaded rasters, risking visible flicker and added latency on a common, unrelated code path. Medium confidence.

Security

  • apps/geolibre-desktop/src-tauri/src/lib.rs:417-422allow_raster_asset now also grants asset-protocol access to a .tif/.tiff merely referenced inside an already-opened QGIS project, with no check that the raster lives under the project's own directory (is_safe_absolute_path only blocks UNC/.., not out-of-tree absolute paths). This weakens the prior "only files the user explicitly selected or dropped" invariant — opening an untrusted .qgs/.qgz can expose an arbitrary local .tif path to the app. Likely an intentional trade-off (documented in the comment), but worth confirming. Medium confidence.
  • apps/geolibre-desktop/src/lib/qgis-project-import.ts:58-68materializeQgisRemoteLayers fires an unconfirmed fetch() to any HTTP datasource embedded in the project as soon as it's opened, on both desktop and browser. A crafted project could use this as a beacon or to probe internal/metadata endpoints before the user sees any content. Medium confidence.

Quality

  • apps/geolibre-desktop/src/lib/qgis-project-import.ts:516-521 — stroke width and marker radius are converted mm→px (* 3.78) unconditionally, ignoring QGIS's outline_width_unit/size_unit options; symbols authored in pixel/point units will render at the wrong size. Low-medium confidence.
  • Minor: qgisSourcePath doesn't strip GDAL's /vsizip/ prefix the way it strips /vsicurl/, so real QGIS zipped-shapefile datasources (which typically use /vsizip/…/data.zip/data.shp) likely won't resolve correctly even though .zip is in SUPPORTED_VECTOR_EXTENSIONS. Low confidence.
  • Minor: in the browser file-picker path, openLocalDataFileWithFallback returns only file.name (no directory) as path, so relative-path resolution in qgisSourcePath produces a nonsensical directory for browser imports. Currently harmless in practice since browser-mode local layers are never loaded anyway (flagged via the "browser-local-file" warning), but the stored sourcePath metadata is misleading. Low confidence.

CLAUDE.md

  • No violations found: new strings go through t()/en.json, dialog styling uses logical Tailwind utilities (no physical ml-/left-), and no node_modules/generated-catalog files were hand-edited.

Comment thread apps/geolibre-desktop/src-tauri/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

Now composing the final summary.

Code review

Bugs

  • qgisSourcePath (apps/geolibre-desktop/src/lib/qgis-project-import.ts:795) drops the |layername=... GDAL sublayer selector from a datasource, keeping only the whole-file path. Because the imported layer has no geojson yet, restoreLocalFileLayers re-reads the entire file right after import and matches sublayers back to store layers by display name — which commonly differs from the QGIS project's custom layer name — silently falling back to the first sublayer in the file on a mismatch. This can import the wrong table from a multi-layer GeoPackage/FileGDB with no visible error. Confidence: medium-high.

Security

  • allow_raster_asset in apps/geolibre-desktop/src-tauri/src/lib.rs:405-431 now also grants asset-protocol access to a GeoTIFF path when any previously-selected .qgs/.qgz project is passed as qgis_project_path — the check doesn't verify that project actually references the raster, nor does it confine the raster path to the project's directory. Since .qgs is plain, easily-crafted XML, importing one untrusted project effectively grants blanket access to read any .tif/.tiff on the filesystem the process can reach, weakening the existing "user explicitly selected this file" model. Confidence: medium.

Performance

  • No notable issues found in the changed code.

Quality

  • unsupportedReason (qgis-project-import.ts:866-880) labels every unsupported raster case (UNC path, unsupported provider, non-OSM remote raster) as "non-vector", producing a misleading "Only file-based vector layers are supported" message for rasters. Cosmetic only. Confidence: low-medium.
  • The new allow_raster_asset bypass branch (qgis_project_path) has no accompanying Rust unit test, unlike is_safe_absolute_path's existing test coverage in the same file. Confidence: low.

CLAUDE.md

  • No violations found: i18n strings use t() and are added to en.json; new Tailwind classes use logical utilities (me-2) consistent with RTL conventions; no new external tile/map host is introduced (the OSM-basemap mapping reuses the existing DEFAULT_BASEMAP); no touched files fall under the Whitebox catalog, PMTiles zoom mirror, or backend uv.lock rules.

@giswqs
giswqs merged commit 8362f7e into main Jul 31, 2026
16 checks passed
@giswqs
giswqs deleted the feat/issue-1562-qgis-project-import branch July 31, 2026 04:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Open QGIS project files (.qgz and .qgs)

2 participants