Merge pull request #3871 from HeyPuter/juancastro/put-1798-sharing-email-notification-changes

feat: name the issuing app in share emails, and cut the digest window to 5s
This commit is contained in:
Juan Fernando Castro
2026-09-21 09:58:09 -04:00
committed by GitHub
8 changed files with 266 additions and 17 deletions
+3 -3
View File
@@ -184,7 +184,7 @@ const SHARE_LIST_ROW = `
<table role="presentation" width="100%" cellpadding="0" cellspacing="0" border="0" style="width: 100%;">
{{#each shares}}
<tr>
<td class="ink rule" style="padding: 12px 0;{{#unless @first}} border-top: 1px solid ${RULE};{{/unless}} font-family: ${FONT}; font-size: 16px; line-height: 24px; color: ${INK};"><strong style="font-weight: 600;">{{this.sender}}</strong> shared ${SHARE_ITEM_LIST}</td>
<td class="ink rule" style="padding: 12px 0;{{#unless @first}} border-top: 1px solid ${RULE};{{/unless}} font-family: ${FONT}; font-size: 16px; line-height: 24px; color: ${INK};"><strong style="font-weight: 600;">{{this.sender}}</strong> shared ${SHARE_ITEM_LIST}{{#if this.via}} via {{this.via}}{{/if}}</td>
</tr>
{{/each}}
</table>
@@ -458,7 +458,7 @@ immediately</p>
Shared with you on Puter:
{{#each shares}}
- {{this.sender}} shared {{this.what}}
- {{this.sender}} shared {{this.what}}{{#if this.via}} via {{this.via}}{{/if}}
{{#each this.items}}{{#if this.link}} {{this.name}}: {{this.link}}
{{/if}}{{/each}}{{/each}}
@@ -496,7 +496,7 @@ immediately</p>
Shared with you on Puter:
{{#each shares}}
- {{this.sender}} shared {{this.what}}
- {{this.sender}} shared {{this.what}}{{#if this.via}} via {{this.via}}{{/if}}
{{/each}}
There's no Puter account for {{email}} yet. Create one with this
@@ -63,7 +63,7 @@ export const SHARE_NOTIFY_RECIPIENT_DAILY_LIMIT = 50;
* span goes as a single message. Email can't be rewritten the way the in-app
* notification can, so it gets the grouped wording by waiting instead.
*/
export const SHARE_EMAIL_BATCH_SECONDS = 30;
export const SHARE_EMAIL_BATCH_SECONDS = 5;
// Long enough to list, claim and send; short enough that a crashed flusher
// doesn't strand the digest.
@@ -130,6 +130,15 @@ interface ShareNotificationTarget {
name: string;
}
/**
* The issuing app: `label` is what the mail says, `match` what the link
* carries.
*/
interface IssuingApp {
label: string | null;
match: string;
}
/**
* A queued send's durable form: persisted to KV so it survives the node that
* queued it and is visible to every other node's flush.
@@ -146,6 +155,8 @@ interface DigestEntryRecord {
names: string[];
/** As `names`, plus links. Absent on records queued before this shipped. */
items?: DigestItem[];
/** The app the shares came through, when one did. */
app?: IssuingApp | null;
/** Arrival order — KV lists by key, which is a uuid and says nothing. */
queuedAt: number;
}
@@ -257,6 +268,7 @@ export class ShareNotificationService extends PuterService {
const issuerId = actor.user?.id;
const issuer = actor.user?.username;
if (typeof issuerId !== 'number') return;
const app = await this.#issuingApp(actor);
const counts = new Map<number, number>();
const named = new Map<number, DigestItem[]>();
@@ -267,7 +279,7 @@ export class ShareNotificationService extends PuterService {
issuerId,
)) {
counts.set(holderId, (counts.get(holderId) ?? 0) + 1);
const item = this.#digestItem(share);
const item = this.#digestItem(share, app);
if (item) {
const items = named.get(holderId) ?? [];
if (items.length < DIGEST_NAMES_PER_SENDER) items.push(item);
@@ -305,6 +317,7 @@ export class ShareNotificationService extends PuterService {
count,
named.get(holderId) ?? [],
interrupt,
app,
);
} catch (err) {
console.warn(
@@ -317,7 +330,7 @@ export class ShareNotificationService extends PuterService {
);
try {
await this.#emailInvites(actor, shares);
await this.#emailInvites(actor, shares, app);
} catch (err) {
console.warn('[share-notify] could not email invites:', err);
}
@@ -630,6 +643,7 @@ export class ShareNotificationService extends PuterService {
count: number,
items: DigestItem[],
mayOpen: boolean,
app: IssuingApp | null,
): Promise<void> {
// Explicitly false, not falsy: unset means on.
if (this.config.share_email_notifications === false) {
@@ -673,21 +687,61 @@ export class ShareNotificationService extends PuterService {
count,
items,
mayOpen,
app,
);
}
/** The app this call shares through (label: title, then name, then host). */
async #issuingApp(actor: Actor): Promise<IssuingApp | null> {
const uid = actor.effectiveApp?.uid;
if (!uid) return null;
try {
const app = (await this.stores.app.getByUid(uid)) as {
name?: string | null;
title?: string | null;
index_url?: string | null;
} | null;
if (!app) return { label: null, match: uid };
return {
label:
app.title ||
app.name ||
this.#originOf(app.index_url) ||
null,
match: app.name || uid,
};
} catch (err) {
// The mail still goes; it just can't name the app.
console.warn('[share-notify] could not resolve issuing app:', err);
return { label: null, match: uid };
}
}
/** `https://draw.example.com/v2/` → `draw.example.com`. */
#originOf(indexUrl: string | null | undefined): string | null {
if (!indexUrl) return null;
try {
return new URL(indexUrl).host || null;
} catch {
return null;
}
}
/**
* One named, linked item for the digest. Built from the uuid and owner, not
* `share.path` — that is the owner's real path here, not the recipient's to
* see. Both forms name the owner first, which is where it comes from.
*/
#digestItem(share: ResolvedShare): DigestItem | null {
#digestItem(
share: ResolvedShare,
app: IssuingApp | null,
): DigestItem | null {
if (!share.name) return null;
const path = this.#targetPath(share);
if (!path) return { name: share.name };
return {
name: share.name,
link: shareDeepLink(this.#appLink(), path),
link: shareDeepLink(this.#appLink(), path, app?.match),
path,
};
}
@@ -726,6 +780,7 @@ export class ShareNotificationService extends PuterService {
count: number,
items: DigestItem[],
mayOpen: boolean,
app: IssuingApp | null,
): Promise<void> {
// A digest is one email, so the budget is spent opening one, not per
// share — anything arriving while one collects joins it for free.
@@ -742,6 +797,7 @@ export class ShareNotificationService extends PuterService {
// can flush this entry; `items` is what this one reads.
names: items.map((item) => item.name),
items,
app,
queuedAt: Date.now(),
};
await this.stores.kv.set({
@@ -876,8 +932,17 @@ export class ShareNotificationService extends PuterService {
record.sender,
record.count,
this.#recordItems(record),
record.app?.label,
);
}
// Name the app on the button only when the whole mail is its doing.
const matches = new Set(
claimed.map(({ record }) => record.app?.match ?? null),
);
const linkApp =
matches.size === 1
? (matches.values().next().value ?? null)
: null;
const [{ record: first }] = claimed;
console.log('[share-notify] sending digest:', {
key,
@@ -901,6 +966,7 @@ export class ShareNotificationService extends PuterService {
link: sharedViewLink(
this.#appLink(),
digestItemPaths(entries),
linkApp,
),
// The template composes the unsubscribe URL from
// the origin, so `?` and `=` stay literal instead
@@ -959,7 +1025,11 @@ export class ShareNotificationService extends PuterService {
* `share_email_notifications` says — there is no Puter inbox to use instead
* — but still budgeted: an invite reaches someone who never asked for it.
*/
async #emailInvites(actor: Actor, shares: ResolvedShare[]): Promise<void> {
async #emailInvites(
actor: Actor,
shares: ResolvedShare[],
app: IssuingApp | null,
): Promise<void> {
if (!this.config.email) return;
const issuerId = actor.user?.id;
if (typeof issuerId !== 'number') return;
@@ -1003,6 +1073,7 @@ export class ShareNotificationService extends PuterService {
count,
items,
mayOpen,
app,
);
} catch (err) {
console.warn('[share-notify] invite email failed:', err);
@@ -161,6 +161,37 @@ describe('sharedViewLink', () => {
expect(shared).toEqual(paths.slice(0, SHARE_DEEP_LINK_ITEMS_LIMIT));
});
it('carries the issuing app as its own parameter, or not at all', () => {
const path = `/alice/${UID}/a.txt`;
const link = sharedViewLink('https://puter.com', [path], 'draw-app');
expect(link).toBe(
`https://puter.com/?shared=${encodeURIComponent(path)}&shared_app=draw-app`,
);
expect(new URL(link).searchParams.get('shared_app')).toBe('draw-app');
// No app, no parameter — for a null the same as for an omission.
expect(sharedViewLink('https://puter.com', [path], null)).toBe(
sharedViewLink('https://puter.com', [path]),
);
expect(shareDeepLink('https://puter.com', path, 'draw-app')).toBe(link);
});
it('encodes an app value that would otherwise break the query string', () => {
const link = sharedViewLink('https://puter.com', [], 'a&b=c');
expect(new URL(link).searchParams.get('shared_app')).toBe('a&b=c');
});
it('reserves room for the app before spending the length cap on items', () => {
const paths = Array.from(
{ length: SHARE_DEEP_LINK_ITEMS_LIMIT },
(_, i) => `/alice/${UID}/${'quarterly report '.repeat(8)}${i}.pdf`,
);
const app = `app-${'x'.repeat(60)}`;
const link = sharedViewLink('https://puter.com', paths, app);
expect(link.length).toBeLessThanOrEqual(SHARE_DEEP_LINK_MAX_LENGTH);
// The app survives however many items wanted the space.
expect(new URL(link).searchParams.get('shared_app')).toBe(app);
});
// Twenty ordinary names already run to several kilobytes once encoded, so
// the count alone is no guard; the link itself has to stay short enough.
it('stops adding items before the link outgrows what mail clients tolerate', () => {
+21 -5
View File
@@ -27,6 +27,9 @@
/** The query parameter the GUI routes on. */
export const SHARE_DEEP_LINK_PARAM = 'shared';
/** The issuing app (its `name`, or uid), so the GUI can match the share to it. */
export const SHARE_DEEP_LINK_APP_PARAM = 'shared_app';
export interface ShareTarget {
/** The entry's own name, which the masked path's last segment must be. */
name: string;
@@ -74,13 +77,22 @@ export const SHARE_DEEP_LINK_MAX_LENGTH = 2000;
* they are signed in. Only masked paths travel — each one's second segment is
* the uuid, so a rename is recoverable and there is no second copy to disagree
* with the first. With no paths the link still lands on Shared.
*
* `app` rides along as `shared_app`, its length reserved up front.
*/
export const sharedViewLink = (origin: string, paths: string[]): string => {
export const sharedViewLink = (
origin: string,
paths: string[],
app?: string | null,
): string => {
const base = `${origin.replace(/\/+$/, '')}/?`;
const appParam = app
? `&${SHARE_DEEP_LINK_APP_PARAM}=${encodeURIComponent(app)}`
: '';
// The first items that fit, in order — never a later one over an
// earlier, so what is highlighted reads as the top of the list.
const params: string[] = [];
let length = base.length;
let length = base.length + appParam.length;
for (const path of new Set(paths)) {
if (params.length === SHARE_DEEP_LINK_ITEMS_LIMIT) break;
const param = `${SHARE_DEEP_LINK_PARAM}=${encodeURIComponent(path)}`;
@@ -92,13 +104,17 @@ export const sharedViewLink = (origin: string, paths: string[]): string => {
}
return (
base +
(params.length === 0 ? `${SHARE_DEEP_LINK_PARAM}=` : params.join('&'))
(params.length === 0 ? `${SHARE_DEEP_LINK_PARAM}=` : params.join('&')) +
appParam
);
};
/** A link that opens `path`: the Shared view with that one item highlighted. */
export const shareDeepLink = (origin: string, path: string): string =>
sharedViewLink(origin, [path]);
export const shareDeepLink = (
origin: string,
path: string,
app?: string | null,
): string => sharedViewLink(origin, [path], app);
/** The link for a target, or `null` when it isn't addressable. */
export const shareTargetLink = (
+95 -1
View File
@@ -32,6 +32,8 @@
*/
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
import { makeActor } from '../../core/actor.js';
import { runWithContext } from '../../core/context.js';
import { setupPuterTestEnv, type PuterTestEnv } from '../../testUtil.js';
const BOOT_TIMEOUT_MS = 120_000;
@@ -250,7 +252,11 @@ describe('share email', () => {
const username = `se${uniqueSuffix()}`;
const signup = await fetch(new URL('/signup', env.origin), {
method: 'POST',
headers: { 'content-type': 'application/json' },
headers: {
'content-type': 'application/json',
// Each signup is its own person; don't share one signup budget.
'user-agent': `share-email-suite/${username}`,
},
body: JSON.stringify({
username,
email,
@@ -500,6 +506,9 @@ describe('share email', () => {
expect(mail.text).toContain(link);
// The owner's real path is theirs alone; only the mask travels.
expect(mail.html).not.toContain(`/${owner.username}/deeplink`);
// No app issued this, so nothing claims it.
expect(mail.html).not.toContain('shared_app=');
expect(mail.html).not.toContain(' via ');
});
it('links every file when several are shared at once', async () => {
@@ -534,6 +543,91 @@ describe('share email', () => {
expect(mail.text).toContain(`Open Puter: ${href}`);
});
/** An app of `owner`'s with reach over `file`, and a token to share as them. */
const makeSharingApp = async (
owner: { username: string },
file: { uid: string },
title: string,
) => {
const user = await env.server.stores.user.getByUsername(owner.username);
const actor = makeActor({ user: user! });
const app = await env.server.stores.app.create(
{
name: `mail-app-${crypto.randomUUID().slice(0, 8)}`,
title,
index_url: 'https://mail-app.test/',
},
{ ownerUserId: user!.id },
);
await runWithContext({ actor }, () =>
env.server.services.permission.grantUserAppPermission(
actor,
app.uid,
`fs:${file.uid}:read`,
),
);
const token = await env.server.services.auth.getUserAppToken(
actor,
app.uid,
);
return { ...app, token };
};
it('names the issuing app and stamps it on the links', async () => {
const owner = env.users.user;
const recipient = await signUpAndConfirm(uninvitedAddress());
sent = [];
const file = await makeFile(owner, 'via-app');
const { token, ...app } = await makeSharingApp(owner, file, 'Mail App');
await shareWith({ token }, recipient.email, [{ uid: file.uid }]);
const mail = await waitForMail({ to: recipient.email });
expect(mail.html).toContain('via Mail App');
expect(mail.text).toContain(`shared ${file.name} via Mail App`);
const href = openPuterHref(mail.html);
expect(new URL(href!).searchParams.get('shared_app')).toBe(app.name);
const masked = `/${owner.username}/${file.uid}/${file.name}`;
expect(mail.html).toContain(
`?shared=${encodeURIComponent(masked)}&shared_app=${app.name}`,
);
// The invite names the app too; with no links, no parameter to ride.
const invitee = uninvitedAddress();
await shareWith({ token }, invitee, [{ uid: file.uid }]);
const invite = await waitForMail({ to: invitee });
expect(invite.html).toContain('via Mail App');
expect(invite.html).not.toContain('shared_app=');
});
it('keeps the app off the button when the digest is not all its doing', async () => {
const appSender = env.users.user;
const plainSender = env.users.admin;
const recipient = await signUpAndConfirm(uninvitedAddress());
sent = [];
const viaApp = await makeFile(appSender, 'mixed-app');
const plain = await makeFile(plainSender, 'mixed-plain');
const app = await makeSharingApp(appSender, viaApp, 'Mixer');
await withDigestWindow(env, DIGEST_WINDOW_SECONDS, async () => {
await shareWith({ token: app.token }, recipient.email, [
{ uid: viaApp.uid },
]);
await shareWith(plainSender, recipient.email, [{ uid: plain.uid }]);
});
const mail = await waitForMail({ to: recipient.email });
await sleep(SETTLE_MS);
expect(mailTo(recipient.email)).toHaveLength(1);
// The app's own line and item link keep their attribution...
expect(mail.html).toContain('via Mixer');
expect(mail.html).toContain(`&shared_app=${app.name}`);
// ...but the button speaks for the whole mail, so it names no app.
const href = openPuterHref(mail.html);
expect(new URL(href!).searchParams.get('shared_app')).toBeNull();
});
// Nothing to route to yet, so the names stay plain and the call to action
// is still "create an account".
it('does not link the files in an invite', async () => {
@@ -246,4 +246,33 @@ describe('email digests', () => {
},
]);
});
it('carries the issuing app onto the line, as "via"', () => {
const merged = mergeDigestEntry([], 'alice', 1, [item('a.txt')], 'Draw');
expect(merged).toEqual([
{ username: 'alice', count: 1, items: [item('a.txt')], app: 'Draw' },
]);
expect(digestLines(merged)).toMatchObject([
{ sender: 'alice', what: 'a.txt', via: 'Draw' },
]);
// No app, no `via` key — the template's {{#if}} must see nothing.
expect(digestLines([{ username: 'bob', count: 1, items: [] }])[0])
.not.toHaveProperty('via');
});
it('drops the app when one sender arrives through two sources', () => {
const viaApp = mergeDigestEntry([], 'alice', 1, [item('a.txt')], 'Draw');
// Same app again: attribution holds.
expect(
mergeDigestEntry(viaApp, 'alice', 1, [item('b.txt')], 'Draw')[0].app,
).toBe('Draw');
// A plain share, or another app, makes the line nobody's to claim.
expect(mergeDigestEntry(viaApp, 'alice', 1, [])[0].app).toBeNull();
expect(
mergeDigestEntry(viaApp, 'alice', 1, [], 'Notes')[0].app,
).toBeNull();
// A different sender keeps their own attribution.
const two = mergeDigestEntry(viaApp, 'bob', 1, [], 'Notes');
expect(two.map((entry) => entry.app)).toEqual(['Draw', 'Notes']);
});
});
@@ -136,6 +136,8 @@ export interface DigestEntry {
count: number;
/** Named items, newest last. */
items: DigestItem[];
/** The app the shares came through, when they all came through one. */
app?: string | null;
}
/** How many item names one digest line spells out before counting the rest. */
@@ -147,6 +149,7 @@ export const mergeDigestEntry = (
username: string | undefined,
count: number,
items: DigestItem[] = [],
app?: string | null,
): DigestEntry[] => {
const name = username || 'Someone';
const merged = entries.map((entry) => ({
@@ -157,9 +160,11 @@ export const mergeDigestEntry = (
if (existing) {
existing.count += count;
existing.items.push(...items);
// One sender through two sources is nobody's app to name.
if ((existing.app ?? null) !== (app ?? null)) existing.app = null;
return merged;
}
merged.push({ username: name, count, items: [...items] });
merged.push({ username: name, count, items: [...items], app: app ?? null });
return merged;
};
@@ -206,6 +211,8 @@ export interface DigestLine {
lead: string;
items: DigestItem[];
trail: string;
/** The issuing app's display name, rendered as "via {{via}}". */
via?: string;
}
/** One rendered line per sender: who, and what they shared. */
@@ -230,5 +237,6 @@ export const digestLines = (entries: DigestEntry[]): DigestLine[] =>
lead,
items: named,
trail,
...(entry.app ? { via: entry.app } : {}),
};
});
+1 -1
View File
@@ -181,7 +181,7 @@ Separately, the notification and email that tell a recipient about a share are b
Recipients are emailed by default and opt out with the unsubscribe link the mail carries; a deployment can turn share email off entirely with `share_email_notifications: false`.
Over these, **the share still succeeds** — only the announcement is dropped. The recipient's notification is kept up to date either way, and folds several senders into one ("alice and bob shared 5 items with you"), so nothing is lost; it just doesn't interrupt them again. Emails are additionally batched: everything triggered for one recipient within a 90-second window goes as a single digest message. Recipients can also refuse shares outright — from one sender, or from everyone — which fails that sender's `share` call with `recipient_not_accepting_shares`. Both are managed from **Settings → Security → Blocked people**.
Over these, **the share still succeeds** — only the announcement is dropped. The recipient's notification is kept up to date either way, and folds several senders into one ("alice and bob shared 5 items with you"), so nothing is lost; it just doesn't interrupt them again. Emails are additionally batched: everything triggered for one recipient within a 5-second window goes as a single digest message. Recipients can also refuse shares outright — from one sender, or from everyone — which fails that sender's `share` call with `recipient_not_accepting_shares`. Both are managed from **Settings → Security → Blocked people**.
### Teams and teams