fix(newsletters): make the warning's duration and queue claim honest

Two review findings, both about the warning saying things that are not
true.

The duration contradicted the rate beside it. `rate` came from the live
draft while `minutes` came from the resolution the server computed from
the SAVED rate, so any unsaved edit produced a mismatched pair: 120
recipients switched from 10/min to 1/min still claimed 12 minutes rather
than 120. Recomputed from the rate the send will actually use — queueing
persists the draft first — with the server's own formula and the same
1..10 clamp, so the two numbers cannot disagree.

The queue claim was overstated. "other email queues behind it" is only
true when the campaign saturates the queue: scheduled_at is staggered
and processEmailQueue excludes future rows before taking its batch, so a
campaign paced below the 10/minute ceiling leaves capacity for a
password reset on the next tick. Now says other email CAN be delayed
behind it, which is what the shared queue actually guarantees.

The new test fails against the previous pairing.
This commit is contained in:
Paul Nothaft
2026-09-05 14:20:49 +02:00
parent 49197be329
commit 7b4a65ecc7
4 changed files with 52 additions and 10 deletions
+1 -1
View File
@@ -6978,7 +6978,7 @@
"title": "Großer Versand — prüfen Sie zuerst Ihre Versandreputation", "title": "Großer Versand — prüfen Sie zuerst Ihre Versandreputation",
"body": "{{count}} Personen auf einmal von einer Domain anzuschreiben, die sonst nur Transaktionsmails versendet, lässt Spamfilter aufmerksam werden. Anbieter können den gesamten Versand drosseln, als Spam einstufen oder blockieren — und ein schlechter Lauf beeinträchtigt auch die Zustellung Ihrer Galerie-E-Mails.", "body": "{{count}} Personen auf einmal von einer Domain anzuschreiben, die sonst nur Transaktionsmails versendet, lässt Spamfilter aufmerksam werden. Anbieter können den gesamten Versand drosseln, als Spam einstufen oder blockieren — und ein schlechter Lauf beeinträchtigt auch die Zustellung Ihrer Galerie-E-Mails.",
"advice": "Stellen Sie sicher, dass SPF, DKIM und DMARC für Ihre Versanddomain eingerichtet sind, senden Sie sich zuerst einen Test, und teilen Sie eine erste Kampagne nach Möglichkeit in mehrere kleinere Sendungen auf.", "advice": "Stellen Sie sicher, dass SPF, DKIM und DMARC für Ihre Versanddomain eingerichtet sind, senden Sie sich zuerst einen Test, und teilen Sie eine erste Kampagne nach Möglichkeit in mehrere kleinere Sendungen auf.",
"duration": "Bei {{rate}}/Minute dauert dies etwa {{minutes}} Minuten; andere E-Mails — Galerie-Einladungen, Passwort-Zurücksetzungen — warten so lange in der Warteschlange." "duration": "Bei {{rate}}/Minute dauert dies etwa {{minutes}} Minuten. Die Versand-Warteschlange wird geteilt: Während der Kampagne können andere E-Mails — Galerie-Einladungen, Passwort-Zurücksetzungen — dahinter verzögert werden."
} }
} }
} }
+1 -1
View File
@@ -6977,7 +6977,7 @@
"title": "Large send — check your sending reputation first", "title": "Large send — check your sending reputation first",
"body": "Mailing {{count}} people at once from a domain that usually sends only transactional email is what makes spam filters take notice. Providers may throttle, junk or block the whole batch, and a bad run damages delivery of your gallery emails too.", "body": "Mailing {{count}} people at once from a domain that usually sends only transactional email is what makes spam filters take notice. Providers may throttle, junk or block the whole batch, and a bad run damages delivery of your gallery emails too.",
"advice": "Confirm SPF, DKIM and DMARC are set up for your sending domain, send yourself a test first, and consider splitting a first campaign across several smaller sends.", "advice": "Confirm SPF, DKIM and DMARC are set up for your sending domain, send yourself a test first, and consider splitting a first campaign across several smaller sends.",
"duration": "At {{rate}}/minute this takes about {{minutes}} minutes, and other email — gallery invitations, password resets — queues behind it." "duration": "At {{rate}}/minute this takes about {{minutes}} minutes. The send queue is shared, so while it runs other email — gallery invitations, password resets — can be delayed behind it."
} }
} }
} }
@@ -38,6 +38,14 @@ import { usePermissions } from '../../../contexts/PermissionsContext';
*/ */
const LARGE_SEND_THRESHOLD = 50; const LARGE_SEND_THRESHOLD = 50;
/**
* Queue throughput ceiling, mirroring newsletterService.clampRate. The server
* clamps the stored rate to this, so the estimate has to clamp identically or
* it would promise a speed the queue cannot deliver.
*/
const MIN_RATE_PER_MINUTE = 1;
const MAX_RATE_PER_MINUTE = 10;
/** Variables the server substitutes per recipient. */ /** Variables the server substitutes per recipient. */
const VARIABLES = [ const VARIABLES = [
'customer_name', 'first_name', 'last_name', 'salutation', 'customer_name', 'first_name', 'last_name', 'salutation',
@@ -181,6 +189,21 @@ export const NewsletterComposerPage: React.FC = () => {
// A campaign with no subject, no body or nobody to send to must not be // A campaign with no subject, no body or nobody to send to must not be
// sendable — the button is the last place to catch that before 2 000 // sendable — the button is the last place to catch that before 2 000
// people get a blank email. // people get a blank email.
// The rate the send will actually use: queueing persists the draft first,
// so an edited rate is the one that takes effect. `estimatedMinutes` from
// the resolution is computed from the SAVED rate, so pairing the two showed
// a contradiction after any unsaved edit — 120 recipients switched from
// 10/min to 1/min still claimed 12 minutes instead of 120. Recomputed here
// with the server's own formula (adminNewsletters.js: ceil(count / rate)).
const effectiveRate = Math.min(
MAX_RATE_PER_MINUTE,
Math.max(MIN_RATE_PER_MINUTE, Number(draft?.sendRatePerMinute) || MAX_RATE_PER_MINUTE)
);
const estimatedMinutes = Math.max(
1,
Math.ceil((resolution?.recipientCount ?? 0) / effectiveRate)
);
const canQueue = useMemo(() => Boolean( const canQueue = useMemo(() => Boolean(
draft draft
&& draft.status === 'draft' && draft.status === 'draft'
@@ -445,12 +468,10 @@ export const NewsletterComposerPage: React.FC = () => {
</p> </p>
<p> <p>
{t('newsletters.largeSend.duration', {t('newsletters.largeSend.duration',
'At {{rate}}/minute this takes about {{minutes}} minutes, and other ' 'At {{rate}}/minute this takes about {{minutes}} minutes. The send '
+ 'email — gallery invitations, password resets — queues behind it.', + 'queue is shared, so while it runs other email — gallery '
{ + 'invitations, password resets — can be delayed behind it.',
rate: draft.sendRatePerMinute, { rate: effectiveRate, minutes: estimatedMinutes })}
minutes: resolution?.estimatedMinutes ?? 1,
})}
</p> </p>
</div> </div>
</div> </div>
@@ -162,9 +162,30 @@ describe('newsletter composer', () => {
expect(warning).toHaveTextContent(/sending reputation/i); expect(warning).toHaveTextContent(/sending reputation/i);
expect(warning).toHaveTextContent(/spam filters/i); expect(warning).toHaveTextContent(/spam filters/i);
expect(warning).toHaveTextContent(/SPF, DKIM and DMARC/i); expect(warning).toHaveTextContent(/SPF, DKIM and DMARC/i);
// The operator is told what it costs: duration, and what waits behind it. // The operator is told what it costs: duration, and what may wait behind
// it. 120 recipients at the fixture's 20/min clamps to the queue's real
// ceiling of 10/min, so the honest estimate is 12 minutes.
expect(warning).toHaveTextContent(/12 minutes/); expect(warning).toHaveTextContent(/12 minutes/);
expect(warning).toHaveTextContent(/queues behind it/i); expect(warning).toHaveTextContent(/can be delayed behind it/i);
});
it('recomputes the duration when the rate is edited, before saving', async () => {
// The estimate the server returns is computed from the SAVED rate, while
// the input shows the edited one. Pairing them meant the warning could
// claim 12 minutes for a send that would actually take 120.
resolution = { ...resolution, recipientCount: 120, estimatedMinutes: 12 };
renderComposer();
await screen.findByTestId('recipient-summary');
await screen.findByTestId('large-send-warning');
const rate = screen.getByLabelText(/Send rate/i);
await userEvent.clear(rate);
await userEvent.type(rate, '1');
await waitFor(() => {
expect(screen.getByTestId('large-send-warning')).toHaveTextContent(/At 1\/minute/);
expect(screen.getByTestId('large-send-warning')).toHaveTextContent(/120 minutes/);
});
}); });
it('does not warn on a send small enough not to matter', async () => { it('does not warn on a send small enough not to matter', async () => {