From 0682021680a093e46fbc58fc6e80bbea3a5d2612 Mon Sep 17 00:00:00 2001 From: Jack Neto Date: Sat, 18 Jan 2020 19:48:39 -0500 Subject: [PATCH] Fix several issues --- src/common/ModalDialog/index.jsx | 8 ++- src/common/SubmitButtonWithProgress/index.jsx | 14 +++- src/core/Accounts/Transactions/RuleFields.jsx | 11 ++- .../Transactions/TransactionDialog.jsx | 8 ++- .../Transactions/TransactionsTable.jsx | 2 +- .../Transactions/TransactionsToolbar.jsx | 45 ++++++++---- .../__tests__/RuleFields.test.jsx | 58 ++++++++++++--- .../TransactionsTable.test.jsx.snap | 70 +++++++++++-------- .../TransactionsToolbar.test.jsx.snap | 11 ++- .../__snapshots__/form.test.jsx.snap | 4 +- .../__snapshots__/form.test.jsx.snap | 28 ++++---- src/core/BudgetCategories/index.jsx | 28 +++++++- src/store/budget/__tests__/reducer.test.js | 40 +++++++++-- src/store/budget/reducer.js | 1 + src/store/index.js | 1 + src/store/transactions/actions.js | 3 - 16 files changed, 233 insertions(+), 99 deletions(-) diff --git a/src/common/ModalDialog/index.jsx b/src/common/ModalDialog/index.jsx index bcd8230..e4ee99c 100644 --- a/src/common/ModalDialog/index.jsx +++ b/src/common/ModalDialog/index.jsx @@ -11,6 +11,7 @@ import IconButton from '@material-ui/core/IconButton' import CloseIcon from '@material-ui/icons/Close' import Typography from '@material-ui/core/Typography' import Grid from '@material-ui/core/Grid' +import SubmitButtonWithProgress from '../SubmitButtonWithProgress' const useStyles = makeStyles((theme) => ({ dialogMargin: { @@ -45,6 +46,7 @@ const ModalDialog = ({ title, children, onSubmit, + isSubmitting, onCancel, onDelete, open, @@ -84,7 +86,7 @@ const ModalDialog = ({ )} - + @@ -99,6 +101,7 @@ ModalDialog.propTypes = { title: PropTypes.string.isRequired, children: PropTypes.node.isRequired, onSubmit: PropTypes.func.isRequired, + isSubmitting: PropTypes.bool, onCancel: PropTypes.func.isRequired, onDelete: PropTypes.func, className: PropTypes.string, @@ -110,7 +113,8 @@ ModalDialog.defaultProps = { className: null, onDelete: null, maxWidth: null, - fullWidth: false + fullWidth: false, + isSubmitting: false } export default ModalDialog diff --git a/src/common/SubmitButtonWithProgress/index.jsx b/src/common/SubmitButtonWithProgress/index.jsx index be6bf94..b306998 100644 --- a/src/common/SubmitButtonWithProgress/index.jsx +++ b/src/common/SubmitButtonWithProgress/index.jsx @@ -20,13 +20,19 @@ const styles = (theme) => ({ } }) -const SubmitButtonWithProgress = ({ classes, label, isSubmitting }) => { +const SubmitButtonWithProgress = ({ + classes, + className, + label, + isSubmitting +}) => { return ( -
+
@@ -37,8 +43,12 @@ const SubmitButtonWithProgress = ({ classes, label, isSubmitting }) => { SubmitButtonWithProgress.propTypes = { classes: PropTypes.object.isRequired, + className: PropTypes.string, label: PropTypes.string.isRequired, isSubmitting: PropTypes.bool.isRequired } +SubmitButtonWithProgress.defaultProps = { + className: null +} export default withStyles(styles)(SubmitButtonWithProgress) diff --git a/src/core/Accounts/Transactions/RuleFields.jsx b/src/core/Accounts/Transactions/RuleFields.jsx index 4c87d03..1af2795 100644 --- a/src/core/Accounts/Transactions/RuleFields.jsx +++ b/src/core/Accounts/Transactions/RuleFields.jsx @@ -38,8 +38,7 @@ const useStyles = makeStyles((theme) => ({ textTransform: 'none' }, smallIcon: { - width: 15, - height: 15 + fontSize: 14 } })) @@ -55,11 +54,11 @@ const RuleFields = ({ showTransactions }) => { const classes = useStyles() - const [checkboxesState, SetCheckboxesState] = useState({ applyToExistin: true, applyToFuture: true }) + const [oldCheckboxesState, SetOldCheckboxesState] = useState({ applyToExistin: true, applyToFuture: true }) const handleChangeApplyToOtherTransactions = (_, checked) => { if (!checked) { - SetCheckboxesState({ + SetOldCheckboxesState({ applyToExisting: values.applyToExisting, applyToFuture: values.applyToFuture }) @@ -68,8 +67,8 @@ const RuleFields = ({ setFieldValue('applyToFuture', false) } else { setFieldValue('applyToOtherTransactions', true) - setFieldValue('applyToExisting', checkboxesState.applyToExisting) - setFieldValue('applyToFuture', checkboxesState.applyToFuture) + setFieldValue('applyToExisting', oldCheckboxesState.applyToExisting) + setFieldValue('applyToFuture', oldCheckboxesState.applyToFuture) } } diff --git a/src/core/Accounts/Transactions/TransactionDialog.jsx b/src/core/Accounts/Transactions/TransactionDialog.jsx index 00a3678..60085a2 100644 --- a/src/core/Accounts/Transactions/TransactionDialog.jsx +++ b/src/core/Accounts/Transactions/TransactionDialog.jsx @@ -136,6 +136,7 @@ const getRule = (values, accountId) => { export const TransactionDialogComponent = ({ handleSubmit, + isSubmitting, setFieldValue, values, errors, @@ -174,6 +175,7 @@ export const TransactionDialogComponent = ({ open={open} title={transaction ? 'Edit transaction' : 'New transaction'} onSubmit={handleSubmit} + isSubmitting={isSubmitting} onCancel={onCancel} onDelete={transaction ? onDelete : undefined} className={classes.root} @@ -407,6 +409,7 @@ export const TransactionDialogComponent = ({ TransactionDialogComponent.propTypes = { handleSubmit: PropTypes.func.isRequired, + isSubmitting: PropTypes.bool.isRequired, values: PropTypes.object.isRequired, errors: PropTypes.object.isRequired, touched: PropTypes.object.isRequired, @@ -452,7 +455,6 @@ export default compose( } } - console.log({ ruleId: transaction.ruleId, rules: budget.rules }) const ruleAttributes = {} if (transaction.ruleId) { const rule = budget.rules[transaction.ruleId] @@ -502,7 +504,7 @@ export default compose( createdAt: Yup.date() .required('Please select the date of this transaction') }), - handleSubmit: (values, { props, setSubmitting, resetForm }) => { + handleSubmit: async (values, { props, setSubmitting, resetForm }) => { setSubmitting(true) const { transactionType, @@ -529,7 +531,7 @@ export default compose( } const rule = getRule(values, props.account.id) if (!rule) transaction.ruleId = null - props.onSave(transaction, { rule, applyToExisting, applyToFuture }) + await props.onSave(transaction, { rule, applyToExisting, applyToFuture }) resetForm() setSubmitting(false) } diff --git a/src/core/Accounts/Transactions/TransactionsTable.jsx b/src/core/Accounts/Transactions/TransactionsTable.jsx index 3862a6f..ccbc58e 100644 --- a/src/core/Accounts/Transactions/TransactionsTable.jsx +++ b/src/core/Accounts/Transactions/TransactionsTable.jsx @@ -347,7 +347,7 @@ export class TransactionsTableComponent extends React.Component { ) } - if (allowClickOnRow) { + if (allowClickOnRow && rowData.type !== 'openingBalance') { return ( toolbarProps.handleEdit(rowData)} diff --git a/src/core/Accounts/Transactions/TransactionsToolbar.jsx b/src/core/Accounts/Transactions/TransactionsToolbar.jsx index f897347..c5b25f5 100644 --- a/src/core/Accounts/Transactions/TransactionsToolbar.jsx +++ b/src/core/Accounts/Transactions/TransactionsToolbar.jsx @@ -11,6 +11,7 @@ import { mdiImport } from '@mdi/js' import InputBase from '@material-ui/core/InputBase' import InputAdornment from '@material-ui/core/InputAdornment' import SearchIcon from '@material-ui/icons/Search' +import CloseIcon from '@material-ui/icons/Close' import { fade } from '@material-ui/core/styles/colorManipulator' import confirm from '../../../util/confirm' import TableToolbar from '../../../common/TableToolbar' @@ -23,24 +24,18 @@ const styles = (theme) => ({ buttons: { display: 'flex' }, - search: { - position: 'relative', - borderRadius: theme.shape.borderRadius * 2, - backgroundColor: fade(theme.palette.grey[400], 0.15), - marginRight: theme.spacing(2), - marginLeft: 0, - width: '100%', - '&:hover': { - backgroundColor: fade(theme.palette.grey[400], 0.25) - } - }, inputRoot: { color: 'inherit', width: '100%', padding: theme.spacing(2), paddingTop: theme.spacing(1), - paddingBottom: theme.spacing(1) - + paddingBottom: theme.spacing(1), + borderRadius: theme.shape.borderRadius * 2, + backgroundColor: fade(theme.palette.grey[400], 0.15), + marginRight: theme.spacing(4), + '&:hover': { + backgroundColor: fade(theme.palette.grey[400], 0.25) + } }, inputInput: { transition: theme.transitions.create('width'), @@ -48,6 +43,12 @@ const styles = (theme) => ({ [theme.breakpoints.up('md')]: { minWidth: 240 } + }, + smallIcon: { + fontSize: 16 + }, + iconSpacer: { + width: theme.spacing(3) } }) @@ -94,9 +95,9 @@ export class TransactionsToolbarComponent extends React.Component { ) : (
-
+
} + endAdornment={( + + {filterProps.filters.description && ( + this.onChangeSearch({ target: { name: 'description', value: '' } })} + > + + + )} + {!filterProps.filters.description && (
)} + + )} />
diff --git a/src/core/Accounts/Transactions/__tests__/RuleFields.test.jsx b/src/core/Accounts/Transactions/__tests__/RuleFields.test.jsx index c6be435..5e01e25 100644 --- a/src/core/Accounts/Transactions/__tests__/RuleFields.test.jsx +++ b/src/core/Accounts/Transactions/__tests__/RuleFields.test.jsx @@ -3,6 +3,7 @@ import { render, queryByAttribute } from '@testing-library/react' import '@testing-library/jest-dom/extend-expect' import configureMockStore from 'redux-mock-store' import { Provider } from 'react-redux' +import faker from 'faker' import RuleFields from '../RuleFields' import ThemeProvider from '../../../ThemeProvider' import { initialState as settingsInitialState } from '../../../../store/settings/reducer' @@ -23,7 +24,7 @@ const defaultValues = { applyToFuture: true } -const renderContent = (props) => { +const renderContent = (props = {}) => { const mockStore = configureMockStore() const store = mockStore({ transactions: transactionsInitialState, @@ -61,14 +62,17 @@ describe('RuleFields', () => { const { getByText, container } = renderContent() expect(getByText('Apply this category to')).toBeInTheDocument() expect(getByText('0 other existing transactions')).toBeInTheDocument() - expect(getByName(container, 'applyToOtherTransactions')).toHaveProperty('disabled', true) - expect(getByName(container, 'applyToExisting')).toHaveProperty('disabled', true) - expect(getByName(container, 'applyToFuture')).toHaveProperty('disabled', true) + const applyToOtherTransactionsCheckbox = getByName(container, 'applyToOtherTransactions') + const applyToExistingCheckbox = getByName(container, 'applyToExisting') + const applyToFutureCheckbox = getByName(container, 'applyToFuture') + expect(applyToOtherTransactionsCheckbox).toHaveProperty('disabled', true) + expect(applyToExistingCheckbox).toHaveProperty('disabled', true) + expect(applyToFutureCheckbox).toHaveProperty('disabled', true) expect(getByName(container, 'filterText')).not.toBeInTheDocument() expect(getByName(container, 'matchAmount')).toHaveProperty('disabled', true) - expect(getByName(container, 'applyToOtherTransactions')).toHaveProperty('checked', true) - expect(getByName(container, 'applyToExisting')).toHaveProperty('checked', true) - expect(getByName(container, 'applyToFuture')).toHaveProperty('checked', true) + expect(applyToOtherTransactionsCheckbox).toHaveProperty('checked', true) + expect(applyToExistingCheckbox).toHaveProperty('checked', true) + expect(applyToFutureCheckbox).toHaveProperty('checked', true) }) it('should enable some fields if a category is selected', async () => { @@ -76,9 +80,12 @@ describe('RuleFields', () => { const { getByText, container } = renderContent({ values: { ...defaultValues, categoryId } }) expect(getByText('Apply this category to')).toBeInTheDocument() expect(getByText('0 other existing transactions')).toBeInTheDocument() - expect(getByName(container, 'applyToOtherTransactions')).toHaveProperty('disabled', false) - expect(getByName(container, 'applyToExisting')).toHaveProperty('disabled', true) - expect(getByName(container, 'applyToFuture')).toHaveProperty('disabled', false) + const applyToOtherTransactionsCheckbox = getByName(container, 'applyToOtherTransactions') + const applyToExistingCheckbox = getByName(container, 'applyToExisting') + const applyToFutureCheckbox = getByName(container, 'applyToFuture') + expect(applyToOtherTransactionsCheckbox).toHaveProperty('disabled', false) + expect(applyToExistingCheckbox).toHaveProperty('disabled', true) + expect(applyToFutureCheckbox).toHaveProperty('disabled', false) expect(getByName(container, 'filterText')).not.toBeInTheDocument() expect(getByName(container, 'matchAmount')).toHaveProperty('disabled', false) }) @@ -94,4 +101,35 @@ describe('RuleFields', () => { expect(getByName(container, 'matchAmount')).toHaveProperty('disabled', true) }) }) + + describe('transaction with ruleId', () => { + const transaction = { + id: faker.random.uuid(), + accountId: faker.random.uuid(), + description: 'Buy groceries', + amount: { + accountCurrency: faker.finance.amount(), + localCurrency: faker.finance.amount() + }, + createdAt: Date.now(), + ruleId: faker.random.uuid() + } + + it('should show remove if no category selected', () => { + const { getByText, container } = renderContent({ transaction }) + expect(getByText('Remove this category from')).toBeInTheDocument() + expect(getByText('0 other existing transactions')).toBeInTheDocument() + const applyToOtherTransactionsCheckbox = getByName(container, 'applyToOtherTransactions') + const applyToExistingCheckbox = getByName(container, 'applyToExisting') + const applyToFutureCheckbox = getByName(container, 'applyToFuture') + expect(applyToOtherTransactionsCheckbox).toHaveProperty('disabled', true) + expect(applyToExistingCheckbox).toHaveProperty('disabled', true) + expect(applyToFutureCheckbox).toHaveProperty('disabled', true) + expect(getByName(container, 'filterText')).not.toBeInTheDocument() + expect(getByName(container, 'matchAmount')).toHaveProperty('disabled', true) + expect(applyToOtherTransactionsCheckbox).toHaveProperty('checked', true) + expect(applyToExistingCheckbox).toHaveProperty('checked', true) + expect(applyToFutureCheckbox).toHaveProperty('checked', true) + }) + }) }) diff --git a/src/core/Accounts/Transactions/__tests__/__snapshots__/TransactionsTable.test.jsx.snap b/src/core/Accounts/Transactions/__tests__/__snapshots__/TransactionsTable.test.jsx.snap index 477561b..3e5869a 100644 --- a/src/core/Accounts/Transactions/__tests__/__snapshots__/TransactionsTable.test.jsx.snap +++ b/src/core/Accounts/Transactions/__tests__/__snapshots__/TransactionsTable.test.jsx.snap @@ -5,10 +5,10 @@ exports[`TransactionsTable matches snapshot with no transactions 1`] = ` className=" " >
-
+
+
+
+
-
+
+
+
+
+
+
+