Fix several issues

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 = ({
                 )}
               </Grid>
               <Grid item xs={4}>
-                <Button type="submit" color="secondary" className={classes.saveButton}>Save</Button>
+                <SubmitButtonWithProgress label="Save" isSubmitting={isSubmitting} className={classes.saveButton} />
               </Grid>
             </Grid>
           </DialogActions>
@@ -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

src/common/SubmitButtonWithProgress/index.jsx

@@ -20,13 +20,19 @@ const styles = (theme) => ({
   }
 })
 
-const SubmitButtonWithProgress = ({ classes, label, isSubmitting }) => {
+const SubmitButtonWithProgress = ({
+  classes,
+  className,
+  label,
+  isSubmitting
+}) => {
   return (
-    <div className={classes.wrapper}>
+    <div className={[classes.wrapper, className].join(' ')}>
       <Button
         type="submit"
         color="secondary"
         disabled={isSubmitting}
+        disableFocusRipple={true}
       >
         {label}
       </Button>
@@ -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)

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)
     }
   }
 

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)
     }

src/core/Accounts/Transactions/TransactionsTable.jsx

@@ -347,7 +347,7 @@ export class TransactionsTableComponent extends React.Component {
                         </Tooltip>
                       )
                     }
-                    if (allowClickOnRow) {
+                    if (allowClickOnRow && rowData.type !== 'openingBalance') {
                       return (
                         <Link
                           onClick={() => toolbarProps.handleEdit(rowData)}

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,31 +24,31 @@ 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'),
     width: '100%',
     [theme.breakpoints.up('md')]: {
       minWidth: 240
     }
+  },
+  smallIcon: {
+    fontSize: 16
+  },
+  iconSpacer: {
+    width: theme.spacing(3)
   }
 })
 
@@ -94,9 +95,9 @@ export class TransactionsToolbarComponent extends React.Component {
           </Tooltip>
         ) : (
           <div className={classes.buttons}>
-            <div className={classes.search}>
+            <div>
               <InputBase
-                type="search"
+                type="input"
                 placeholder="Search transactions"
                 onChange={this.onChangeSearch}
                 value={filterProps.filters.description || ''}
@@ -105,6 +106,20 @@ export class TransactionsToolbarComponent extends React.Component {
                   input: classes.inputInput
                 }}
                 startAdornment={<InputAdornment position="start"><SearchIcon /></InputAdornment>}
+                endAdornment={(
+                  <InputAdornment position="end">
+                    {filterProps.filters.description && (
+                      <IconButton
+                        size="small"
+                        aria-label="clear search"
+                        onClick={() => this.onChangeSearch({ target: { name: 'description', value: '' } })}
+                      >
+                        <CloseIcon className={classes.smallIcon} />
+                      </IconButton>
+                    )}
+                    {!filterProps.filters.description && (<div className={classes.iconSpacer} />)}
+                  </InputAdornment>
+                )}
               />
             </div>
             <Tooltip title="Import transaction">

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,24 +62,30 @@ 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 () => {
       const categoryId = budgetInitialState.categoryTree[0].options[0].id
       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)
+    })
+  })
 })