fix: Prevent excessive konnector alerts notifications #2516
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
taratatach
requested review from
JF-Cozy,
Merkur39 and
PolariTOON
as code owners
taratatach
force-pushed
the
fix/prevent-excessive-konnector-alerts-notifications
branch
from
aacbb2d to
74f5aa5
Compare
BundleMonFiles updated (1)
Unchanged files (47)
Total files change +456B 0% Final result: ✅ View report in BundleMon website ➡️ |
Crash--
reviewed
Oct 4, 2022
Crash--
reviewed
Oct 4, 2022
taratatach
force-pushed
the
fix/prevent-excessive-konnector-alerts-notifications
branch
from
74f5aa5 to
90b4fce
Compare
JF-Cozy
approved these changes
Oct 5, 2022
| @@ -115,3 +121,24 @@ export const fetchRelatedFuturAtTriggers = async (client, id) => { | |||
|
|
|||
| return data | |||
| } | |||
|
|
|||
| export const timeAgo = ({ | |||
There was a problem hiding this comment.
nit: est-ce qu'on a regardé dans date-fns s'il y a un équivalent ?
There was a problem hiding this comment.
Non, je n'ai pas regardé, je ne savais pas qu'il y avait ce module. On pourrait même remplacer isOlderThan j'imagine. Je vais regarder.
There was a problem hiding this comment.
attention par contre à la version utilisée de la lib
There was a problem hiding this comment.
Oui, il n'ya pas la fonction add dans la version actuelle donc je vais passer à la dernière version (je suis en train de mettre à jour les imports dont la manière de faire a visiblement changé).
There was a problem hiding this comment.
attention au peerdep des libs. Peut-être que cette version est nécessaire pour harvest par exemple
There was a problem hiding this comment.
harvest déclare sa propre dépendance à date-fns. Je serais par ailleurs surpris qu'un tel changement puisse être un breaking change pour d'autres dépendances de Banks. Ce serait amha un gros problème d'architecture.
There was a problem hiding this comment.
Je suis retourné plus ou moins à l'état avant la mise à jour de date-fns puisque cela pose des soucis avec nos autres bibliothèques.
J'ai mis la mise à jour dans une autre PR : #2522
taratatach
force-pushed
the
fix/prevent-excessive-konnector-alerts-notifications
branch
5 times, most recently
from
f0adb0d to
94e5117
Compare
We don't want users to be notified (neither by e-mail nor push notifications) when a konnector failed a long time ago since there's no real need for a quick action from their part. This will be particularly useful when activating these notifications for a large audience. To achieve that result, we create a specific reason not to notify that we should not ignore via the `forcedIgnored` argument of the service. We set the limit to 7 days and 15 minutes after the last failure as this is the maximum delay we can wait for before triggering the service to send the last reminder notification to the user. Setting the limit below this would prevent that last notification to be sent.
When a konnector fails for an actionnable reason (i.e. the user has the power to fix the underlying issue), we send a notification after the konnector has run and schedule 2 `konnectorAlerts` triggers 3 days and 7 days after to send notifications again in case the konnector is still failing to run. However, we set the starting point of these delays not at the moment when the `konnectorAlerts` service runs for the first time but at the moment the konnector failed for the last time. This date will always be in the past but it can be quite old thus generating reminders scheduled dates very close to the moment the service ran first and sent a notification to the user. It can even be in the past in which case `cozy-stack` will execute the trigger immediately after it is scheduled. In both situations, the user gets spammed with notifications. To prevent these, we will now make sure the reminders are scheduled to be sent at least 2 days after the moment the service first ran.
taratatach
force-pushed
the
fix/prevent-excessive-konnector-alerts-notifications
branch
from
94e5117 to
fe27012
Compare
Crash--
approved these changes
Oct 11, 2022
taratatach
deleted the
fix/prevent-excessive-konnector-alerts-notifications
branch
JF-Cozy
pushed a commit
that referenced
this pull request
Oct 14, 2022
``` * Don't notify users about very old konnector run failures (i.e. older than 7 days and 15 minutes which is the maximum delay between the last failure and the scheduled 7 days reminder). * Prevent scheduling `konnectorAlerts` triggers in the past which would be executed right away by `cozy-stack` ending up sending multiple notifications to the user about the failed konnector run. * ``` We don't want users to be notified (neither by e-mail nor push notifications) when a konnector failed a long time ago since there's no real need for a quick action from their part and we want to avoid spamming them with lots of notifications at the same time.
taratatach
added a commit
that referenced
this pull request
Dec 5, 2022
``` ### ✨ Features * Don't notify users about very old konnector run failures (i.e. older than 7 days and 15 minutes which is the maximum delay between the last failure and the scheduled 7 days reminder). (#2516) ### 🐛 Bug Fixes * Prevent scheduling `konnectorAlerts` triggers in the past which would be executed right away by `cozy-stack` ending up sending multiple notifications to the user about the failed konnector run. (#2516) * Prevent service call loops by only updating transactions for which we've updated the associated recurrence. (#2429) ### 🔧 Tech * Run recurrence service only if flag is set (#2451) ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We don't want users to be notified (neither by e-mail nor push
notifications) when a konnector failed a long time ago since there's
no real need for a quick action from their part and we want to avoid
spamming them with lots of notifications at the same time.