feat(secret-reminders): addressed PR suggestions and improvements

This commit is contained in:
Carlos Monastyrski
2025-07-04 11:58:09 -03:00
parent 0efc314f33
commit e2559f10bc
26 changed files with 276 additions and 186 deletions
+15 -27
View File
@@ -16,7 +16,7 @@ import { ormify, selectAllTableCols, sqlNestRelationships } from "@app/lib/knex"
export type TReminderDALFactory = ReturnType<typeof reminderDALFactory>;
export const reminderDALFactory = (db: TDbClient) => {
const reminderOrm = ormify(db, TableName.Reminders);
const reminderOrm = ormify(db, TableName.Reminder);
const getTodayDateRange = () => {
const now = new Date();
@@ -29,15 +29,11 @@ export const reminderDALFactory = (db: TDbClient) => {
const findSecretDailyReminders = async (tx?: Knex) => {
const { startOfDay, endOfDay } = getTodayDateRange();
const rawReminders = await (tx || db)(TableName.Reminders)
const rawReminders = await (tx || db)(TableName.Reminder)
.whereBetween("nextReminderDate", [startOfDay, endOfDay])
.leftJoin(
TableName.RemindersRecipients,
`${TableName.Reminders}.id`,
`${TableName.RemindersRecipients}.reminderId`
)
.leftJoin<TUsers>(TableName.Users, `${TableName.RemindersRecipients}.userId`, `${TableName.Users}.id`)
.leftJoin<TSecretsV2>(TableName.SecretV2, `${TableName.Reminders}.secretId`, `${TableName.SecretV2}.id`)
.leftJoin(TableName.ReminderRecipient, `${TableName.Reminder}.id`, `${TableName.ReminderRecipient}.reminderId`)
.leftJoin<TUsers>(TableName.Users, `${TableName.ReminderRecipient}.userId`, `${TableName.Users}.id`)
.leftJoin<TSecretsV2>(TableName.SecretV2, `${TableName.Reminder}.secretId`, `${TableName.SecretV2}.id`)
.leftJoin<TSecretFolders>(
TableName.SecretFolder,
`${TableName.SecretV2}.folderId`,
@@ -50,7 +46,7 @@ export const reminderDALFactory = (db: TDbClient) => {
)
.leftJoin<TProjects>(TableName.Project, `${TableName.Environment}.projectId`, `${TableName.Project}.id`)
.leftJoin<TOrganizations>(TableName.Organization, `${TableName.Project}.orgId`, `${TableName.Organization}.id`)
.select(selectAllTableCols(TableName.Reminders))
.select(selectAllTableCols(TableName.Reminder))
.select(db.ref("email").withSchema(TableName.Users))
.select(db.ref("name").withSchema(TableName.Project).as("projectName"))
.select(db.ref("id").withSchema(TableName.Project).as("projectId"))
@@ -84,30 +80,22 @@ export const reminderDALFactory = (db: TDbClient) => {
const futureDate = new Date(startOfDay);
futureDate.setDate(futureDate.getDate() + daysAhead);
const reminders = await (tx || db)(TableName.Reminders)
const reminders = await (tx || db)(TableName.Reminder)
.where("nextReminderDate", ">=", startOfDay)
.where("nextReminderDate", "<=", futureDate)
.orderBy("nextReminderDate", "asc")
.leftJoin(
TableName.RemindersRecipients,
`${TableName.Reminders}.id`,
`${TableName.RemindersRecipients}.reminderId`
)
.select(selectAllTableCols(TableName.Reminders))
.select(db.ref("userId").withSchema(TableName.RemindersRecipients));
.leftJoin(TableName.ReminderRecipient, `${TableName.Reminder}.id`, `${TableName.ReminderRecipient}.reminderId`)
.select(selectAllTableCols(TableName.Reminder))
.select(db.ref("userId").withSchema(TableName.ReminderRecipient));
return reminders;
};
const findSecretReminder = async (secretId: string, tx?: Knex) => {
const rawReminders = await (tx || db)(TableName.Reminders)
.where(`${TableName.Reminders}.secretId`, secretId)
.leftJoin(
TableName.RemindersRecipients,
`${TableName.Reminders}.id`,
`${TableName.RemindersRecipients}.reminderId`
)
.select(selectAllTableCols(TableName.Reminders))
.select(db.ref("userId").withSchema(TableName.RemindersRecipients));
const rawReminders = await (tx || db)(TableName.Reminder)
.where(`${TableName.Reminder}.secretId`, secretId)
.leftJoin(TableName.ReminderRecipient, `${TableName.Reminder}.id`, `${TableName.ReminderRecipient}.reminderId`)
.select(selectAllTableCols(TableName.Reminder))
.select(db.ref("userId").withSchema(TableName.ReminderRecipient));
const reminders = sqlNestRelationships({
data: rawReminders,
key: "id",
+25 -19
View File
@@ -11,7 +11,7 @@ import { TReminderServiceFactory } from "./reminder-service";
type TDailyReminderQueueServiceFactoryDep = {
reminderService: TReminderServiceFactory;
queueService: TQueueServiceFactory;
secretDAL: Pick<TSecretV2BridgeDALFactory, "findSecretsWithReminderRecipients">;
secretDAL: Pick<TSecretV2BridgeDALFactory, "transaction" | "findSecretsWithReminderRecipients">;
secretReminderRecipientsDAL: Pick<TSecretReminderRecipientsDALFactory, "delete">;
};
@@ -51,11 +51,12 @@ export const dailyReminderQueueServiceFactory = ({
if (match) {
map.set(match[0], {
timestamp: job.timestamp,
delay: job.delay
delay: job.delay,
data: job.data
});
}
return map;
}, new Map<string, { timestamp: number; delay: number }>());
}, new Map<string, { timestamp: number; delay: number; data: unknown }>());
if (reminderJobs.length === 0) {
logger.info(`${QueueName.SecretReminderMigration}: no reminder jobs found`);
return;
@@ -96,21 +97,26 @@ export const dailyReminderQueueServiceFactory = ({
}
}
await reminderService.batchCreateReminders(
secretsWithReminder.map((secret) => {
const delayedJob = reminderDelayedJobs.get(secret.id);
const nextDate = delayedJob ? new Date(delayedJob.timestamp + delayedJob.delay) : undefined;
return {
secretId: secret.id,
message: secret.reminderNote,
repeatDays: secret.reminderRepeatDays,
nextReminderDate: nextDate,
recipients: secret.recipients || []
};
})
);
await secretDAL.transaction(async (tx) => {
await reminderService.batchCreateReminders(
secretsWithReminder.map((secret) => {
const delayedJob = reminderDelayedJobs.get(secret.id);
const projectId = (delayedJob?.data as { projectId?: string })?.projectId;
const nextDate = delayedJob ? new Date(delayedJob.timestamp + delayedJob.delay) : undefined;
return {
secretId: secret.id,
message: secret.reminderNote,
repeatDays: secret.reminderRepeatDays,
nextReminderDate: nextDate,
recipients: secret.recipients || [],
projectId
};
}),
tx
);
await secretReminderRecipientsDAL.delete({ $in: { secretId: secretsWithReminder.map((s) => s.id) } });
await secretReminderRecipientsDAL.delete({ $in: { secretId: secretsWithReminder.map((s) => s.id) } }, tx);
});
numberOfRetryOnFailure = 0;
} catch (error) {
@@ -176,11 +182,11 @@ export const dailyReminderQueueServiceFactory = ({
};
queueService.listen(QueueName.DailyReminders, "failed", (_, err) => {
logger.error(err, `${QueueName.DailyReminders}: resource cleanup failed`);
logger.error(err, `${QueueName.DailyReminders}: daily reminder processing failed`);
});
queueService.listen(QueueName.SecretReminderMigration, "failed", (_, err) => {
logger.error(err, `${QueueName.SecretReminderMigration}: resource cleanup failed`);
logger.error(err, `${QueueName.SecretReminderMigration}: secret reminder migration failed`);
});
return {
@@ -9,7 +9,8 @@ import { logger } from "@app/lib/logger";
import { ActorAuthMethod, ActorType } from "../auth/auth-type";
import { TProjectMembershipDALFactory } from "../project-membership/project-membership-dal";
import { TReminderRecipientDALFactory } from "../reminder-recipients/reminder-dal";
import { TReminderRecipientDALFactory } from "../reminder-recipients/reminder-recipient-dal";
import { TSecretV2BridgeDALFactory } from "../secret-v2-bridge/secret-v2-bridge-dal";
import { SmtpTemplates, TSmtpService } from "../smtp/smtp-service";
import { TReminderDALFactory } from "./reminder-dal";
import { TBatchCreateReminderDTO, TCreateReminderDTO } from "./reminder-types";
@@ -20,6 +21,7 @@ type TReminderServiceFactoryDep = {
smtpService: TSmtpService;
projectMembershipDAL: Pick<TProjectMembershipDALFactory, "findAllProjectMembers">;
permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
secretV2BridgeDAL: Pick<TSecretV2BridgeDALFactory, "invalidateSecretCacheByProjectId">;
};
export type TReminderServiceFactory = ReturnType<typeof reminderServiceFactory>;
@@ -29,7 +31,8 @@ export const reminderServiceFactory = ({
reminderRecipientDAL,
smtpService,
projectMembershipDAL,
permissionService
permissionService,
secretV2BridgeDAL
}: TReminderServiceFactoryDep) => {
const addDays = (days: number, fromDate: Date = new Date()): Date => {
const result = new Date(fromDate);
@@ -76,13 +79,15 @@ export const reminderServiceFactory = ({
message,
repeatDays,
nextReminderDate: nextReminderDateInput,
recipients
recipients,
projectId
}: {
secretId?: string;
message?: string | null;
repeatDays?: number | null;
nextReminderDate?: string | null;
recipients?: string[] | null;
projectId: string;
}) => {
if (!secretId) {
throw new BadRequestError({ message: "secretId is required" });
@@ -124,7 +129,7 @@ export const reminderServiceFactory = ({
// Manage recipients (add/update/delete as needed)
await $manageReminderRecipients(reminderId, recipients);
await secretV2BridgeDAL.invalidateSecretCacheByProjectId(projectId);
return { id: reminderId, created: !existingReminder };
};
@@ -145,7 +150,10 @@ export const reminderServiceFactory = ({
});
ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionSecretActions.Edit, ProjectPermissionSub.Secrets);
const response = await createReminderInternal(reminder);
const response = await createReminderInternal({
...reminder,
projectId
});
return response;
};
@@ -184,28 +192,30 @@ export const reminderServiceFactory = ({
for (const reminder of remindersToSend) {
try {
const recipients: string[] = reminder.recipients
.map((r) => r.email)
.filter((email): email is string => Boolean(email));
if (recipients.length === 0) {
const members = await projectMembershipDAL.findAllProjectMembers(reminder.projectId);
recipients.push(...members.map((m) => m.user.email).filter((email): email is string => Boolean(email)));
}
await smtpService.sendMail({
template: SmtpTemplates.SecretReminder,
subjectLine: "Infisical secret reminder",
recipients,
substitutions: {
reminderNote: reminder.message || "",
projectName: reminder.projectName || "",
organizationName: reminder.organizationName || ""
await reminderDAL.transaction(async (tx) => {
const recipients: string[] = reminder.recipients
.map((r) => r.email)
.filter((email): email is string => Boolean(email));
if (recipients.length === 0) {
const members = await projectMembershipDAL.findAllProjectMembers(reminder.projectId);
recipients.push(...members.map((m) => m.user.email).filter((email): email is string => Boolean(email)));
}
await smtpService.sendMail({
template: SmtpTemplates.SecretReminder,
subjectLine: "Infisical secret reminder",
recipients,
substitutions: {
reminderNote: reminder.message || "",
projectName: reminder.projectName || "",
organizationName: reminder.organizationName || ""
}
});
if (reminder.repeatDays) {
await reminderDAL.updateById(reminder.id, { nextReminderDate: addDays(reminder.repeatDays) }, tx);
} else {
await reminderDAL.deleteById(reminder.id, tx);
}
});
if (reminder.repeatDays) {
await reminderDAL.updateById(reminder.id, { nextReminderDate: addDays(reminder.repeatDays) });
} else {
await reminderDAL.deleteById(reminder.id);
}
} catch (error) {
logger.error(
error,
@@ -240,27 +250,30 @@ export const reminderServiceFactory = ({
ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionSecretActions.Edit, ProjectPermissionSub.Secrets);
await reminderDAL.delete({ secretId });
await secretV2BridgeDAL.invalidateSecretCacheByProjectId(projectId);
};
const removeReminderRecipients = async (secretId: string, tx?: Knex) => {
const removeReminderRecipients = async (secretId: string, projectId: string, tx?: Knex) => {
const reminder = await reminderDAL.findOne({ secretId }, tx);
if (!reminder) {
return;
}
await reminderRecipientDAL.delete({ reminderId: reminder.id }, tx);
await secretV2BridgeDAL.invalidateSecretCacheByProjectId(projectId);
};
const deleteReminderBySecretId = async (secretId: string, tx?: Knex) => {
const deleteReminderBySecretId = async (secretId: string, projectId: string, tx?: Knex) => {
await reminderDAL.delete({ secretId }, tx);
await secretV2BridgeDAL.invalidateSecretCacheByProjectId(projectId);
};
const batchCreateReminders = async (remindersData: TBatchCreateReminderDTO) => {
const batchCreateReminders = async (remindersData: TBatchCreateReminderDTO, tx?: Knex) => {
if (!remindersData || remindersData.length === 0) {
return { created: 0, reminderIds: [] };
}
const processedReminders = remindersData.map(
({ secretId, message, repeatDays, nextReminderDate: nextReminderDateInput, recipients }) => {
({ secretId, message, repeatDays, nextReminderDate: nextReminderDateInput, recipients, projectId }) => {
let nextReminderDate;
if (nextReminderDateInput) {
nextReminderDate = new Date(nextReminderDateInput);
@@ -281,18 +294,21 @@ export const reminderServiceFactory = ({
message,
repeatDays,
nextReminderDate,
recipients: recipients ? [...new Set(recipients)] : []
recipients: recipients ? [...new Set(recipients)] : [],
projectId
};
}
);
const newReminders = await reminderDAL.insertMany(
processedReminders.map(({ secretId, message, repeatDays, nextReminderDate }) => ({
processedReminders.map(({ secretId, message, repeatDays, nextReminderDate, projectId }) => ({
secretId,
message,
repeatDays,
nextReminderDate
}))
nextReminderDate,
projectId
})),
tx
);
const allRecipientInserts: Array<{ reminderId: string; userId: string }> = [];
@@ -310,7 +326,12 @@ export const reminderServiceFactory = ({
});
if (allRecipientInserts.length > 0) {
await reminderRecipientDAL.insertMany(allRecipientInserts);
await reminderRecipientDAL.insertMany(allRecipientInserts, tx);
}
const projectIds = new Set(processedReminders.map((r) => r.projectId).filter((id): id is string => Boolean(id)));
for (const projectId of projectIds) {
await secretV2BridgeDAL.invalidateSecretCacheByProjectId(projectId);
}
return {
@@ -24,21 +24,11 @@ export type TCreateReminderDTO = {
};
};
export type TCreateSecretReminderDTO = {
secretName: string;
projectId: string;
environment: string;
secretPath: string;
message?: string | null;
repeatDays?: number | null;
nextReminderDate?: string | null;
recipients?: string[] | null;
};
export type TBatchCreateReminderDTO = {
secretId: string;
message?: string | null;
repeatDays?: number | null;
nextReminderDate?: string | Date | null;
recipients?: string[] | null;
projectId?: string;
}[];