Properly delete reminders when secret is deleted

Prior to this fix, the update function would try to update a repeated job based on the NEW info, which it will of course fail to find. This results in a new reminder being created, without the old reminder being deleted first.

backend/src/helpers/reminder.ts

@@ -1,9 +1,9 @@
-import { ISecret } from "../models";
 import {
   createRecurringSecretReminder,
   deleteRecurringSecretReminder,
   updateRecurringSecretReminder
 } from "../queues/reminders/sendSecretReminders";
+import type { ISecret } from "../models";
 
 type TPartialSecret = Pick<
   ISecret,
@@ -25,10 +25,12 @@ export const createReminder = async (oldSecret: TPartialSecret, newSecret: TPart
 
   if (oldSecret.secretReminderRepeatDays) {
     // This will first delete the existing recurring job, and then create a new one.
+
     await updateRecurringSecretReminder({
       workspaceId,
       secretId,
-      repeatDays: newSecret.secretReminderRepeatDays,
+      newRepeatDays: newSecret.secretReminderRepeatDays,
+      oldRepeatDays: oldSecret.secretReminderRepeatDays,
       note: newSecret.secretReminderNote
     });
   } else {

backend/src/helpers/secrets.ts

@@ -48,6 +48,8 @@ import { getFolderByPath, getFolderIdFromServiceToken } from "../services/Folder
 import picomatch from "picomatch";
 import path from "path";
 import { getAnImportedSecret } from "../services/SecretImportService";
+import { deleteReminder } from "./reminder";
+import { logger } from "../utils/logging";
 
 /**
  * Validate scope for service token v2
@@ -1057,6 +1059,30 @@ export const deleteSecretHelper = async ({
     folderId: secret?.folder
   });
 
+  if (secrets.length) {
+    for (const s of secrets) {
+      if (s.secretReminderRepeatDays !== null && s.secretReminderRepeatDays !== undefined) {
+        await deleteReminder({
+          _id: s._id,
+          secretReminderRepeatDays: s.secretReminderRepeatDays
+        }).catch((err) =>
+          logger.error(err, `Failed to delete reminder during secret deletion for ${s._id}`)
+        );
+      }
+    }
+  }
+
+  if (secret) {
+    if (secret.secretReminderRepeatDays !== null && secret.secretReminderRepeatDays !== undefined) {
+      await deleteReminder({
+        _id: secret._id,
+        secretReminderRepeatDays: secret.secretReminderRepeatDays
+      }).catch((err) =>
+        logger.error(err, `Failed to delete reminder during secret deletion for ${secret?._id}`)
+      );
+    }
+  }
+
   const postHogClient = await TelemetryService.getPostHogClient();
 
   if (postHogClient) {
@@ -1675,7 +1701,7 @@ export const deleteSecretBatchHelper = async ({
         ...(type === SECRET_PERSONAL ? getAuthDataPayloadUserObj(authData) : {})
       }))
     )
-    .select({ secretBlindIndexes: 1 })
+    .select({ secretBlindIndexes: 1, secretReminderRepeatDays: 1 })
     .lean()
     .exec();
 
@@ -1723,6 +1749,17 @@ export const deleteSecretBatchHelper = async ({
     folderId
   });
 
+  for (const s of deletedSecrets) {
+    if (s.secretReminderRepeatDays !== null && s.secretReminderRepeatDays !== undefined) {
+      await deleteReminder({
+        _id: s._id,
+        secretReminderRepeatDays: s.secretReminderRepeatDays
+      }).catch((err) =>
+        logger.error(err, `Failed to delete reminder during bulk secret deletion for ${s._id}`)
+      );
+    }
+  }
+
   const postHogClient = await TelemetryService.getPostHogClient();
 
   if (postHogClient) {

backend/src/queues/reminders/sendSecretReminders.ts

@@ -1,5 +1,5 @@
-import Queue, { Job } from "bull";
 import { IUser, Membership, Organization, Workspace } from "../../models";
+import Queue, { Job } from "bull";
 import { Types } from "mongoose";
 import { sendMail } from "../../helpers";
 
@@ -10,6 +10,15 @@ type TSendSecretReminders = {
   note: string | undefined | null;
 };
 
+type TUpdateSecretReminder = {
+  oldRepeatDays: number;
+  newRepeatDays: number;
+
+  note: string | undefined | null;
+  secretId: string;
+  workspaceId: string;
+};
+
 type TDeleteSecretReminder = {
   secretId: string;
   repeatDays: number;
@@ -76,8 +85,16 @@ export const deleteRecurringSecretReminder = (jobDetails: TDeleteSecretReminder)
   });
 };
 
-export const updateRecurringSecretReminder = async (jobDetails: TSendSecretReminders) => {
+export const updateRecurringSecretReminder = async (jobDetails: TUpdateSecretReminder) => {
   // We need to delete the potentially existing reminder job first, or the new one won't be created.
-  await deleteRecurringSecretReminder(jobDetails);
-  await createRecurringSecretReminder(jobDetails);
+  await deleteRecurringSecretReminder({
+    repeatDays: jobDetails.oldRepeatDays,
+    secretId: jobDetails.secretId
+  });
+  await createRecurringSecretReminder({
+    repeatDays: jobDetails.newRepeatDays,
+    secretId: jobDetails.secretId,
+    workspaceId: jobDetails.workspaceId,
+    note: jobDetails.note
+  });
 };