Skip to content

fix(favorites): follow-up fixes for saved query folders - #1183

Open
Asymtode712 wants to merge 1 commit into
TabularisDB:mainfrom
Asymtode712:fix/saved-query-folders-followup
Open

Asymtode712 wants to merge 1 commit into
TabularisDB:mainfrom
Asymtode712:fix/saved-query-folders-followup

Conversation

@Asymtode712

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1181 (organize saved queries in connection folders), addressing the review notes.

Changes

  • Modals: SavedQueryFolderModal and MoveSavedQueryModal no longer reset state with a synchronous setState in useEffect (.rules/react.md Not loading on macos. #2). The form is mounted only while the modal is open and keyed by folder/query, so state initializes on mount. Props are unchanged.
  • Modal styling: aligned with .rules/modals.md (600px width, subtitle, standard content/footer padding, rounded-lg text-sm buttons, label style). Focus-visible outlines and keyboard access are kept.
  • Lenient folder listing: get_saved_query_folders now uses the lenient read, matching the read_folders_or_empty doc comment. A corrupt folders.json shows no folders and queries stay Unfiled. Create, rename, delete and move keep the strict read so they never overwrite a bad file.
  • Readable corruption error: an unreadable or invalid folders.json now returns a single user-facing message instead of the raw serde error. The technical detail goes to the log.
  • Delete-folder title: the confirmation uses a new sidebar.confirmDeleteFolderTitle ("Delete Folder") instead of reusing the query title.
  • i18n: 4 new sidebar.* strings (3 subtitles and the delete title) in all 11 locales.

Not changed

Folders of deleted connections remain in folders.json. Saved queries already behave the same way, so this is not a regression. Cleaning it up is left for a later milestone.

Plugin drivers

No change. Folders stay driver-independent organization of saved queries, with no SQL, driver-id, capability or dialect dependency and no plugin contract changes.

Testing

  • Rust: the strict read returns the friendly message with no serde detail. The lenient read, atomic write and migration tests still pass.
  • Frontend: tests for the delete-folder title, a fresh dialog on every open, the backend message shown in the create dialog, and the move dialog preselecting the current folder.
  • cargo test --lib saved_quer, pnpm exec tsc -b, pnpm lint and the full vitest suite pass.

@kilo-code-bot

kilo-code-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Notes from verification against the current head (b7942d3):

  • Both modals now mount their form only while open, keyed by folder/query, so state starts fresh on each open without a reset effect. This matches .rules/react.md.
  • get_saved_query_folders now uses the lenient read, while create/rename/delete/move keep the strict read, so a corrupt folders.json is never overwritten.
  • The strict read returns a single user-facing message and logs the technical detail. On the lenient path the technical detail is now logged once (first warning) and the generic message a second time, which is harmless noise.
  • The 4 new sidebar.* keys are present in all 11 locales.
  • Tests cover the delete-folder title, fresh dialog on reopen, backend message display, and move-dialog preselection.

Not executed (read-only review): the Rust and vitest suites were not run.

Files Reviewed (18 files)
  • src-tauri/src/saved_queries.rs
  • src-tauri/src/saved_query_folders.rs
  • src-tauri/src/saved_query_folders_tests.rs
  • src/components/layout/ExplorerSidebar.tsx
  • src/components/modals/MoveSavedQueryModal.tsx
  • src/components/modals/SavedQueryFolderModal.tsx
  • src/i18n/locales/de.json
  • src/i18n/locales/en.json
  • src/i18n/locales/es.json
  • src/i18n/locales/fr.json
  • src/i18n/locales/it.json
  • src/i18n/locales/ja.json
  • src/i18n/locales/ko.json
  • src/i18n/locales/pt-BR.json
  • src/i18n/locales/ru.json
  • src/i18n/locales/tl.json
  • src/i18n/locales/zh.json
  • tests/components/layout/ExplorerSidebar.test.tsx

Reviewed by claude-sonnet-5.5 · Input: 254 · Output: 2.8K · Cached: 295.4K

Review guidance: REVIEW.md from base branch main

This branch has not been deployed

No deployments
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.

1 participant