Merge pull request #2608 from cozy/fix/prevent-recurrence-actions-on-…

…operations-with-existing-recurrence

The recurrence service is built in such a way that each call modifying
transactions will trigger another service call to make sure all
transaction-recurrence associations that could be made have been made.
This can result in a long chain of calls, potentially infinite, if
transactions are modified on each run.

We try to limit this effect by not modifying transactions that are
already associated with a recurrence. This should prevent situations
were the service juggles between 2 recurrences with a given transaction.

The fix has the added benefit of not modifying associations that were
made by the user and to present a stable automatic association.

```
### 🐛 Bug Fixes

* The recurrence service should not modify transactions already associated with a recurrence

### 🔧 Tech

* Clarified recurrence service functions and variables names
```

src/ducks/recurrence/search-with-existing-recurrence.spec.js

@@ -1166,7 +1166,7 @@ describe('recurrence bundles (with existing recurrences)', () => {
     ]
 
     const transactions = fixtures[TRANSACTION_DOCTYPE]
-    const spyUpdate = jest.spyOn(utils, 'addTransactionToBundles')
+    const spyUpdate = jest.spyOn(utils, 'addTransactionsToBundles')
 
     findAndUpdateRecurrences(recurrences, transactions)
     expect(spyUpdate).toHaveBeenCalled()

src/ducks/recurrence/search.js

@@ -12,7 +12,7 @@ import {
   getRulesFromConfig,
   groupBundles
 } from 'ducks/recurrence/rules'
-import { addTransactionToBundles } from 'ducks/recurrence/utils'
+import { addTransactionsToBundles } from 'ducks/recurrence/utils'
 import { logRecurrencesLabelAndTransactionsNumber } from 'ducks/recurrence/service'
 import { log } from './logger'
 
@@ -108,7 +108,7 @@ export const updateRecurrences = (bundles, newTransactions, rules) => {
   let updatedBundles = []
 
   const { updatedBundles: newUpdatedBundles, transactionsForUpdatedBundles } =
-    addTransactionToBundles(bundles, newTransactions)
+    addTransactionsToBundles(bundles, newTransactions)
 
   updatedBundles = newUpdatedBundles
   const remainingTransactions = differenceBy(

src/ducks/recurrence/service.js

@@ -106,10 +106,12 @@ export const logRecurrencesLabelAndTransactionsNumber = ({
 
 /**
  * Fetches
- *   - transactions in the last 100 days
+ *   - transactions from the last 100 days with no recurrences
  *   - current recurrences
- * and update recurrences and operations according to recurrence matching algorithm.
- * The date taken into account to create recurrences is `transaction.date` and not `transaction.realizationDate`.
+ * and updates recurrences and transactions according to the recurrence matching
+ * algorithm.
+ * The date taken into account to create recurrences is `transaction.date` and
+ * not `transaction.realizationDate`.
  *
  * Called inside service.
  */
@@ -129,15 +131,23 @@ const main = async ({ client }) => {
     )
     const transactions = await client.queryAll(transactionQuery)
 
+    // Filter out transactions already associated with a recurrence as we don't
+    // want to change it.
+    const newTransactions = transactions.filter(
+      transaction =>
+        transaction.relationships == null ||
+        transaction.relationships.recurrence == null
+    )
+
     if (recurrences.length > 0) {
       log(
         'info',
-        `Loaded transactions from ${NB_DAYS_LOOKBACK} days back, ${transactions.length} transactions to consider`
+        `Loaded transactions from ${NB_DAYS_LOOKBACK} days back, ${newTransactions.length} transactions to consider`
       )
     } else {
       log(
         'info',
-        `Loaded all transactions (since there were no recurrences yet), ${transactions.length} transactions to consider`
+        `Loaded all transactions (since there were no recurrences yet), ${newTransactions.length} transactions to consider`
       )
     }
 
@@ -146,7 +156,7 @@ const main = async ({ client }) => {
 
     const updatedRecurrences = findAndUpdateRecurrences(
       recurrencesAmountsCatIdsUpdated.map(r => ({ ...r })),
-      transactions
+      newTransactions
     ).map(x => omit(x, '_type'))
 
     const { true: emptyRecurrences = [], false: nonEmptyRecurrences = [] } =

src/ducks/recurrence/utils.js

@@ -139,26 +139,26 @@ export const isDeprecatedBundle = recurrence => {
 }
 
 /**
- * Allows to add transactions to bundles which match with these conditions:
+ * Adds new transactions to bundles which match with these conditions:
  * - Amount (with +/- percentage)
  * - CategoryId
  * - Account
  *
  * @param {Array<Recurrence>} bundles
- * @param {Array<Transaction>} transactions
+ * @param {Array<Transaction>} newTransactions
  *
  * @returns {{transactionsForUpdatedBundles: Array<Transaction>, updatedBundles: Array<Recurrence>}}
  */
-export const addTransactionToBundles = (bundles, transactions) => {
+export const addTransactionsToBundles = (bundles, newTransactions) => {
   let transactionsForUpdatedBundles = []
 
   const updatedBundles = [...bundles].map(b => {
-    const bundle = { ...b }
+    const bundle = { ...b } // WARNING: this only creates a shallow copy of `b`
 
     const minAmount = getMinAmount(bundle)
     const maxAmount = getMaxAmount(bundle)
 
-    const transactionFounds = transactions.filter(transaction => {
+    const transactionsFound = newTransactions.filter(transaction => {
       const hasSomeSameCategoryId = bundle.categoryIds.some(
         catId => getCategoryId(transaction) === catId
       )
@@ -178,10 +178,10 @@ export const addTransactionToBundles = (bundles, transactions) => {
       )
     })
 
-    if (transactionFounds?.length > 0) {
-      bundle.ops = uniqBy([...bundle.ops, ...transactionFounds], o => o._id)
+    if (transactionsFound?.length > 0) {
+      bundle.ops = uniqBy([...bundle.ops, ...transactionsFound], o => o._id)
       transactionsForUpdatedBundles =
-        transactionsForUpdatedBundles.concat(transactionFounds)
+        transactionsForUpdatedBundles.concat(transactionsFound)
     }
 
     return bundle