From 8999631af86efa34cae82c4ad603391a80e2a27f Mon Sep 17 00:00:00 2001 From: Andy Ruiz Garramones Date: Fri, 11 Sep 2026 22:45:56 +0200 Subject: [PATCH 1/6] feat(inbox): add createdAt and changedFiles to pull request rows Both are scalars on a fragment the query already requests, so they add no cost, and createdAt was being fetched and thrown away. The digest needs them to sort rows by age and to show how big a review is. updatedAt can't do the sorting: it moves on every comment, so the most argued-about pull request would look like the newest. Co-Authored-By: Claude Opus 5 --- .../inbox/core/entities/inbox.interface.ts | 3 ++ backend/src/inbox/dto/inbox.response.ts | 6 ++++ .../src/inbox/github-inbox-data.service.ts | 35 +++++++++++++------ backend/src/inbox/inbox-classifier.ts | 11 +++--- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/backend/src/inbox/core/entities/inbox.interface.ts b/backend/src/inbox/core/entities/inbox.interface.ts index 5dcec29..e3cb7fe 100644 --- a/backend/src/inbox/core/entities/inbox.interface.ts +++ b/backend/src/inbox/core/entities/inbox.interface.ts @@ -60,6 +60,9 @@ export interface InboxPullRequest { title: string; url: string; isDraft: boolean; + /** ISO 8601, as GitHub gave it. When the pull request was opened, not when it last moved. */ + createdAt: string; + changedFiles: number; repositoryId: string; repositoryFullName: string; author: InboxAuthor; diff --git a/backend/src/inbox/dto/inbox.response.ts b/backend/src/inbox/dto/inbox.response.ts index f45f817..27ddea9 100644 --- a/backend/src/inbox/dto/inbox.response.ts +++ b/backend/src/inbox/dto/inbox.response.ts @@ -25,6 +25,12 @@ export class InboxPullRequestResponse { @ApiProperty() isDraft: boolean; + @ApiProperty({ description: 'ISO 8601. When the pull request was opened, not when it moved.' }) + createdAt: string; + + @ApiProperty({ description: 'How many files it touches.' }) + changedFiles: number; + @ApiProperty({ description: "GitHub's numeric repository id, as a string" }) repositoryId: string; diff --git a/backend/src/inbox/github-inbox-data.service.ts b/backend/src/inbox/github-inbox-data.service.ts index 41b70ea..c4ba71d 100644 --- a/backend/src/inbox/github-inbox-data.service.ts +++ b/backend/src/inbox/github-inbox-data.service.ts @@ -24,6 +24,12 @@ export interface GithubInboxPullRequest { * and it answers the question the ordering is actually asking - what moved most recently. */ updatedAt: string; + /** + * When it was opened, and the clock the digest ages rows by. Not `updatedAt`, which moves on + * every comment, so the row argued about hardest would read as the newest. + */ + createdAt: string; + changedFiles: number; repositoryId: string; repositoryFullName: string; authorLogin: string; @@ -91,17 +97,22 @@ export class GithubInboxDataService { let response: Response; try { - response = await githubFetch(this.metrics, 'graphql_inbox', 'https://api.github.com/graphql', { - method: 'POST', - headers: { - Authorization: `Bearer ${accessToken}`, - 'Content-Type': 'application/json', + response = await githubFetch( + this.metrics, + 'graphql_inbox', + 'https://api.github.com/graphql', + { + method: 'POST', + headers: { + Authorization: `Bearer ${accessToken}`, + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + query: INBOX_QUERY, + variables: { yours: MAX_YOURS, waiting: MAX_WAITING, threads: MAX_THREADS }, + }), }, - body: JSON.stringify({ - query: INBOX_QUERY, - variables: { yours: MAX_YOURS, waiting: MAX_WAITING, threads: MAX_THREADS }, - }), - }); + ); } catch (error) { this.logger.warn(`Could not reach GitHub for an inbox: ${error}`); return null; @@ -159,6 +170,8 @@ function normalizeSearch(search: any): GithubInboxPullRequest[] { url: node.url ?? '', isDraft: Boolean(node.isDraft), updatedAt: node.updatedAt ?? '', + createdAt: node.createdAt ?? '', + changedFiles: Number(node.changedFiles ?? 0), repositoryId: node.repository.id, repositoryFullName: node.repository.nameWithOwner, authorLogin: login, @@ -231,6 +244,8 @@ fragment Row on PullRequest { url isDraft updatedAt + createdAt + changedFiles reviewDecision repository { id nameWithOwner } author { __typename login avatarUrl } diff --git a/backend/src/inbox/inbox-classifier.ts b/backend/src/inbox/inbox-classifier.ts index 19c6d81..12579ca 100644 --- a/backend/src/inbox/inbox-classifier.ts +++ b/backend/src/inbox/inbox-classifier.ts @@ -1,7 +1,4 @@ -import { - GithubInboxPullRequest, - GithubInbox, -} from './github-inbox-data.service'; +import { GithubInboxPullRequest, GithubInbox } from './github-inbox-data.service'; import { InboxBuildFilters, InboxViewFilters, @@ -161,6 +158,8 @@ function toPullRequest(pullRequest: GithubInboxPullRequest): InboxPullRequest { title: pullRequest.title, url: pullRequest.url, isDraft: pullRequest.isDraft, + createdAt: pullRequest.createdAt, + changedFiles: pullRequest.changedFiles, repositoryId: pullRequest.repositoryId, repositoryFullName: pullRequest.repositoryFullName, author: { @@ -224,9 +223,7 @@ function group( keys: readonly InboxSectionKey[], sectionOf: (pullRequest: GithubInboxPullRequest) => InboxSectionKey, ): InboxSectionContent[] { - const buckets = new Map( - keys.map((key) => [key, []]), - ); + const buckets = new Map(keys.map((key) => [key, []])); for (const pullRequest of pullRequests) { buckets.get(sectionOf(pullRequest))?.push(pullRequest); From eb05ea621fae61667badbb73bab4c4c4895879da Mon Sep 17 00:00:00 2001 From: Andy Ruiz Garramones Date: Fri, 11 Sep 2026 22:45:56 +0200 Subject: [PATCH 2/6] feat(shared): add a local-day helper and extract the warmer's pool localMomentIn returns the day and hour in a given IANA zone, using Intl rather than arithmetic on a stored offset. A stored offset is wrong as soon as the clocks move, and the digest promises nine in the morning where the user is. pool is the inbox warmer's concurrency helper, moved to shared unchanged so the digest sweep can use it instead of copying it. Co-Authored-By: Claude Opus 5 --- .../src/inbox/warm/inbox-warmer.service.ts | 20 +----- backend/src/shared/async/pool.ts | 22 +++++++ backend/src/shared/time/local-day.ts | 63 +++++++++++++++++++ 3 files changed, 86 insertions(+), 19 deletions(-) create mode 100644 backend/src/shared/async/pool.ts create mode 100644 backend/src/shared/time/local-day.ts diff --git a/backend/src/inbox/warm/inbox-warmer.service.ts b/backend/src/inbox/warm/inbox-warmer.service.ts index 994e856..3d27b92 100644 --- a/backend/src/inbox/warm/inbox-warmer.service.ts +++ b/backend/src/inbox/warm/inbox-warmer.service.ts @@ -1,5 +1,6 @@ import { Injectable, Logger, OnApplicationBootstrap, OnModuleDestroy } from '@nestjs/common'; import { MetricsService } from '../../analytics/metrics.service'; +import { pool } from '../../shared/async/pool'; import { getEnvConfig } from '../../shared/configs/env-configs'; import { InboxWarmTarget, UserReadService } from '../../user/read/user-read.service'; import { buildFiltersOf } from '../core/entities/inbox-filters.interface'; @@ -198,25 +199,6 @@ export class InboxWarmerService implements OnApplicationBootstrap, OnModuleDestr } } -/** - * Runs `work` over `items`, at most `limit` at a time. - * - * Workers pulling from a shared cursor rather than fixed slices, so one slow user delays the - * next item and not a whole quarter of the list. Never rejects: `warm` handles its own - * failures, and a pool that threw would take the sweep down with it. - */ -async function pool(items: T[], limit: number, work: (item: T) => Promise): Promise { - let next = 0; - - const workers = Array.from({ length: Math.min(limit, items.length) }, async () => { - for (let index = next++; index < items.length; index = next++) { - await work(items[index]); - } - }); - - await Promise.all(workers); -} - function describe(error: unknown): string { return error instanceof Error ? error.message : String(error); } diff --git a/backend/src/shared/async/pool.ts b/backend/src/shared/async/pool.ts new file mode 100644 index 0000000..e09275c --- /dev/null +++ b/backend/src/shared/async/pool.ts @@ -0,0 +1,22 @@ +/** + * Runs `work` over `items`, at most `limit` at a time. + * + * Workers pulling from a shared cursor rather than fixed slices, so one slow item delays the next + * one and not a whole quarter of the list. Never rejects: callers handle their own failures, and + * a pool that threw would take its caller down with it. + */ +export async function pool( + items: T[], + limit: number, + work: (item: T) => Promise, +): Promise { + let next = 0; + + const workers = Array.from({ length: Math.min(limit, items.length) }, async () => { + for (let index = next++; index < items.length; index = next++) { + await work(items[index]); + } + }); + + await Promise.all(workers); +} diff --git a/backend/src/shared/time/local-day.ts b/backend/src/shared/time/local-day.ts new file mode 100644 index 0000000..5553984 --- /dev/null +++ b/backend/src/shared/time/local-day.ts @@ -0,0 +1,63 @@ +/** + * What day and hour it is where somebody is. + * + * Through Intl rather than arithmetic on a stored offset, which is only true until the clocks + * move. + */ + +export interface LocalMoment { + /** `YYYY-MM-DD` in the zone. */ + day: string; + hour: number; +} + +/** Constructing one loads zone data, and the sweep asks for the same few zones all day. */ +const FORMATTERS = new Map(); + +export function isTimezone(value: unknown): value is string { + if (typeof value !== 'string' || value.length === 0) { + return false; + } + + try { + formatterFor(value); + + return true; + } catch { + return false; + } +} + +export function localMomentIn(timezone: string, at: Date): LocalMoment { + const parts = formatterFor(timezone).formatToParts(at); + const read = (type: Intl.DateTimeFormatPartTypes): string => + parts.find((part) => part.type === type)?.value ?? ''; + + return { + day: `${read('year')}-${read('month')}-${read('day')}`, + hour: Number(read('hour')), + }; +} + +function formatterFor(timezone: string): Intl.DateTimeFormat { + const existing = FORMATTERS.get(timezone); + + if (existing) { + return existing; + } + + // Throws RangeError on an unknown zone, which is what isTimezone reads. + const formatter = new Intl.DateTimeFormat('en-GB', { + timeZone: timezone, + year: 'numeric', + month: '2-digit', + day: '2-digit', + hour: '2-digit', + // `hourCycle` rather than `hour12: false`, which renders midnight as 24 on some runtimes. + hourCycle: 'h23', + }); + + FORMATTERS.set(timezone, formatter); + + return formatter; +} From b98c5f182d78d1a5245214d0ecb7f5c83fdcb57e Mon Sep 17 00:00:00 2001 From: Andy Ruiz Garramones Date: Fri, 11 Sep 2026 22:45:57 +0200 Subject: [PATCH 3/6] feat(settings): add a daily digest switch, hour and timezone Off by default. Every other poke answers a webhook the user already asked to hear about, while this one is a new scheduled message. The settings write sets each field rather than replacing the whole pokeSettings subdocument, so an older client saving the fields it knows can't wipe a digest schedule it has never heard of. Mutes keep the whole-set replacement, which is how unmuting is expressed. claimDigest reserves one digest per user per local day in a single conditional write, so two sweeps or two replicas can't both send. It stores a local date rather than an instant, so the rule holds across daylight saving and half-hour zones. Enabling the digest marks today as already sent, but only if the chosen hour has passed. Turning it on at 16:00 with an hour of 09:00 won't fire minutes later; turning it on at 08:00 still gets today's at 09:00. Co-Authored-By: Claude Opus 5 --- .../src/notifications/core/poke-settings.ts | 36 +++++++- .../dto/poke-settings.response.ts | 19 ++++ .../dto/update-poke-settings.body.ts | 35 +++++++- .../settings/poke-settings.controller.ts | 19 ++-- backend/src/user/core/entities/user.entity.ts | 22 +++++ backend/src/user/read/user-read.service.ts | 48 ++++++++++ backend/src/user/write/user-write.service.ts | 89 +++++++++++++++++-- .../test/notifications/poke-settings.spec.ts | 6 ++ 8 files changed, 260 insertions(+), 14 deletions(-) diff --git a/backend/src/notifications/core/poke-settings.ts b/backend/src/notifications/core/poke-settings.ts index a257a1f..779e464 100644 --- a/backend/src/notifications/core/poke-settings.ts +++ b/backend/src/notifications/core/poke-settings.ts @@ -53,14 +53,42 @@ export function isReviewRequestResolution(value: unknown): value is ReviewReques export interface PokeSettings { mutedTypes: NotificationType[]; reviewRequestResolution: ReviewRequestResolution; + digestEnabled: boolean; + /** Nought to twenty-three, read in the timezone on the user row. */ + digestHour: number; } -/** Opting in is already an explicit act; the useful default afterwards is everything. */ +/** + * Opting in is already an explicit act; the useful default afterwards is everything. + * + * Except the digest, which is a message on a schedule rather than an answer to a webhook, and so + * is asked for rather than assumed. + */ export const DEFAULT_POKE_SETTINGS: PokeSettings = { mutedTypes: [], reviewRequestResolution: 'any_review', + digestEnabled: false, + digestHour: 9, }; +export function isDigestHour(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value >= 0 && value <= 23; +} + +/** + * A save, which is the whole set for the mutes and only what it mentions for the digest. + * + * Absence means different things: a type left out of `mutedTypes` is how unmuting is spelled, + * while a body without the digest fields is a client that predates them, and a stale tab must + * not be able to undo a schedule it has never heard of. + */ +export interface PokeSettingsUpdate { + mutedTypes: NotificationType[]; + reviewRequestResolution: ReviewRequestResolution; + digestEnabled?: boolean; + digestHour?: number; +} + /** * The settings as they sit on the user row: absent for anybody who has never moved a switch, * and plain strings rather than members of the enum. @@ -72,6 +100,8 @@ export const DEFAULT_POKE_SETTINGS: PokeSettings = { export interface PokeStoredSettings { mutedTypes?: string[]; reviewRequestResolution?: string; + digestEnabled?: boolean; + digestHour?: number; } /** @@ -96,6 +126,10 @@ export function normalizePokeSettings(stored: PokeStoredSettings | null | undefi reviewRequestResolution: isReviewRequestResolution(resolution) ? resolution : DEFAULT_POKE_SETTINGS.reviewRequestResolution, + digestEnabled: stored?.digestEnabled === true, + digestHour: isDigestHour(stored?.digestHour) + ? stored.digestHour + : DEFAULT_POKE_SETTINGS.digestHour, }; } diff --git a/backend/src/notifications/dto/poke-settings.response.ts b/backend/src/notifications/dto/poke-settings.response.ts index 3755adf..64a4d3f 100644 --- a/backend/src/notifications/dto/poke-settings.response.ts +++ b/backend/src/notifications/dto/poke-settings.response.ts @@ -36,4 +36,23 @@ export class PokeSettingsResponse implements PokeSettings { 'An account that has never touched the settings answers `any_review`.', }) reviewRequestResolution: ReviewRequestResolution; + + @ApiProperty({ + description: + 'Whether this user gets a daily digest of the pull requests still waiting on their ' + + 'review. Off for an account that has never touched the settings: unlike every other ' + + 'kind here it is a message on a schedule rather than an answer to a webhook, so it is ' + + 'asked for rather than assumed.', + }) + digestEnabled: boolean; + + @ApiProperty({ + minimum: 0, + maximum: 23, + description: + "The hour the digest is sent, read in this user's own timezone. Nine for an account that " + + 'has never touched the settings. A digest is sent at most once a local day, so changing ' + + "this after today's has gone takes effect tomorrow.", + }) + digestHour: number; } diff --git a/backend/src/notifications/dto/update-poke-settings.body.ts b/backend/src/notifications/dto/update-poke-settings.body.ts index 094880f..00b493c 100644 --- a/backend/src/notifications/dto/update-poke-settings.body.ts +++ b/backend/src/notifications/dto/update-poke-settings.body.ts @@ -1,5 +1,16 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; -import { ArrayMaxSize, IsArray, IsEnum, IsIn, IsOptional } from 'class-validator'; +import { + ArrayMaxSize, + IsArray, + IsBoolean, + IsEnum, + IsIn, + IsInt, + IsOptional, + IsTimeZone, + Max, + Min, +} from 'class-validator'; import { ALL_NOTIFICATION_TYPES, NotificationType } from '../core/entities/notification-type.enum'; import { REVIEW_REQUEST_RESOLUTIONS, ReviewRequestResolution } from '../core/poke-settings'; @@ -29,4 +40,26 @@ export class UpdatePokeSettingsBody { @IsOptional() @IsIn(REVIEW_REQUEST_RESOLUTIONS) reviewRequestResolution?: ReviewRequestResolution; + + @ApiPropertyOptional() + @IsOptional() + @IsBoolean() + digestEnabled?: boolean; + + @ApiPropertyOptional({ minimum: 0, maximum: 23 }) + @IsOptional() + @IsInt() + @Min(0) + @Max(23) + digestHour?: number; + + /** + * The reader's IANA zone, which only the browser knows, sent with every save so somebody who + * moves is followed. Rejected rather than dropped when unknown: a digest stored against a zone + * nothing can read is one that silently never arrives. + */ + @ApiPropertyOptional() + @IsOptional() + @IsTimeZone() + timezone?: string; } diff --git a/backend/src/notifications/settings/poke-settings.controller.ts b/backend/src/notifications/settings/poke-settings.controller.ts index 5c58c20..ff68f67 100644 --- a/backend/src/notifications/settings/poke-settings.controller.ts +++ b/backend/src/notifications/settings/poke-settings.controller.ts @@ -38,11 +38,18 @@ export class PokeSettingsController { @CurrentUserId() userId: string, @Body() body: UpdatePokeSettingsBody, ): Promise { - const settings = await this.userWriteService.updatePokeSettings(userId, { - mutedTypes: body.mutedTypes, - reviewRequestResolution: - body.reviewRequestResolution ?? DEFAULT_POKE_SETTINGS.reviewRequestResolution, - }); + const settings = await this.userWriteService.updatePokeSettings( + userId, + { + mutedTypes: body.mutedTypes, + reviewRequestResolution: + body.reviewRequestResolution ?? DEFAULT_POKE_SETTINGS.reviewRequestResolution, + // Absences passed through: a client that has not heard of the digest cannot reset it. + digestEnabled: body.digestEnabled, + digestHour: body.digestHour, + }, + body.timezone, + ); // The names go in whole, unlike the inbox's team and author lists: these are our own closed // set rather than somebody else's data, and which kinds people actually switch off is the @@ -52,6 +59,8 @@ export class PokeSettingsController { muted_types: settings.mutedTypes, muted_count: settings.mutedTypes.length, review_request_resolution: settings.reviewRequestResolution, + digest_enabled: settings.digestEnabled, + digest_hour: settings.digestHour, }); return settings; diff --git a/backend/src/user/core/entities/user.entity.ts b/backend/src/user/core/entities/user.entity.ts index 16fe75c..aa6179d 100644 --- a/backend/src/user/core/entities/user.entity.ts +++ b/backend/src/user/core/entities/user.entity.ts @@ -55,6 +55,14 @@ export class PokeSettingsEntity implements PokeStoredSettings { // existed, which reads as the default - and a value this deploy cannot spell reads as it too. @Prop() reviewRequestResolution?: string; + + @Prop() + digestEnabled?: boolean; + + // An hour rather than a time: a stored "09:00+02:00" is wrong twice a year, and again when + // they move. + @Prop() + digestHour?: number; } export const PokeSettingsSchema = SchemaFactory.createForClass(PokeSettingsEntity); @@ -132,6 +140,16 @@ export class UserEntity { @Prop({ type: Date }) inboxLastUsedAt?: Date; + // An IANA zone the browser resolved: GitHub exposes none and nothing here implies one. Only + // the digest reads it, so most rows never have one. + @Prop() + timezone?: string; + + // The last local date a digest was claimed for, `YYYY-MM-DD` in this user's own zone. A date + // rather than an instant, so the once-a-day rule survives daylight saving and half-hour zones. + @Prop() + digestSentOn?: string; + @Prop() createdAt: Date; @@ -146,3 +164,7 @@ export const UserSchema = SchemaFactory.createForClass(UserEntity); // The warmer's one query is a range over this, every five minutes, across every user there is. // Sparse because most rows never get the field, and a row without it can never match a `$gte`. UserSchema.index({ inboxLastUsedAt: 1 }, { sparse: true }); + +// The digest sweep asks for everybody who has turned it on, several times an hour. Sparse for +// the same reason: absent on most rows, and `true` is the only value worth finding. +UserSchema.index({ 'pokeSettings.digestEnabled': 1 }, { sparse: true }); diff --git a/backend/src/user/read/user-read.service.ts b/backend/src/user/read/user-read.service.ts index cbef566..5132b52 100644 --- a/backend/src/user/read/user-read.service.ts +++ b/backend/src/user/read/user-read.service.ts @@ -5,7 +5,9 @@ import { InboxFilters, normalizeInboxSettings, } from '../../inbox/core/entities/inbox-filters.interface'; +import { normalizePokeSettings } from '../../notifications/core/poke-settings'; import { TokenCipherService } from '../../shared/crypto/token-cipher.service'; +import { isTimezone } from '../../shared/time/local-day'; import { UserEntity } from '../core/entities/user.entity'; import { UserNormalized } from '../core/entities/user.interface'; import { UserSerializer } from '../core/entities/user.serializer'; @@ -16,6 +18,17 @@ export interface InboxWarmTarget { settings: InboxFilters; } +/** One person who has asked for a digest, and what it takes to decide whether one is due. */ +export interface DigestTarget { + userId: string; + timezone: string; + hour: number; + /** The local day their last digest was claimed for. The claim still decides; this saves a write. */ + sentOn?: string; + /** Their inbox settings, so the digest lists what their inbox would. */ + settings: InboxFilters; +} + @Injectable() export class UserReadService { constructor( @@ -95,6 +108,41 @@ export class UserReadService { })); } + /** + * Everybody who has turned the digest on and still holds a token. Whose hour has come is the + * sweep's question, not Mongo's - it cannot evaluate a timezone. Projected for the reason the + * warmer's targets are: normalising would decrypt every stored token to answer it. + */ + public async readDigestTargets(): Promise { + const users = await this.userModel + .find({ + 'pokeSettings.digestEnabled': true, + githubAccessToken: { $exists: true, $ne: null }, + }) + .select({ _id: 1, pokeSettings: 1, timezone: 1, digestSentOn: 1, inboxSettings: 1 }) + .lean< + Pick[] + >() + .exec(); + + return users.flatMap((user) => { + // Without a zone there is no hour to be due at. + if (!isTimezone(user.timezone)) { + return []; + } + + return [ + { + userId: user._id.toString(), + timezone: user.timezone, + hour: normalizePokeSettings(user.pokeSettings).digestHour, + sentOn: user.digestSentOn, + settings: normalizeInboxSettings(user.inboxSettings), + }, + ]; + }); + } + private normalize(user: UserEntity): UserNormalized { return UserSerializer.normalize(user, (value) => this.tokenCipher.decrypt(value)); } diff --git a/backend/src/user/write/user-write.service.ts b/backend/src/user/write/user-write.service.ts index 9268bdb..b0bb89a 100644 --- a/backend/src/user/write/user-write.service.ts +++ b/backend/src/user/write/user-write.service.ts @@ -5,8 +5,13 @@ import { InboxFilters, normalizeInboxSettings, } from '../../inbox/core/entities/inbox-filters.interface'; -import { PokeSettings, normalizePokeSettings } from '../../notifications/core/poke-settings'; +import { + PokeSettings, + PokeSettingsUpdate, + normalizePokeSettings, +} from '../../notifications/core/poke-settings'; import { TokenCipherService } from '../../shared/crypto/token-cipher.service'; +import { isTimezone, localMomentIn } from '../../shared/time/local-day'; import { UserEntity } from '../core/entities/user.entity'; import { UserNormalized } from '../core/entities/user.interface'; import { UserSerializer } from '../core/entities/user.serializer'; @@ -150,21 +155,91 @@ export class UserWriteService { * as the inbox settings above. Unmuting is spelled by sending a set without that type in it, * so a merge would make it unspellable. */ - public async updatePokeSettings(userId: string, settings: PokeSettings): Promise { - const user = await this.userModel + public async updatePokeSettings( + userId: string, + settings: PokeSettingsUpdate, + timezone?: string, + now: Date = new Date(), + ): Promise { + // Field by field, not one subdocument: a body that says nothing about the digest leaves it. + const previous = await this.userModel .findOneAndUpdate( { _id: new Types.ObjectId(userId) }, - { $set: { pokeSettings: settings } }, - { returnDocument: 'after' }, + { + $set: { + 'pokeSettings.mutedTypes': settings.mutedTypes, + 'pokeSettings.reviewRequestResolution': settings.reviewRequestResolution, + ...(settings.digestEnabled === undefined + ? {} + : { 'pokeSettings.digestEnabled': settings.digestEnabled }), + ...(settings.digestHour === undefined + ? {} + : { 'pokeSettings.digestHour': settings.digestHour }), + ...(timezone ? { timezone } : {}), + }, + }, + { returnDocument: 'before' }, ) .lean() .exec(); - if (!user) { + if (!previous) { throw new NotFoundException('User not found'); } - return normalizePokeSettings(user.pokeSettings); + const before = normalizePokeSettings(previous.pokeSettings); + const after: PokeSettings = { + mutedTypes: settings.mutedTypes, + reviewRequestResolution: settings.reviewRequestResolution, + digestEnabled: settings.digestEnabled ?? before.digestEnabled, + digestHour: settings.digestHour ?? before.digestHour, + }; + + // Also when there is no stamp at all: enabling from a client that sent no timezone claimed + // nothing, and the save that later supplies one must not fire a digest minutes afterwards. + if (after.digestEnabled && (!before.digestEnabled || !previous.digestSentOn)) { + await this.claimDigestOnEnable(userId, after.digestHour, timezone ?? previous.timezone, now); + } + + return after; + } + + /** + * Takes one digest per person per day, and answers whether this caller got it. + * + * One conditional write rather than a read and a write, so two passes - or two replicas - + * cannot both find the day unclaimed and both send. + */ + public async claimDigest(userId: string, localDay: string): Promise { + const result = await this.userModel.updateOne( + { _id: new Types.ObjectId(userId), digestSentOn: { $ne: localDay } }, + { $set: { digestSentOn: localDay } }, + ); + + return result.modifiedCount === 1; + } + + /** Spends today only where their hour has been, so turning it on at four sends nothing at four. */ + private async claimDigestOnEnable( + userId: string, + hour: number, + timezone: string | undefined, + now: Date, + ): Promise { + if (!isTimezone(timezone)) { + return; + } + + const moment = localMomentIn(timezone, now); + + if (moment.hour < hour) { + return; + } + + await this.userModel.updateOne( + { _id: new Types.ObjectId(userId) }, + { $set: { digestSentOn: moment.day } }, + ); } /** diff --git a/backend/test/notifications/poke-settings.spec.ts b/backend/test/notifications/poke-settings.spec.ts index 9e04878..97043a5 100644 --- a/backend/test/notifications/poke-settings.spec.ts +++ b/backend/test/notifications/poke-settings.spec.ts @@ -53,6 +53,8 @@ describe('Poke settings', () => { expect(response.body.pokeSettings).toEqual({ mutedTypes: [], reviewRequestResolution: 'any_review', + digestEnabled: false, + digestHour: 9, }); }); @@ -169,6 +171,8 @@ describe('Poke settings', () => { expect(response.body).toEqual({ mutedTypes: [NotificationType.IssueComment], reviewRequestResolution: 'any_review', + digestEnabled: false, + digestHour: 9, }); }); @@ -183,6 +187,8 @@ describe('Poke settings', () => { expect(response.body).toEqual({ mutedTypes: [NotificationType.IssueMention], reviewRequestResolution: 'strict', + digestEnabled: false, + digestHour: 9, }); }); From 3e548fd435a86ca814c9dc819127e037652b6a23 Mon Sep 17 00:00:00 2001 From: Andy Ruiz Garramones Date: Fri, 11 Sep 2026 22:45:57 +0200 Subject: [PATCH 4/6] feat(slack): render the digest message One message: a heading, then the rows under it oldest first. Delivery is a fourth wrapper alongside the test and welcome messages, since the private send() already resolves the workspace, opens and caches the DM, and records failures. No avatars. Slack fetches every image while posting and rejects the whole message if one fails. A poke carries a single avatar; a digest would carry one per row. Co-Authored-By: Claude Opus 5 --- backend/src/analytics/analytics-events.ts | 2 +- .../notifications/delivery/slack-message.ts | 131 ++++++++++++++++++ .../slack-notification-delivery.service.ts | 25 +++- 3 files changed, 156 insertions(+), 2 deletions(-) diff --git a/backend/src/analytics/analytics-events.ts b/backend/src/analytics/analytics-events.ts index 94fe5cb..77e8f41 100644 --- a/backend/src/analytics/analytics-events.ts +++ b/backend/src/analytics/analytics-events.ts @@ -83,4 +83,4 @@ export type AnalyticsEvent = * the dashboard's test button and the message that proves a fresh connection works both go out * the same pipe, and both are worth telling apart from a real poke rather than hiding. */ -export type PokeTrigger = 'github_webhook' | 'test' | 'welcome'; +export type PokeTrigger = 'github_webhook' | 'test' | 'welcome' | 'digest'; diff --git a/backend/src/notifications/delivery/slack-message.ts b/backend/src/notifications/delivery/slack-message.ts index fcc0bf7..4bafdf1 100644 --- a/backend/src/notifications/delivery/slack-message.ts +++ b/backend/src/notifications/delivery/slack-message.ts @@ -409,6 +409,137 @@ export function buildTestMessage(githubLogin?: string): SlackMessage { }; } +export interface DigestPullRequest { + number: number; + title: string; + url: string; + repositoryFullName: string; + authorLogin: string; + /** ISO 8601, as GitHub gave it. When it was opened. */ + createdAt: string; + changedFiles: number; +} + +/** Past twenty, a count of what is left is more use than more rows. */ +const MAX_DIGEST_ROWS = 20; + +/** Slack rejects a section over 3,000 characters. */ +const MAX_SECTION_CHARS = 2800; + +const MAX_DIGEST_TITLE_CHARS = 80; + +/** + * The day's list of what is waiting on somebody, oldest first. + * + * No avatars: Slack fetches every image while posting and rejects the whole message if one + * fails, and a digest would carry one per row. + */ +export function buildDigestMessage(pullRequests: DigestPullRequest[], now: Date): SlackMessage { + const shown = pullRequests.slice(0, MAX_DIGEST_ROWS); + const hidden = pullRequests.length - shown.length; + const heading = + pullRequests.length === 1 + ? '*1 pull request is waiting on your review.*' + : `*${pullRequests.length} pull requests are waiting on your review.*`; + + const blocks: unknown[] = [{ type: 'section', text: { type: 'mrkdwn', text: heading } }]; + + for (const chunk of chunkLines(shown.map((pullRequest) => digestLine(pullRequest, now)))) { + blocks.push({ type: 'section', text: { type: 'mrkdwn', text: chunk } }); + } + + if (hidden > 0) { + blocks.push({ + type: 'context', + elements: [ + { + type: 'mrkdwn', + text: `and ${hidden} more waiting on you.`, + }, + ], + }); + } + + return { text: digestFallback(pullRequests), blocks }; +} + +/** All somebody sees in the notification preview, so it has to carry the point alone. */ +function digestFallback(pullRequests: DigestPullRequest[]): string { + const [first] = pullRequests; + + if (pullRequests.length === 1) { + return `#${first.number} ${first.title} is waiting on your review.`; + } + + return `${pullRequests.length} pull requests are waiting on your review.`; +} + +function digestLine(pullRequest: DigestPullRequest, now: Date): string { + const title = clamp(escape(pullRequest.title) || 'Untitled', MAX_DIGEST_TITLE_CHARS); + const facts = [ + age(pullRequest.createdAt, now), + files(pullRequest.changedFiles), + `\`${escape(pullRequest.repositoryFullName)}\``, + handleLink(pullRequest.authorLogin), + ]; + + return `• ${link(pullRequest.url, `#${pullRequest.number} ${title}`)} · ${facts.join(' · ')}`; +} + +/** The largest unit that still fits, rounded down, so nothing reads as older than it is. */ +function age(createdAt: string, now: Date): string { + const opened = Date.parse(createdAt); + + if (Number.isNaN(opened)) { + return 'age unknown'; + } + + const hours = Math.floor((now.getTime() - opened) / (60 * 60_000)); + + if (hours < 1) { + return 'just opened'; + } + + if (hours < 24) { + return hours === 1 ? '1 hour old' : `${hours} hours old`; + } + + const days = Math.floor(hours / 24); + + return days === 1 ? '1 day old' : `${days} days old`; +} + +function files(changed: number): string { + return changed === 1 ? '1 file' : `${changed} files`; +} + +function chunkLines(lines: string[]): string[] { + const chunks: string[] = []; + let current = ''; + + for (const line of lines) { + const candidate = current ? `${current}\n${line}` : line; + + if (candidate.length > MAX_SECTION_CHARS && current) { + chunks.push(current); + current = line; + continue; + } + + current = candidate; + } + + if (current) { + chunks.push(current); + } + + return chunks; +} + +function clamp(text: string, limit: number): string { + return text.length <= limit ? text : `${text.slice(0, limit - 1).trimEnd()}…`; +} + /** * What the link says: the title, then the number people actually use to refer to it. The * number alone is unreadable and the title alone is unsearchable. diff --git a/backend/src/notifications/delivery/slack-notification-delivery.service.ts b/backend/src/notifications/delivery/slack-notification-delivery.service.ts index 6264b6b..3d53cc7 100644 --- a/backend/src/notifications/delivery/slack-notification-delivery.service.ts +++ b/backend/src/notifications/delivery/slack-notification-delivery.service.ts @@ -19,7 +19,13 @@ import { UserNormalized } from '../../user/core/entities/user.interface'; import { GithubNotificationNormalized } from '../core/entities/github-notification.interface'; import { NotificationType } from '../core/entities/notification-type.enum'; import { PokeMessageWriteService } from '../messages/write/poke-message-write.service'; -import { buildPokeMessage, buildTestMessage, buildWelcomeMessage } from './slack-message'; +import { + buildDigestMessage, + buildPokeMessage, + buildTestMessage, + buildWelcomeMessage, + DigestPullRequest, +} from './slack-message'; /** * Why a poke did not reach Slack. Only `sent` and `failed` are unusual; the rest are ordinary @@ -191,6 +197,23 @@ export class SlackNotificationDeliveryService { return outcome; } + /** + * The day's list, from the sweep rather than anything that just happened. Not remembered the + * way a poke is: a digest is a list, with nothing in it to strike through later. + */ + public async deliverDigest( + userId: string, + pullRequests: DigestPullRequest[], + now: Date, + ): Promise { + const { outcome } = await this.send(userId, buildDigestMessage(pullRequests, now), { + trigger: 'digest', + pokeType: 'digest', + }); + + return outcome; + } + private async send( userId: string, message: SlackMessage, From b9ec954c93d989eab45fce0e0bcacc45059fab6f Mon Sep 17 00:00:00 2001 From: Andy Ruiz Garramones Date: Fri, 11 Sep 2026 22:45:57 +0200 Subject: [PATCH 5/6] feat(digest): add the sweep that sends the daily digest A timer shaped like the inbox warmer: bootstrap hook, setInterval, and 0 to disable it, which is what the test suite runs with. sweep takes the instant to run at, so a spec can cover two timezones at once. A pass sends to anyone whose hour has passed, not only those whose hour it is right now, so a deploy across 09:00 costs minutes rather than the whole day. The day is claimed after the list is built, not before. Building only reads, so two passes both building costs one GitHub query each, while claiming first would let a single 502 burn the day. An empty list still claims the day, or the rest of it costs an inbox rebuild every sweep. The list goes through groupWaitingOnYou rather than reading the snapshot directly, because author filters are applied on the way out. Reading the rows directly would list authors the user has told the inbox to ignore. Co-Authored-By: Claude Opus 5 --- backend/.env.example | 10 + backend/src/analytics/metrics-catalog.ts | 24 +- backend/src/app.module.ts | 3 + .../src/notifications/digest/digest.module.ts | 19 + .../notifications/digest/digest.service.ts | 180 +++++ backend/src/shared/configs/env-configs.ts | 9 + backend/test/notifications/digest.spec.ts | 620 ++++++++++++++++++ backend/test/utils/bootstrap.ts | 8 + 8 files changed, 872 insertions(+), 1 deletion(-) create mode 100644 backend/src/notifications/digest/digest.module.ts create mode 100644 backend/src/notifications/digest/digest.service.ts create mode 100644 backend/test/notifications/digest.spec.ts diff --git a/backend/.env.example b/backend/.env.example index 1d5f860..f48365b 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -104,3 +104,13 @@ POSTHOG_HOST=https://eu.i.posthog.com # 0 turns the sweep off entirely without a release, which is also what the e2e suite runs with: # a timer reaching for GitHub in the background would make every spec's mocks a race. INBOX_WARM_SWEEP_INTERVAL_MS=300000 + +# --- Daily digest --- +# How often the sweep looks for somebody whose chosen hour has come, in milliseconds. Not how +# often anybody is messaged: one digest per person per local day is settled by a conditional +# write whatever this is set to, so it only decides how soon after their hour they hear. +# +# A quarter hour is the coarsest that still honours zones offset by half and three-quarter hours. +# 0 turns it off entirely without a release, which is what the e2e suite runs with - and more so +# than the sweep above, because this timer posts to Slack rather than only reading. +DIGEST_SWEEP_INTERVAL_MS=900000 diff --git a/backend/src/analytics/metrics-catalog.ts b/backend/src/analytics/metrics-catalog.ts index 4bbf6af..ba02f79 100644 --- a/backend/src/analytics/metrics-catalog.ts +++ b/backend/src/analytics/metrics-catalog.ts @@ -47,7 +47,9 @@ export type CounterName = | 'proke.poke.delivered' | 'proke.cache.lookups' | 'proke.inbox.warmed' - | 'proke.inbox.warm.sweeps'; + | 'proke.inbox.warm.sweeps' + | 'proke.digest.sent' + | 'proke.digest.sweeps'; /** Gauges. A value that moves both ways, read at a moment. */ export type GaugeName = 'proke.event_loop.delay'; @@ -61,6 +63,7 @@ export type GaugeName = 'proke.event_loop.delay'; */ export type HistogramName = | 'proke.inbox.warm.duration' + | 'proke.digest.duration' | 'proke.webhook.duration' | 'proke.poke.latency' | 'proke.github.request.duration' @@ -151,6 +154,22 @@ export type WarmOutcome = 'refreshed' | 'no_token' | 'github_unavailable' | 'fai */ export type WarmSweepOutcome = 'completed' | 'overlapped' | 'failed'; +/** + * What one person's digest came to. + * + * `empty` is the healthy majority rather than a failure, and kept apart from `sent` because the + * ratio between them says whether the digest is worth sending at all. `claimed_already` on a + * single-instance deploy would mean the claim is not doing its job. + */ +export type DigestOutcome = + | 'sent' + | 'empty' + | 'claimed_already' + | 'no_token' + | 'github_unavailable' + | 'undeliverable' + | 'failed'; + /** * Which GitHub call this was. Hand-written labels rather than URLs, which carry ids. * @@ -250,6 +269,9 @@ export interface MetricAttributeMap { // Undimensioned: there is one sweep, and splitting it by anything would only make the series // smaller without making it answer a different question. 'proke.inbox.warm.duration': Record; + 'proke.digest.sent': { outcome: DigestOutcome }; + 'proke.digest.sweeps': { outcome: WarmSweepOutcome }; + 'proke.digest.duration': Record; 'proke.event_loop.delay': { quantile: EventLoopQuantile }; 'http.server.duration': { route: string; method: string; status: string }; } diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 4721ccc..948f7f6 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -5,6 +5,7 @@ import { AuthCoreModule } from './auth/core/auth-core.module'; import { ConnectionsModule } from './connections/connections.module'; import { InboxModule } from './inbox/inbox.module'; import { InboxWarmModule } from './inbox/warm/inbox-warm.module'; +import { DigestModule } from './notifications/digest/digest.module'; import { PokeSettingsModule } from './notifications/settings/poke-settings.module'; import { getEnvConfig } from './shared/configs/env-configs'; import { HttpMetricsModule } from './shared/http/http-metrics.middleware'; @@ -30,6 +31,8 @@ import { SlackEventsModule } from './webhooks/slack/slack-events.module'; // What kinds of poke somebody wants, account-wide. The delivery side of notifications is // wired in under the webhook module; this is only the settings route. PokeSettingsModule, + // The other scheduler: the daily list of what is still waiting on somebody. + DigestModule, SlackModule, GithubWebhookModule, SlackEventsModule, diff --git a/backend/src/notifications/digest/digest.module.ts b/backend/src/notifications/digest/digest.module.ts new file mode 100644 index 0000000..853661c --- /dev/null +++ b/backend/src/notifications/digest/digest.module.ts @@ -0,0 +1,19 @@ +import { Module } from '@nestjs/common'; +import { InboxModule } from '../../inbox/inbox.module'; +import { UserReadModule } from '../../user/read/user-read.module'; +import { UserWriteModule } from '../../user/write/user-write.module'; +import { NotificationsCoreModule } from '../core/notifications-core.module'; +import { DigestService } from './digest.service'; + +/** + * The timer that sends the daily digest. + * + * Above the modules it uses, like InboxWarmModule: neither the inbox that builds the list nor + * the delivery that sends it should know about a schedule. + */ +@Module({ + imports: [InboxModule, UserReadModule, UserWriteModule, NotificationsCoreModule], + providers: [DigestService], + exports: [DigestService], +}) +export class DigestModule {} diff --git a/backend/src/notifications/digest/digest.service.ts b/backend/src/notifications/digest/digest.service.ts new file mode 100644 index 0000000..325f2ee --- /dev/null +++ b/backend/src/notifications/digest/digest.service.ts @@ -0,0 +1,180 @@ +import { Injectable, Logger, OnApplicationBootstrap, OnModuleDestroy } from '@nestjs/common'; +import { MetricsService } from '../../analytics/metrics.service'; +import { buildFiltersOf } from '../../inbox/core/entities/inbox-filters.interface'; +import { InboxPullRequest } from '../../inbox/core/entities/inbox.interface'; +import { groupWaitingOnYou } from '../../inbox/inbox-classifier'; +import { InboxRefreshService } from '../../inbox/inbox-refresh.service'; +import { pool } from '../../shared/async/pool'; +import { getEnvConfig } from '../../shared/configs/env-configs'; +import { localMomentIn } from '../../shared/time/local-day'; +import { DigestTarget, UserReadService } from '../../user/read/user-read.service'; +import { UserWriteService } from '../../user/write/user-write.service'; +import { SlackNotificationDeliveryService } from '../delivery/slack-notification-delivery.service'; +import { DigestPullRequest } from '../delivery/slack-message'; + +/** Without it, a deploy during somebody's digest hour holds their digest until the next sweep. */ +const FIRST_SWEEP_DELAY_MS = 20_000; + +/** As many at once as the warmer allows itself, and for the same reason: see InboxWarmerService. */ +const CONCURRENCY = 4; + +/** + * The daily list of what is still waiting on somebody. + * + * The warmer only reads. This sends Slack messages, which cannot be taken back, so a day is + * claimed and only the pass that wins it posts. See UserWriteService.claimDigest. + */ +@Injectable() +export class DigestService implements OnApplicationBootstrap, OnModuleDestroy { + private readonly logger = new Logger(DigestService.name); + private firstSweep?: NodeJS.Timeout; + private timer?: NodeJS.Timeout; + private sweeping = false; + + constructor( + private readonly userReadService: UserReadService, + private readonly userWriteService: UserWriteService, + private readonly inboxRefreshService: InboxRefreshService, + private readonly deliveryService: SlackNotificationDeliveryService, + private readonly metrics: MetricsService, + ) {} + + public onApplicationBootstrap(): void { + const intervalMs = getEnvConfig().notifications.digestSweepIntervalMs; + + if (intervalMs <= 0) { + this.logger.log('The digest is off (DIGEST_SWEEP_INTERVAL_MS is 0)'); + + return; + } + + this.firstSweep = setTimeout(() => void this.sweep(), FIRST_SWEEP_DELAY_MS); + this.firstSweep.unref?.(); + + this.timer = setInterval(() => void this.sweep(), intervalMs); + this.timer.unref?.(); + + this.logger.log(`Digest sweep every ${Math.round(intervalMs / 60_000)}m`); + } + + public onModuleDestroy(): void { + clearTimeout(this.firstSweep); + clearInterval(this.timer); + } + + /** `now` is passed so a spec can pin the clock and test two timezones at one instant. */ + public async sweep(now: Date = new Date()): Promise { + if (this.sweeping) { + this.metrics.count('proke.digest.sweeps', { outcome: 'overlapped' }); + this.logger.warn('Skipping a digest sweep: the previous one is still running'); + + return; + } + + this.sweeping = true; + const startedAt = Date.now(); + + try { + const targets = await this.userReadService.readDigestTargets(); + + await pool( + targets.filter((target) => isDue(target, now)), + CONCURRENCY, + (target) => this.send(target, now), + ); + + this.metrics.count('proke.digest.sweeps', { outcome: 'completed' }); + } catch (error) { + this.metrics.count('proke.digest.sweeps', { outcome: 'failed' }); + this.logger.error(`Digest sweep failed: ${describe(error)}`); + } finally { + this.sweeping = false; + this.metrics.duration('proke.digest.duration', Date.now() - startedAt, {}); + } + } + + private async send(target: DigestTarget, now: Date): Promise { + try { + const result = await this.inboxRefreshService.refresh( + target.userId, + buildFiltersOf(target.settings), + ); + + // Unclaimed, so a GitHub blip at nine costs one sweep rather than the whole day. + if (!result.ok) { + this.metrics.count('proke.digest.sent', { + outcome: result.reason === 'no-token' ? 'no_token' : 'github_unavailable', + }); + + return; + } + + // Through the grouping, not the stored rows: ignoredAuthors is applied on the way out. + const waiting = groupWaitingOnYou(result.snapshot.waitingOnYou, target.settings) + .flatMap((section) => section.pullRequests) + .sort(byOldestFirst); + + const day = localMomentIn(target.timezone, now).day; + + if (!(await this.userWriteService.claimDigest(target.userId, day))) { + this.metrics.count('proke.digest.sent', { outcome: 'claimed_already' }); + + return; + } + + // Below the claim, so a quiet day is spent rather than rebuilt every quarter of an hour. + if (waiting.length === 0) { + this.metrics.count('proke.digest.sent', { outcome: 'empty' }); + + return; + } + + const outcome = await this.deliveryService.deliverDigest( + target.userId, + waiting.map(toDigestPullRequest), + now, + ); + + this.metrics.count('proke.digest.sent', { + outcome: outcome === 'sent' ? 'sent' : outcome === 'failed' ? 'failed' : 'undeliverable', + }); + } catch (error) { + this.metrics.count('proke.digest.sent', { outcome: 'failed' }); + this.logger.error(`Failed to send the digest of user ${target.userId}: ${describe(error)}`); + } + } +} + +/** `sentOn` is checked here, not left to the claim, so the rest of the day costs no writes. */ +function isDue(target: DigestTarget, now: Date): boolean { + const moment = localMomentIn(target.timezone, now); + + return moment.hour >= target.hour && target.sentOn !== moment.day; +} + +function byOldestFirst(left: InboxPullRequest, right: InboxPullRequest): number { + return openedMs(left) - openedMs(right); +} + +/** Zero rather than NaN, which would leave the whole list unordered. Same as `updatedMs`. */ +function openedMs(pullRequest: InboxPullRequest): number { + const parsed = Date.parse(pullRequest.createdAt || ''); + + return Number.isNaN(parsed) ? 0 : parsed; +} + +function toDigestPullRequest(pullRequest: InboxPullRequest): DigestPullRequest { + return { + number: pullRequest.number, + title: pullRequest.title, + url: pullRequest.url, + repositoryFullName: pullRequest.repositoryFullName, + authorLogin: pullRequest.author.login, + createdAt: pullRequest.createdAt, + changedFiles: pullRequest.changedFiles, + }; +} + +function describe(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/backend/src/shared/configs/env-configs.ts b/backend/src/shared/configs/env-configs.ts index fb5fc8c..a1920fc 100644 --- a/backend/src/shared/configs/env-configs.ts +++ b/backend/src/shared/configs/env-configs.ts @@ -91,6 +91,12 @@ interface EnvConfig { * and once per member it picks out by name. */ reviewBatchWindowMs: number; + /** + * How often the digest sweep looks for somebody whose hour has come, and nought to turn it + * off. Not how often anybody is messaged: the claim allows one digest per local day whatever + * this is set to, so it only decides how soon after their hour they hear. + */ + digestSweepIntervalMs: number; }; } @@ -164,6 +170,9 @@ export function getEnvConfig(): EnvConfig { // Five seconds: long enough that the pieces of one review reliably meet, short enough that // a poke is still a poke. Configurable mostly so the e2e suite need not sit through it. reviewBatchWindowMs: Number(process.env.REVIEW_BATCH_WINDOW_MS ?? 5000), + // A quarter hour: the coarsest that still honours zones offset by half and three-quarter + // hours, and four passes an hour means only a long deploy costs anybody their digest. + digestSweepIntervalMs: Number(process.env.DIGEST_SWEEP_INTERVAL_MS ?? 15 * 60_000), }, }; } diff --git a/backend/test/notifications/digest.spec.ts b/backend/test/notifications/digest.spec.ts new file mode 100644 index 0000000..d7af55e --- /dev/null +++ b/backend/test/notifications/digest.spec.ts @@ -0,0 +1,620 @@ +import * as nock from 'nock'; +import * as request from 'supertest'; +import { DEFAULT_POKE_SETTINGS } from '../../src/notifications/core/poke-settings'; +import { createTestApp } from '../utils/bootstrap'; + +const TEAM_ID = 'T0ACME'; + +/** + * The daily digest: who it is sent to, when, and what it says. + * + * 2026-09-11T08:30:00Z is 09:30 in Lisbon, 01:30 in Los Angeles and 17:30 in Tokyo, so a user + * whose hour is nine is due in two of those three and not the third. + */ +describe('The daily digest', () => { + let bootstrap: Awaited>; + + const MORNING_IN_LISBON = new Date('2026-09-11T08:30:00Z'); + const AFTERNOON_IN_LISBON = new Date('2026-09-11T13:30:00Z'); + const NEXT_MORNING_IN_LISBON = new Date('2026-09-12T08:30:00Z'); + + beforeAll(async () => { + process.env.TOKEN_ENCRYPTION_KEY = 'test-encryption-key'; + + bootstrap = await createTestApp(); + }); + + beforeEach(async () => { + await bootstrap.methods.beforeEach(); + }); + + afterAll(async () => { + await bootstrap.methods.afterAll(); + }); + + const server = () => bootstrap.app.getHttpServer(); + const digest = () => bootstrap.services.digestService; + const auth = (token: string) => ({ Authorization: `Bearer ${token}` }); + + const pullRequest = (overrides: Record = {}) => ({ + id: `node-${Math.random()}`, + number: 1, + title: 'A change', + url: 'https://github.com/acme/api/pull/1', + isDraft: false, + updatedAt: '2026-09-11T00:00:00Z', + createdAt: '2026-09-08T08:30:00Z', + changedFiles: 4, + repository: { id: 'repo-1', nameWithOwner: 'acme/api' }, + author: { __typename: 'User', login: 'bob', avatarUrl: 'https://avatars/bob' }, + reviewThreads: { nodes: [] }, + ...overrides, + }); + + /** One GraphQL answer and one teams answer: exactly what building one inbox costs. */ + const mockOneRefresh = (waitingOnYou: any[] = [pullRequest()]) => { + nock('https://api.github.com') + .post('/graphql') + .reply(200, { + data: { + viewer: { login: 'ada' }, + yours: { nodes: [] }, + waitingOnYou: { nodes: waitingOnYou }, + }, + }); + nock('https://api.github.com').get('/user/teams').query(true).reply(200, []); + }; + + /** Registered even where nothing should be sent, so an accidental post fails an assertion. */ + const capturePost = () => { + const posts: any[] = []; + + nock('https://slack.com') + .post('/api/chat.postMessage', (body) => { + posts.push(body); + return true; + }) + .times(10) + .reply(200, { ok: true }); + + return posts; + }; + + /** Counting posts alone cannot tell a skipped sweep from one GitHub refused. */ + const untouchedGithubMocks = () => + nock.pendingMocks().some((mock) => mock.includes('api.github.com')); + + /** + * Somebody connected end to end, with the digest already on. Written onto the row rather than + * through the settings route, because enabling it there deliberately uses up the current day. + */ + const digestUser = async ( + options: { + timezone?: string; + hour?: number; + enabled?: boolean; + githubAccessToken?: string | undefined; + slack?: boolean; + sentOn?: string; + } = {}, + ) => { + const { user, token } = await bootstrap.utils.authUtils.setupUser({ + githubLogin: 'ada', + githubAccessToken: 'githubAccessToken' in options ? options.githubAccessToken : 'gho_token', + }); + + await bootstrap.models.userModel.updateOne( + { _id: user.id }, + { + $set: { + timezone: options.timezone ?? 'Europe/Lisbon', + pokeSettings: { + mutedTypes: [], + digestEnabled: options.enabled ?? true, + digestHour: options.hour ?? 9, + }, + ...(options.sentOn ? { digestSentOn: options.sentOn } : {}), + }, + }, + ); + + if (options.slack !== false) { + await connectSlack(user.id); + } + + return { user, token }; + }; + + const connectSlack = async (userId: string) => { + await bootstrap.models.slackWorkspaceModel.create({ + teamId: `${TEAM_ID}-${userId}`, + teamName: 'Acme', + botUserId: 'B0PROKE', + botToken: 'xoxb-workspace-token', + }); + await bootstrap.models.slackLinkModel.create({ + userId, + teamId: `${TEAM_ID}-${userId}`, + slackUserId: 'U0ADA', + dmChannelId: 'D0ADA', + }); + }; + + /** Not the route: whether this spends today depends on the instant, so the spec holds it. */ + const enableDigest = (userId: string, hour: number, now: Date) => + bootstrap.services.userWriteService.updatePokeSettings( + userId, + { + mutedTypes: [], + reviewRequestResolution: 'any_review', + digestEnabled: true, + digestHour: hour, + }, + 'Europe/Lisbon', + now, + ); + + const storedUser = async (userId: string) => bootstrap.models.userModel.findById(userId).lean(); + + describe('who it goes to', () => { + it('sends to somebody whose hour has come', async () => { + await digestUser(); + mockOneRefresh(); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(1); + expect(posts[0].channel).toEqual('D0ADA'); + }); + + // The same instant is 09:30 in Lisbon and 01:30 in Los Angeles. + it('leaves alone somebody for whom it is not yet that hour', async () => { + await digestUser({ timezone: 'America/Los_Angeles' }); + mockOneRefresh(); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(0); + expect(untouchedGithubMocks()).toBe(true); + }); + + it('leaves alone somebody who has not asked for one', async () => { + await digestUser({ enabled: false }); + mockOneRefresh(); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(0); + expect(untouchedGithubMocks()).toBe(true); + }); + + // The rule is whether their hour has been, not whether it is now, so a deploy across it + // costs minutes rather than the day. + it('still sends to somebody whose hour went by earlier in their day', async () => { + await digestUser({ timezone: 'Asia/Tokyo' }); + mockOneRefresh(); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(1); + }); + + it('leaves alone somebody whose GitHub authorization is gone', async () => { + await digestUser({ githubAccessToken: undefined }); + mockOneRefresh(); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(0); + expect(untouchedGithubMocks()).toBe(true); + }); + + it('survives somebody who has never connected Slack', async () => { + await digestUser({ slack: false }); + mockOneRefresh(); + + await expect(digest().sweep(MORNING_IN_LISBON)).resolves.toBeUndefined(); + }); + + // Asserts the second person's message, not only that nothing threw: carrying on is the point. + it('carries on past somebody whose inbox fails to build', async () => { + await digestUser(); + await digestUser(); + + nock('https://api.github.com').post('/graphql').reply(500, {}); + mockOneRefresh(); + const posts = capturePost(); + + await expect(digest().sweep(MORNING_IN_LISBON)).resolves.toBeUndefined(); + + expect(posts).toHaveLength(1); + }); + + // ignoredAuthors is applied when the inbox is served, so a digest reading the stored rows + // straight off the snapshot would list them anyway. + it('leaves out the authors their inbox is set to ignore', async () => { + const { token } = await digestUser(); + + await request(server()) + .put('/inbox/settings') + .send({ ignoredAuthors: ['dependabot'] }) + .set(auth(token)) + .expect(200); + + mockOneRefresh([ + pullRequest({ + number: 1, + title: 'Bump lodash', + author: { __typename: 'Bot', login: 'dependabot', avatarUrl: 'https://avatars/dep' }, + }), + pullRequest({ number: 2, title: 'A real change' }), + ]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(1); + expect(posts[0].text).toEqual('#2 A real change is waiting on your review.'); + expect(JSON.stringify(posts[0].blocks)).not.toContain('Bump lodash'); + }); + + it('says nothing at all when nothing is waiting', async () => { + await digestUser(); + mockOneRefresh([]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(0); + }); + }); + + describe('the once-a-day claim', () => { + it('sends one message however many times the sweep runs', async () => { + const { user } = await digestUser(); + mockOneRefresh(); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + await digest().sweep(MORNING_IN_LISBON); + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(1); + expect((await storedUser(user.id))?.digestSentOn).toEqual('2026-09-11'); + }); + + // Otherwise a quiet day costs an inbox rebuild on every pass until midnight. + it('spends the day even when there was nothing to send', async () => { + const { user } = await digestUser(); + mockOneRefresh([]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(0); + expect((await storedUser(user.id))?.digestSentOn).toEqual('2026-09-11'); + }); + + it('tries again later in the day when GitHub could not be reached', async () => { + const { user } = await digestUser(); + nock('https://api.github.com').post('/graphql').reply(502, {}); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(0); + expect((await storedUser(user.id))?.digestSentOn).toBeUndefined(); + + mockOneRefresh(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(1); + }); + + it('sends again the next day', async () => { + const { user } = await digestUser(); + mockOneRefresh(); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + mockOneRefresh(); + await digest().sweep(NEXT_MORNING_IN_LISBON); + + expect(posts).toHaveLength(2); + expect((await storedUser(user.id))?.digestSentOn).toEqual('2026-09-12'); + }); + }); + + describe('what it says', () => { + const lines = (posts: any[]) => + posts[0].blocks + .filter((block: any) => block.type === 'section') + .map((block: any) => block.text.text); + + it('leads with the count and lists the oldest first', async () => { + await digestUser(); + mockOneRefresh([ + pullRequest({ number: 2, title: 'Newer', createdAt: '2026-09-11T04:30:00Z' }), + pullRequest({ number: 1, title: 'Older', createdAt: '2026-09-08T08:30:00Z' }), + ]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + const [heading, list] = lines(posts); + + expect(heading).toEqual('*2 pull requests are waiting on your review.*'); + expect(list.indexOf('Older')).toBeLessThan(list.indexOf('Newer')); + }); + + it('ages each row from when the pull request was opened', async () => { + await digestUser(); + mockOneRefresh([ + pullRequest({ number: 1, createdAt: '2026-09-08T08:30:00Z' }), + pullRequest({ number: 2, createdAt: '2026-09-11T04:30:00Z' }), + pullRequest({ number: 3, createdAt: '2026-09-11T08:00:00Z' }), + ]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + const [, list] = lines(posts); + + expect(list).toContain('3 days old'); + expect(list).toContain('4 hours old'); + expect(list).toContain('just opened'); + }); + + it('carries the size, the repository and the author of each row', async () => { + await digestUser(); + mockOneRefresh([pullRequest({ number: 7, title: 'Retry the paginator', changedFiles: 1 })]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + const [, list] = lines(posts); + + expect(list).toContain('#7 Retry the paginator'); + expect(list).toContain('1 file'); + expect(list).toContain('`acme/api`'); + expect(list).toContain(''); + }); + + it('references no images at all', async () => { + await digestUser(); + mockOneRefresh([pullRequest(), pullRequest({ number: 2 })]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(JSON.stringify(posts[0].blocks)).not.toContain('avatars'); + expect(posts[0].blocks.some((block: any) => block.type === 'image')).toBe(false); + }); + + it('stops listing past twenty and says how many more there were', async () => { + await digestUser(); + mockOneRefresh( + Array.from({ length: 23 }, (_unused, index) => pullRequest({ number: index + 1 })), + ); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + const context = posts[0].blocks.find((block: any) => block.type === 'context'); + + expect(lines(posts).join('\n')).toContain('*23 pull requests are waiting on your review.*'); + expect(context.elements[0].text).toEqual('and 3 more waiting on you.'); + }); + + it('carries the whole point in the notification preview', async () => { + await digestUser(); + mockOneRefresh([pullRequest({ number: 7, title: 'Retry the paginator' })]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts[0].text).toEqual('#7 Retry the paginator is waiting on your review.'); + }); + + // Twenty rows of a real repository run to ~4,400 characters, so splitting is the normal path. + it('splits a long list into sections Slack will accept', async () => { + await digestUser(); + mockOneRefresh( + Array.from({ length: 20 }, (_unused, index) => + pullRequest({ + number: index + 1, + title: 'Retry the paginator when the upstream cursor expires mid-page', + url: `https://github.com/acme/platform-services/pull/${index + 1}`, + repository: { id: 'repo-1', nameWithOwner: 'acme/platform-services' }, + }), + ), + ); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + const [heading, ...chunks] = lines(posts); + + expect(heading).toEqual('*20 pull requests are waiting on your review.*'); + expect(chunks.length).toBeGreaterThan(1); + + for (const chunk of chunks) { + expect(chunk.length).toBeLessThan(3000); + } + + // None of them fell down the gap between two sections. + for (let number = 1; number <= 20; number += 1) { + expect(chunks.join('\n')).toContain(`#${number} Retry`); + } + }); + + it('cuts a title too long to sit on one row', async () => { + await digestUser(); + mockOneRefresh([ + pullRequest({ + number: 7, + title: + 'Rework the reconciliation job so a partial page never leaves the cursor behind, ' + + 'and backfill the ones it already did', + }), + ]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + const [, list] = lines(posts); + + expect(list).toContain('Rework the reconciliation job so a partial page'); + expect(list).toContain('…'); + expect(list).not.toContain('backfill'); + }); + + it('keeps the order when one row has no readable opening time', async () => { + await digestUser(); + // `updatedAt` sets the arrival order, and this one is where NaN takes the rest with it. + mockOneRefresh([ + pullRequest({ + number: 1, + title: 'Newer', + createdAt: '2026-09-11T04:30:00Z', + updatedAt: '2026-09-11T04:00:00Z', + }), + pullRequest({ + number: 2, + title: 'Undated', + createdAt: null, + updatedAt: '2026-09-11T03:00:00Z', + }), + pullRequest({ + number: 3, + title: 'Older', + createdAt: '2026-09-05T08:30:00Z', + updatedAt: '2026-09-11T02:00:00Z', + }), + pullRequest({ + number: 4, + title: 'Middling', + createdAt: '2026-09-08T08:30:00Z', + updatedAt: '2026-09-11T01:00:00Z', + }), + ]); + const posts = capturePost(); + + await digest().sweep(MORNING_IN_LISBON); + + const [, list] = lines(posts); + + expect(list.indexOf('Older')).toBeLessThan(list.indexOf('Middling')); + expect(list.indexOf('Middling')).toBeLessThan(list.indexOf('Newer')); + expect(list).toContain('age unknown'); + }); + }); + + describe('the settings', () => { + it('is off, at nine, for somebody who has never touched it', async () => { + const { token } = await bootstrap.utils.authUtils.setupUser(); + + const { body } = await request(server()).get('/users/me').set(auth(token)).expect(200); + + expect(body.pokeSettings.digestEnabled).toBe(DEFAULT_POKE_SETTINGS.digestEnabled); + expect(body.pokeSettings.digestHour).toBe(DEFAULT_POKE_SETTINGS.digestHour); + }); + + it('stores the hour and the zone, and hands them back with the user', async () => { + const { token, user } = await bootstrap.utils.authUtils.setupUser(); + + const updated = await request(server()) + .put('/notifications/settings') + .send({ mutedTypes: [], digestEnabled: true, digestHour: 7, timezone: 'Europe/Lisbon' }) + .set(auth(token)) + .expect(200); + + expect(updated.body.digestEnabled).toBe(true); + expect(updated.body.digestHour).toBe(7); + + const me = await request(server()).get('/users/me').set(auth(token)).expect(200); + + expect(me.body.pokeSettings.digestHour).toBe(7); + expect((await storedUser(user.id))?.timezone).toEqual('Europe/Lisbon'); + }); + + it('rejects an hour and a zone it cannot read rather than storing a default', async () => { + const { token } = await bootstrap.utils.authUtils.setupUser(); + + await request(server()) + .put('/notifications/settings') + .send({ mutedTypes: [], digestHour: 24 }) + .set(auth(token)) + .expect(400); + + await request(server()) + .put('/notifications/settings') + .send({ mutedTypes: [], timezone: 'Middle/Earth' }) + .set(auth(token)) + .expect(400); + }); + + it('starts tomorrow when it is switched on after the hour has been', async () => { + const { user } = await bootstrap.utils.authUtils.setupUser({ + githubAccessToken: 'gho_token', + }); + await connectSlack(user.id); + mockOneRefresh(); + const posts = capturePost(); + + await enableDigest(user.id, 9, MORNING_IN_LISBON); + + expect((await storedUser(user.id))?.digestSentOn).toEqual('2026-09-11'); + + await digest().sweep(MORNING_IN_LISBON); + + expect(posts).toHaveLength(0); + expect(untouchedGithubMocks()).toBe(true); + }); + + it('starts today when it is switched on before the hour has been', async () => { + const { user } = await bootstrap.utils.authUtils.setupUser({ + githubAccessToken: 'gho_token', + }); + await connectSlack(user.id); + mockOneRefresh(); + const posts = capturePost(); + + await enableDigest(user.id, 14, MORNING_IN_LISBON); + + expect((await storedUser(user.id))?.digestSentOn).toBeUndefined(); + + await digest().sweep(AFTERNOON_IN_LISBON); + + expect(posts).toHaveLength(1); + }); + + it('leaves an existing schedule alone when a client saves without it', async () => { + const { token, user } = await bootstrap.utils.authUtils.setupUser(); + + await request(server()) + .put('/notifications/settings') + .send({ mutedTypes: [], digestEnabled: true, digestHour: 7, timezone: 'Europe/Lisbon' }) + .set(auth(token)) + .expect(200); + + // A tab that predates the feature: the whole set, as it knows it. + await request(server()) + .put('/notifications/settings') + .send({ mutedTypes: [] }) + .set(auth(token)) + .expect(200); + + expect((await storedUser(user.id))?.timezone).toEqual('Europe/Lisbon'); + expect((await storedUser(user.id))?.pokeSettings?.digestHour).toBe(7); + }); + }); +}); diff --git a/backend/test/utils/bootstrap.ts b/backend/test/utils/bootstrap.ts index 3a1f9fb..021adec 100644 --- a/backend/test/utils/bootstrap.ts +++ b/backend/test/utils/bootstrap.ts @@ -8,6 +8,8 @@ import { RefreshTokenEntity } from '../../src/auth/session/entities/refresh-toke import { ConnectionsModule } from '../../src/connections/connections.module'; import { InboxModule } from '../../src/inbox/inbox.module'; import { InboxWarmModule } from '../../src/inbox/warm/inbox-warm.module'; +import { DigestModule } from '../../src/notifications/digest/digest.module'; +import { DigestService } from '../../src/notifications/digest/digest.service'; import { PokeSettingsModule } from '../../src/notifications/settings/poke-settings.module'; import { InboxWarmerService } from '../../src/inbox/warm/inbox-warmer.service'; import { InstallationEntity } from '../../src/installations/core/entities/installation.entity'; @@ -41,6 +43,9 @@ import { closeInMemoryMongoServer, rootMongooseTestModule } from './mongo-in-mem */ process.env.REVIEW_BATCH_WINDOW_MS ??= '150'; +/** Off for the reason below, and more so: this timer posts to Slack rather than only reading. */ +process.env.DIGEST_SWEEP_INTERVAL_MS ??= '0'; + /** * No sweeping in the background. * @@ -84,6 +89,7 @@ export async function createTestApp() { InboxModule, InboxWarmModule, PokeSettingsModule, + DigestModule, SlackModule, GithubWebhookModule, SlackEventsModule, @@ -163,6 +169,8 @@ export async function createTestApp() { // So a spec can run one pass of the warmer on demand. The timer is off in the suite - see // INBOX_WARM_SWEEP_INTERVAL_MS above - so this is the only thing that makes it sweep. inboxWarmerService: app.get(InboxWarmerService), + // Its sweep takes the instant to run at, so a spec can test two timezones at one instant. + digestService: app.get(DigestService), userReadService: app.get(UserReadService), userWriteService: app.get(UserWriteService), inMemoryCacheService, From f62ed79663f5525f355d67e52b4ebf3bb028b6b9 Mon Sep 17 00:00:00 2001 From: Andy Ruiz Garramones Date: Fri, 11 Sep 2026 22:45:57 +0200 Subject: [PATCH 6/6] feat(dashboard): add the digest switch and hour picker The switch sits under the reel rather than as a tenth row in the list above it. Those rows are kinds of event, counted in the header and previewed in the window, and this is a schedule. The hour picker appears only once the switch is on. The browser's timezone is sent on every settings save. Only the browser knows it, since GitHub exposes none and there is nothing on the session to derive one from, so sending it each time follows a user who moves without ever asking them. Co-Authored-By: Claude Opus 5 --- .../src/components/dashboard/Dashboard.tsx | 21 ++++- .../src/components/dashboard/PokesPanel.tsx | 94 +++++++++++++++++++ frontend/src/components/ui/Select.tsx | 15 ++- frontend/src/lib/api/user.api.ts | 15 ++- frontend/src/lib/logics/pokeSettingsLogic.ts | 26 +++++ 5 files changed, 160 insertions(+), 11 deletions(-) diff --git a/frontend/src/components/dashboard/Dashboard.tsx b/frontend/src/components/dashboard/Dashboard.tsx index aeeb2cf..a8d7a39 100644 --- a/frontend/src/components/dashboard/Dashboard.tsx +++ b/frontend/src/components/dashboard/Dashboard.tsx @@ -24,10 +24,19 @@ export function Dashboard() { } = useValues(slackLogic); const { loadConnection, disconnect, sendTestPoke } = useActions(slackLogic); // No load of its own: these ride in on the profile, which authLogic has already read. - const { mutedTypes, reviewRequestResolution, notice } = - useValues(pokeSettingsLogic); - const { toggleType, setReviewRequestResolution } = - useActions(pokeSettingsLogic); + const { + mutedTypes, + reviewRequestResolution, + digestEnabled, + digestHour, + notice, + } = useValues(pokeSettingsLogic); + const { + toggleType, + setReviewRequestResolution, + setDigestEnabled, + setDigestHour, + } = useActions(pokeSettingsLogic); useEffect(() => { loadConnections(); @@ -85,9 +94,13 @@ export function Dashboard() { pokes={{ mutedTypes, reviewRequestResolution, + digestEnabled, + digestHour, notice, onToggleType: toggleType, onSetReviewRequestResolution: setReviewRequestResolution, + onSetDigestEnabled: setDigestEnabled, + onSetDigestHour: setDigestHour, }} slack={{ connection: slackConnection, diff --git a/frontend/src/components/dashboard/PokesPanel.tsx b/frontend/src/components/dashboard/PokesPanel.tsx index 76742c6..41e1a8f 100644 --- a/frontend/src/components/dashboard/PokesPanel.tsx +++ b/frontend/src/components/dashboard/PokesPanel.tsx @@ -24,6 +24,11 @@ export interface PokesPanelProps { /** When a review request is struck through once somebody else reviews. */ reviewRequestResolution: ReviewRequestResolution; onSetReviewRequestResolution: (resolution: ReviewRequestResolution) => void; + /** Whether the daily list of what is still waiting arrives, and at what hour. */ + digestEnabled: boolean; + digestHour: number; + onSetDigestEnabled: (enabled: boolean) => void; + onSetDigestHour: (hour: number) => void; /** A refused save, in words. Optional so the drafts gallery renders the panel without one. */ notice?: string | null; } @@ -66,6 +71,10 @@ export function PokesPanel({ onToggleType, reviewRequestResolution, onSetReviewRequestResolution, + digestEnabled, + digestHour, + onSetDigestEnabled, + onSetDigestHour, notice, }: PokesPanelProps) { // The row the reel is showing. Follows the pointer, and stays where it was left afterwards - @@ -149,6 +158,13 @@ export function PokesPanel({ */} + + {/* Nothing under the list unless something has actually happened - a refused save, or every kind switched off. There is no standing footnote: the rows say what they do, and a line @@ -336,6 +352,84 @@ function ResolutionRow({ ); } +const CLOCK = + "M8 0a8 8 0 1 1 0 16A8 8 0 0 1 8 0ZM1.5 8a6.5 6.5 0 1 0 13 0 6.5 6.5 0 0 0-13 0Zm7-3.25v2.992l2.028.812a.75.75 0 0 1-.557 1.392l-2.5-1A.751.751 0 0 1 7 8.25v-3.5a.75.75 0 0 1 1.5 0Z"; + +/** No detail line under any of them: "9am" is the whole of what picking it does. */ +const DIGEST_HOURS: readonly SelectOption[] = Array.from( + { length: 24 }, + (_unused, hour) => ({ + value: String(hour), + title: hourName(hour), + }) +); + +function hourName(hour: number): string { + if (hour === 0) return "12am"; + if (hour === 12) return "12pm"; + + return hour < 12 ? `${hour}am` : `${hour - 12}pm`; +} + +/** + * The digest, which is a schedule rather than a kind of poke. + * + * Under the reel rather than in the list above it: a row up there is counted in the header and + * previewed in the window, and this would be the tenth of nine kinds with no card to show. + */ +function DigestRow({ + enabled, + hour, + onSetEnabled, + onSetHour, +}: { + enabled: boolean; + hour: number; + onSetEnabled: (enabled: boolean) => void; + onSetHour: (hour: number) => void; +}) { + return ( +
+
+ + + {enabled ? ( + + ) : null} +
+
+ ); +} + /** A tick, or the ring that holds its place. The same width either way, so nothing shuffles. */ function Tick({ on }: { on: boolean }) { return ( diff --git a/frontend/src/components/ui/Select.tsx b/frontend/src/components/ui/Select.tsx index 3d14e7c..5737426 100644 --- a/frontend/src/components/ui/Select.tsx +++ b/frontend/src/components/ui/Select.tsx @@ -19,8 +19,11 @@ export interface SelectOption { value: T; /** The choice, in a word or two. What the trigger shows once it is picked. */ title: string; - /** What picking it does, in one sentence. Shown under the title in the list, never on the trigger. */ - detail: string; + /** + * What picking it does, in one sentence. Shown under the title in the list, never on the + * trigger. Optional, for lists where the title is the whole answer - an hour explains itself. + */ + detail?: string; } export interface SelectProps { @@ -269,9 +272,11 @@ export function Select({ {option.title} - - {option.detail} - + {option.detail ? ( + + {option.detail} + + ) : null} {/* The same width whether or not it is drawn, so the titles line up. */} { + // The zone rides along on every save: only the browser knows it, and moving changes it. const response = await axios.put( "/notifications/settings", - settings, + { ...settings, timezone: Intl.DateTimeFormat().resolvedOptions().timeZone }, { headers: { Authorization: `Bearer ${jwtToken}` } } ); diff --git a/frontend/src/lib/logics/pokeSettingsLogic.ts b/frontend/src/lib/logics/pokeSettingsLogic.ts index b7a4552..6df06a1 100644 --- a/frontend/src/lib/logics/pokeSettingsLogic.ts +++ b/frontend/src/lib/logics/pokeSettingsLogic.ts @@ -54,6 +54,10 @@ export const pokeSettingsLogic = kea([ setReviewRequestResolution: (resolution: ReviewRequestResolution) => ({ resolution, }), + /** Turn the daily digest on or off. Same timing as a flip. */ + setDigestEnabled: (enabled: boolean) => ({ enabled }), + /** Choose the hour it arrives, in this browser's timezone. */ + setDigestHour: (hour: number) => ({ hour }), /** The whole set as it now stands on screen, ahead of the server agreeing. */ edit: (settings: PokeSettings) => ({ settings }), dismissNotice: true, @@ -119,6 +123,8 @@ export const pokeSettingsLogic = kea([ "Couldn't save that, so it's back to how it was.", toggleType: () => null, setReviewRequestResolution: () => null, + setDigestEnabled: () => null, + setDigestHour: () => null, dismissNotice: () => null, }, ], @@ -161,6 +167,14 @@ export const pokeSettingsLogic = kea([ (settings: PokeSettings): ReviewRequestResolution => settings.reviewRequestResolution, ], + digestEnabled: [ + (s) => [s.settings], + (settings: PokeSettings): boolean => settings.digestEnabled, + ], + digestHour: [ + (s) => [s.settings], + (settings: PokeSettings): number => settings.digestHour, + ], }), listeners(({ actions, values }) => ({ @@ -186,6 +200,18 @@ export const pokeSettingsLogic = kea([ reviewRequestResolution: resolution, }; + actions.edit(next); + actions.saveSettings({ settings: next }); + }, + setDigestEnabled: ({ enabled }) => { + const next: PokeSettings = { ...values.settings, digestEnabled: enabled }; + + actions.edit(next); + actions.saveSettings({ settings: next }); + }, + setDigestHour: ({ hour }) => { + const next: PokeSettings = { ...values.settings, digestHour: hour }; + actions.edit(next); actions.saveSettings({ settings: next }); },