Ask before deleting accounts and categories

ROADMAP.md

@@ -24,7 +24,7 @@ Features that build directly on what exists today.
 - [ ] **Auto calculate charges for Ecocash**
   - For payments and tranfers done using Ecocash specify amount spend and left amount then auto calculate charges
 
-- [ ] **Protect account deletion**
+- [x] **Protect account deletion**
   - If a user press delete account prompt them to confirm before deletion
 
 - [ ] **Correct account amounts**

src/commands/mod.rs

@@ -286,16 +286,42 @@ pub fn update_account(
     Ok(account)
 }
 
+/// Deletes the account along with its transactions, so they stop counting
+/// towards totals. Subscriptions billed to it are kept but unlinked.
 pub fn delete_account(state: &DbState, id: String) -> Result<(), String> {
-    let conn = state.0.lock().map_err(|e| e.to_string())?;
-    conn.execute(
+    let mut conn = state.0.lock().map_err(|e| e.to_string())?;
+    let tx = conn.transaction().map_err(|e| e.to_string())?;
+    tx.execute(
         "UPDATE accounts SET deleted_at = datetime('now') WHERE id = ?1",
         params![id],
     )
     .map_err(|e| e.to_string())?;
+    tx.execute(
+        "UPDATE transactions SET deleted_at = datetime('now') WHERE account_id = ?1 AND deleted_at IS NULL",
+        params![id],
+    )
+    .map_err(|e| e.to_string())?;
+    tx.execute(
+        "UPDATE subscriptions SET account_id = NULL, updated_at = datetime('now') WHERE account_id = ?1",
+        params![id],
+    )
+    .map_err(|e| e.to_string())?;
+    tx.commit().map_err(|e| e.to_string())?;
     Ok(())
 }
 
+/// How many live transactions belong to an account.
+pub fn count_account_transactions(state: &DbState, id: &str) -> Result<usize, String> {
+    let conn = state.0.lock().map_err(|e| e.to_string())?;
+    conn.query_row(
+        "SELECT COUNT(*) FROM transactions WHERE account_id = ?1 AND deleted_at IS NULL",
+        params![id],
+        |row| row.get::<_, i64>(0),
+    )
+    .map(|n| n as usize)
+    .map_err(|e| e.to_string())
+}
+
 fn get_account_by_id(conn: &rusqlite::Connection, id: &str) -> Result<Account, String> {
     conn.query_row(
         "SELECT id, name, account_type, currency, balance, color, icon, is_active, sort_order, created_at, updated_at
@@ -1690,4 +1716,69 @@ mod tests {
         assert_eq!(overview.total_balance, accounts_total);
         assert!(overview.total_income > 0 && overview.total_expenses > 0);
     }
+
+    /// The seeded account with the most transactions.
+    fn busiest_account(db: &DbState) -> String {
+        let txs = get_transactions(db).unwrap();
+        get_accounts(db)
+            .unwrap()
+            .into_iter()
+            .max_by_key(|a| txs.iter().filter(|t| t.account_id == a.id).count())
+            .unwrap()
+            .id
+    }
+
+    #[test]
+    fn deleting_an_account_hides_its_transactions() {
+        let db = seeded();
+        let id = busiest_account(&db);
+        let before = get_transactions(&db).unwrap();
+        let owned = before.iter().filter(|t| t.account_id == id).count();
+        assert!(owned > 0, "seed gives the account some transactions");
+        assert_eq!(count_account_transactions(&db, &id).unwrap(), owned);
+
+        delete_account(&db, id.clone()).unwrap();
+
+        let after = get_transactions(&db).unwrap();
+        assert_eq!(after.len(), before.len() - owned);
+        assert!(after.iter().all(|t| t.account_id != id));
+        assert!(get_overview(&db).unwrap().recent_transactions.iter().all(|t| t.account_id != id));
+        assert_eq!(count_account_transactions(&db, &id).unwrap(), 0);
+    }
+
+    #[test]
+    fn deleting_an_account_keeps_its_subscriptions_unlinked() {
+        let db = seeded();
+        let id = busiest_account(&db);
+        let sub = create_subscription(
+            &db,
+            CreateSubscriptionRequest {
+                name: "Streaming".into(),
+                amount: 999,
+                currency: "USD".into(),
+                frequency: "monthly".into(),
+                category_id: None,
+                account_id: Some(id.clone()),
+                start_date: None,
+            },
+        )
+        .unwrap();
+
+        delete_account(&db, id).unwrap();
+
+        let kept = get_subscriptions(&db).unwrap().into_iter().find(|s| s.id == sub.id).expect("subscription kept");
+        assert_eq!(kept.account_id, None);
+    }
+
+    #[test]
+    fn migrations_clean_up_accounts_deleted_the_old_way() {
+        let db = seeded();
+        let id = busiest_account(&db);
+        {
+            let conn = db.0.lock().unwrap();
+            conn.execute("UPDATE accounts SET deleted_at = datetime('now') WHERE id = ?1", params![id]).unwrap();
+            crate::database::run_migrations(&conn).unwrap();
+        }
+        assert!(get_transactions(&db).unwrap().iter().all(|t| t.account_id != id));
+    }
 }

src/database/mod.rs

@@ -104,6 +104,18 @@ pub fn run_migrations(conn: &Connection) -> Result<()> {
         ",
     )?;
 
+    // Accounts deleted before deletes cascaded left their transactions behind.
+    conn.execute_batch(
+        "
+        UPDATE transactions SET deleted_at = datetime('now')
+        WHERE deleted_at IS NULL
+          AND account_id IN (SELECT id FROM accounts WHERE deleted_at IS NOT NULL);
+
+        UPDATE subscriptions SET account_id = NULL
+        WHERE account_id IN (SELECT id FROM accounts WHERE deleted_at IS NOT NULL);
+        ",
+    )?;
+
     Ok(())
 }
 

src/ui/accounts.rs

@@ -16,6 +16,7 @@ use std::sync::Arc;
 enum Dialog {
     Add(Entity<AddAccountForm>),
     Transfer(Entity<TransferForm>),
+    Confirm(Entity<ConfirmDialog>, String),
 }
 
 pub struct AccountsScreen {
@@ -79,10 +80,40 @@ impl AccountsScreen {
         cx.notify();
     }
 
+    fn ask_delete(&mut self, id: String, cx: &mut Context<Self>) {
+        let Some(name) = self.accounts().iter().find(|a| a.id == id).map(|a| a.name.clone()) else {
+            return;
+        };
+        let records = commands::count_account_transactions(&self.db, &id).unwrap_or(0);
+        let message = if records == 0 {
+            format!("This will permanently remove {name}. This action cannot be undone.")
+        } else {
+            format!(
+                "This will permanently remove {name} and its {}. This action cannot be undone.",
+                plural(records, "transaction")
+            )
+        };
+        let dialog = cx.new(|cx| ConfirmDialog::new("Delete account?", message, "Delete", cx));
+        self._dialog_sub = Some(cx.subscribe(&dialog, |this, _, event: &ConfirmEvent, cx| {
+            if *event == ConfirmEvent::Confirm {
+                if let Some(Dialog::Confirm(_, id)) = this.dialog.take() {
+                    this.delete(id, cx);
+                }
+            }
+            this.dialog = None;
+            this._dialog_sub = None;
+            cx.notify();
+        }));
+        self.dialog = Some(Dialog::Confirm(dialog, id));
+        cx.notify();
+    }
+
     fn delete(&mut self, id: String, cx: &mut Context<Self>) {
+        let name = self.accounts().iter().find(|a| a.id == id).map(|a| a.name.clone()).unwrap_or_default();
         match commands::delete_account(&self.db, id) {
             Ok(()) => {
                 self.accounts = commands::get_accounts(&self.db);
+                cx.emit(ScreenEvent::ok(format!("Deleted account {name}")));
             }
             Err(e) => cx.emit(ScreenEvent::error(format!("Could not delete account: {e}"))),
         }
@@ -127,7 +158,7 @@ impl Render for AccountsScreen {
                             )
                             .child(
                                 button(SharedString::from(format!("delete-account-{}", a.id)), "Delete", ButtonVariant::Danger, cx)
-                                    .on_click(cx.listener(move |this, _, _, cx| this.delete(id.clone(), cx))),
+                                    .on_click(cx.listener(move |this, _, _, cx| this.ask_delete(id.clone(), cx))),
                             )
                     })
                     .collect();
@@ -138,6 +169,7 @@ impl Render for AccountsScreen {
         let dialog = self.dialog.as_ref().map(|d| match d {
             Dialog::Add(form) => form.clone().into_any_element(),
             Dialog::Transfer(form) => form.clone().into_any_element(),
+            Dialog::Confirm(dialog, _) => dialog.clone().into_any_element(),
         });
 
         div()
@@ -449,15 +481,39 @@ mod tests {
         assert!(matches!(events.borrow().last(), Some(ScreenEvent::Toast { ok: false, .. })));
     }
 
+    fn ask_delete(screen: &Entity<AccountsScreen>, id: &str, cx: &mut VisualTestContext) -> Entity<ConfirmDialog> {
+        cx.update(|_, cx| screen.update(cx, |s, cx| s.ask_delete(id.to_string(), cx)));
+        cx.update(|_, cx| match screen.read(cx).dialog.as_ref() {
+            Some(Dialog::Confirm(d, _)) => d.clone(),
+            _ => panic!("confirm dialog should be open"),
+        })
+    }
+
+    #[gpui_kit::test]
+    fn delete_asks_first_and_removes_on_confirm(cx: &mut TestAppContext) {
+        let (screen, cx, events) = setup(cx);
+        let id = cx.update(|_, cx| screen.read(cx).accounts()[0].id.clone());
+        let dialog = ask_delete(&screen, &id, cx);
+        cx.update(|_, cx| assert_eq!(screen.read(cx).accounts().len(), 4, "nothing deleted until confirmed"));
+        cx.update(|_, cx| dialog.update(cx, |_, cx| cx.emit(ConfirmEvent::Confirm)));
+        cx.update(|_, cx| {
+            let s = screen.read(cx);
+            assert!(!s.has_dialog());
+            assert_eq!(s.accounts().len(), 3);
+            assert!(s.accounts().iter().all(|a| a.id != id));
+        });
+        assert!(matches!(events.borrow().last(), Some(ScreenEvent::Toast { ok: true, .. })));
+    }
+
     #[gpui_kit::test]
-    fn delete_removes_the_account(cx: &mut TestAppContext) {
+    fn cancelling_the_delete_keeps_the_account(cx: &mut TestAppContext) {
         let (screen, cx, _) = setup(cx);
         let id = cx.update(|_, cx| screen.read(cx).accounts()[0].id.clone());
-        cx.update(|_, cx| screen.update(cx, |s, cx| s.delete(id.clone(), cx)));
+        let dialog = ask_delete(&screen, &id, cx);
+        cx.update(|_, cx| dialog.update(cx, |_, cx| cx.emit(ConfirmEvent::Cancel)));
         cx.update(|_, cx| {
-            let accounts = screen.read(cx).accounts();
-            assert_eq!(accounts.len(), 3);
-            assert!(accounts.iter().all(|a| a.id != id));
+            assert!(!screen.read(cx).has_dialog());
+            assert_eq!(screen.read(cx).accounts().len(), 4);
         });
     }
 

src/ui/categories.rs

@@ -36,6 +36,8 @@ pub struct CategoriesScreen {
     counts: HashMap<String, usize>,
     form: Option<Entity<CategoryForm>>,
     _form_sub: Option<Subscription>,
+    confirm: Option<(Entity<ConfirmDialog>, String)>,
+    _confirm_sub: Option<Subscription>,
 }
 
 impl EventEmitter<ScreenEvent> for CategoriesScreen {}
@@ -48,6 +50,8 @@ impl CategoriesScreen {
             counts: HashMap::new(),
             form: None,
             _form_sub: None,
+            confirm: None,
+            _confirm_sub: None,
         };
         screen.load();
         screen
@@ -104,9 +108,40 @@ impl CategoriesScreen {
         cx.notify();
     }
 
+    fn ask_delete(&mut self, id: String, cx: &mut Context<Self>) {
+        let Some(name) = self.categories().iter().find(|c| c.id == id).map(|c| c.name.clone()) else {
+            return;
+        };
+        let records = self.counts.get(&id).copied().unwrap_or(0);
+        let message = if records == 0 {
+            format!("This will permanently remove {name}. This action cannot be undone.")
+        } else {
+            format!(
+                "This will permanently remove {name}. Its {} will be kept without a category. This action cannot be undone.",
+                plural(records, "transaction")
+            )
+        };
+        let dialog = cx.new(|cx| ConfirmDialog::new("Delete category?", message, "Delete", cx));
+        self._confirm_sub = Some(cx.subscribe(&dialog, |this, _, event: &ConfirmEvent, cx| {
+            if let Some((_, id)) = this.confirm.take() {
+                if *event == ConfirmEvent::Confirm {
+                    this.delete(id, cx);
+                }
+            }
+            this._confirm_sub = None;
+            cx.notify();
+        }));
+        self.confirm = Some((dialog, id));
+        cx.notify();
+    }
+
     fn delete(&mut self, id: String, cx: &mut Context<Self>) {
+        let name = self.categories().iter().find(|c| c.id == id).map(|c| c.name.clone()).unwrap_or_default();
         match commands::delete_category(&self.db, id) {
-            Ok(()) => self.load(),
+            Ok(()) => {
+                self.load();
+                cx.emit(ScreenEvent::ok(format!("Deleted category {name}")));
+            }
             Err(e) => cx.emit(ScreenEvent::error(format!("Could not delete category: {e}"))),
         }
         cx.notify();
@@ -175,7 +210,7 @@ impl CategoriesScreen {
                                 ButtonVariant::Danger,
                                 cx,
                             )
-                            .on_click(cx.listener(move |this, _, _, cx| this.delete(id.clone(), cx))),
+                            .on_click(cx.listener(move |this, _, _, cx| this.ask_delete(id.clone(), cx))),
                         ),
                 );
             }
@@ -217,6 +252,7 @@ impl Render for CategoriesScreen {
             .child(page_header("Categories", add, cx))
             .child(div().flex().gap(rems(1.5)).child(expenses).child(income))
             .children(self.form.clone())
+            .children(self.confirm.as_ref().map(|(dialog, _)| dialog.clone()))
     }
 }
 
@@ -453,11 +489,35 @@ mod tests {
         });
     }
 
+    fn ask_delete(screen: &Entity<CategoriesScreen>, id: &str, cx: &mut VisualTestContext) -> Entity<ConfirmDialog> {
+        cx.update(|_, cx| screen.update(cx, |s, cx| s.ask_delete(id.to_string(), cx)));
+        cx.update(|_, cx| screen.read(cx).confirm.as_ref().map(|(d, _)| d.clone()).expect("confirm dialog open"))
+    }
+
     #[gpui_kit::test]
-    fn delete_removes_category(cx: &mut TestAppContext) {
+    fn delete_asks_first_and_removes_on_confirm(cx: &mut TestAppContext) {
         let (screen, cx) = setup(cx);
         let id = cx.update(|_, cx| screen.read(cx).categories()[0].id.clone());
-        cx.update(|_, cx| screen.update(cx, |s, cx| s.delete(id.clone(), cx)));
-        cx.update(|_, cx| assert!(screen.read(cx).categories().iter().all(|c| c.id != id)));
+        let dialog = ask_delete(&screen, &id, cx);
+        cx.update(|_, cx| assert_eq!(screen.read(cx).categories().len(), 8, "nothing deleted until confirmed"));
+        cx.update(|_, cx| dialog.update(cx, |_, cx| cx.emit(ConfirmEvent::Confirm)));
+        cx.update(|_, cx| {
+            let s = screen.read(cx);
+            assert!(s.confirm.is_none());
+            assert!(s.categories().iter().all(|c| c.id != id));
+        });
+    }
+
+    #[gpui_kit::test]
+    fn cancelling_the_delete_keeps_the_category(cx: &mut TestAppContext) {
+        let (screen, cx) = setup(cx);
+        let id = cx.update(|_, cx| screen.read(cx).categories()[0].id.clone());
+        let dialog = ask_delete(&screen, &id, cx);
+        cx.update(|_, cx| dialog.update(cx, |_, cx| cx.emit(ConfirmEvent::Cancel)));
+        cx.update(|_, cx| {
+            let s = screen.read(cx);
+            assert!(s.confirm.is_none());
+            assert_eq!(s.categories().len(), 8);
+        });
     }
 }