diff --git a/ROADMAP.md b/ROADMAP.md index 81f6106..bee608a 100644 --- a/ROADMAP.md +++ b/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** diff --git a/src/commands/mod.rs b/src/commands/mod.rs index a7a7632..c0d823e 100644 --- a/src/commands/mod.rs +++ b/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 { + 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 { 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)); + } } diff --git a/src/database/mod.rs b/src/database/mod.rs index d298ca2..b52b6a8 100644 --- a/src/database/mod.rs +++ b/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(()) } diff --git a/src/ui/accounts.rs b/src/ui/accounts.rs index 23e4253..a1077b7 100644 --- a/src/ui/accounts.rs +++ b/src/ui/accounts.rs @@ -16,6 +16,7 @@ use std::sync::Arc; enum Dialog { Add(Entity), Transfer(Entity), + Confirm(Entity, String), } pub struct AccountsScreen { @@ -79,10 +80,40 @@ impl AccountsScreen { cx.notify(); } + fn ask_delete(&mut self, id: String, cx: &mut Context) { + 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) { + 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, id: &str, cx: &mut VisualTestContext) -> Entity { + 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); }); } diff --git a/src/ui/categories.rs b/src/ui/categories.rs index 1435568..532f128 100644 --- a/src/ui/categories.rs +++ b/src/ui/categories.rs @@ -36,6 +36,8 @@ pub struct CategoriesScreen { counts: HashMap, form: Option>, _form_sub: Option, + confirm: Option<(Entity, String)>, + _confirm_sub: Option, } impl EventEmitter 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) { + 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) { + 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, id: &str, cx: &mut VisualTestContext) -> Entity { + 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); + }); } }