From a25327d37000cbc1c7f4bcaf11526cc4d1f9fb61 Mon Sep 17 00:00:00 2001 From: Jed Fox <git@jedfox.com> Date: Tue, 6 Jun 2023 16:41:46 -0400 Subject: [PATCH] Remove account types (#948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #944, closes #532. ~WIP because something is causing the test budget to fail to create because it’s using INSERT instead of UPDATE sql queries. (Or not? I have no idea)~ --- packages/desktop-client/e2e/accounts.test.js | 1 - .../e2e/page-models/navigation.js | 1 - .../src/components/accounts/MobileAccounts.js | 22 ------- .../modals/ConfigureLinkedAccounts.js | 13 +--- .../components/modals/CreateLocalAccount.js | 40 +----------- packages/import-ynab4/importer.js | 19 ------ packages/import-ynab5/importer.js | 20 ------ .../1682265217543_remove_account_type.sql | 5 ++ .../loot-core/src/client/actions/queries.ts | 4 +- packages/loot-core/src/mocks/budget.ts | 62 +++++++++---------- packages/loot-core/src/mocks/index.ts | 3 +- .../loot-core/src/server/accounts/link.ts | 3 - packages/loot-core/src/server/api-models.ts | 1 - packages/loot-core/src/server/api.ts | 1 - .../loot-core/src/server/aql/schema/index.ts | 2 - packages/loot-core/src/server/db/index.ts | 6 -- packages/loot-core/src/server/main.ts | 5 -- packages/loot-core/src/server/models.ts | 21 +------ packages/loot-core/src/shared/accounts.ts | 55 ++-------------- .../loot-core/src/types/main.handlers.d.ts | 1 - .../loot-core/src/types/models/account.d.ts | 1 - upcoming-release-notes/948.md | 6 ++ 22 files changed, 52 insertions(+), 240 deletions(-) create mode 100644 packages/loot-core/migrations/1682265217543_remove_account_type.sql create mode 100644 upcoming-release-notes/948.md diff --git a/packages/desktop-client/e2e/accounts.test.js b/packages/desktop-client/e2e/accounts.test.js index da28d63c1..631cc3070 100644 --- a/packages/desktop-client/e2e/accounts.test.js +++ b/packages/desktop-client/e2e/accounts.test.js @@ -24,7 +24,6 @@ test.describe('Accounts', () => { test('creates a new account and views the initial balance transaction', async () => { const accountPage = await navigation.createAccount({ name: 'New Account', - type: 'Checking / Cash', offBudget: false, balance: 100, }); diff --git a/packages/desktop-client/e2e/page-models/navigation.js b/packages/desktop-client/e2e/page-models/navigation.js index ad04a6387..00a6d801d 100644 --- a/packages/desktop-client/e2e/page-models/navigation.js +++ b/packages/desktop-client/e2e/page-models/navigation.js @@ -60,7 +60,6 @@ export class Navigation { // Fill the form await this.page.getByLabel('Name:').fill(data.name); - await this.page.getByLabel('Type:').selectOption({ label: data.type }); await this.page.getByLabel('Balance:').fill(String(data.balance)); if (data.offBudget) { diff --git a/packages/desktop-client/src/components/accounts/MobileAccounts.js b/packages/desktop-client/src/components/accounts/MobileAccounts.js index 6604e1960..3f0946095 100644 --- a/packages/desktop-client/src/components/accounts/MobileAccounts.js +++ b/packages/desktop-client/src/components/accounts/MobileAccounts.js @@ -4,9 +4,7 @@ import { useNavigate } from 'react-router-dom-v5-compat'; import * as actions from 'loot-core/src/client/actions'; import * as queries from 'loot-core/src/client/queries'; -import { prettyAccountType } from 'loot-core/src/shared/accounts'; -import Wallet from '../../icons/v1/Wallet'; import { colors, styles } from '../../style'; import { withThemeColor } from '../../util/withThemeColor'; import { Button, Text, TextOneLine, View } from '../common'; @@ -104,26 +102,6 @@ export function AccountCard({ account, updated, getBalanceQuery, onSelect }) { /> )} </View> - <View - style={{ - flexDirection: 'row', - alignItems: 'center', - marginTop: '4px', - }} - > - <Text style={[styles.smallText, { color: colors.n5 }]}> - {prettyAccountType(account.type)} - </Text> - <Wallet - style={{ - width: 15, - height: 15, - color: colors.n9, - marginLeft: 8, - marginBottom: 2, - }} - /> - </View> </View> <CellValue binding={getBalanceQuery(account)} diff --git a/packages/desktop-client/src/components/modals/ConfigureLinkedAccounts.js b/packages/desktop-client/src/components/modals/ConfigureLinkedAccounts.js index f54c50c6c..dedafb957 100644 --- a/packages/desktop-client/src/components/modals/ConfigureLinkedAccounts.js +++ b/packages/desktop-client/src/components/modals/ConfigureLinkedAccounts.js @@ -1,10 +1,6 @@ import React, { useState } from 'react'; -import { - fromPlaidAccountType, - determineOffBudget, - prettyAccountType, -} from 'loot-core/src/shared/accounts'; +import { determineOffBudget } from 'loot-core/src/shared/accounts'; import Checkmark from '../../icons/v1/Checkmark'; import { styles, colors } from '../../style'; @@ -47,9 +43,6 @@ function Account({ account, offbudget, onSelect }) { flexDirection: 'row', }} > - {prettyAccountType( - fromPlaidAccountType(account.type, account.subtype), - )} <Text style={{ marginLeft: 4 }}> ... {account.mask} @@ -79,9 +72,7 @@ export default function ConfigureLinkedAccounts({ actions, }) { let [offbudgetAccounts, setOffbudgetAccounts] = useState(() => - accounts - .filter(acct => determineOffBudget(fromPlaidAccountType(acct.type))) - .map(acct => acct.id), + accounts.filter(acct => determineOffBudget(acct.type)).map(acct => acct.id), ); function toggleAccount(id) { diff --git a/packages/desktop-client/src/components/modals/CreateLocalAccount.js b/packages/desktop-client/src/components/modals/CreateLocalAccount.js index 07d97666e..38d729b0a 100644 --- a/packages/desktop-client/src/components/modals/CreateLocalAccount.js +++ b/packages/desktop-client/src/components/modals/CreateLocalAccount.js @@ -2,7 +2,6 @@ import React from 'react'; import { Formik } from 'formik'; -import { determineOffBudget } from 'loot-core/src/shared/accounts'; import { toRelaxedNumber } from 'loot-core/src/shared/util'; import { colors } from '../../style'; @@ -12,7 +11,6 @@ import { ModalButtons, Button, Input, - Select, InlineField, FormError, InitialFocus, @@ -26,17 +24,10 @@ function CreateLocalAccount({ modalProps, actions, history }) { <View> <Formik validateOnChange={false} - initialValues={{ - name: '', - type: 'checking', - balance: '0', - }} + initialValues={{ name: '', balance: '0' }} validate={() => ({})} onSubmit={async (values, { setErrors }) => { const errors = {}; - if (!values.type) { - errors.type = 'required'; - } if (!values.name) { errors.name = 'required'; } @@ -49,7 +40,6 @@ function CreateLocalAccount({ modalProps, actions, history }) { modalProps.onClose(); let id = await actions.createAccount( values.name, - values.type, toRelaxedNumber(values.balance), values.offbudget, ); @@ -84,34 +74,6 @@ function CreateLocalAccount({ modalProps, actions, history }) { </FormError> )} - <InlineField label="Type" width="75%"> - <Select - name="type" - value={values.type} - onChange={e => { - setFieldValue( - 'offbudget', - determineOffBudget(e.target.value), - ); - handleChange(e); - }} - onBlur={handleBlur} - > - <option value="checking">Checking / Cash</option> - <option value="savings">Savings</option> - <option value="credit">Credit Card</option> - <option value="investment">Investment</option> - <option value="mortgage">Mortgage</option> - <option value="debt">Debt</option> - <option value="other">Other</option> - </Select> - </InlineField> - {errors.type && ( - <FormError style={{ marginLeft: 75 }}> - You must select a type - </FormError> - )} - <View style={{ width: '75%', diff --git a/packages/import-ynab4/importer.js b/packages/import-ynab4/importer.js index 6c53124b1..e71c96011 100644 --- a/packages/import-ynab4/importer.js +++ b/packages/import-ynab4/importer.js @@ -11,24 +11,6 @@ import uuid from 'uuid'; // Utils -function mapAccountType(type) { - switch (type) { - case 'Cash': - case 'Checking': - return 'checking'; - case 'CreditCard': - return 'credit'; - case 'Savings': - return 'savings'; - case 'InvestmentAccount': - return 'investment'; - case 'Mortgage': - return 'mortgage'; - default: - return 'other'; - } -} - function sortByKey(arr, key) { return [...arr].sort((item1, item2) => { if (item1[key] < item2[key]) { @@ -82,7 +64,6 @@ async function importAccounts(data, entityIdMap) { accounts.map(async account => { if (!account.isTombstone) { const id = await actual.createAccount({ - type: mapAccountType(account.accountType), name: account.accountName, offbudget: account.onBudget ? false : true, closed: account.hidden ? true : false, diff --git a/packages/import-ynab5/importer.js b/packages/import-ynab5/importer.js index 554bcd51c..25a48c7e6 100644 --- a/packages/import-ynab5/importer.js +++ b/packages/import-ynab5/importer.js @@ -16,25 +16,6 @@ function monthFromDate(date) { return parts[0] + '-' + parts[1]; } -function mapAccountType(type) { - switch (type) { - case 'cash': - case 'checking': - return 'checking'; - case 'creditCard': - case 'lineOfCredit': - return 'credit'; - case 'savings': - return 'savings'; - case 'investmentAccount': - return 'investment'; - case 'mortgage': - return 'mortgage'; - default: - return 'other'; - } -} - function sortByKey(arr, key) { return [...arr].sort((item1, item2) => { if (item1[key] < item2[key]) { @@ -62,7 +43,6 @@ function importAccounts(data, entityIdMap) { data.accounts.map(async account => { if (!account.deleted) { let id = await actual.createAccount({ - type: mapAccountType(account.type), name: account.name, offbudget: account.on_budget ? false : true, closed: account.closed, diff --git a/packages/loot-core/migrations/1682265217543_remove_account_type.sql b/packages/loot-core/migrations/1682265217543_remove_account_type.sql new file mode 100644 index 000000000..34fe10429 --- /dev/null +++ b/packages/loot-core/migrations/1682265217543_remove_account_type.sql @@ -0,0 +1,5 @@ +BEGIN TRANSACTION; + +ALTER TABLE accounts DROP COLUMN type; + +COMMIT; diff --git a/packages/loot-core/src/client/actions/queries.ts b/packages/loot-core/src/client/actions/queries.ts index 70ab734fa..3308684a2 100644 --- a/packages/loot-core/src/client/actions/queries.ts +++ b/packages/loot-core/src/client/actions/queries.ts @@ -230,9 +230,9 @@ export function updateAccount(account) { }; } -export function createAccount(name, type, balance, offBudget) { +export function createAccount(name, balance, offBudget) { return async function (dispatch) { - let id = await send('account-create', { name, type, balance, offBudget }); + let id = await send('account-create', { name, balance, offBudget }); await dispatch(getAccounts()); await dispatch(getPayees()); return id; diff --git a/packages/loot-core/src/mocks/budget.ts b/packages/loot-core/src/mocks/budget.ts index b90ca9958..ba559dc74 100644 --- a/packages/loot-core/src/mocks/budget.ts +++ b/packages/loot-core/src/mocks/budget.ts @@ -564,14 +564,14 @@ export async function createTestBudget(handlers) { await db.runQuery('DELETE FROM category_groups'); let accounts: AccountEntity[] = [ - { name: 'Bank of America', type: 'checking' }, - { name: 'Ally Savings', type: 'savings' }, - { name: 'Capital One Checking', type: 'checking' }, - { name: 'HSBC', type: 'checking' }, - { name: 'Vanguard 401k', type: 'investment', offBudget: 1 }, - { name: 'Mortgage', type: 'mortgage', offBudget: 1 }, - { name: 'House Asset', type: 'other', offBudget: 1 }, - { name: 'Roth IRA', type: 'investment', offBudget: 1 }, + { name: 'Bank of America' }, + { name: 'Ally Savings' }, + { name: 'Capital One Checking' }, + { name: 'HSBC' }, + { name: 'Vanguard 401k', offBudget: 1 }, + { name: 'Mortgage', offBudget: 1 }, + { name: 'House Asset', offBudget: 1 }, + { name: 'Roth IRA', offBudget: 1 }, ]; await runMutator(() => batchMessages(async () => { @@ -664,31 +664,27 @@ export async function createTestBudget(handlers) { await runMutator(() => batchMessages(async () => { for (let account of accounts) { - switch (account.type) { - case 'checking': - if (account.name === 'Bank of America') { - await fillPrimaryChecking(handlers, account, payees, allGroups); - } else { - await fillChecking(handlers, account, payees, allGroups); - } - break; - case 'investment': - await fillInvestment(handlers, account, payees, allGroups); - break; - case 'savings': - await fillSavings(handlers, account, payees, allGroups); - break; - case 'mortgage': - await fillMortgage(handlers, account, payees, allGroups); - break; - case 'other': - if (account.name === 'House Asset') { - await fillOther(handlers, account, payees, allGroups); - } else { - await fillChecking(handlers, account, payees, allGroups); - } - break; - default: + if (account.name === 'Bank of America') { + await fillPrimaryChecking(handlers, account, payees, allGroups); + } else if ( + account.name === 'Capital One Checking' || + account.name === 'HSBC' + ) { + await fillChecking(handlers, account, payees, allGroups); + } else if (account.name === 'Ally Savings') { + await fillSavings(handlers, account, payees, allGroups); + } else if ( + account.name === 'Vanguard 401k' || + account.name === 'Roth IRA' + ) { + await fillInvestment(handlers, account, payees, allGroups); + } else if (account.name === 'Mortgage') { + await fillMortgage(handlers, account, payees, allGroups); + } else if (account.name === 'House Asset') { + await fillOther(handlers, account, payees, allGroups); + } else { + console.error('Unknown account name for test budget: ', account.name); + await fillChecking(handlers, account, payees, allGroups); } } }), diff --git a/packages/loot-core/src/mocks/index.ts b/packages/loot-core/src/mocks/index.ts index f0e055c35..2d4fcee59 100644 --- a/packages/loot-core/src/mocks/index.ts +++ b/packages/loot-core/src/mocks/index.ts @@ -2,7 +2,7 @@ import * as uuid from '../platform/uuid'; import * as monthUtils from '../shared/months'; import type { TransactionEntity } from '../types/models'; -export function generateAccount(name, isConnected, type, offbudget) { +export function generateAccount(name, isConnected, offbudget) { return { id: uuid.v4Sync(), name, @@ -10,7 +10,6 @@ export function generateAccount(name, isConnected, type, offbudget) { bank: isConnected ? Math.floor(Math.random() * 10000) : null, bankId: isConnected ? Math.floor(Math.random() * 10000) : null, bankName: isConnected ? 'boa' : null, - type: type || 'checking', offbudget: offbudget ? 1 : 0, closed: 0, }; diff --git a/packages/loot-core/src/server/accounts/link.ts b/packages/loot-core/src/server/accounts/link.ts index eb12b9fc8..5815e89b4 100644 --- a/packages/loot-core/src/server/accounts/link.ts +++ b/packages/loot-core/src/server/accounts/link.ts @@ -1,6 +1,5 @@ import * as asyncStorage from '../../platform/server/asyncStorage'; import * as uuid from '../../platform/uuid'; -import { fromPlaidAccountType } from '../../shared/accounts'; import { amountToInteger } from '../../shared/util'; import * as db from '../db'; import { runMutator } from '../mutators'; @@ -81,7 +80,6 @@ export async function addAccounts(bankId, accountIds, offbudgetIds = []) { account_id: acct.account_id, name: acct.name, official_name: acct.official_name, - type: fromPlaidAccountType(acct.type), balance_current: amountToInteger(acct.balances.current), mask: acct.mask, bank: bankId, @@ -128,7 +126,6 @@ export async function addNordigenAccounts( account_id: acct.account_id, name: acct.name, official_name: acct.official_name, - type: fromPlaidAccountType(acct.type), balance_current: amountToInteger(acct.balances.current), mask: acct.mask, bank: bankId, diff --git a/packages/loot-core/src/server/api-models.ts b/packages/loot-core/src/server/api-models.ts index 482a527a7..1678ae15a 100644 --- a/packages/loot-core/src/server/api-models.ts +++ b/packages/loot-core/src/server/api-models.ts @@ -84,7 +84,6 @@ export const accountModel = { return { id: account.id, name: account.name, - type: account.type, offbudget: account.offbudget ? true : false, closed: account.closed ? true : false, }; diff --git a/packages/loot-core/src/server/api.ts b/packages/loot-core/src/server/api.ts index c36da3398..f24931269 100644 --- a/packages/loot-core/src/server/api.ts +++ b/packages/loot-core/src/server/api.ts @@ -463,7 +463,6 @@ handlers['api/account-create'] = withMutation(async function ({ checkFileOpen(); return handlers['account-create']({ name: account.name, - type: account.type, offBudget: account.offbudget, closed: account.closed, // Current the API expects an amount but it really should expect diff --git a/packages/loot-core/src/server/aql/schema/index.ts b/packages/loot-core/src/server/aql/schema/index.ts index e409b28be..5f4a84acf 100644 --- a/packages/loot-core/src/server/aql/schema/index.ts +++ b/packages/loot-core/src/server/aql/schema/index.ts @@ -61,8 +61,6 @@ export const schema = { accounts: { id: f('id'), name: f('string', { required: true }), - // TODO: enum - type: f('string'), offbudget: f('boolean'), closed: f('boolean'), sort_order: f('float'), diff --git a/packages/loot-core/src/server/db/index.ts b/packages/loot-core/src/server/db/index.ts index 0a1e4b48e..9c7be1dae 100644 --- a/packages/loot-core/src/server/db/index.ts +++ b/packages/loot-core/src/server/db/index.ts @@ -547,12 +547,6 @@ export function getAccounts() { } export async function insertAccount(account) { - // Default to checking. Makes it a lot easier for tests and is - // generally harmless. - if (account.type === undefined) { - account = { ...account, type: 'checking' }; - } - const accounts = await all( 'SELECT * FROM accounts WHERE offbudget = ? ORDER BY sort_order, name', [account.offbudget != null ? account.offbudget : 0], diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts index 12da8fdea..95c10bd46 100644 --- a/packages/loot-core/src/server/main.ts +++ b/packages/loot-core/src/server/main.ts @@ -11,7 +11,6 @@ import * as fs from '../platform/server/fs'; import logger from '../platform/server/log'; import * as sqlite from '../platform/server/sqlite'; import * as uuid from '../platform/uuid'; -import { fromPlaidAccountType } from '../shared/accounts'; import { isNonProductionEnvironment } from '../shared/environment'; import * as monthUtils from '../shared/months'; import q, { Query } from '../shared/query'; @@ -756,7 +755,6 @@ handlers['accounts-link'] = async function ({ id: upgradingId, account_id: account.account_id, official_name: account.official_name, - type: fromPlaidAccountType(account.type), balance_current: amountToInteger(account.balances.current), balance_available: amountToInteger(account.balances.available), balance_limit: amountToInteger(account.balances.limit), @@ -806,7 +804,6 @@ handlers['nordigen-accounts-link'] = async function ({ mask: account.mask, name: account.name, official_name: account.official_name, - type: account.type, bank: bank.id, }); await db.insertPayee({ @@ -855,7 +852,6 @@ handlers['nordigen-accounts-connect'] = async function ({ handlers['account-create'] = mutator(async function ({ name, - type, balance, offBudget, closed, @@ -863,7 +859,6 @@ handlers['account-create'] = mutator(async function ({ return withUndo(async () => { const id = await db.insertAccount({ name, - type, offbudget: offBudget ? 1 : 0, closed: closed ? 1 : 0, }); diff --git a/packages/loot-core/src/server/models.ts b/packages/loot-core/src/server/models.ts index 40cb1002a..12968c9ad 100644 --- a/packages/loot-core/src/server/models.ts +++ b/packages/loot-core/src/server/models.ts @@ -36,30 +36,11 @@ export function fromDateRepr(number) { } export const accountModel = { - validateAccountType(account) { - const { type } = account; - if ( - type !== 'checking' && - type !== 'savings' && - type !== 'investment' && - type !== 'credit' && - type !== 'mortgage' && - type !== 'debt' && - type !== 'other' - ) { - throw new Error('Invalid account type: ' + type); - } - }, - validate(account, { update }: { update?: boolean } = {}) { - if (!update || account.type != null) { - accountModel.validateAccountType(account); - } - requiredFields( 'account', account, - update ? ['name', 'type', 'offbudget', 'closed'] : ['name', 'type'], + update ? ['name', 'offbudget', 'closed'] : ['name'], update, ); diff --git a/packages/loot-core/src/shared/accounts.ts b/packages/loot-core/src/shared/accounts.ts index 668ca00c1..cb74510ff 100644 --- a/packages/loot-core/src/shared/accounts.ts +++ b/packages/loot-core/src/shared/accounts.ts @@ -1,58 +1,13 @@ -export function fromPlaidAccountType(type, subtype?: string) { - switch (type) { +export function determineOffBudget(plaidAccountType) { + switch (plaidAccountType) { case 'brokerage': case 'investment': - return 'investment'; - case 'credit': - return 'credit'; case 'loan': - return 'debt'; - case 'other': - return 'other'; - case 'depository': - default: - switch (subtype) { - case 'money market': - case 'savings': - return 'savings'; - case 'cd': - return 'cd'; - default: - return 'checking'; - } - } -} - -export function prettyAccountType(type) { - switch (type) { - case 'checking': - return 'Checking'; - case 'savings': - return 'Savings'; - case 'cd': - return 'CD'; - case 'investment': - return 'Investment'; - case 'credit': - return 'Credit Card'; - case 'mortgage': - return 'Mortgage'; - case 'debt': - return 'Debt'; - case 'other': - default: - return 'Other'; - } -} - -export function determineOffBudget(type) { - switch (type) { - case 'investment': - case 'mortgage': - case 'debt': case 'other': return true; + case 'credit': + case 'depository': default: + return false; } - return false; } diff --git a/packages/loot-core/src/types/main.handlers.d.ts b/packages/loot-core/src/types/main.handlers.d.ts index 12fed1f4a..f7158fb2a 100644 --- a/packages/loot-core/src/types/main.handlers.d.ts +++ b/packages/loot-core/src/types/main.handlers.d.ts @@ -156,7 +156,6 @@ export interface MainHandlers { 'account-create': (arg: { name; - type; balance; offBudget; closed?; diff --git a/packages/loot-core/src/types/models/account.d.ts b/packages/loot-core/src/types/models/account.d.ts index b00123056..5d062d8a4 100644 --- a/packages/loot-core/src/types/models/account.d.ts +++ b/packages/loot-core/src/types/models/account.d.ts @@ -1,7 +1,6 @@ export interface AccountEntity { id?: string; name: string; - type?: string; offbudget?: boolean; closed?: boolean; sort_order?: number; diff --git a/upcoming-release-notes/948.md b/upcoming-release-notes/948.md new file mode 100644 index 000000000..e8bc80d0b --- /dev/null +++ b/upcoming-release-notes/948.md @@ -0,0 +1,6 @@ +--- +category: Enhancements +authors: [j-f1] +--- + +Remove support for storing account types on the account (they didn’t do anything in the budget) -- GitLab