Skip to content

Commit 750cbcf

Browse files
committed
Address code review feedback
- Rename TableOperationType.delete to .drop for clarity - Reset dialog state (ignoreForeignKeys, cascade) on open - Restore pending operations on failure for retry capability - Include SQLite in transaction wrapping - Rename 'opts' variable to 'tableOptions' for readability - Add note when multiple tables selected (same options apply to all) - Use SET CONSTRAINTS ALL DEFERRED for PostgreSQL (no superuser required)
1 parent b9be15f commit 750cbcf

4 files changed

Lines changed: 50 additions & 18 deletions

File tree

TablePro/Models/TableOperationOptions.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ struct TableOperationOptions: Codable, Equatable {
1717
/// Type of table operation
1818
enum TableOperationType: String, Codable {
1919
case truncate
20-
case delete
20+
case drop
2121
}

TablePro/Views/Main/MainContentCoordinator.swift

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -613,8 +613,8 @@ final class MainContentCoordinator: ObservableObject {
613613
options[tableName]?.ignoreForeignKeys == true
614614
}
615615

616-
// Wrap in transaction for atomicity (except SQLite which handles differently)
617-
let needsTransaction = (sortedTruncates.count + sortedDeletes.count) > 1 && dbType != .sqlite
616+
// Wrap in transaction for atomicity
617+
let needsTransaction = (sortedTruncates.count + sortedDeletes.count) > 1
618618
if needsTransaction {
619619
statements.append("BEGIN")
620620
}
@@ -625,14 +625,14 @@ final class MainContentCoordinator: ObservableObject {
625625

626626
for tableName in sortedTruncates {
627627
let quotedName = dbType.quoteIdentifier(tableName)
628-
let opts = options[tableName] ?? TableOperationOptions()
629-
statements.append(truncateStatement(tableName: quotedName, options: opts, dbType: dbType))
628+
let tableOptions = options[tableName] ?? TableOperationOptions()
629+
statements.append(truncateStatement(tableName: quotedName, options: tableOptions, dbType: dbType))
630630
}
631631

632632
for tableName in sortedDeletes {
633633
let quotedName = dbType.quoteIdentifier(tableName)
634-
let opts = options[tableName] ?? TableOperationOptions()
635-
statements.append(dropTableStatement(tableName: quotedName, options: opts, dbType: dbType))
634+
let tableOptions = options[tableName] ?? TableOperationOptions()
635+
statements.append(dropTableStatement(tableName: quotedName, options: tableOptions, dbType: dbType))
636636
}
637637

638638
if needsDisableFK {
@@ -700,11 +700,16 @@ final class MainContentCoordinator: ObservableObject {
700700
let truncatedTables = Set(pendingTruncates)
701701
let conn = connection
702702

703-
// Clear operations immediately (before async execution)
703+
// Capture options before clearing (for potential restore on failure)
704+
var capturedOptions: [String: TableOperationOptions] = [:]
705+
for table in deletedTables.union(truncatedTables) {
706+
capturedOptions[table] = tableOperationOptions[table]
707+
}
708+
709+
// Clear operations immediately (to prevent double-execution)
704710
if clearTableOps {
705711
pendingTruncates.removeAll()
706712
pendingDeletes.removeAll()
707-
// Clear options for processed tables
708713
for table in deletedTables.union(truncatedTables) {
709714
tableOperationOptions.removeValue(forKey: table)
710715
}
@@ -791,6 +796,17 @@ final class MainContentCoordinator: ObservableObject {
791796
if let index = tabManager.selectedTabIndex {
792797
tabManager.tabs[index].errorMessage = "Save failed: \(error.localizedDescription)"
793798
}
799+
800+
// Restore operations on failure so user can retry
801+
if clearTableOps {
802+
DatabaseManager.shared.updateSession(conn.id) { session in
803+
session.pendingTruncates = truncatedTables
804+
session.pendingDeletes = deletedTables
805+
for (table, opts) in capturedOptions {
806+
session.tableOperationOptions[table] = opts
807+
}
808+
}
809+
}
794810
}
795811
}
796812
}

TablePro/Views/Sidebar/SidebarView.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ struct SidebarView: View {
321321
pendingDeletes = updated
322322
} else {
323323
// Show dialog to confirm operation
324-
pendingOperationType = .delete
324+
pendingOperationType = .drop
325325
pendingOperationTables = tablesToToggle
326326
showOperationDialog = true
327327
}

TablePro/Views/Sidebar/TableOperationDialog.swift

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ struct TableOperationDialog: View {
2828

2929
private var title: String {
3030
switch operationType {
31-
case .delete:
32-
return "Delete table '\(tableName)'"
31+
case .drop:
32+
return "Drop table '\(tableName)'"
3333
case .truncate:
3434
return "Truncate table '\(tableName)'"
3535
}
@@ -40,10 +40,14 @@ struct TableOperationDialog: View {
4040
databaseType != .sqlite
4141
}
4242

43+
private var isMultipleTables: Bool {
44+
tableName.contains("tables")
45+
}
46+
4347
private var cascadeDescription: String {
4448
switch operationType {
45-
case .delete:
46-
return "Delete all rows linked by foreign keys"
49+
case .drop:
50+
return "Drop all tables that depend on this table"
4751
case .truncate:
4852
if databaseType == .mysql || databaseType == .mariadb {
4953
return "Not supported for TRUNCATE in MySQL/MariaDB"
@@ -74,6 +78,13 @@ struct TableOperationDialog: View {
7478

7579
// Options
7680
VStack(alignment: .leading, spacing: 16) {
81+
// Note for multiple tables
82+
if isMultipleTables {
83+
Text("Same options will be applied to all selected tables.")
84+
.font(.system(size: 11))
85+
.foregroundStyle(.secondary)
86+
}
87+
7788
// Ignore foreign key checks
7889
Toggle(isOn: $ignoreForeignKeys) {
7990
Text("Ignore foreign key checks")
@@ -120,6 +131,11 @@ struct TableOperationDialog: View {
120131
}
121132
.frame(width: 320)
122133
.background(Color(nsColor: .windowBackgroundColor))
134+
.onAppear {
135+
// Reset state when dialog opens
136+
ignoreForeignKeys = false
137+
cascade = false
138+
}
123139
.onExitCommand {
124140
isPresented = false
125141
}
@@ -137,11 +153,11 @@ struct TableOperationDialog: View {
137153

138154
// MARK: - Preview
139155

140-
#Preview("Delete Table - MySQL") {
156+
#Preview("Drop Table - MySQL") {
141157
TableOperationDialog(
142158
isPresented: .constant(true),
143159
tableName: "users",
144-
operationType: .delete,
160+
operationType: .drop,
145161
databaseType: .mysql,
146162
onConfirm: { options in
147163
print("Options: \(options)")
@@ -161,11 +177,11 @@ struct TableOperationDialog: View {
161177
)
162178
}
163179

164-
#Preview("Delete Table - SQLite") {
180+
#Preview("Drop Table - SQLite") {
165181
TableOperationDialog(
166182
isPresented: .constant(true),
167183
tableName: "products",
168-
operationType: .delete,
184+
operationType: .drop,
169185
databaseType: .sqlite,
170186
onConfirm: { options in
171187
print("Options: \(options)")

0 commit comments

Comments
 (0)