From a6d2140197314086ec82c82df1adc84abf4986a8 Mon Sep 17 00:00:00 2001 From: Philipinho <16838612+Philipinho@users.noreply.github.com> Date: Mon, 30 Mar 2026 20:20:51 +0100 Subject: [PATCH] clean up --- .../page-update-email-rate-limiter.ts | 4 +- .../services/page.notification.ts | 118 ++++++++++++++---- .../emails/page-update-digest-email.tsx | 60 +++++++-- .../emails/page-update-email.tsx | 6 +- .../transactional/partials/partials.tsx | 4 + 5 files changed, 149 insertions(+), 43 deletions(-) diff --git a/apps/server/src/core/notification/services/page-update-email-rate-limiter.ts b/apps/server/src/core/notification/services/page-update-email-rate-limiter.ts index 29841c37..f91c54d6 100644 --- a/apps/server/src/core/notification/services/page-update-email-rate-limiter.ts +++ b/apps/server/src/core/notification/services/page-update-email-rate-limiter.ts @@ -4,8 +4,8 @@ import type { Redis } from 'ioredis'; const KEY_PREFIX = 'page-update:emails:'; const DIGEST_PREFIX = 'page-update:digest:'; -const TTL_SECONDS = 86400; // 24 hours -const MAX_IMMEDIATE_EMAILS = 10; +const TTL_SECONDS = 28800; // 8 hours +const MAX_IMMEDIATE_EMAILS = 4; @Injectable() export class PageUpdateEmailRateLimiter { diff --git a/apps/server/src/core/notification/services/page.notification.ts b/apps/server/src/core/notification/services/page.notification.ts index 651a648b..bf4b631e 100644 --- a/apps/server/src/core/notification/services/page.notification.ts +++ b/apps/server/src/core/notification/services/page.notification.ts @@ -186,8 +186,10 @@ export class PageNotificationService { const candidateIds = watcherIds.filter((id) => !actorSet.has(id)); if (candidateIds.length === 0) return; - const afterPrefs = await this.getEligiblePageUpdateUserIds(candidateIds); - if (afterPrefs.length === 0) return; + const eligibleUsers = await this.getEligiblePageUpdateUsers(candidateIds); + if (eligibleUsers.size === 0) return; + + const afterPrefs = [...eligibleUsers.keys()]; const recentlyNotified = await this.notificationRepo.getRecentlyNotifiedUserIds( @@ -237,6 +239,7 @@ export class PageNotificationService { notification.id, `${actor.name} updated ${pageTitle}`, PageUpdateEmail({ + userName: eligibleUsers.get(userId) ?? '', actorName: actor.name, pageTitle, pageUrl: basePageUrl, @@ -255,37 +258,38 @@ export class PageNotificationService { } } - private async getEligiblePageUpdateUserIds( + private async getEligiblePageUpdateUsers( userIds: string[], - ): Promise { - if (userIds.length === 0) return []; + ): Promise> { + if (userIds.length === 0) return new Map(); const users = await this.db .selectFrom('users') - .select(['id', 'settings']) + .select(['id', 'name', 'settings']) .where('id', 'in', userIds) .where('deletedAt', 'is', null) .where('deactivatedAt', 'is', null) .execute(); - return users - .filter((u) => { - const settings = u.settings as any; - return settings?.notifications?.['page.updated'] !== false; - }) - .map((u) => u.id); + const eligible = new Map(); + for (const u of users) { + const settings = u.settings as any; + if (settings?.notifications?.['page.updated'] !== false) { + eligible.set(u.id, u.name); + } + } + return eligible; } private async scheduleDigest( userId: string, workspaceId: string, ): Promise { - const jobId = `page-update-digest:${userId}`; await this.notificationQueue .add( QueueJob.PAGE_UPDATE_DIGEST, { userId, workspaceId }, - { jobId, delay: DIGEST_DELAY_MS }, + { delay: DIGEST_DELAY_MS, removeOnComplete: true }, ) .catch((err) => { this.logger.error( @@ -298,40 +302,102 @@ export class PageNotificationService { const notificationIds = await this.rateLimiter.popDigest(userId); if (notificationIds.length === 0) return; - const notifications = await this.db - .selectFrom('notifications') - .select(['id', 'pageId']) - .where('id', 'in', notificationIds) - .execute(); + const [user, notifications] = await Promise.all([ + this.db + .selectFrom('users') + .select(['id', 'name']) + .where('id', '=', userId) + .executeTakeFirst(), + this.db + .selectFrom('notifications') + .select(['id', 'pageId', 'actorId']) + .where('id', 'in', notificationIds) + .execute(), + ]); - if (notifications.length === 0) return; + if (!user || notifications.length === 0) return; - const pageIds = [...new Set(notifications.map((n) => n.pageId).filter(Boolean))]; + const pageIds = [ + ...new Set(notifications.map((n) => n.pageId).filter(Boolean)), + ]; + const actorIds = [ + ...new Set(notifications.map((n) => n.actorId).filter(Boolean)), + ]; - const pages = await this.db + const allPages = await this.db .selectFrom('pages') .innerJoin('spaces', 'spaces.id', 'pages.spaceId') .select([ 'pages.id', 'pages.title', 'pages.slugId', + 'pages.spaceId', 'spaces.slug as spaceSlug', ]) .where('pages.id', 'in', pageIds) .execute(); + if (allPages.length === 0) return; + + const spaceIds = [...new Set(allPages.map((p) => p.spaceId))]; + + const accessibleSpaceIds = new Set(); + for (const spaceId of spaceIds) { + const usersWithAccess = + await this.spaceMemberRepo.getUserIdsWithSpaceAccess([userId], spaceId); + if (usersWithAccess.has(userId)) accessibleSpaceIds.add(spaceId); + } + + const spaceFilteredPages = allPages.filter((p) => + accessibleSpaceIds.has(p.spaceId), + ); + if (spaceFilteredPages.length === 0) return; + + const accessiblePageIds = new Set(); + for (const p of spaceFilteredPages) { + const hasAccess = await this.pagePermissionRepo.getUserIdsWithPageAccess( + p.id, + [userId], + ); + if (hasAccess.includes(userId)) accessiblePageIds.add(p.id); + } + + const pages = spaceFilteredPages.filter((p) => accessiblePageIds.has(p.id)); + if (pages.length === 0) return; + + const actors = actorIds.length > 0 + ? await this.db + .selectFrom('users') + .select(['id', 'name']) + .where('id', 'in', actorIds) + .execute() + : []; + + const actorMap = new Map(actors.map((a) => [a.id, a.name])); + const pageActors = new Map>(); + for (const n of notifications) { + if (!n.pageId || !n.actorId) continue; + const names = pageActors.get(n.pageId) ?? new Set(); + const name = actorMap.get(n.actorId); + if (name) names.add(name); + pageActors.set(n.pageId, names); + } + const pageUpdates = pages.map((p) => ({ title: getPageTitle(p.title), url: `${appUrl}/s/${p.spaceSlug}/p/${p.slugId}`, + updatedBy: [...(pageActors.get(p.id) ?? [])], })); - if (pageUpdates.length === 0) return; - await this.notificationService.queueEmail( userId, notificationIds[0], - `${pageUpdates.length} pages were updated`, - PageUpdateDigestEmail({ pageUpdates }), + `Your digest: ${pageUpdates.length} page updates`, + PageUpdateDigestEmail({ + userName: user.name, + pageUpdates, + totalUpdates: pageUpdates.length, + }), NotificationType.PAGE_UPDATED, ); } diff --git a/apps/server/src/integrations/transactional/emails/page-update-digest-email.tsx b/apps/server/src/integrations/transactional/emails/page-update-digest-email.tsx index 72066b68..35f2d613 100644 --- a/apps/server/src/integrations/transactional/emails/page-update-digest-email.tsx +++ b/apps/server/src/integrations/transactional/emails/page-update-digest-email.tsx @@ -1,42 +1,76 @@ import { Link, Section, Text } from '@react-email/components'; import * as React from 'react'; import { content, link, paragraph } from '../css/styles'; -import { MailBody } from '../partials/partials'; +import { getGreetingName, MailBody } from '../partials/partials'; interface PageUpdate { title: string; url: string; + updatedBy: string[]; } interface Props { + userName: string; pageUpdates: PageUpdate[]; + totalUpdates: number; } -export const PageUpdateDigestEmail = ({ pageUpdates }: Props) => { +export const PageUpdateDigestEmail = ({ + userName, + pageUpdates, + totalUpdates, +}: Props) => { return (
- Hi there, - The following {pageUpdates.length} pages you watch were updated: + Hi {getGreetingName(userName)}, + + There {totalUpdates === 1 ? 'has' : 'have'} been{' '} + + {totalUpdates} update{totalUpdates === 1 ? '' : 's'} + {' '} + since your last update. + + {pageUpdates.map((page, i) => ( - - {'• '} - - {page.title} - - +
+ + + {page.title} + + + {page.updatedBy.length > 0 && ( + + {page.updatedBy.join(', ')} made edits + + )} +
))}
); }; -const listItem = { +const pageCard = { + borderLeft: '3px solid #e8e5ef', + paddingLeft: '12px', + marginBottom: '12px', +}; + +const pageTitle = { ...paragraph, - margin: '4px 0', - lineHeight: 1.4, + margin: '0 0 2px 0', + fontSize: 14, + fontWeight: 'bold' as const, +}; + +const updatedByText = { + ...paragraph, + margin: '0', + fontSize: 13, + color: '#666', }; export default PageUpdateDigestEmail; diff --git a/apps/server/src/integrations/transactional/emails/page-update-email.tsx b/apps/server/src/integrations/transactional/emails/page-update-email.tsx index 531f9dd0..188d8a34 100644 --- a/apps/server/src/integrations/transactional/emails/page-update-email.tsx +++ b/apps/server/src/integrations/transactional/emails/page-update-email.tsx @@ -1,15 +1,17 @@ import { Link, Section, Text } from '@react-email/components'; import * as React from 'react'; import { content, link, paragraph } from '../css/styles'; -import { EmailButton, MailBody } from '../partials/partials'; +import { EmailButton, getGreetingName, MailBody } from '../partials/partials'; interface Props { + userName: string; actorName: string; pageTitle: string; pageUrl: string; } export const PageUpdateEmail = ({ + userName, actorName, pageTitle, pageUrl, @@ -17,7 +19,7 @@ export const PageUpdateEmail = ({ return (
- Hi there, + Hi {getGreetingName(userName)}, {actorName} updated{' '} diff --git a/apps/server/src/integrations/transactional/partials/partials.tsx b/apps/server/src/integrations/transactional/partials/partials.tsx index f97eb989..c5b30e6b 100644 --- a/apps/server/src/integrations/transactional/partials/partials.tsx +++ b/apps/server/src/integrations/transactional/partials/partials.tsx @@ -87,3 +87,7 @@ export function MailFooter() {
); } + +export function getGreetingName(name?: string): string { + return name?.split(' ')[0] || 'there'; +}