Merge pull request #2659 from cozy/fix/Balance(Lower|Greater)-notific…

…ation fix: Balance(Lower|Greater) notifications
cozy · May 3, 2023 · fdd870f · fdd870f
2 parents 029e32c + 4a90a3a
commit fdd870f
Show file tree

Hide file tree

Showing 8 changed files with 142 additions and 27 deletions.
diff --git a/src/ducks/notifications/BalanceGreater/index.js b/src/ducks/notifications/BalanceGreater/index.js
@@ -19,6 +19,7 @@ import {
 import { ruleAccountFilter } from 'ducks/settings/ruleUtils'
 
 import template from './template.hbs'
+import { fetchSettings } from 'ducks/settings/helpers'
 
 const addCurrency = o => ({ ...o, currency: '€' })
 
@@ -70,8 +71,15 @@ class BalanceGreater extends NotificationView {
     )
   }
 
-  filterForRule(rule, account) {
-    const isBalanceOver = getAccountBalance(account) > rule.value
+  filterForRule(rule, account, balancesNotifications) {
+    const lastAccountBalance =
+      balancesNotifications[account._id] != null
+        ? balancesNotifications[account._id]
+        : null
+    const isLastBalanceOver =
+      lastAccountBalance !== null ? lastAccountBalance <= rule.value : true
+    const isBalanceOver =
+      isLastBalanceOver && getAccountBalance(account) > rule.value
     const accountFilter = ruleAccountFilter(rule, this.data.groups)
     const correspondsAccountToGroup = accountFilter(account)
     return isBalanceOver && correspondsAccountToGroup
@@ -82,20 +90,22 @@ class BalanceGreater extends NotificationView {
    * For each rule, returns a list of matching accounts
    * Rules that do not match any accounts are discarded
    */
-  findMatchingRules() {
+  findMatchingRules(balancesNotifications) {
     return this.rules
       .filter(rule => rule.enabled)
       .map(rule => ({
         rule,
         accounts: this.data.accounts.filter(acc =>
-          this.filterForRule(rule, acc)
+          this.filterForRule(rule, acc, balancesNotifications)
         )
       }))
       .filter(({ accounts }) => accounts.length > 0)
   }
 
-  fetchData() {
-    const matchingRules = this.findMatchingRules()
+  async fetchData() {
+    const { balancesNotifications } = await fetchSettings(this.client)
+
+    const matchingRules = this.findMatchingRules(balancesNotifications)
     const accountsFiltered = uniqBy(
       flatten(matchingRules.map(x => x.accounts)),
       x => x._id

diff --git a/src/ducks/notifications/BalanceGreater/index.spec.js b/src/ducks/notifications/BalanceGreater/index.spec.js
@@ -9,13 +9,20 @@ import BalanceGreater from './index'
 import fixtures from 'test/fixtures/unit-tests.json'
 import enLocale from 'locales/en.json'
 
+import { fetchSettings } from 'ducks/settings/helpers'
+
 const unique = arr => Array.from(new Set(arr))
 
 const minValueBy = (arr, fn) => fn(minBy(arr, fn))
 const maxValueBy = (arr, fn) => fn(maxBy(arr, fn))
 const getIDFromAccount = account => account._id
 const getAccountBalance = account => account.balance
 
+jest.mock('../../settings/helpers', () => ({
+  ...jest.requireActual('../../settings/helpers'),
+  fetchSettings: jest.fn()
+}))
+
 describe('BalanceGreater', () => {
   beforeEach(() => {
     jest.spyOn(console, 'warn').mockImplementation(msg => {
@@ -27,11 +34,19 @@ describe('BalanceGreater', () => {
     jest.restoreAllMocks()
   })
 
-  const setup = ({ value, accountOrGroup, rules } = {}) => {
+  const setup = ({
+    ruleValue,
+    accountOrGroup,
+    rules,
+    balancesNotifications = {}
+  } = {}) => {
     const cozyUrl = 'http://cozy.tools:8080'
     const client = new CozyClient({
       uri: cozyUrl
     })
+    client.query = jest.fn()
+    fetchSettings.mockResolvedValue({ balancesNotifications })
+
     const locales = {
       en: enLocale
     }
@@ -43,7 +58,7 @@ describe('BalanceGreater', () => {
     const config = {
       rules: rules || [
         {
-          value: value || 1000,
+          value: ruleValue || 1000,
           accountOrGroup: accountOrGroup || null,
           enabled: true
         }
@@ -68,14 +83,41 @@ describe('BalanceGreater', () => {
 
   describe('without accountOrGroup', () => {
     it('should compute relevant accounts', async () => {
-      const { notification } = setup({ value: 1000 })
+      const { notification } = setup({ ruleValue: 1000 })
+      const { accounts } = await notification.buildData()
+      expect(accounts).toHaveLength(5)
+      expect(maxValueBy(accounts, getAccountBalance)).toBeGreaterThan(1000)
+    })
+
+    it('should return only accounts where their previous balances were not already positive to the rule', async () => {
+      // Original balances: "compteisa4": 1421.20, "compteisa1": 3974.20
+      const { notification } = setup({
+        ruleValue: 1000,
+        balancesNotifications: { compteisa4: 2000, compteisa1: 999 }
+      })
       const { accounts } = await notification.buildData()
+
+      expect(accounts).toHaveLength(4)
+      expect(maxValueBy(accounts, getAccountBalance)).toBeGreaterThan(1000)
+    })
+
+    it('should not return accounts if previous balances were already positive to the rule', async () => {
+      // Original balances: "compteisa4": 1421.20, "compteisa1": 3974.20
+      const { notification } = setup({
+        ruleValue: 1000,
+        balancesNotifications: {
+          compteisa4: 100,
+          compteisa1: 1000
+        }
+      })
+      const { accounts } = await notification.buildData()
+
       expect(accounts).toHaveLength(5)
       expect(maxValueBy(accounts, getAccountBalance)).toBeGreaterThan(1000)
     })
 
     it('should compute relevant accounts for a different value', async () => {
-      const { notification } = setup({ value: 2000 })
+      const { notification } = setup({ ruleValue: 2000 })
       const { accounts } = await notification.buildData()
       expect(accounts).toHaveLength(4)
       expect(maxValueBy(accounts, getAccountBalance)).toBeGreaterThan(2000)
@@ -105,7 +147,7 @@ describe('BalanceGreater', () => {
 
     it('should compute relevant accounts for a different value', async () => {
       const { notification } = setup({
-        value: 4000,
+        ruleValue: 4000,
         accountOrGroup: compteisa1
       })
       const data = await notification.buildData()
@@ -122,7 +164,7 @@ describe('BalanceGreater', () => {
     it('should compute relevant accounts', async () => {
       const { notification } = setup({
         accountOrGroup: isabelleGroup,
-        value: 4000
+        ruleValue: 4000
       })
       const { accounts } = await notification.buildData()
       expect(accounts).toHaveLength(1)
@@ -133,7 +175,7 @@ describe('BalanceGreater', () => {
 
     it('should compute relevant accounts for a different value', async () => {
       const { notification } = setup({
-        value: 900,
+        ruleValue: 900,
         accountOrGroup: isabelleGroup
       })
       const { accounts } = await notification.buildData()

diff --git a/src/ducks/notifications/BalanceLower/index.js b/src/ducks/notifications/BalanceLower/index.js
@@ -18,6 +18,7 @@ import {
 } from 'ducks/notifications/helpers'
 import template from './template.hbs'
 import { ruleAccountFilter } from 'ducks/settings/ruleUtils'
+import { fetchSettings } from 'ducks/settings/helpers'
 
 const addCurrency = o => ({ ...o, currency: '€' })
 
@@ -69,8 +70,16 @@ class BalanceLower extends NotificationView {
     )
   }
 
-  filterForRule(rule, account) {
-    const isBalanceUnder = getAccountBalance(account) < rule.value
+  filterForRule(rule, account, balancesNotifications) {
+    const lastAccountBalance =
+      balancesNotifications[account._id] != null
+        ? balancesNotifications[account._id]
+        : null
+    const isLastBalanceOver =
+      lastAccountBalance !== null ? lastAccountBalance >= rule.value : true
+    const isBalanceUnder =
+      isLastBalanceOver && getAccountBalance(account) < rule.value
+
     const accountFilter = ruleAccountFilter(rule, this.data.groups)
     const correspondsAccountToGroup = accountFilter(account)
     const isNotCreditCard = account.type !== 'CreditCard'
@@ -82,20 +91,22 @@ class BalanceLower extends NotificationView {
    * For each rule, returns a list of matching accounts
    * Rules that do not match any accounts are discarded
    */
-  findMatchingRules() {
+  findMatchingRules(balancesNotifications) {
     return this.rules
       .filter(rule => rule.enabled)
       .map(rule => ({
         rule,
         accounts: this.data.accounts.filter(acc =>
-          this.filterForRule(rule, acc)
+          this.filterForRule(rule, acc, balancesNotifications)
         )
       }))
       .filter(({ accounts }) => accounts.length > 0)
   }
 
-  fetchData() {
-    const matchingRules = this.findMatchingRules()
+  async fetchData() {
+    const { balancesNotifications } = await fetchSettings(this.client)
+
+    const matchingRules = this.findMatchingRules(balancesNotifications)
     const accountsFiltered = uniqBy(
       flatten(matchingRules.map(x => x.accounts)),
       x => x._id

diff --git a/src/ducks/notifications/BalanceLower/index.spec.js b/src/ducks/notifications/BalanceLower/index.spec.js
@@ -7,13 +7,20 @@ import maxBy from 'lodash/maxBy'
 import minBy from 'lodash/minBy'
 import enLocale from 'locales/en.json'
 
+import { fetchSettings } from 'ducks/settings/helpers'
+
 const unique = arr => Array.from(new Set(arr))
 
 const minValueBy = (arr, fn) => fn(minBy(arr, fn))
 const maxValueBy = (arr, fn) => fn(maxBy(arr, fn))
 const getIDFromAccount = account => account._id
 const getAccountBalance = account => account.balance
 
+jest.mock('../../settings/helpers', () => ({
+  ...jest.requireActual('../../settings/helpers'),
+  fetchSettings: jest.fn()
+}))
+
 describe('balance lower', () => {
   beforeEach(() => {
     jest.spyOn(console, 'warn').mockImplementation(msg => {
@@ -25,11 +32,19 @@ describe('balance lower', () => {
     jest.restoreAllMocks()
   })
 
-  const setup = ({ value, accountOrGroup, rules } = {}) => {
+  const setup = ({
+    ruleValue,
+    accountOrGroup,
+    rules,
+    balancesNotifications = {}
+  } = {}) => {
     const cozyUrl = 'http://cozy.tools:8080'
     const client = new CozyClient({
       uri: cozyUrl
     })
+    client.query = jest.fn()
+    fetchSettings.mockResolvedValue({ balancesNotifications })
+
     const locales = {
       en: enLocale
     }
@@ -41,7 +56,7 @@ describe('balance lower', () => {
     const config = {
       rules: rules || [
         {
-          value: value || 5000,
+          value: ruleValue || 5000,
           accountOrGroup: accountOrGroup || null,
           enabled: true
         }
@@ -66,14 +81,41 @@ describe('balance lower', () => {
 
   describe('without accountOrGroup', () => {
     it('should compute relevant accounts', async () => {
-      const { notification } = setup({ value: 5000 })
+      const { notification } = setup({ ruleValue: 5000 })
       const { accounts } = await notification.buildData()
       expect(accounts).toHaveLength(4)
       expect(maxValueBy(accounts, getAccountBalance)).toBeLessThan(5000)
     })
 
+    it('should return only accounts where their previous balances were not already positive to the rule', async () => {
+      // Original balances: "compteisa4": 1421.20, "compteisa1": 3974.20
+      const { notification } = setup({
+        ruleValue: 5000,
+        balancesNotifications: { compteisa4: 2000, compteisa1: 5000 }
+      })
+      const { accounts } = await notification.buildData()
+
+      expect(accounts).toHaveLength(3)
+      expect(maxValueBy(accounts, getAccountBalance)).toBeLessThan(5000)
+    })
+
+    it('should not return accounts if previous balances were already positive to the rule', async () => {
+      // Original balances: "compteisa4": 1421.20, "compteisa1": 3974.20
+      const { notification } = setup({
+        ruleValue: 5000,
+        balancesNotifications: {
+          compteisa4: 100,
+          compteisa1: 4999
+        }
+      })
+      const { accounts } = await notification.buildData()
+
+      expect(accounts).toHaveLength(2)
+      expect(maxValueBy(accounts, getAccountBalance)).toBeLessThan(5000)
+    })
+
     it('should compute relevant accounts for a different value', async () => {
-      const { notification } = setup({ value: 3000 })
+      const { notification } = setup({ ruleValue: 3000 })
       const { accounts } = await notification.buildData()
       expect(accounts).toHaveLength(2)
       expect(maxValueBy(accounts, getAccountBalance)).toBeLessThan(3000)
@@ -101,7 +143,7 @@ describe('balance lower', () => {
 
     it('should compute relevant accounts for a different value', async () => {
       const { notification } = setup({
-        value: 3000,
+        ruleValue: 3000,
         accountOrGroup: compteisa1
       })
       const data = await notification.buildData()
@@ -118,7 +160,7 @@ describe('balance lower', () => {
     it('should compute relevant accounts', async () => {
       const { notification } = setup({
         accountOrGroup: isabelleGroup,
-        value: 5000
+        ruleValue: 5000
       })
       const { accounts } = await notification.buildData()
       expect(accounts).toHaveLength(2)
@@ -132,7 +174,7 @@ describe('balance lower', () => {
 
     it('should compute relevant accounts for a different value', async () => {
       const { notification } = setup({
-        value: 500,
+        ruleValue: 500,
         accountOrGroup: isabelleGroup
       })
       const { accounts } = await notification.buildData()

diff --git a/src/ducks/settings/__snapshots__/helpers.spec.jsx.snap b/src/ducks/settings/__snapshots__/helpers.spec.jsx.snap
@@ -10,6 +10,7 @@ Object {
   "autogroups": Object {
     "processedAccounts": Array [],
   },
+  "balancesNotifications": Object {},
   "billsMatching": Object {
     "billsLastSeq": "0",
     "transactionsLastSeq": "0",

diff --git a/src/ducks/settings/constants.js b/src/ducks/settings/constants.js
@@ -5,6 +5,7 @@ export const DEFAULTS_SETTINGS = {
   _type: 'io.cozy.bank.settings',
   _id: 'configuration',
   id: 'configuration',
+  balancesNotifications: {},
   autogroups: {
     processedAccounts: []
   },

diff --git a/src/targets/services/onOperationOrBillCreateHelpers.js b/src/targets/services/onOperationOrBillCreateHelpers.js
@@ -6,7 +6,10 @@ import set from 'lodash/set'
 import logger from 'cozy-logger'
 import flag from 'cozy-flags'
 
-import { sendNotifications } from 'ducks/notifications/services'
+import {
+  fetchTransactionAccounts,
+  sendNotifications
+} from 'ducks/notifications/services'
 import matchFromBills from 'ducks/billsMatching/matchFromBills'
 import matchFromTransactions from 'ducks/billsMatching/matchFromTransactions'
 import { logResult } from 'ducks/billsMatching/utils'
@@ -96,7 +99,11 @@ export const doSendNotifications = async (setting, notifChanges) => {
   log('info', '⌛ Do send notifications...')
   try {
     const transactionsToNotify = notifChanges.documents
+    const accounts = await fetchTransactionAccounts(transactionsToNotify)
     await sendNotifications(setting, transactionsToNotify)
+    for (const account of accounts) {
+      set(setting, `balancesNotifications.${account._id}`, account.balance)
+    }
     set(
       setting,
       'notifications.lastSeq',

diff --git a/test/fixtures/unit-tests.json b/test/fixtures/unit-tests.json
@@ -1717,6 +1717,7 @@
   "io.cozy.bank.settings": [
     {
       "_id": "settings-1",
+      "balancesNotifications": {},
       "autogroups": {
         "processedAccounts": []
       },