feat: error handling, query history, and release-ready UI polish#30
feat: error handling, query history, and release-ready UI polish#30remcostoeten wants to merge 3 commits intomasterfrom
Conversation
Error Handling: - Add ErrorBoundary component wrapping all major features - Add ErrorFallback with smart error categorization - Add mapConnectionError utility for friendly error messages - Connection errors now show helpful messages instead of technical details Query History: - Add query history panel with zustand store - Track query execution time, success/failure, row counts - Filter history by connection - Click to reuse queries UI Polish: - Add EmptyState for no connections view - Disable SSH tunnel UI with "Soon" badge (not yet wired to backend) - Update Dora AI view to use NotImplemented component - Remove dead handleToolbarAction code - Add skeleton, error-state, not-implemented, disabled-feature components Backend: - Add soft delete support for LibSQL - Add truncate table support - Schema and mutation improvements Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… logs - Replace alert() with toast in seed-view.tsx - Replace confirm() with AlertDialog in database-sidebar.tsx for bulk drop/truncate - Fix stray "3" text in studio-toolbar.tsx - Remove debug console.log from database-studio.tsx, sql-console/api.ts, disabled-feature.tsx - Update RELEASE_TRACKER.md with session progress Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @remcostoeten, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis PR introduces comprehensive improvements across the Dora application: new error handling and shared UI components (ErrorBoundary, EmptyState, ErrorFallback), query history tracking with localStorage persistence, validation schemas for connections, CSV/SQL export capabilities, clipboard operations in the data grid, and backend LibSQL support for maintenance operations (dump, truncate, soft delete). Additionally, it includes multiple test suites for accessibility utilities and database functionality, plus documentation of release progress. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src-tauri/src/database/services/mutation.rs (1)
728-734:⚠️ Potential issue | 🟠 MajorLibSQL
execute_batchlacks transaction support.The Postgres branch (lines 702-717) wraps batch execution in a transaction with proper rollback on failure, and SQLite (lines 719-726) uses
conn.transaction(). However, the LibSQL branch executes statements individually without transaction boundaries, which could leave the database in an inconsistent state if a statement fails mid-batch.🔧 Proposed fix to add transaction support for LibSQL batch execution
DatabaseClient::LibSQL { connection } => { + // Start transaction + connection.execute("BEGIN", ()).await + .map_err(|e| Error::Any(anyhow!("LibSQL BEGIN failed: {}", e)))?; + + let result: Result<(), Error> = async { + for stmt in &statements { + let res = connection.execute(stmt, ()).await + .map_err(|e| Error::Any(anyhow!("LibSQL execution failed: {}", e)))?; + affected_rows += res as usize; + } + Ok(()) + }.await; + + if result.is_ok() { + connection.execute("COMMIT", ()).await + .map_err(|e| Error::Any(anyhow!("LibSQL COMMIT failed: {}", e)))?; + } else { + let _ = connection.execute("ROLLBACK", ()).await; + result?; + } - for stmt in &statements { - let res = connection.execute(stmt, ()).await - .map_err(|e| Error::Any(anyhow!("LibSQL execution failed: {}", e)))?; - affected_rows += res as usize; - } }apps/desktop/src/features/sidebar/components/bottom-toolbar.tsx (1)
29-188:⚠️ Potential issue | 🟠 MajorGuard
onActionbefore calling it.
onActionis optional now, but it’s invoked unconditionally, which can throw at runtime and fail strict TS checks.🔧 Suggested fix
- onClick={function () { - onAction(item.id) - }} + onClick={function () { + onAction?.(item.id) + }}
🤖 Fix all issues with AI agents
In `@apps/desktop/src-tauri/src/database/maintenance.rs`:
- Around line 321-345: truncate_table_libsql currently deletes all rows but does
not reset the autoincrement counter like the SQLite path does; after the DELETE
in truncate_table_libsql call the same sequence-reset SQL (e.g. DELETE FROM
sqlite_sequence WHERE name = ?) for the given table_name and map any error into
Error::Any (same style as existing map_err), so the autoincrement sequence is
cleared; update TruncateResult.message/flow as needed and use the function name
truncate_table_libsql to locate the change.
In `@apps/desktop/src/features/connections/components/connection-dialog.tsx`:
- Around line 298-306: handleSave currently runs validateConnection and calls
setValidationError but the component never renders validationError so users get
no feedback; update the connection-dialog component to display validation
failures by rendering validationError (from state setValidationError) inline on
the corresponding input (map validationError.field to the field's error
prop/message) and/or trigger a visible toast when validation.success is false;
ensure the UI blocks the save flow until validationError is cleared and that
validateConnection, setValidationError, validationError, formData,
useConnectionString and handleSave are the referenced symbols you update.
In `@apps/desktop/src/features/connections/validation.ts`:
- Around line 91-109: The validation currently lets missing/invalid
formData.type pass through and return success; update the validation in the
function that reads formData.type to explicitly reject unknown or missing types
by returning { success: false, error: 'Unsupported connection type', field:
'type' } (or similar) when type is falsy or not one of the handled values; keep
existing branches that call sqliteConnectionSchema.parse(formData),
libsqlConnectionSchema.parse(formData), and the postgres/mysql branch that
checks useConnectionString and calls connectionStringSchema.parse(formData) or
connectionFieldsSchema.parse(formData), but add a final else that returns the
failure object for unrecognized types.
- Around line 112-118: The Zod v4 compatibility fix: in the Zod error handling
inside the validation logic (where you check `if (error instanceof z.ZodError)`
and use `error.errors` and `firstError`), replace uses of `error.errors` with
`error.issues` (e.g., set `const firstError = error.issues[0]`) and keep
extracting `firstError.message` and `firstError.path[0]` as before so the
function returns the same shape (`{ success: false, error: ..., field: ... }`)
without causing a runtime exception.
- Around line 62-75: The current refine validator in validation.ts checks
data.authMethod but always sets path: ['password'], causing keyfile errors to
point to the wrong field; replace the .refine(...) with .superRefine((data, ctx)
=> { if authMethod === 'password' and missing password then ctx.addIssue({ code:
z.ZodIssueCode.custom, message: 'Password is required', path: ['password'] });
if authMethod === 'keyfile' and missing privateKeyPath then ctx.addIssue({ code:
z.ZodIssueCode.custom, message: 'Private key path is required', path:
['privateKeyPath'] }); }) so the validation error is mapped to the correct field
(reference symbols: .refine -> .superRefine, ctx.addIssue, data.authMethod,
data.password, data.privateKeyPath).
In `@apps/desktop/src/features/database-studio/components/data-grid.tsx`:
- Around line 537-595: The paste handler builds an edits array but calls
onBatchCellEdit(edits) while onBatchCellEdit currently expects the signature
(rowIndex, columnName, newValue); fix by aligning APIs: either (A) map the paste
edits into the existing call pattern by iterating edits and calling
onBatchCellEdit(edit.rowIndex, edit.columnName, edit.value) (or batching via
existing helper) inside the paste case in data-grid.tsx, or (B) update
onBatchCellEdit’s contract to accept an edits array (Array<{rowIndex,
columnName, value}>) and then update all callers/implementations (e.g.,
DatabaseStudio.handleBatchCellEdit and the “set-null” batch action) to accept
and process the new edits-array format; update type defs for onBatchCellEdit
accordingly so TypeScript validation passes.
In `@apps/desktop/src/features/database-studio/components/studio-toolbar.tsx`:
- Around line 287-297: The Export button is icon-only and currently relies on
title for accessibility; update the Button inside DropdownMenuTrigger (the
Button wrapping the Download icon in studio-toolbar.tsx) to include an explicit
aria-label (e.g., aria-label="Export") so screen readers and keyboard users get
a reliable name; keep the existing title if desired but ensure aria-label is
present on the Button element containing the Download icon.
In `@apps/desktop/src/features/database-studio/database-studio.tsx`:
- Around line 1016-1071: The export path in handleExportSqlAll currently ignores
errors from commands.exportTable and non-'ok' responses; wrap the export call in
a try/catch and explicitly handle non-ok results: call commands.exportTable
inside try, check result.status !== 'ok' and show a user-facing toast/error
(including result.error or message if available), and in the success branch
proceed to create the Blob and trigger download; ensure URL.revokeObjectURL is
invoked in a finally block (or always called) to avoid leaking object URLs, and
in the catch show a toast with the caught error and return. Use the existing
handleExportSqlAll function, commands.exportTable call, result.status check, and
URL.revokeObjectURL to locate and implement the changes.
In `@apps/desktop/src/features/sql-console/api.ts`:
- Around line 90-96: The result object incorrectly maps columns with
columns.map(c => c.name) which turns an already string[] into undefined entries;
update the return in the function that constructs the response (the object
containing columns, rows, rowCount, executionTime, error, queryType) to set
columns: columns (i.e., return the columns array directly) so the API returns
the original string[]; keep the rest of the properties (rows, rowCount,
executionTime, error, queryType) unchanged.
In `@apps/desktop/src/features/sql-console/components/console-toolbar.tsx`:
- Around line 207-229: The CSV menu item currently renders even when the
optional handler onExportCsv is undefined, producing a visible no-op; update the
DropdownMenu content to either conditionally render the DropdownMenuItem for CSV
only when onExportCsv is truthy or render it disabled when onExportCsv is
absent. Locate the DropdownMenuItem that references onExportCsv in
console-toolbar.tsx and wrap it in a guard (e.g., if (onExportCsv) render the
item) or add disabled and cursor styles plus no-op protection so clicking cannot
occur when onExportCsv is undefined.
In `@apps/desktop/src/features/sql-console/sql-console.tsx`:
- Around line 294-318: The CSV export in handleExportCsv assumes each row is an
object keyed by string column names and that result.columns are strings; change
it to normalize columns to an array of string keys (e.g., map result.columns to
colName via col.name || String(col)) and when serializing rows handle both
object rows and array rows (if Array.isArray(row) use row[index], otherwise use
row[colName]); also ensure missing values (undefined/null) become empty strings
and still apply CSV escaping for commas, quotes and newlines before building the
Blob and triggering download.
In `@apps/desktop/src/shared/ui/disabled-feature.tsx`:
- Around line 31-38: The Button currently uses the disabled prop which prevents
onClick from firing; remove the disabled attribute and instead set
aria-disabled={true} on the Button (keeping the cursor-not-allowed/opacity
styles) so the control remains clickable and handleClick can fire the toast;
ensure the existing onClick={handleClick} and title={`${feature} - Coming soon`}
remain unchanged (update in disabled-feature.tsx where Button is rendered).
In `@apps/desktop/src/shared/ui/skeleton.tsx`:
- Line 1: Update the import for the classnames helper: replace the non-existent
import from '@/shared/lib/utils' with the correct module '@/shared/utils/cn' so
the `cn` symbol used in this file resolves; update the import statement that
currently imports `cn` to reference `@/shared/utils/cn` to match other
components in shared/ui.
In `@RELEASE_TRACKER.md`:
- Around line 54-58: The Markdown fenced code blocks in RELEASE_TRACKER.md lack
language identifiers which triggers MD040; update each triple-backtick block
that lists file paths or ASCII trees (the blocks around the lists like the ones
containing "apps/desktop/src/shared/ui/error-boundary.tsx" and the ASCII tree
under "apps/desktop/") to use a language tag (use "text") by changing ``` to
```text for all occurrences (also apply the same fix to the other ranges noted
around lines 61-95 and 277-301).
🧹 Nitpick comments (24)
apps/desktop/src-tauri/src/database/commands.rs (1)
254-255: Silent error swallowing may hide data issues.Using
unwrap_or(serde_json::Value::Null)silently discards parse errors, making it impossible for callers to distinguish between "no data" and "malformed data." This is inconsistent with Line 845 whereserde_json::from_str(json_data.get())?propagates errors.Consider logging the error or propagating it:
♻️ Option 1: Propagate the error
let raw = svc.fetch_page(query_id, page_index).await?; - Ok(raw.map(|r| serde_json::from_str(r.get()).unwrap_or(serde_json::Value::Null))) + Ok(raw.map(|r| serde_json::from_str(r.get())).transpose()?)♻️ Option 2: Log parse failures for debugging
let raw = svc.fetch_page(query_id, page_index).await?; - Ok(raw.map(|r| serde_json::from_str(r.get()).unwrap_or(serde_json::Value::Null))) + Ok(raw.map(|r| { + serde_json::from_str(r.get()).unwrap_or_else(|e| { + tracing::warn!("Failed to parse page data: {}", e); + serde_json::Value::Null + }) + }))apps/desktop/src/features/docker-manager/components/seed-view.tsx (1)
3-3: AlignuseToastimport with shared UI index.Use the shared UI entrypoint for UI primitives to keep imports consistent across the app.
Suggested change
-import { useToast } from '@/components/ui/use-toast' +import { useToast } from '@/shared/components/ui'As per coding guidelines, Import shared UI primitives from
@/shared/components/ui.apps/desktop/src-tauri/src/database/libsql/schema.rs (2)
228-233: Inconsistent error handling silently swallows errors during index info retrieval.Line 228 uses
.ok().flatten()which silently ignores errors frominfo_rows.next().await, whereas the outer loop at line 213-216 properly maps errors. This inconsistency could hide issues when retrieving column information for indexes.Consider aligning the error handling approach for consistency:
♻️ Suggested fix for consistent error handling
let mut column_names = Vec::new(); - while let Some(info_row) = info_rows.next().await.ok().flatten() { + while let Some(info_row) = info_rows + .next() + .await + .map_err(|e| Error::Any(anyhow::anyhow!("Failed to fetch index info row: {}", e)))? + { let col_name: String = info_row.get(2).unwrap_or_default(); if !col_name.is_empty() { column_names.push(col_name); } }
235-240: Consider detecting primary key indexes fromPRAGMA index_listorigin column.
PRAGMA index_listreturns anorigincolumn (index 3) that indicates'pk'for primary key indexes. Currentlyis_primaryis hardcoded tofalse, which may be inaccurate.♻️ Suggested enhancement to detect primary key indexes
while let Some(row) = rows .next() .await .map_err(|e| Error::Any(anyhow::anyhow!("Failed to fetch index row: {}", e)))? { let name: String = row.get(1).unwrap_or_default(); let is_unique: i64 = row.get(2).unwrap_or(0); + let origin: String = row.get(3).unwrap_or_default(); + let is_primary = origin == "pk"; // ... column fetching ... indexes.push(IndexInfo { name, column_names, is_unique: is_unique != 0, - is_primary: false, + is_primary, }); }apps/desktop/src-tauri/src/database/maintenance.rs (1)
588-599:format_libsql_value_for_sqlcannot distinguish NULL from valid values.The function returns a stringified value on success but falls through to
"NULL"on error. Ifrow.get::<i64>(idx)fails because the actual value is a string, the function incorrectly tries subsequent types. However, if all type attempts fail (e.g., the column contains a BLOB), the function returns"NULL"even if the value isn't actually null.This mirrors the Postgres helper's limitation but could produce incorrect dump output for BLOB columns.
♻️ Consider using libsql::Value for accurate type handling
fn format_libsql_value_for_sql(row: &libsql::Row, idx: i32) -> String { + match row.get_value(idx) { + Ok(libsql::Value::Null) => "NULL".to_string(), + Ok(libsql::Value::Integer(i)) => i.to_string(), + Ok(libsql::Value::Real(f)) => f.to_string(), + Ok(libsql::Value::Text(s)) => format!("'{}'", s.replace('\'', "''")), + Ok(libsql::Value::Blob(b)) => format!("X'{}'", hex::encode(&b)), + Err(_) => "NULL".to_string(), + } - if let Ok(v) = row.get::<i64>(idx) { - return v.to_string(); - } - if let Ok(v) = row.get::<f64>(idx) { - return v.to_string(); - } - if let Ok(v) = row.get::<String>(idx) { - return format!("'{}'", v.replace('\'', "''")); - } - "NULL".to_string() }Note: This would require adding the
hexcrate dependency for BLOB support.apps/desktop/src/shared/ui/error-state.tsx (1)
1-13: AlignErrorStateimports/exports with component conventions.Use the shared UI index import path and export the component as default.
As per coding guidelines, Import shared UI primitives from `@/shared/components/ui` and Export components as default from component files.♻️ Suggested refactor
-import { Button } from './button' +import { Button } from '@/shared/components/ui' -export function ErrorState({ title, message, onRetry, className }: Props) { +export default function ErrorState({ title, message, onRetry, className }: Props) {__tests__/apps/desktop/unit/accessibility-utilities.test.ts (1)
3-174: These tests validate local helpers, not production code.Consider importing the actual accessibility utilities (or testing the public API) so the tests catch regressions in real logic instead of re-implemented functions.
apps/desktop/src/shared/ui/disabled-feature.tsx (1)
1-22: AlignDisabledFeatureimports/exports with component conventions.Use the shared UI index import path and export the component as default.
As per coding guidelines, Import shared UI primitives from `@/shared/components/ui` and Export components as default from component files.♻️ Suggested refactor
-import { Button } from './button' +import { Button } from '@/shared/components/ui' -export function DisabledFeature({ +export default function DisabledFeature({__tests__/apps/desktop/unit/database-studio.test.ts (1)
3-165: Tests re-implement logic instead of exercising production utilities.Please consider importing the real export/clipboard utilities (or testing the public API) so these tests provide meaningful regression coverage.
apps/desktop/src/features/sql-console/stores/query-history-store.tsx (1)
64-73: Prefer a non-deprecated, collision-safe ID generator.
substris deprecated andMath.randomis weak for IDs. Considercrypto.randomUUID()with a fallback for older environments.🔧 Suggested update
- id: `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + id: + typeof globalThis.crypto?.randomUUID === 'function' + ? globalThis.crypto.randomUUID() + : `${Date.now()}-${Math.random().toString(36).slice(2, 9)}`,apps/desktop/src/shared/ui/skeleton.tsx (1)
9-71: Consider splitting skeleton components into separate files (and default-export each).This file defines five components. Splitting them keeps each component focused and aligns with the component organization guidelines.
As per coding guidelines, "One component per file" and "Export components as default from component files".apps/desktop/src/shared/ui/error-boundary.tsx (1)
16-16: Use a default export for component files.
This keeps component exports consistent with the codebase guidelines; update imports accordingly.As per coding guidelines, Export components as default from component files.💡 Suggested change
-export class ErrorBoundary extends Component<Props, State> { +export default class ErrorBoundary extends Component<Props, State> {apps/desktop/src/shared/ui/empty-state.tsx (2)
1-3: Import shared UI primitives via@/shared/components/ui.
This keeps shared UI access consistent across the codebase.As per coding guidelines, Import shared UI primitives from `@/shared/components/ui`.💡 Suggested change
-import { Button } from '@/shared/ui/button' +import { Button } from '@/shared/components/ui'
16-16: Export EmptyState as default per component-file convention.
Please update its import sites to use default import.As per coding guidelines, Export components as default from component files.💡 Suggested change
-export function EmptyState({ icon, title, description, action, className }: Props) { +export default function EmptyState({ icon, title, description, action, className }: Props) {apps/desktop/src/features/sql-console/components/query-history-panel.tsx (2)
1-3: Import shared UI primitives via@/shared/components/ui.
This avoids deep imports from shared UI.As per coding guidelines, Import shared UI primitives from `@/shared/components/ui`.💡 Suggested change
-import { Button } from '@/shared/ui/button' -import { ScrollArea } from '@/shared/ui/scroll-area' +import { Button, ScrollArea } from '@/shared/components/ui'
12-12: Export QueryHistoryPanel as default.
Aligns component-file exports; update import sites accordingly.As per coding guidelines, Export components as default from component files.💡 Suggested change
-export function QueryHistoryPanel({ onSelectQuery, currentConnectionId }: Props) { +export default function QueryHistoryPanel({ onSelectQuery, currentConnectionId }: Props) {apps/desktop/src/features/connections/components/connection-dialog.tsx (1)
34-437: ConnectionDialog is very large—consider splitting.
Extract form sections and side-effects into smaller components/hooks to keep this under the size guidelines.As per coding guidelines, Keep components under 300 lines; Break down components over 200 lines into smaller, focused components.
apps/desktop/src/features/connections/components/connection-dialog/connection-form.tsx (1)
4-9: Import shared UI primitives via@/shared/components/ui.
Consider consolidating the shared UI imports into the index barrel for consistency.As per coding guidelines, Import shared UI primitives from `@/shared/components/ui`.💡 Suggested change
-import { Badge } from '@/shared/ui/badge' -import { Button } from '@/shared/ui/button' -import { Checkbox } from '@/shared/ui/checkbox' -import { Input } from '@/shared/ui/input' -import { Label } from '@/shared/ui/label' -import { Tooltip, TooltipContent, TooltipTrigger } from '@/shared/ui/tooltip' +import { + Badge, + Button, + Checkbox, + Input, + Label, + Tooltip, + TooltipContent, + TooltipTrigger +} from '@/shared/components/ui'apps/desktop/src/pages/Index.tsx (2)
33-36: Import shared UI primitives via@/shared/components/ui.
Please pull ErrorBoundary/EmptyState/NotImplemented from the shared UI barrel to match the project convention.As per coding guidelines, Import shared UI primitives from `@/shared/components/ui`.💡 Suggested change
-import { ErrorBoundary } from '@/shared/ui/error-boundary' -import { EmptyState } from '@/shared/ui/empty-state' -import { NotImplemented } from '@/shared/ui/not-implemented' +import { ErrorBoundary, EmptyState, NotImplemented } from '@/shared/components/ui'
39-607: Index component is too large—consider splitting.
Extract view renderers and data handlers into focused components/hooks to keep this manageable.As per coding guidelines, Keep components under 300 lines; Break down components over 200 lines into smaller, focused components.
apps/desktop/src/shared/ui/not-implemented.tsx (1)
8-22: Default-export the component to match component-file conventions.
This file defines a single component but exports it as a named function; switch to a default export and update imports accordingly.✅ Suggested change
-export function NotImplemented({ feature, description }: Props) { +export default function NotImplemented({ feature, description }: Props) { return ( <div className="flex flex-col items-center justify-center gap-3 p-8 text-center"> <div className="flex h-12 w-12 items-center justify-center rounded-full bg-muted"> <AlertTriangle className="h-6 w-6 text-muted-foreground" /> </div> <div className="space-y-1"> <h3 className="text-sm font-medium text-foreground">{feature}</h3> <p className="text-xs text-muted-foreground"> {description || 'This feature is coming soon'} </p> </div> </div> ) }As per coding guidelines: Export components as default from component files.
apps/desktop/src/features/sql-console/sql-console.tsx (1)
460-465: Prefer functional state updates for the history toggle shortcut.
This avoids stale closures if the shortcut handler outlives the render it was created in.✅ Suggested change
- setShowHistory(!showHistory) + setShowHistory((prev) => !prev)apps/desktop/src/shared/ui/error-fallback.tsx (2)
1-3: ImportButtonfrom the shared UI index.✅ Suggested change
-import { Button } from './button' +import { Button } from '@/shared/components/ui'Based on learnings: Applies to **/*.{ts,tsx} : Import shared UI primitives from
@/shared/components/ui.
84-126: Default-export the component from this component file.✅ Suggested change
-export function ErrorFallback({ error, feature, onRetry, className }: Props) { +export default function ErrorFallback({ error, feature, onRetry, className }: Props) { const mapping = mapError(error) const Icon = mapping.iconAs per coding guidelines: Export components as default from component files.
| /// Truncate a single table (LibSQL) - uses DELETE since LibSQL has no TRUNCATE | ||
| pub async fn truncate_table_libsql( | ||
| conn: &libsql::Connection, | ||
| table_name: &str, | ||
| ) -> Result<TruncateResult, Error> { | ||
| let count_query = format!("SELECT COUNT(*) FROM \"{}\"", table_name); | ||
| let mut rows = conn.query(&count_query, ()).await | ||
| .map_err(|e| Error::Any(anyhow::anyhow!("Failed to count rows: {}", e)))?; | ||
|
|
||
| let row_count: i64 = if let Some(row) = rows.next().await.ok().flatten() { | ||
| row.get::<i64>(0).unwrap_or(0) | ||
| } else { | ||
| 0 | ||
| }; | ||
|
|
||
| conn.execute(&format!("DELETE FROM \"{}\"", table_name), ()).await | ||
| .map_err(|e| Error::Any(anyhow::anyhow!("Truncate failed: {}", e)))?; | ||
|
|
||
| Ok(TruncateResult { | ||
| success: true, | ||
| affected_rows: row_count as usize, | ||
| tables_truncated: vec![table_name.to_string()], | ||
| message: Some(format!("Truncated table '{}', removed {} rows", table_name, row_count)), | ||
| }) | ||
| } |
There was a problem hiding this comment.
Missing: LibSQL truncate does not reset autoincrement sequence.
The SQLite implementation at lines 250-254 resets the autoincrement counter via sqlite_sequence, but the LibSQL implementation omits this step. This could lead to unexpected ID gaps after truncation.
🔧 Proposed fix to reset autoincrement
conn.execute(&format!("DELETE FROM \"{}\"", table_name), ()).await
.map_err(|e| Error::Any(anyhow::anyhow!("Truncate failed: {}", e)))?;
+ // Reset autoincrement counter (best effort, ignore errors if table doesn't use autoincrement)
+ let _ = conn.execute(
+ "DELETE FROM sqlite_sequence WHERE name = ?",
+ vec![libsql::Value::Text(table_name.to_string())],
+ ).await;
+
Ok(TruncateResult {🤖 Prompt for AI Agents
In `@apps/desktop/src-tauri/src/database/maintenance.rs` around lines 321 - 345,
truncate_table_libsql currently deletes all rows but does not reset the
autoincrement counter like the SQLite path does; after the DELETE in
truncate_table_libsql call the same sequence-reset SQL (e.g. DELETE FROM
sqlite_sequence WHERE name = ?) for the given table_name and map any error into
Error::Any (same style as existing map_err), so the autoincrement sequence is
cleared; update TruncateResult.message/flow as needed and use the function name
truncate_table_libsql to locate the change.
| function handleSave() { | ||
| if (!formData.name) return | ||
| const validation = validateConnection(formData as Record<string, unknown>, useConnectionString) | ||
|
|
||
| if (!validation.success) { | ||
| setValidationError({ field: validation.field, message: validation.error }) | ||
| return | ||
| } | ||
|
|
||
| setValidationError(null) |
There was a problem hiding this comment.
Surface validation failures to the user.
handleSave sets validationError and returns, but there’s no UI feedback in this file, so the save appears to do nothing. Consider inline field errors or a toast when validation fails.
🤖 Prompt for AI Agents
In `@apps/desktop/src/features/connections/components/connection-dialog.tsx`
around lines 298 - 306, handleSave currently runs validateConnection and calls
setValidationError but the component never renders validationError so users get
no feedback; update the connection-dialog component to display validation
failures by rendering validationError (from state setValidationError) inline on
the corresponding input (map validationError.field to the field's error
prop/message) and/or trigger a visible toast when validation.success is false;
ensure the UI blocks the save flow until validationError is cleared and that
validateConnection, setValidationError, validationError, formData,
useConnectionString and handleSave are the referenced symbols you update.
| }).refine( | ||
| (data) => { | ||
| if (data.authMethod === 'password') { | ||
| return !!data.password | ||
| } | ||
| if (data.authMethod === 'keyfile') { | ||
| return !!data.privateKeyPath | ||
| } | ||
| return true | ||
| }, | ||
| { | ||
| message: 'Password or private key is required based on auth method', | ||
| path: ['password'], | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "validation.ts" | grep -E "connections|desktop"Repository: remcostoeten/dora
Length of output: 114
🏁 Script executed:
cat -n apps/desktop/src/features/connections/validation.ts | head -100Repository: remcostoeten/dora
Length of output: 3684
🏁 Script executed:
rg "sshTunnelSchema" --type ts --type tsxRepository: remcostoeten/dora
Length of output: 88
🏁 Script executed:
rg "sshTunnelSchema" -t tsRepository: remcostoeten/dora
Length of output: 154
🏁 Script executed:
rg "sshTunnelSchema\|from.*validation" -t tsRepository: remcostoeten/dora
Length of output: 43
🌐 Web query:
Zod superRefine API ZodIssueCode.custom documentation
💡 Result:
In Zod, .superRefine((value, ctx) => { ... }) gives you a ctx.addIssue(...) API so you can manually emit validation issues (including ZodIssueCode.custom). (odocs-zod.vercel.app)
ZodIssueCode.custom in superRefine
Use it when you want to raise your own error (often for cross-field / multi-rule validation):
import { z } from "zod";
const Schema = z.object({
password: z.string(),
confirm: z.string(),
}).superRefine((data, ctx) => {
if (data.password !== data.confirm) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
path: ["confirm"], // optional: point error at a field
message: "Passwords don't match",
// params: { ... } // optional: custom payload for error maps
// fatal: true // optional: abort early (then return z.NEVER)
});
}
});- Notes from the docs:
.superRefinecan add multiple issues; if you don’t callctx.addIssue, validation passes. (odocs-zod.vercel.app)- Plain
.refine(...)always produces issues with codecustom(you can’t pick a different issue code there). (odocs-zod.vercel.app) - You can abort early by passing
fatal: truetoaddIssueand returningz.NEVER. (odocs-zod.vercel.app)
params + ZodIssueCode.custom (for error maps)
If you attach params to a custom issue, you can read them in a ZodErrorMap when issue.code === ZodIssueCode.custom. (odocs-zod.vercel.app)
Citations:
- 1: https://odocs-zod.vercel.app/?utm_source=openai
- 2: https://odocs-zod.vercel.app/?utm_source=openai
- 3: https://odocs-zod.vercel.app/?utm_source=openai
- 4: https://odocs-zod.vercel.app/?utm_source=openai
- 5: https://odocs-zod.vercel.app/ERROR_HANDLING?utm_source=openai
🏁 Script executed:
rg "sshTunnel|SSH.*tunnel" -t ts -i --max-count 20Repository: remcostoeten/dora
Length of output: 1912
🏁 Script executed:
cat -n apps/desktop/src/features/connections/validation.ts | tail -50Repository: remcostoeten/dora
Length of output: 1728
🏁 Script executed:
rg "sshTunnelSchema\.parse|validate.*[Ss]sh" -t ts -A 3Repository: remcostoeten/dora
Length of output: 43
Map SSH auth errors to the correct field.
The refine validator correctly checks both auth methods but always sets path: ['password'], so when authMethod='keyfile' and privateKeyPath is missing, the error incorrectly points to the password field instead. Use superRefine() with ctx.addIssue() to set the correct path per field.
Suggested change
-export const sshTunnelSchema = z.object({
+export const sshTunnelSchema = z
+ .object({
enabled: z.literal(true),
host: z.string().min(1, 'SSH host is required'),
port: z.number().int().min(1).max(65535).default(22),
username: z.string().min(1, 'SSH username is required'),
authMethod: z.enum(['password', 'keyfile']),
password: z.string().optional(),
privateKeyPath: z.string().optional(),
- }).refine(
- (data) => {
- if (data.authMethod === 'password') {
- return !!data.password
- }
- if (data.authMethod === 'keyfile') {
- return !!data.privateKeyPath
- }
- return true
- },
- {
- message: 'Password or private key is required based on auth method',
- path: ['password'],
- }
-)
+ })
+ .superRefine((data, ctx) => {
+ if (data.authMethod === 'password' && !data.password) {
+ ctx.addIssue({
+ code: z.ZodIssueCode.custom,
+ message: 'Password is required for password auth',
+ path: ['password'],
+ })
+ }
+ if (data.authMethod === 'keyfile' && !data.privateKeyPath) {
+ ctx.addIssue({
+ code: z.ZodIssueCode.custom,
+ message: 'Private key path is required for keyfile auth',
+ path: ['privateKeyPath'],
+ })
+ }
+ })🤖 Prompt for AI Agents
In `@apps/desktop/src/features/connections/validation.ts` around lines 62 - 75,
The current refine validator in validation.ts checks data.authMethod but always
sets path: ['password'], causing keyfile errors to point to the wrong field;
replace the .refine(...) with .superRefine((data, ctx) => { if authMethod ===
'password' and missing password then ctx.addIssue({ code: z.ZodIssueCode.custom,
message: 'Password is required', path: ['password'] }); if authMethod ===
'keyfile' and missing privateKeyPath then ctx.addIssue({ code:
z.ZodIssueCode.custom, message: 'Private key path is required', path:
['privateKeyPath'] }); }) so the validation error is mapped to the correct field
(reference symbols: .refine -> .superRefine, ctx.addIssue, data.authMethod,
data.password, data.privateKeyPath).
| const type = formData.type as string | ||
|
|
||
| // Validate name first | ||
| if (!formData.name || (formData.name as string).trim() === '') { | ||
| return { success: false, error: 'Connection name is required', field: 'name' } | ||
| } | ||
|
|
||
| if (type === 'sqlite') { | ||
| sqliteConnectionSchema.parse(formData) | ||
| } else if (type === 'libsql') { | ||
| libsqlConnectionSchema.parse(formData) | ||
| } else if (type === 'postgres' || type === 'mysql') { | ||
| if (useConnectionString) { | ||
| connectionStringSchema.parse(formData) | ||
| } else { | ||
| connectionFieldsSchema.parse(formData) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Reject unsupported connection types instead of silently succeeding.
If type is missing or invalid, validation currently returns success.
✅ Suggested change
if (type === 'sqlite') {
sqliteConnectionSchema.parse(formData)
} else if (type === 'libsql') {
libsqlConnectionSchema.parse(formData)
} else if (type === 'postgres' || type === 'mysql') {
if (useConnectionString) {
connectionStringSchema.parse(formData)
} else {
connectionFieldsSchema.parse(formData)
}
+ } else {
+ return { success: false, error: 'Unsupported connection type', field: 'type' }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const type = formData.type as string | |
| // Validate name first | |
| if (!formData.name || (formData.name as string).trim() === '') { | |
| return { success: false, error: 'Connection name is required', field: 'name' } | |
| } | |
| if (type === 'sqlite') { | |
| sqliteConnectionSchema.parse(formData) | |
| } else if (type === 'libsql') { | |
| libsqlConnectionSchema.parse(formData) | |
| } else if (type === 'postgres' || type === 'mysql') { | |
| if (useConnectionString) { | |
| connectionStringSchema.parse(formData) | |
| } else { | |
| connectionFieldsSchema.parse(formData) | |
| } | |
| } | |
| const type = formData.type as string | |
| // Validate name first | |
| if (!formData.name || (formData.name as string).trim() === '') { | |
| return { success: false, error: 'Connection name is required', field: 'name' } | |
| } | |
| if (type === 'sqlite') { | |
| sqliteConnectionSchema.parse(formData) | |
| } else if (type === 'libsql') { | |
| libsqlConnectionSchema.parse(formData) | |
| } else if (type === 'postgres' || type === 'mysql') { | |
| if (useConnectionString) { | |
| connectionStringSchema.parse(formData) | |
| } else { | |
| connectionFieldsSchema.parse(formData) | |
| } | |
| } else { | |
| return { success: false, error: 'Unsupported connection type', field: 'type' } | |
| } |
🤖 Prompt for AI Agents
In `@apps/desktop/src/features/connections/validation.ts` around lines 91 - 109,
The validation currently lets missing/invalid formData.type pass through and
return success; update the validation in the function that reads formData.type
to explicitly reject unknown or missing types by returning { success: false,
error: 'Unsupported connection type', field: 'type' } (or similar) when type is
falsy or not one of the handled values; keep existing branches that call
sqliteConnectionSchema.parse(formData), libsqlConnectionSchema.parse(formData),
and the postgres/mysql branch that checks useConnectionString and calls
connectionStringSchema.parse(formData) or
connectionFieldsSchema.parse(formData), but add a final else that returns the
failure object for unrecognized types.
| if (error instanceof z.ZodError) { | ||
| const firstError = error.errors[0] | ||
| return { | ||
| success: false, | ||
| error: firstError.message, | ||
| field: firstError.path[0] as string, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "package.json" -type f | head -5 | xargs grep -l "zod" 2>/dev/nullRepository: remcostoeten/dora
Length of output: 88
🏁 Script executed:
find . -name "package.json" -path "*/apps/desktop/*" -o -name "package.json" -path "*/package.json" | xargs cat 2>/dev/null | grep -A 2 -B 2 "zod"Repository: remcostoeten/dora
Length of output: 171
🌐 Web query:
Zod v4 ZodError API error.issues vs error.errors property
💡 Result:
In Zod v4, ZodError exposes validation problems on error.issues. The old error.errors property from v3 (an alias for the same array) was removed, so v4 code should switch to issues. [1], [2]
Practical migration:
const issues = (err as any).issues ?? (err as any).errors; // if you need v3+v4 compatType-wise, v4’s ZodError interface includes issues: ZodIssue[] (and does not list errors). [3]
Sources: Zod v4 docs / type declarations showing issues [3], and Zod maintainers/users noting error.errors is gone and “it’s error.issues now” [1], [2].
🏁 Script executed:
cat -n apps/desktop/src/features/connections/validation.ts | head -130 | tail -30Repository: remcostoeten/dora
Length of output: 1040
Switch error.errors to error.issues for Zod v4 compatibility.
The project uses Zod v4, which removed the error.errors property and replaced it with error.issues. The current code will throw a runtime error when validation fails.
Suggested change
} catch (error) {
if (error instanceof z.ZodError) {
- const firstError = error.errors[0]
+ const issues = (error.issues ?? (error as any).errors ?? [])
+ const firstError = issues[0]
+ if (!firstError) return { success: false, error: 'Validation failed' }
return {
success: false,
error: firstError.message,
field: firstError.path[0] as string,
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (error instanceof z.ZodError) { | |
| const firstError = error.errors[0] | |
| return { | |
| success: false, | |
| error: firstError.message, | |
| field: firstError.path[0] as string, | |
| } | |
| } catch (error) { | |
| if (error instanceof z.ZodError) { | |
| const issues = (error.issues ?? (error as any).errors ?? []) | |
| const firstError = issues[0] | |
| if (!firstError) return { success: false, error: 'Validation failed' } | |
| return { | |
| success: false, | |
| error: firstError.message, | |
| field: firstError.path[0] as string, | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/desktop/src/features/connections/validation.ts` around lines 112 - 118,
The Zod v4 compatibility fix: in the Zod error handling inside the validation
logic (where you check `if (error instanceof z.ZodError)` and use `error.errors`
and `firstError`), replace uses of `error.errors` with `error.issues` (e.g., set
`const firstError = error.issues[0]`) and keep extracting `firstError.message`
and `firstError.path[0]` as before so the function returns the same shape (`{
success: false, error: ..., field: ... }`) without causing a runtime exception.
| {onExport && ( | ||
| <Button | ||
| variant='ghost' | ||
| size='icon' | ||
| className={cn( | ||
| 'h-7 w-7 text-muted-foreground hover:text-foreground', | ||
| !hasResults && 'opacity-50 cursor-not-allowed' | ||
| )} | ||
| onClick={onExport} | ||
| disabled={!hasResults} | ||
| title='Export results as JSON' | ||
| > | ||
| <Download className='h-3.5 w-3.5' /> | ||
| </Button> | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button | ||
| variant='ghost' | ||
| size='icon' | ||
| className={cn( | ||
| 'h-7 w-7 text-muted-foreground hover:text-foreground', | ||
| !hasResults && 'opacity-50 cursor-not-allowed' | ||
| )} | ||
| disabled={!hasResults} | ||
| title='Export results' | ||
| > | ||
| <Download className='h-3.5 w-3.5' /> | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align='end'> | ||
| <DropdownMenuItem onClick={onExport}> | ||
| Export as JSON | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem onClick={onExportCsv}> | ||
| Export as CSV | ||
| </DropdownMenuItem> |
There was a problem hiding this comment.
Guard the CSV menu item when onExportCsv is absent.
onExportCsv is optional, but the menu item always renders; this creates a visible no-op.
✅ Suggested change
- <DropdownMenuItem onClick={onExportCsv}>
- Export as CSV
- </DropdownMenuItem>
+ {onExportCsv && (
+ <DropdownMenuItem onClick={onExportCsv}>
+ Export as CSV
+ </DropdownMenuItem>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {onExport && ( | |
| <Button | |
| variant='ghost' | |
| size='icon' | |
| className={cn( | |
| 'h-7 w-7 text-muted-foreground hover:text-foreground', | |
| !hasResults && 'opacity-50 cursor-not-allowed' | |
| )} | |
| onClick={onExport} | |
| disabled={!hasResults} | |
| title='Export results as JSON' | |
| > | |
| <Download className='h-3.5 w-3.5' /> | |
| </Button> | |
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild> | |
| <Button | |
| variant='ghost' | |
| size='icon' | |
| className={cn( | |
| 'h-7 w-7 text-muted-foreground hover:text-foreground', | |
| !hasResults && 'opacity-50 cursor-not-allowed' | |
| )} | |
| disabled={!hasResults} | |
| title='Export results' | |
| > | |
| <Download className='h-3.5 w-3.5' /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent align='end'> | |
| <DropdownMenuItem onClick={onExport}> | |
| Export as JSON | |
| </DropdownMenuItem> | |
| <DropdownMenuItem onClick={onExportCsv}> | |
| Export as CSV | |
| </DropdownMenuItem> | |
| {onExport && ( | |
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild> | |
| <Button | |
| variant='ghost' | |
| size='icon' | |
| className={cn( | |
| 'h-7 w-7 text-muted-foreground hover:text-foreground', | |
| !hasResults && 'opacity-50 cursor-not-allowed' | |
| )} | |
| disabled={!hasResults} | |
| title='Export results' | |
| > | |
| <Download className='h-3.5 w-3.5' /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent align='end'> | |
| <DropdownMenuItem onClick={onExport}> | |
| Export as JSON | |
| </DropdownMenuItem> | |
| {onExportCsv && ( | |
| <DropdownMenuItem onClick={onExportCsv}> | |
| Export as CSV | |
| </DropdownMenuItem> | |
| )} |
🤖 Prompt for AI Agents
In `@apps/desktop/src/features/sql-console/components/console-toolbar.tsx` around
lines 207 - 229, The CSV menu item currently renders even when the optional
handler onExportCsv is undefined, producing a visible no-op; update the
DropdownMenu content to either conditionally render the DropdownMenuItem for CSV
only when onExportCsv is truthy or render it disabled when onExportCsv is
absent. Locate the DropdownMenuItem that references onExportCsv in
console-toolbar.tsx and wrap it in a guard (e.g., if (onExportCsv) render the
item) or add disabled and cursor styles plus no-op protection so clicking cannot
occur when onExportCsv is undefined.
| const handleExportCsv = useCallback(function () { | ||
| if (!result || result.rows.length === 0) return | ||
|
|
||
| const headers = result.columns.join(',') | ||
| const rows = result.rows.map(function (row) { | ||
| return result.columns.map(function (col) { | ||
| const value = row[col] | ||
| if (value === null || value === undefined) return '' | ||
| const stringValue = String(value) | ||
| if (stringValue.includes(',') || stringValue.includes('"') || stringValue.includes('\n')) { | ||
| return '"' + stringValue.replace(/"/g, '""') + '"' | ||
| } | ||
| return stringValue | ||
| }).join(',') | ||
| }).join('\n') | ||
|
|
||
| const csvContent = headers + '\n' + rows | ||
| const blob = new Blob([csvContent], { type: 'text/csv' }) | ||
| const url = URL.createObjectURL(blob) | ||
| const a = document.createElement('a') | ||
| a.href = url | ||
| a.download = 'query-results.csv' | ||
| a.click() | ||
| URL.revokeObjectURL(url) | ||
| }, [result]) |
There was a problem hiding this comment.
CSV export assumes object rows (and string columns), which can produce empty CSVs.
If rows are arrays or columns are objects, row[col] yields undefined. Normalize columns and handle array rows.
✅ Suggested fix
const handleExportCsv = useCallback(function () {
if (!result || result.rows.length === 0) return
- const headers = result.columns.join(',')
- const rows = result.rows.map(function (row) {
- return result.columns.map(function (col) {
- const value = row[col]
- if (value === null || value === undefined) return ''
- const stringValue = String(value)
- if (stringValue.includes(',') || stringValue.includes('"') || stringValue.includes('\n')) {
- return '"' + stringValue.replace(/"/g, '""') + '"'
- }
- return stringValue
- }).join(',')
- }).join('\n')
+ const columns = result.columns.map((c: any) => (typeof c === 'string' ? c : c.name))
+ const escape = function (value: unknown) {
+ if (value === null || value === undefined) return ''
+ const stringValue = String(value)
+ if (stringValue.includes(',') || stringValue.includes('"') || stringValue.includes('\n')) {
+ return '"' + stringValue.replace(/"/g, '""') + '"'
+ }
+ return stringValue
+ }
+ const headers = columns.map(escape).join(',')
+ const rows = result.rows
+ .map(function (row) {
+ if (Array.isArray(row)) {
+ return columns.map((_, i) => escape(row[i])).join(',')
+ }
+ if (row && typeof row === 'object') {
+ return columns.map((col) => escape((row as Record<string, unknown>)[col])).join(',')
+ }
+ return columns.map(() => '').join(',')
+ })
+ .join('\n')
const csvContent = headers + '\n' + rows
const blob = new Blob([csvContent], { type: 'text/csv' })
const url = URL.createObjectURL(blob)
const a = document.createElement('a')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleExportCsv = useCallback(function () { | |
| if (!result || result.rows.length === 0) return | |
| const headers = result.columns.join(',') | |
| const rows = result.rows.map(function (row) { | |
| return result.columns.map(function (col) { | |
| const value = row[col] | |
| if (value === null || value === undefined) return '' | |
| const stringValue = String(value) | |
| if (stringValue.includes(',') || stringValue.includes('"') || stringValue.includes('\n')) { | |
| return '"' + stringValue.replace(/"/g, '""') + '"' | |
| } | |
| return stringValue | |
| }).join(',') | |
| }).join('\n') | |
| const csvContent = headers + '\n' + rows | |
| const blob = new Blob([csvContent], { type: 'text/csv' }) | |
| const url = URL.createObjectURL(blob) | |
| const a = document.createElement('a') | |
| a.href = url | |
| a.download = 'query-results.csv' | |
| a.click() | |
| URL.revokeObjectURL(url) | |
| }, [result]) | |
| const handleExportCsv = useCallback(function () { | |
| if (!result || result.rows.length === 0) return | |
| const columns = result.columns.map((c: any) => (typeof c === 'string' ? c : c.name)) | |
| const escape = function (value: unknown) { | |
| if (value === null || value === undefined) return '' | |
| const stringValue = String(value) | |
| if (stringValue.includes(',') || stringValue.includes('"') || stringValue.includes('\n')) { | |
| return '"' + stringValue.replace(/"/g, '""') + '"' | |
| } | |
| return stringValue | |
| } | |
| const headers = columns.map(escape).join(',') | |
| const rows = result.rows | |
| .map(function (row) { | |
| if (Array.isArray(row)) { | |
| return columns.map((_, i) => escape(row[i])).join(',') | |
| } | |
| if (row && typeof row === 'object') { | |
| return columns.map((col) => escape((row as Record<string, unknown>)[col])).join(',') | |
| } | |
| return columns.map(() => '').join(',') | |
| }) | |
| .join('\n') | |
| const csvContent = headers + '\n' + rows | |
| const blob = new Blob([csvContent], { type: 'text/csv' }) | |
| const url = URL.createObjectURL(blob) | |
| const a = document.createElement('a') | |
| a.href = url | |
| a.download = 'query-results.csv' | |
| a.click() | |
| URL.revokeObjectURL(url) | |
| }, [result]) |
🤖 Prompt for AI Agents
In `@apps/desktop/src/features/sql-console/sql-console.tsx` around lines 294 -
318, The CSV export in handleExportCsv assumes each row is an object keyed by
string column names and that result.columns are strings; change it to normalize
columns to an array of string keys (e.g., map result.columns to colName via
col.name || String(col)) and when serializing rows handle both object rows and
array rows (if Array.isArray(row) use row[index], otherwise use row[colName]);
also ensure missing values (undefined/null) become empty strings and still apply
CSV escaping for commas, quotes and newlines before building the Blob and
triggering download.
| return ( | ||
| <Button | ||
| variant={variant} | ||
| size={size} | ||
| disabled | ||
| className={cn('cursor-not-allowed opacity-50', className)} | ||
| onClick={handleClick} | ||
| title={`${feature} - Coming soon`} |
There was a problem hiding this comment.
disabled blocks clicks, so the toast never fires.
A disabled button won’t emit onClick. Use aria-disabled (or a wrapper) if you still want to show a toast.
🔧 Suggested fix
- <Button
+ <Button
variant={variant}
size={size}
- disabled
+ aria-disabled
+ data-disabled
className={cn('cursor-not-allowed opacity-50', className)}
- onClick={handleClick}
+ onClick={(event) => {
+ event.preventDefault()
+ handleClick()
+ }}
title={`${feature} - Coming soon`}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <Button | |
| variant={variant} | |
| size={size} | |
| disabled | |
| className={cn('cursor-not-allowed opacity-50', className)} | |
| onClick={handleClick} | |
| title={`${feature} - Coming soon`} | |
| return ( | |
| <Button | |
| variant={variant} | |
| size={size} | |
| aria-disabled | |
| data-disabled | |
| className={cn('cursor-not-allowed opacity-50', className)} | |
| onClick={(event) => { | |
| event.preventDefault() | |
| handleClick() | |
| }} | |
| title={`${feature} - Coming soon`} |
🤖 Prompt for AI Agents
In `@apps/desktop/src/shared/ui/disabled-feature.tsx` around lines 31 - 38, The
Button currently uses the disabled prop which prevents onClick from firing;
remove the disabled attribute and instead set aria-disabled={true} on the Button
(keeping the cursor-not-allowed/opacity styles) so the control remains clickable
and handleClick can fire the toast; ensure the existing onClick={handleClick}
and title={`${feature} - Coming soon`} remain unchanged (update in
disabled-feature.tsx where Button is rendered).
| @@ -0,0 +1,71 @@ | |||
| import { cn } from '@/shared/lib/utils' | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if `@/shared/lib/utils` exists and exports cn
echo "=== Checking for utils files in shared directory ==="
find apps/desktop/src/shared -type f \( -name "utils.ts" -o -name "utils.tsx" -o -name "cn.ts" -o -name "cn.tsx" \) 2>/dev/null | head -20
echo -e "\n=== Directory structure of shared ==="
tree -L 3 apps/desktop/src/shared 2>/dev/null || find apps/desktop/src/shared -type d | head -20
echo -e "\n=== Check for cn exports in shared directory ==="
rg -n "export\s+(function\s+)?cn|export\s*\{\s*cn" apps/desktop/src/shared -A 2Repository: remcostoeten/dora
Length of output: 1404
🏁 Script executed:
#!/bin/bash
# Check how cn is imported in other files in this PR/branch
echo "=== How cn is imported across the codebase ==="
rg "import.*cn.*from.*['\"]@/" apps/desktop/src/shared -n
rg "import.*cn.*from.*['\"]@/" apps/desktop/src --include="*.ts" --include="*.tsx" | head -15Repository: remcostoeten/dora
Length of output: 2526
🏁 Script executed:
#!/bin/bash
# Verify the actual path and structure - check the skeleton.tsx file
echo "=== Content of skeleton.tsx to see the import ==="
head -10 apps/desktop/src/shared/ui/skeleton.tsxRepository: remcostoeten/dora
Length of output: 310
Change import path from @/shared/lib/utils to @/shared/utils/cn
The path @/shared/lib/utils does not exist. The cn utility is located at @/shared/utils/cn. This import will fail at build time. Align with all other UI components in the shared/ui directory that correctly import from @/shared/utils/cn.
🤖 Prompt for AI Agents
In `@apps/desktop/src/shared/ui/skeleton.tsx` at line 1, Update the import for the
classnames helper: replace the non-existent import from '@/shared/lib/utils'
with the correct module '@/shared/utils/cn' so the `cn` symbol used in this file
resolves; update the import statement that currently imports `cn` to reference
`@/shared/utils/cn` to match other components in shared/ui.
| ``` | ||
| apps/desktop/src/shared/ui/error-boundary.tsx | ||
| apps/desktop/src/shared/ui/error-fallback.tsx | ||
| apps/desktop/src/shared/utils/error-messages.ts | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
This resolves markdownlint warnings and improves syntax highlighting.
✅ Suggested change
-```
+```text
apps/desktop/src/shared/ui/error-boundary.tsx
apps/desktop/src/shared/ui/error-fallback.tsx
apps/desktop/src/shared/utils/error-messages.ts
-```
+```
-```
+```text
apps/desktop/src/pages/Index.tsx
- Added ErrorBoundary wrapping
- Added EmptyState for no connections
@@
apps/desktop/src/features/connections/components/connection-dialog/connection-form.tsx
- Disabled SSH tunnel checkbox with "Soon" badge and tooltip
- Removed unused SshTunnelConfigForm import
-```
+```
-```
+```text
apps/desktop/
├── src/
│ ├── features/
@@
│ └── Index.tsx # Main app shell
├── src-tauri/
│ └── src/
│ ├── database/ # DB operations
│ └── commands/ # Tauri commands
-```
+```Also applies to: 61-95, 277-301
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@RELEASE_TRACKER.md` around lines 54 - 58, The Markdown fenced code blocks in
RELEASE_TRACKER.md lack language identifiers which triggers MD040; update each
triple-backtick block that lists file paths or ASCII trees (the blocks around
the lists like the ones containing
"apps/desktop/src/shared/ui/error-boundary.tsx" and the ASCII tree under
"apps/desktop/") to use a language tag (use "text") by changing ``` to ```text
for all occurrences (also apply the same fix to the other ranges noted around
lines 61-95 and 277-301).
|
All changes from this PR have already been integrated into master. Closing as duplicate work. |
1 participant
Summary by CodeRabbit
Release Notes
New Features
Improvements
Tests