From b6e40b9a2ac48571fcb11a9a0688f61cecec56f4 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:24:02 +0200 Subject: [PATCH] feat(email): global signature footer from the business profile (#1264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The business profile already carried the operator's full issuer block — address, phone, email, website, VAT id — but none of it reached an email. Those columns only fed the quote/invoice PDF renderer, so every outgoing mail footer was the fixed logo + company name + copyright line. The signature is rendered by wrapEmailHtml and nowhere else, so no template, no per-type send path and no queue row needed a change. Two new columns on business_profile (migration 198) carry the toggle and one free-text legal line; everything else is read from the address fields the operator already maintains. Default off, with a test pinning that the disabled path is byte-identical to a no-profile install. Includes three rounds of external review fixes: the plain-text MIME part also carries the signature; string booleans ('false'/'0') no longer invert the toggle; the status line stays silent rather than asserting "off" while unauthorised or loading; and the preview's Text tab mirrors the send path's htmlToText fallback. Manual Messages replies deliberately keep no signature — they bypass the wrapper by design — and the UI copy names that exception. Closes #1264 (Part A) --- .../businessProfileSignature.test.js | 144 +++++++++ .../integration/emailSignatureFooter.test.js | 276 ++++++++++++++++++ .../198_business_profile_email_signature.js | 46 +++ backend/src/routes/adminBusinessProfile.js | 14 + backend/src/routes/adminEmail.js | 26 +- .../src/services/businessProfileService.js | 120 +++++++- backend/src/services/emailProcessor.js | 175 ++++++++++- backend/src/services/guestRecoveryService.js | 5 +- frontend/src/i18n/locales/de.json | 19 +- frontend/src/i18n/locales/en.json | 19 +- frontend/src/pages/admin/EmailConfigPage.tsx | 41 +++ .../settings/SettingsBusinessProfilePage.tsx | 112 ++++++- .../__tests__/emailSignatureCard.test.tsx | 164 +++++++++++ .../src/services/businessProfile.service.ts | 10 + 14 files changed, 1155 insertions(+), 16 deletions(-) create mode 100644 backend/__tests__/integration/businessProfileSignature.test.js create mode 100644 backend/__tests__/integration/emailSignatureFooter.test.js create mode 100644 backend/migrations/core/198_business_profile_email_signature.js create mode 100644 frontend/src/pages/admin/settings/__tests__/emailSignatureCard.test.tsx diff --git a/backend/__tests__/integration/businessProfileSignature.test.js b/backend/__tests__/integration/businessProfileSignature.test.js new file mode 100644 index 00000000..9a7a3c19 --- /dev/null +++ b/backend/__tests__/integration/businessProfileSignature.test.js @@ -0,0 +1,144 @@ +/** + * PUT /api/admin/business-profile — email signature fields (migration 198). + * + * The two new columns are boolean + free text, which is exactly the shape + * that goes wrong quietly: `optional({ values: 'falsy' })` on the boolean + * would silently drop `false`, leaving the admin unable to switch the + * signature back off. The round-trip below is what pins that. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-bpsig-test-')); +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'db.sqlite'); +process.env.STORAGE_PATH = path.join(tmpDir, 'storage'); +fs.mkdirSync(process.env.STORAGE_PATH, { recursive: true }); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'bpsig-route-test-secret'; + +const request = require('supertest'); +const { + bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken, buildRouteApp, +} = require('./helpers/crmDb'); + +describe('business profile — email signature round-trip', () => { + let db; + let cleanup; + let app; + let token; + + const put = (payload) => request(app) + .put('/api/admin/business-profile') + .set('Authorization', `Bearer ${token}`) + .send(payload); + + const get = () => request(app) + .get('/api/admin/business-profile') + .set('Authorization', `Bearer ${token}`); + + // GET returns the snapshot at the top level; PUT wraps it in + // successResponse's `data` envelope. Read either. + const profileOf = (res) => (res.body.data || res.body).profile; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + const { adminId } = await seedMinimal(db); + await assignAdminRole(db, adminId, 'super_admin'); + token = mintAdminToken(adminId); + app = buildRouteApp('/api/admin/business-profile', require('../../src/routes/adminBusinessProfile')); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + it('defaults to off with an empty legal line', async () => { + const res = await get(); + expect(res.status).toBe(200); + const profile = profileOf(res); + expect(profile.emailSignatureEnabled).toBe(false); + expect(profile.emailSignatureExtra).toBe(''); + }); + + it('persists the toggle and the legal line', async () => { + const res = await put({ + emailSignatureEnabled: true, + emailSignatureExtra: 'Handelsregister Vaduz FL-0002.123.456-7', + }); + expect(res.status).toBe(200); + + const profile = profileOf(await get()); + expect(profile.emailSignatureEnabled).toBe(true); + expect(profile.emailSignatureExtra).toBe('Handelsregister Vaduz FL-0002.123.456-7'); + }); + + it('switches the toggle back off — `false` is not dropped as falsy', async () => { + await put({ emailSignatureEnabled: true }); + const res = await put({ emailSignatureEnabled: false }); + expect(res.status).toBe(200); + + expect(profileOf(await get()).emailSignatureEnabled).toBe(false); + }); + + it('clears the legal line with an empty string', async () => { + await put({ emailSignatureExtra: 'something' }); + const res = await put({ emailSignatureExtra: '' }); + expect(res.status).toBe(200); + + expect(profileOf(await get()).emailSignatureExtra).toBe(''); + }); + + it('trims surrounding whitespace off the legal line', async () => { + await put({ emailSignatureExtra: ' Registered in Vaduz ' }); + expect(profileOf(await get()).emailSignatureExtra).toBe('Registered in Vaduz'); + }); + + // Codex review: express-validator's isBoolean() accepts the STRINGS + // 'false' and '0', and Boolean('false') is true — so a form-encoded client + // trying to switch the signature OFF switched it on instead. + it.each([['false'], ['0']])('treats the string %s as off, not on', async (value) => { + await put({ emailSignatureEnabled: true }); + expect(profileOf(await get()).emailSignatureEnabled).toBe(true); + + const res = await put({ emailSignatureEnabled: value }); + expect(res.status).toBe(200); + expect(profileOf(await get()).emailSignatureEnabled).toBe(false); + }); + + it.each([['true'], ['1']])('treats the string %s as on', async (value) => { + await put({ emailSignatureEnabled: false }); + const res = await put({ emailSignatureEnabled: value }); + expect(res.status).toBe(200); + expect(profileOf(await get()).emailSignatureEnabled).toBe(true); + }); + + it('rejects a non-boolean toggle and a legal line over 500 chars', async () => { + expect((await put({ emailSignatureEnabled: 'yes please' })).status).toBe(400); + expect((await put({ emailSignatureExtra: 'x'.repeat(501) })).status).toBe(400); + }); + + it('does not let an unmapped column ride in on the payload', async () => { + // ALLOWED_PROFILE_FIELDS is the whitelist; the route's camel→snake map + // is the second gate. Neither should pass a raw snake_case key through. + const before = await db('business_profile').where({ id: 1 }).first(); + await put({ email_signature_enabled: true, id: 999 }); + const after = await db('business_profile').where({ id: 1 }).first(); + + expect(after.id).toBe(before.id); + expect(after.email_signature_enabled).toBe(before.email_signature_enabled); + }); + + it('invalidates the wrapper signature cache on write', async () => { + const { wrapEmailHtml } = require('../../src/services/emailProcessor'); + + await put({ emailSignatureEnabled: false }); + expect(await wrapEmailHtml('
x
', 'S')).not.toContain('Bahnhofstrasse 9'); + + // Same request cycle, well inside the 60 s memo window: the PUT must + // clear the cache or the operator sees a stale footer for a minute. + await put({ emailSignatureEnabled: true, addressLine1: 'Bahnhofstrasse 9' }); + expect(await wrapEmailHtml('x
', 'S')).toContain('Bahnhofstrasse 9'); + }); +}); diff --git a/backend/__tests__/integration/emailSignatureFooter.test.js b/backend/__tests__/integration/emailSignatureFooter.test.js new file mode 100644 index 00000000..9b7c8be3 --- /dev/null +++ b/backend/__tests__/integration/emailSignatureFooter.test.js @@ -0,0 +1,276 @@ +/** + * Global email footer signature (migration 198, issue #1264). + * + * The signature is rendered by `wrapEmailHtml` and nowhere else, which is + * the whole point of the design: every template, preview, test mail and + * manual send passes through that one wrapper, so none of them needed a + * per-template change. These tests pin that contract at the wrapper. + * + * The load-bearing case is the DISABLED one — an upgraded install must keep + * a byte-identical footer until an admin opts in. + */ + +const { bootCrmDb } = require('./helpers/crmDb'); + +describe('wrapEmailHtml — business-profile signature footer', () => { + let db; + let cleanup; + let wrapEmailHtml; + let renderEmailSignatureText; + let businessProfileService; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + ({ wrapEmailHtml, renderEmailSignatureText } = require('../../src/services/emailProcessor')); + businessProfileService = require('../../src/services/businessProfileService'); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + beforeEach(async () => { + await db('business_profile').where({ id: 1 }).update({ + company_name: null, + address_line1: null, + address_line2: null, + postal_code: null, + city: null, + country_code: null, + country_name: null, + phone: null, + mobile: null, + email: null, + website: null, + vat_id: null, + email_signature_enabled: false, + email_signature_extra: null, + }); + businessProfileService.invalidateEmailSignatureCache(); + }); + + const fullProfile = { + company_name: 'Müller Fotografie GmbH', + address_line1: 'Bahnhofstrasse 1', + address_line2: 'Postfach 42', + postal_code: '9494', + city: 'Schaan', + country_code: 'li', + country_name: 'Liechtenstein', + phone: '+41 79 123 45 67', + mobile: '+41 78 000 11 22', + email: 'hello@example.com', + website: 'example.com', + vat_id: 'CHE-123.456.789', + email_signature_enabled: true, + email_signature_extra: 'Handelsregister Vaduz\nFL-0002.123.456-7', + }; + + async function enable(overrides = {}) { + await db('business_profile').where({ id: 1 }).update({ ...fullProfile, ...overrides }); + businessProfileService.invalidateEmailSignatureCache(); + } + + it('renders nothing extra when the toggle is off', async () => { + // Profile fully populated, signature switched OFF: the operator's + // address must not leak into mail just because they filled in the + // invoice issuer block. + await enable({ email_signature_enabled: false }); + + const html = await wrapEmailHtml('Body
', 'Subject'); + + expect(html).not.toContain('Bahnhofstrasse 1'); + expect(html).not.toContain('hello@example.com'); + expect(html).not.toContain('CHE-123.456.789'); + }); + + it('produces a byte-identical footer to a no-profile install when disabled', async () => { + const withEmptyProfile = await wrapEmailHtml('Body
', 'Subject'); + await enable({ email_signature_enabled: false }); + const withDisabledSignature = await wrapEmailHtml('Body
', 'Subject'); + + expect(withDisabledSignature).toBe(withEmptyProfile); + }); + + it('renders address, contacts, VAT id and the legal line when enabled', async () => { + await enable(); + + const html = await wrapEmailHtml('Body
', 'Subject'); + + expect(html).toContain('Müller Fotografie GmbH'); + expect(html).toContain('Bahnhofstrasse 1'); + expect(html).toContain('Postfach 42'); + // "LI-9494 Schaan / Liechtenstein" — same shape as the PDF issuer block. + expect(html).toContain('LI-9494 Schaan / Liechtenstein'); + expect(html).toContain('VAT ID: CHE-123.456.789'); + expect(html).toContain('Handelsregister VaduzBody
', 'Subject'); + + // Separators stripped from the tel: href, kept in the visible text. + expect(html).toContain('href="tel:+41791234567"'); + expect(html).toContain('href="tel:+41780001122"'); + expect(html).toContain('href="mailto:hello@example.com"'); + // A bare hostname is promoted to https:// rather than left relative. + expect(html).toContain('href="https://example.com"'); + }); + + it('keeps an already-absolute website URL as typed', async () => { + await enable({ website: 'http://legacy.example.org/studio' }); + + const html = await wrapEmailHtml('Body
', 'Subject'); + + expect(html).toContain('href="http://legacy.example.org/studio"'); + }); + + it('neutralises a javascript: website into an inert https URL', async () => { + await enable({ website: 'javascript:alert(1)' }); + + const html = await wrapEmailHtml('Body
', 'Subject'); + + expect(html).not.toContain('href="javascript:'); + expect(html).toContain('href="https://javascript:alert(1)"'); + }); + + it('HTML-escapes every signature field', async () => { + await enable({ + company_name: '', + address_line1: 'Rue "des" Fleurs & Co', + vat_id: 'Body
', 'Subject'); + + // Escaped, so the markup is inert text — the tags never open. + expect(html).not.toContain(''); + expect(html).not.toContain(''); + expect(html).not.toContain('Body
', 'Subject'); + + expect(html.match(/Müller Fotografie GmbH/g).length).toBe( + // header alt, footer alt, footer name line, copyright line — the + // signature must not add a fifth. + (await wrapEmailHtml('Body
', 'Subject', 'en')).match(/Müller Fotografie GmbH/g).length + ); + expect(html.match(/Müller Fotografie GmbH/g).length).toBe(4); + + await db('app_settings').where({ setting_key: 'branding_company_name' }).del(); + }); + + it('uses the German VAT label for a German mail', async () => { + await enable(); + + const de = await wrapEmailHtml('Body
', 'Subject', 'de'); + const en = await wrapEmailHtml('Body
', 'Subject', 'en'); + + expect(de).toContain('USt-IdNr.: CHE-123.456.789'); + expect(en).toContain('VAT ID: CHE-123.456.789'); + }); + + it('omits empty fields instead of rendering blank rows', async () => { + await enable({ + address_line2: null, mobile: null, website: null, vat_id: null, email_signature_extra: null, + }); + + const html = await wrapEmailHtml('Body
', 'Subject'); + + expect(html).toContain('hello@example.com'); + expect(html).not.toContain('VAT ID:'); + expect(html).not.toMatch(/·\s*·/); + }); + + it('renders no signature block when enabled but the profile is blank', async () => { + await db('business_profile').where({ id: 1 }).update({ email_signature_enabled: true }); + businessProfileService.invalidateEmailSignatureCache(); + + const html = await wrapEmailHtml('Body
', 'Subject'); + + // The signature${escapeHtml(signature.companyName)}
`); + } + + // A literal middle dot, not `·`: the plain-text part of every mail + // is derived from this HTML by htmlToText, which decodes only the five + // core entities — an `·` would survive verbatim into the text body. + if (signature.addressLines.length) { + rows.push(`${signature.addressLines.map(escapeHtml).join(' \u00b7 ')}
`); + } + + const contact = []; + for (const number of [signature.phone, signature.mobile]) { + const href = signatureTelHref(number); + if (href) contact.push(renderSignatureLink(href, number, mutedTextColor)); + } + if (signature.email) { + contact.push(renderSignatureLink(`mailto:${signature.email}`, signature.email, mutedTextColor)); + } + const website = signatureWebsiteHref(signature.website); + if (website) contact.push(renderSignatureLink(website, signature.website, mutedTextColor)); + if (contact.length) { + rows.push(`${contact.join(' \u00b7 ')}
`); + } + + if (signature.vatId) { + const label = SIGNATURE_VAT_LABELS[language] || SIGNATURE_VAT_LABELS.en; + rows.push(`${escapeHtml(label)}: ${escapeHtml(signature.vatId)}
`); + } + + // Free text (Handelsregister line, disclaimer, …). Plain text, never + // HTML — escaped, then newlines become${extra}
`); + } + + if (!rows.length) return ''; + + return ` +