fix: Prevent excessive konnector alerts notifications

src/ducks/transactions/queries.js

@@ -100,35 +100,33 @@ export const makeFilteredTransactionsConn = options => {
  * @return {[QueryDefinition, QueryDefinition]} - Earliest and latest queries
  */
 export const makeEarliestLatestQueries = baseQuery => {
-
   /**
-   * Big hack. We worked to optimize this query see 
+   * Big hack. We worked to optimize this query see
    * commit 2bb11660ec40c683898673b3270c763404552cd8
-   * 
-   * But by doing so, we had an issue where when selecting 
+   *
+   * But by doing so, we had an issue where when selecting
    * "all accounts", the result was not right. This is because
    * we indexed by _id first and then date.
-   * So the current fix is : 
-   * - If we have a selector on an account, then the index should 
-   * be indexed by account first and then we ensure that we have 
+   * So the current fix is :
+   * - If we have a selector on an account, then the index should
+   * be indexed by account first and then we ensure that we have
    * the date within the index
    * - If we don't have a slector on an account, then the index
    * should be indexed first by the date and then by the selector
-   * 
-   * @todo: We should really split all this query creation to several 
+   *
+   * @todo: We should really split all this query creation to several
    * methods. We can't have optimized query with such generic methods
-   */ 
+   */
 
   const selectors = Object.keys(baseQuery.selector)
   let indexedFields
-  if(selectors.includes('account')){
+  if (selectors.includes('account')) {
     indexedFields = selectors
-    indexedFields.push('date')  
+    indexedFields.push('date')
   } else {
     indexedFields = ['date']
     indexedFields.push(...selectors)
   }
-
 
   const sortByDesc = []
   const sortByAsc = []

src/targets/services/konnectorAlerts/createTriggerAt.js

@@ -1,14 +1,23 @@
 import { TRIGGER_DOCTYPE } from 'doctypes'
 import { logger } from 'ducks/konnectorAlerts'
-import { ONE_DAY } from 'ducks/recurrence/constants'
-import { getTriggerStates, fetchRelatedFuturAtTriggers } from './helpers'
-
-const dateInDays = (referenceDate, n) => {
-  return new Date(+new Date(referenceDate) + n * ONE_DAY)
-}
+import {
+  add,
+  getTriggerStates,
+  fetchRelatedFuturAtTriggers
+} from 'targets/services/konnectorAlerts/helpers'
 
 const createTriggerAt = async ({ client, date, konnectorTriggerId }) => {
   try {
+    if (date <= add(Date.now(), { days: 2 })) {
+      // If the date is in the past or too close to the current execution of the
+      // service, we don't create a trigger.
+      logger(
+        'info',
+        '@at trigger not created: this konnector trigger would be too close to this execution (less than 2 days)'
+      )
+      return
+    }
+
     await client.save({
       _type: TRIGGER_DOCTYPE,
       type: '@at',
@@ -56,19 +65,19 @@ export const createScheduledTrigger = async client => {
     if (relatedFuturAtTriggers.length > 0) {
       logger(
         'info',
-        `@at triggers not created: @at triggers already existing in the futur for this konnector trigger`
+        `@at triggers not created: @at triggers already existing in the future for this konnector trigger`
       )
       continue
     }
 
     await containerForTesting.createTriggerAt({
       client,
-      date: dateInDays(triggerStates.last_failure, 3),
+      date: add(triggerStates.last_failure, { days: 3 }),
       konnectorTriggerId: id
     })
     await containerForTesting.createTriggerAt({
       client,
-      date: dateInDays(triggerStates.last_failure, 7),
+      date: add(triggerStates.last_failure, { days: 7 }),
       konnectorTriggerId: id
     })
   }

src/targets/services/konnectorAlerts/createTriggerAt.spec.js

@@ -1,7 +1,7 @@
 import { createMockClient } from 'cozy-client'
 
 import { createScheduledTrigger, containerForTesting } from './createTriggerAt'
-import { getTriggerStates, fetchRelatedFuturAtTriggers } from './helpers'
+import { fetchRelatedFuturAtTriggers, getTriggerStates, sub } from './helpers'
 
 jest.mock('./helpers', () => ({
   ...jest.requireActual('./helpers'),
@@ -26,7 +26,7 @@ describe('createTriggerAt', () => {
     expect(containerForTesting.createTriggerAt).not.toHaveBeenCalled()
   })
 
-  it("should not create @at triggers if the konnector doesn't sent notification", async () => {
+  it('should not create @at triggers if the service should not notify', async () => {
     getTriggerStates.mockResolvedValue({
       trigger01Id: { shouldNotify: { ok: false } }
     })
@@ -36,7 +36,7 @@ describe('createTriggerAt', () => {
     expect(containerForTesting.createTriggerAt).not.toHaveBeenCalled()
   })
 
-  it('should not create @at triggers if there are already created in the futur', async () => {
+  it('should not create @at triggers if some were already scheduled in the future', async () => {
     getTriggerStates.mockResolvedValue({
       trigger01Id: {
         shouldNotify: { ok: true }
@@ -51,16 +51,56 @@ describe('createTriggerAt', () => {
     expect(containerForTesting.createTriggerAt).not.toHaveBeenCalled()
   })
 
-  it('should create two @at triggers', async () => {
-    getTriggerStates.mockResolvedValue({
-      trigger01Id: {
-        shouldNotify: { ok: true }
-      }
+  describe('when the last trigger failure occurred less than 24 hours ago', () => {
+    it('should create two @at triggers', async () => {
+      getTriggerStates.mockResolvedValue({
+        trigger01Id: {
+          shouldNotify: { ok: true },
+          last_failure: sub(Date.now(), { hours: 23, minutes: 59, seconds: 59 })
+        }
+      })
+      fetchRelatedFuturAtTriggers.mockResolvedValue([])
+
+      await createScheduledTrigger(client)
+
+      expect(client.save).toHaveBeenCalledTimes(2)
     })
-    fetchRelatedFuturAtTriggers.mockResolvedValue([])
+  })
 
-    await createScheduledTrigger(client)
+  describe('when the last trigger failure occurred less than 5 days ago', () => {
+    it('should create one @at trigger', async () => {
+      getTriggerStates.mockResolvedValue({
+        trigger01Id: {
+          shouldNotify: { ok: true },
+          last_failure: sub(Date.now(), {
+            days: 4,
+            hours: 23,
+            minutes: 59,
+            seconds: 59
+          })
+        }
+      })
+      fetchRelatedFuturAtTriggers.mockResolvedValue([])
+
+      await createScheduledTrigger(client)
+
+      expect(client.save).toHaveBeenCalledTimes(1)
+    })
+  })
 
-    expect(containerForTesting.createTriggerAt).toHaveBeenCalledTimes(2)
+  describe('when the last trigger failure occurred 5 days ago or more', () => {
+    it('should not create @at triggers', async () => {
+      getTriggerStates.mockResolvedValue({
+        trigger01Id: {
+          shouldNotify: { ok: true },
+          last_failure: sub(Date.now(), { days: 5 })
+        }
+      })
+      fetchRelatedFuturAtTriggers.mockResolvedValue([])
+
+      await createScheduledTrigger(client)
+
+      expect(client.save).toHaveBeenCalledTimes(0)
+    })
   })
 })

src/targets/services/konnectorAlerts/helpers.js

@@ -1,6 +1,15 @@
 import memoize from 'lodash/memoize'
 import keyBy from 'lodash/keyBy'
 import get from 'lodash/get'
+import addDays from 'date-fns/add_days'
+import addHours from 'date-fns/add_hours'
+import addMinutes from 'date-fns/add_minutes'
+import addSeconds from 'date-fns/add_seconds'
+import parse from 'date-fns/parse'
+import subDays from 'date-fns/sub_days'
+import subHours from 'date-fns/sub_hours'
+import subMinutes from 'date-fns/sub_minutes'
+import subSeconds from 'date-fns/sub_seconds'
 
 import { Q } from 'cozy-client'
 
@@ -115,3 +124,12 @@ export const fetchRelatedFuturAtTriggers = async (client, id) => {
 
   return data
 }
+
+export const add = (base, { days = 0, hours = 0, minutes = 0, seconds = 0 }) =>
+  addDays(addHours(addMinutes(addSeconds(base, seconds), minutes), hours), days)
+
+export const sub = (base, { days = 0, hours = 0, minutes = 0, seconds = 0 }) =>
+  subDays(subHours(subMinutes(subSeconds(base, seconds), minutes), hours), days)
+
+export const isOlderThan = (referenceDate, { days, hours, minutes, seconds }) =>
+  parse(referenceDate) < sub(Date.now(), { days, hours, minutes, seconds })

src/targets/services/konnectorAlerts/helpers.spec.js

@@ -1,5 +1,7 @@
 import { createMockClient } from 'cozy-client'
-import { storeTriggerStates } from './helpers'
+import MockDate from 'mockdate'
+
+import { add, storeTriggerStates, isOlderThan, sub } from './helpers'
 
 const triggerStatesWithNotifsInfo = [
   {
@@ -55,3 +57,82 @@ describe('storeTriggerStates', () => {
     })
   })
 })
+
+describe('sub', () => {
+  it('returns a Date object', () => {
+    expect(
+      sub(Date.now(), { days: 1, hours: 2, minutes: 3, seconds: 4 })
+    ).toBeInstanceOf(Date)
+    expect(
+      sub(new Date(), { days: 1, hours: 2, minutes: 3, seconds: 4 })
+    ).toBeInstanceOf(Date)
+  })
+
+  it('returns the expected past date', () => {
+    const base = Date.parse('28 Sep 2022 11:14:37 GMT')
+
+    const past = sub(base, { days: 1, hours: 2, minutes: 3, seconds: 4 })
+    expect(past.getUTCFullYear()).toBe(2022)
+    expect(past.getUTCMonth()).toBe(8) // Months are 0 indexed
+    expect(past.getUTCDate()).toBe(27)
+    expect(past.getUTCHours()).toBe(9)
+    expect(past.getUTCMinutes()).toBe(11)
+    expect(past.getUTCSeconds()).toBe(33)
+  })
+})
+
+describe('add', () => {
+  it('returns a Date object', () => {
+    expect(
+      add(Date.now(), {
+        days: 1,
+        hours: 2,
+        minutes: 3,
+        seconds: 4
+      })
+    ).toBeInstanceOf(Date)
+    expect(
+      add(new Date(), {
+        days: 1,
+        hours: 2,
+        minutes: 3,
+        seconds: 4
+      })
+    ).toBeInstanceOf(Date)
+  })
+
+  it('returns the expected future date', () => {
+    const base = Date.parse('28 Sep 2022 11:14:37 GMT')
+
+    const future = add(base, {
+      days: 1,
+      hours: 2,
+      minutes: 3,
+      seconds: 4
+    })
+    expect(future.getUTCFullYear()).toBe(2022)
+    expect(future.getUTCMonth()).toBe(8) // Months are 0 indexed
+    expect(future.getUTCDate()).toBe(29)
+    expect(future.getUTCHours()).toBe(13)
+    expect(future.getUTCMinutes()).toBe(17)
+    expect(future.getUTCSeconds()).toBe(41)
+  })
+})
+
+describe('isOlderThan', () => {
+  afterEach(() => {
+    MockDate.reset()
+  })
+
+  it('returns true if given date is older than given time parameters', () => {
+    MockDate.set(Date.parse('28 Sep 2022 11:14:37 GMT'))
+
+    expect(isOlderThan('2022-09-28T11:14:33Z', { seconds: 3 })).toBe(true)
+  })
+
+  it('returns false if given date is not older than given time parameters', () => {
+    MockDate.set(Date.parse('28 Sep 2022 11:14:37 GMT'))
+
+    expect(isOlderThan('2022-09-28T11:14:33Z', { seconds: 4 })).toBe(false)
+  })
+})

src/targets/services/konnectorAlerts/sendTriggerNotifications.spec.js

@@ -1,5 +1,6 @@
 import CozyClient from 'cozy-client'
 import { sendNotification } from 'cozy-notifications'
+import MockDate from 'mockdate'
 
 import matchAll from 'utils/matchAll'
 import { sendTriggerNotifications } from './sendTriggerNotifications'
@@ -222,6 +223,11 @@ describe('sendTriggerNotifications', () => {
 
   beforeEach(() => {
     sendNotification.mockClear()
+    MockDate.set('2020-01-02')
+  })
+
+  afterEach(() => {
+    MockDate.reset()
   })
 
   const expectTriggerStatesToHaveBeenSaved = client => {

src/targets/services/konnectorAlerts/shouldNotify.js

@@ -6,15 +6,16 @@ import { JOBS_DOCTYPE } from 'doctypes'
 import {
   getKonnectorSlug,
   fetchRegistryInfo,
-  isErrorActionable
-} from './helpers'
+  isErrorActionable,
+  isOlderThan
+} from 'targets/services/konnectorAlerts/helpers'
 
 /**
  * Returns whether we need to send a notification for a trigger
  *
  * @typedef {Object} ShouldNotifyResult
- * @property {number} ok - Whether the trigger generates a notification
- * @property {number} reason - If ok=false, describes why.
+ * @property {boolean} ok - Whether the trigger generates a notification
+ * @property {string} reason - If ok=false, describes why.
  *
  * @return {ShouldNotifyResult}
  */
@@ -37,6 +38,17 @@ export const shouldNotify = async ({ client, trigger, previousStates }) => {
     return { ok: false, reason: 'never-been-in-success' }
   }
 
+  // We do not want to send notifications for connectors that failed a long time
+  // ago without any retry since then.
+  // The last reminder is scheduled 7 days after the last failure plus a few
+  // minutes, with a maximum of 15 minutes, so we prevent notifications to be
+  // sent if we're past that delay.
+  if (
+    isOlderThan(trigger.current_state.last_failure, { days: 7, minutes: 15 })
+  ) {
+    return { ok: false, reason: 'last-failure-too-old' }
+  }
+
   if (
     previousState.status === 'errored' &&
     isErrorActionable(previousState.last_error)