From ac3f6e91f53903fa8311d8a27fc0d4006c301275 Mon Sep 17 00:00:00 2001 From: synAdmin Date: Tue, 8 Sep 2026 10:49:30 -0500 Subject: [PATCH 1/4] feat(analytics): report consenting customers' activity to Brevo (#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loads Brevo's web tracker for a signed-in customer who has consented, reports route changes as page views, and tracks the three events the issue asked for: added_to_cart, favorited, and checkout_completed. The four design questions were settled on the issue in August and this implements those answers. The consent gate is the part worth reading. The decision recorded on the issue was "gate it behind consent", but the sentence customers actually agreed to named only email: "I want to receive occasional emails about new one-of-a-kind items". Gating a tracker on `marketing_consent` while that was the stored wording would have treated "email me about new items" as authorisation to send someone's browsing to a third party, which it does not say — and this project stores the wording verbatim against each customer precisely so that a record says what the customer saw. So the sentence is widened here, and the tracker is gated on `analytics_consent`, a field the server computes by comparing the wording stored against a customer with the current constant. Changing the sentence therefore does not retroactively widen anybody's consent: everyone who agreed to the old text keeps their email consent and is not tracked until they re-consent through the account page. A boolean alone could not tell those two populations apart, which is the whole reason the text is stored per customer. `analyticsConsent` is exported and has its own unit test, because "agreeing to the old wording does not authorise tracking" is the rule that silently tracks people if it regresses — their flag really is true. QA stays out of the live Brevo account by construction rather than by remembering. The key is per-environment, the tracker never loads without one, and `docker-compose.qa.yml` sets an empty literal with no stack variable behind it, so nothing can inherit a value from the host or be pasted in from production's stack. Same reasoning as QA_DB_PASSWORD and the QA_SMTP_ names beside it. Events are reported from the API layer rather than the UI call sites, so no caller can add to the cart or favorite an item without it being counted, and each fires only after the response was accepted — a refused add is not reported as one. The two checkout completions each name their processor, because a demo purchase charges nothing and counting it as a sale would overstate revenue. The privacy policy gains an analytics section in this change rather than a follow-up, since the published policy previously described none of this and would otherwise have lagged the code. It is deliberate about the limits: withdrawing consent stops further reporting, but anything already sent stays with Brevo, and a script already injected cannot be un-injected — `stopBrevoTracking` stops calls, it does not unload sa.js. That is said in the code too, because "tracking stops" reads as a stronger promise than any web tracker can make. Verified: backend tsc clean, both lint suites 0 errors with no new warnings, 474 unit tests passing across 33 suites, and the frontend production build green including the compose-environment guard. Not verified: integration and e2e, which need a database and a Node this machine does not have active, and no real Brevo key was exercised — the tracker has never been observed reporting to an actual account. Closes #56 Co-Authored-By: Claude Opus 5 --- backend/src/app.ts | 11 +- backend/src/routes/customers.ts | 37 +++++- backend/src/utils.ts | 17 ++- backend/tests/unit/analyticsConsent.test.ts | 63 +++++++++ docker-compose.prod.yml | 11 ++ docker-compose.qa.yml | 14 ++ frontend/src/BrevoTracking.tsx | 71 ++++++++++ frontend/src/api.ts | 6 + frontend/src/brevo.ts | 135 ++++++++++++++++++++ frontend/src/cart/cartApi.ts | 29 ++++- frontend/src/customer/AuthForm.tsx | 7 +- frontend/src/customer/PrivacyPolicy.tsx | 16 +++ frontend/src/customer/customerApi.ts | 10 ++ frontend/src/customer/favoritesApi.ts | 8 ++ frontend/src/main.tsx | 5 + frontend/tests/e2e/auth.spec.ts | 7 +- 16 files changed, 439 insertions(+), 8 deletions(-) create mode 100644 backend/tests/unit/analyticsConsent.test.ts create mode 100644 frontend/src/BrevoTracking.tsx create mode 100644 frontend/src/brevo.ts diff --git a/backend/src/app.ts b/backend/src/app.ts index e4acc46..2e1f926 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -60,7 +60,16 @@ app.get('/api/config', (_req, res) => { // // Trailing slash trimmed so callers can join with a stored path, which // always begins with one, without producing a double. - uploadsBaseUrl: trimTrailingSlashes(process.env.UPLOADS_BASE_URL ?? '') + uploadsBaseUrl: trimTrailingSlashes(process.env.UPLOADS_BASE_URL ?? ''), + // Brevo's Marketing Automation key (#56). Not a secret — it ships to the + // browser by design — but it differs per environment, which is the whole + // reason it is here rather than built in. + // + // Null when unset, and the tracker never loads without it. That is what + // keeps QA out of production's Brevo account: QA sets no key, so no QA + // browsing is ever reported, and there is no flag anyone can forget to + // turn off. Same shape as paypalClientId above. + brevoTrackerKey: process.env.BREVO_TRACKER_KEY?.trim() || null }); }); diff --git a/backend/src/routes/customers.ts b/backend/src/routes/customers.ts index f9b8477..170278e 100755 --- a/backend/src/routes/customers.ts +++ b/backend/src/routes/customers.ts @@ -176,7 +176,39 @@ interface CustomerOrderRow { item_name: string; } -function publicCustomer(c: CustomerRow) { +/** + * Whether this customer has agreed to the *current* consent wording, which is + * the only thing that authorises the Brevo tracker (#56). + * + * Not the same question as `marketing_consent`. That flag says the customer + * agreed to something; `marketing_consent_text` says what. The sentence was + * widened to cover analytics, so a customer who consented to the older wording + * agreed to emails and nothing more — they keep receiving email and are not + * tracked until they re-consent to the current text through the account page. + * + * Comparing the stored string is the point rather than an implementation + * detail: it is why the wording is recorded per customer at all. A boolean on + * its own could not tell these two populations apart, and assuming they are the + * same is exactly the retroactive widening this avoids. + * + * Computed here rather than stored, so it can never drift from the constant. + * + * Exported for the unit test, and narrowed to the two fields it actually reads + * rather than taking a whole CustomerRecord — the rule is about those two and + * nothing else, and a test should not have to invent a customer to state it. + */ +export function analyticsConsent( + c: Pick +): boolean { + return c.marketing_consent && c.marketing_consent_text === MARKETING_CONSENT_TEXT; +} + +/** + * Takes a CustomerRecord rather than a CustomerRow because `analytics_consent` + * is derived from `marketing_consent_text`, which is not on the narrower type. + * Every caller already holds a full record — each query is `SELECT *`. + */ +function publicCustomer(c: CustomerRecord) { return { id: c.id, email: c.email, @@ -184,6 +216,9 @@ function publicCustomer(c: CustomerRow) { last_name: c.last_name, email_verified: c.email_verified, marketing_consent: c.marketing_consent, + // Deliberately separate from marketing_consent: the two disagree for every + // customer who consented before the wording was widened. + analytics_consent: analyticsConsent(c), favorite_alerts: c.favorite_alerts, created_at: c.created_at }; diff --git a/backend/src/utils.ts b/backend/src/utils.ts index 9c64e76..ab19c3e 100755 --- a/backend/src/utils.ts +++ b/backend/src/utils.ts @@ -71,8 +71,23 @@ export function tagColorFor(name: string): string { return TAG_COLORS[Math.abs(hash) % TAG_COLORS.length] ?? TAG_COLORS[0]; } +/** + * Widened for #56 to cover analytics as well as email. + * + * The previous wording named only emails. Gating the Brevo tracker on + * `marketing_consent` while that sentence was the thing customers agreed to + * would have treated "email me about new items" as authorisation to send their + * browsing to a third party, which it plainly did not say. + * + * Changing the sentence does not retroactively widen anyone's consent, because + * `marketing_consent_text` records what each customer was actually shown. + * Everyone who agreed to the old wording keeps their email consent and is not + * tracked; see `analyticsConsent` in routes/customers.ts, which is what the + * tracker is gated on. That is the whole reason this string is stored per + * customer rather than assumed. + */ export const MARKETING_CONSENT_TEXT = - 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.'; + 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs, and I agree that what I browse and buy here may be shared with our email provider to help choose what they contain. I can unsubscribe at any time.'; /** * Strips trailing slashes so a base URL can be joined with a stored path. diff --git a/backend/tests/unit/analyticsConsent.test.ts b/backend/tests/unit/analyticsConsent.test.ts new file mode 100644 index 0000000..ea2808a --- /dev/null +++ b/backend/tests/unit/analyticsConsent.test.ts @@ -0,0 +1,63 @@ +import { analyticsConsent } from '../../src/routes/customers'; +import { MARKETING_CONSENT_TEXT } from '../../src/utils'; + +/** + * The rule this file exists for: agreeing to the *old* consent wording does not + * authorise the Brevo tracker (#56). + * + * The sentence customers agree to was widened to mention analytics. Everyone + * who consented before that agreed to a sentence about email and nothing else, + * and `marketing_consent` alone cannot tell the two populations apart — which + * is exactly why the wording is stored per customer. Getting this wrong would + * silently track people who never agreed to it, and would do so invisibly, + * because the flag they set really is `true`. + */ +describe('analyticsConsent', () => { + const OLD_WORDING = + 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.'; + + it('is true for a customer who agreed to the current wording', () => { + expect( + analyticsConsent({ marketing_consent: true, marketing_consent_text: MARKETING_CONSENT_TEXT }) + ).toBe(true); + }); + + it('is false for a customer who agreed to the previous, email-only wording', () => { + // The case the whole mechanism exists for. They consented, and their + // consent does not cover this. + expect( + analyticsConsent({ marketing_consent: true, marketing_consent_text: OLD_WORDING }) + ).toBe(false); + }); + + it('is false when consent was never given, whatever text is stored', () => { + expect( + analyticsConsent({ marketing_consent: false, marketing_consent_text: MARKETING_CONSENT_TEXT }) + ).toBe(false); + }); + + it('is false when no wording was recorded at all', () => { + // Withdrawal writes a sentinel rather than the consent text, and older rows + // may predate the column being populated. Neither is agreement. + expect( + analyticsConsent({ marketing_consent: true, marketing_consent_text: null }) + ).toBe(false); + }); + + it('does not accept a near-miss, so a reworded sentence re-asks rather than assumes', () => { + expect( + analyticsConsent({ + marketing_consent: true, + marketing_consent_text: `${MARKETING_CONSENT_TEXT} ` + }) + ).toBe(false); + }); + + it('guards the wording itself: the current text must mention what is shared', () => { + // Not a tautology — it fails if someone narrows the sentence back to email + // while leaving the tracker gated on it, which would put this project back + // in the position #56 was filed to get it out of. + expect(MARKETING_CONSENT_TEXT).toContain('browse'); + expect(MARKETING_CONSENT_TEXT).not.toBe(OLD_WORDING); + }); +}); diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index b73b560..f4efaec 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -244,6 +244,17 @@ services: # See docs/ops/image-background-removal-stack.md. - REMBG_URL=${REMBG_URL:-} + # Brevo's Marketing Automation key (#56), from + # https://app.brevo.com/automation/parameters. Optional: unset means the + # tracker is never loaded and no browsing is reported to anyone. + # + # Not a secret — it ships to the browser by design — but it is + # per-environment, and this is the only place production's is named. A + # key here is still not sufficient to track anybody: the script loads + # only for a signed-in customer whose stored consent wording covers + # analytics. See the note on MARKETING_CONSENT_TEXT in backend/src/utils.ts. + - BREVO_TRACKER_KEY=${BREVO_TRACKER_KEY:-} + # Signs the regenerate and discard links in the intake notification email # (#224). Optional: absent, the notification still sends and links to the # review queue without shortcuts. Rotating it revokes every outstanding diff --git a/docker-compose.qa.yml b/docker-compose.qa.yml index 5e084a5..963d25d 100644 --- a/docker-compose.qa.yml +++ b/docker-compose.qa.yml @@ -174,6 +174,20 @@ services: # unset stack variable cannot fail a deploy. - REMBG_URL=${QA_REMBG_URL:-} + # Deliberately empty, and deliberately not a stack variable (#56). + # + # This is what keeps QA browsing out of the live Brevo account. Written as + # an empty literal rather than left out entirely so it cannot inherit a + # value from the host environment, and with no `${...}` so there is no + # stack variable anyone could set here by pasting production's in — the + # same reasoning as QA_DB_PASSWORD and the QA_SMTP_* names above. + # + # QA runs against disposable fixtures. Reporting that browsing as though + # it were customer behaviour would corrupt the segmentation the tracker + # exists to feed, and it would be indistinguishable from real traffic + # after the fact. + - BREVO_TRACKER_KEY= + # Signs the regenerate and discard links in the intake notification email # (#224). Optional: absent, the notification still sends and simply links # to the review queue without shortcuts. Anyone holding a link can act on diff --git a/frontend/src/BrevoTracking.tsx b/frontend/src/BrevoTracking.tsx new file mode 100644 index 0000000..fd9164c --- /dev/null +++ b/frontend/src/BrevoTracking.tsx @@ -0,0 +1,71 @@ +import { useEffect, useState } from 'react'; +import { useLocation } from 'react-router-dom'; +import { fetchConfig } from './api'; +import { useCustomerAuth } from './customer/CustomerAuthContext'; +import { brevoPage, startBrevoTracking, stopBrevoTracking } from './brevo'; + +/** + * Drives the Brevo tracker from the two things that decide whether it may run + * at all: the environment's key and the signed-in customer's consent (#56). + * + * Renders nothing. It exists as a component rather than a hook because it needs + * both the router and the auth context, and mounting it inside `AppRoutes` is + * what guarantees it sits under both — a hook called from the wrong place would + * fail at runtime instead of being impossible to misplace. + * + * ## Why the gate is a server-computed field + * + * `analytics_consent` is not `marketing_consent`. The consent sentence was + * widened to mention analytics, and the server reports this flag by comparing + * the wording each customer actually agreed to against the current text. A + * customer who consented to the older, email-only wording is not tracked. Never + * substitute `marketing_consent` here — that is the exact retroactive widening + * the field exists to prevent. + * + * A signed-out visitor has no consent record, so no key is ever used and the + * script is never injected. Anonymous browsing is not reported at all. + */ +export default function BrevoTracking() { + const { customer } = useCustomerAuth(); + const location = useLocation(); + const [trackerKey, setTrackerKey] = useState(null); + + useEffect(() => { + let cancelled = false; + // Swallowed for the same reason main.tsx swallows its warm-up call: a + // config that cannot be fetched is a broken deployment every other request + // will report, and losing analytics is not worth surfacing to a customer. + void fetchConfig() + .then(config => { if (!cancelled) setTrackerKey(config.brevoTrackerKey); }) + .catch(() => {}); + return () => { cancelled = true; }; + }, []); + + const consented = customer?.analytics_consent === true; + const email = customer?.email; + + // Start and stop are driven by the same effect so there is no state in which + // consent has gone away and nothing has acted on it. Withdrawing consent, + // signing out, and deleting the account all arrive here as `consented` + // turning false, because each one ends with the customer no longer being a + // consenting signed-in customer. + useEffect(() => { + if (consented && trackerKey && email) { + startBrevoTracking(trackerKey, email); + } else { + stopBrevoTracking(); + } + }, [consented, trackerKey, email]); + + // Declared after the effect above so that on the render where consent or the + // key first arrives, tracking is started before this reports the route. + // + // The storefront is a single-page app: without this, the tracker's own + // initial call would be the only page view it ever saw, and every customer + // would look like they viewed one page and left. + useEffect(() => { + brevoPage(location.pathname); + }, [location.pathname, consented, trackerKey]); + + return null; +} diff --git a/frontend/src/api.ts b/frontend/src/api.ts index af72130..1463440 100755 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -55,6 +55,12 @@ export interface SiteConfig { currency: string; /** Origin for uploaded images. Empty means the app's own — see uploadUrl. */ uploadsBaseUrl: string; + /** + * Brevo Marketing Automation key (#56). Null when the environment sets none, + * which is how QA avoids reporting test browsing into the live Brevo account. + * A key alone does not start tracking — see brevo.ts. + */ + brevoTrackerKey: string | null; } export async function fetchConfig(): Promise { diff --git a/frontend/src/brevo.ts b/frontend/src/brevo.ts new file mode 100644 index 0000000..7a8a707 --- /dev/null +++ b/frontend/src/brevo.ts @@ -0,0 +1,135 @@ +/** + * Brevo's web tracker (#56). + * + * Same shape as paypal.ts: the script is injected at runtime, so there is no + * package to import types from and this file declares the sliver actually used. + * Deliberately narrow — it describes what is called here, not the whole SDK. + * + * Two rules hold everywhere in this module, and every exported function is + * written so that breaking either is impossible rather than merely discouraged: + * + * 1. **Nothing loads without a key.** No key means no script, no cookie, no + * request. That is what keeps QA out of the production Brevo account. + * 2. **Nothing loads without consent.** `start` is the only thing that injects + * the script, and its caller gates on the customer's `analytics_consent`, + * which the server derives from the wording that customer actually agreed + * to. A signed-out visitor is never tracked, because there is no consent + * record to consult. + */ + +interface SendinblueSdk { + page: (name?: string, properties?: Record) => void; + identify: (email: string, attributes?: Record) => void; + track: (event: string, properties?: Record) => void; +} + +/** The queue the snippet installs so calls made before load are not lost. */ +interface SibQueue { + equeue: unknown[]; + client_key?: string; + [method: string]: unknown; +} + +declare global { + interface Window { + sendinblue?: SendinblueSdk; + sib?: SibQueue; + } +} + +/** + * Gates every call below. False until `start`, and false again after `stop`. + * + * Kept separate from `window.sendinblue` being present because the two answer + * different questions: the SDK stays in the document once injected, but consent + * can be withdrawn within the same page. Checking only for the SDK would keep + * reporting after sign-out. + */ +let enabled = false; +let loaded = false; + +/** + * Installs the method queue and injects the script. + * + * The queue matters: `sa.js` loads asynchronously and the methods have to exist + * before it arrives, or the `page()` for the landing route — the one call that + * always happens immediately — is dropped. This is Brevo's own snippet, written + * out as typed code rather than pasted as an opaque blob. + */ +function inject(key: string): void { + if (loaded) return; + loaded = true; + + const queue: SibQueue = { equeue: [], client_key: key }; + window.sib = queue; + + const sdk = {} as SendinblueSdk; + const methods = ['track', 'identify', 'page'] as const; + for (const method of methods) { + sdk[method] = (...args: unknown[]) => { + const ready = queue[method]; + if (typeof ready === 'function') { + (ready as (...a: unknown[]) => void)(...args); + } else { + queue.equeue.push({ [method]: args }); + } + }; + } + window.sendinblue = sdk; + + const script = document.createElement('script'); + script.id = 'sendinblue-js'; + script.async = true; + script.src = `https://sibautomation.com/sa.js?key=${encodeURIComponent(key)}`; + // A tracker that cannot load must never take the page down with it. There is + // no retry and no error surfaced: losing analytics is not worth telling a + // customer about, and a visible failure here would be noise on every visit + // from anyone running a blocker. + script.onerror = () => { enabled = false; }; + document.head.appendChild(script); +} + +/** + * Begins tracking for a consenting, signed-in customer. + * + * Safe to call repeatedly — React effects will. The script is injected once; + * later calls only re-assert identity, which is what a customer switching + * accounts in one session needs. + */ +export function startBrevoTracking(key: string | null, email: string): void { + if (!key) return; + inject(key); + enabled = true; + window.sendinblue?.identify(email); +} + +/** + * Stops tracking, on sign-out or when consent is withdrawn. + * + * **This stops calls, it does not unload the script.** Nothing can un-inject a + * script tag or take back the cookies it set, so `sa.js` stays in the document + * until the next full page load. What this guarantees is that no further page + * view or event is reported, and no identity is re-asserted, which is the part + * this application actually controls. Said plainly here because "tracking + * stops" is easy to read as a stronger promise than any web tracker can make. + */ +export function stopBrevoTracking(): void { + enabled = false; +} + +/** A route change. No-op unless tracking is currently permitted. */ +export function brevoPage(path: string): void { + if (!enabled) return; + window.sendinblue?.page(path); +} + +/** A named event. No-op unless tracking is currently permitted. */ +export function brevoTrack(event: string, properties?: Record): void { + if (!enabled) return; + window.sendinblue?.track(event, properties); +} + +/** Exported for tests, which need to observe the gate without a real script. */ +export function isBrevoTrackingEnabled(): boolean { + return enabled; +} diff --git a/frontend/src/cart/cartApi.ts b/frontend/src/cart/cartApi.ts index 7d0a0eb..5dfd2f5 100644 --- a/frontend/src/cart/cartApi.ts +++ b/frontend/src/cart/cartApi.ts @@ -1,3 +1,10 @@ +// Reported here rather than at the UI call sites so that no caller can add an +// item or complete a checkout without it being counted (#56). Every one of +// these calls is a no-op unless a consenting customer is signed in and the +// environment has a key, so importing this into the API layer does not make the +// cart depend on analytics being configured. +import { brevoTrack } from '../brevo'; + export interface CartItem { item_id: number; name: string; @@ -34,7 +41,14 @@ export function fetchCart(): Promise<{ items: CartItem[] }> { } export function addToCart(itemId: number): Promise<{ itemId: number; expiresAt: string }> { - return fetch(`/api/cart/items/${itemId}`, { method: 'POST' }).then(res => handle(res)); + // After handle(), which throws on a non-OK response — so a refused add (the + // item was already reserved by someone else) is not reported as one. + return fetch(`/api/cart/items/${itemId}`, { method: 'POST' }) + .then(res => handle<{ itemId: number; expiresAt: string }>(res)) + .then(result => { + brevoTrack('added_to_cart', { itemId }); + return result; + }); } export function removeFromCart(itemId: number): Promise { @@ -80,12 +94,19 @@ export function createCartPaypalOrder(shippingAddressId: number): Promise<{ orde }).then(res => handle(res)); } +// The two checkout completions are separate functions rather than one, so both +// report the event and both say which they were. Without the processor these +// would be indistinguishable in Brevo, and a demo purchase charges nothing — +// counting it as a sale would overstate revenue. QA cannot reach here anyway +// (no key), but DEMO_MODE is not exclusive to QA, so the distinction is real. export function captureCartPaypalOrder(orderID: string): Promise { return fetch('/api/checkout/cart/paypal/capture', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ orderID }) - }).then(res => handle(res)); + }) + .then(res => handle(res)) + .then(() => { brevoTrack('checkout_completed', { processor: 'paypal' }); }); } export function demoCartPurchase(shippingAddressId: number): Promise { @@ -93,5 +114,7 @@ export function demoCartPurchase(shippingAddressId: number): Promise { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ shippingAddressId }) - }).then(res => handle(res)); + }) + .then(res => handle(res)) + .then(() => { brevoTrack('checkout_completed', { processor: 'demo' }); }); } diff --git a/frontend/src/customer/AuthForm.tsx b/frontend/src/customer/AuthForm.tsx index 90751b7..bdd6732 100644 --- a/frontend/src/customer/AuthForm.tsx +++ b/frontend/src/customer/AuthForm.tsx @@ -20,8 +20,13 @@ export type AuthMode = 'register' | 'login'; // this was shared there were three wordings in play — this one, a shorter one in // the cart prompt, and the string the server actually recorded — and none of // them matched. +// +// Widened for #56 so it covers the Brevo tracker as well as email. The tracker +// is gated on whether a customer's *stored* copy of this sentence matches the +// current one, so changing it here does not retroactively widen the consent of +// anyone who agreed to the previous wording. export const MARKETING_CONSENT_TEXT = - 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.'; + 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs, and I agree that what I browse and buy here may be shared with our email provider to help choose what they contain. I can unsubscribe at any time.'; type Props = Readonly<{ mode: AuthMode; diff --git a/frontend/src/customer/PrivacyPolicy.tsx b/frontend/src/customer/PrivacyPolicy.tsx index ef4d23d..95b2c62 100755 --- a/frontend/src/customer/PrivacyPolicy.tsx +++ b/frontend/src/customer/PrivacyPolicy.tsx @@ -28,6 +28,22 @@ export default function PrivacyPolicy() { link included in every marketing email — no login required. + Analytics and tracking + + If — and only if — you have opted in to marketing, we share what you browse and buy on + this site with Brevo, our email provider, so the emails we send you are about things you + are actually interested in. That covers the pages you visit here, items you add to your + cart or favorite, and completed orders, linked to your email address. + + + If you have not opted in, this does not happen: nothing is loaded, nothing is sent, and no + tracking cookie is set. The same is true if you are not signed in — we do not track + visitors who do not have an account. Withdrawing consent from your account page stops any + further activity being sent. Note that anything already shared with Brevo before you + withdrew remains with them, and a tracking cookie set earlier in your visit stays in your + browser until you close the tab or clear it. + + Your rights You may request a copy of your data ("Download my data" in your account page), or delete your diff --git a/frontend/src/customer/customerApi.ts b/frontend/src/customer/customerApi.ts index b1f9e32..eec6e9e 100755 --- a/frontend/src/customer/customerApi.ts +++ b/frontend/src/customer/customerApi.ts @@ -7,6 +7,16 @@ export interface Customer { last_name: string | null; email_verified: boolean; marketing_consent: boolean; + /** + * Whether this customer agreed to the *current* consent wording, which is the + * only thing that authorises the Brevo tracker (#56). + * + * Not a duplicate of marketing_consent: the two disagree for anyone who + * consented before that sentence was widened to mention analytics. Computed + * on the server from the wording stored against the customer — never derive + * it here from marketing_consent, which is the mistake it exists to prevent. + */ + analytics_consent: boolean; favorite_alerts: boolean; created_at: string; } diff --git a/frontend/src/customer/favoritesApi.ts b/frontend/src/customer/favoritesApi.ts index 2691690..36f9f87 100644 --- a/frontend/src/customer/favoritesApi.ts +++ b/frontend/src/customer/favoritesApi.ts @@ -1,4 +1,7 @@ import { Customer } from './customerApi'; +// See the note in cart/cartApi.ts: reported here rather than at the call sites +// so no caller can favorite an item without it being counted (#56). +import { brevoTrack } from '../brevo'; export interface Favorite { item_id: number; @@ -26,6 +29,11 @@ export async function addFavorite(itemId: number): Promise { await fetch(`/api/customers/me/favorites/${itemId}`, { method: 'POST' }), 'failed to save favorite' ); + // After expectOk, which throws on failure, so a favorite that was not saved + // is not reported as one. Only favoriting is tracked, not un-favoriting: + // #56 asked for the act of favoriting as a signal of interest, and removal is + // a different question nobody has asked yet. + brevoTrack('favorited', { itemId }); } export async function removeFavorite(itemId: number): Promise { diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index d377603..9a5cba3 100755 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -27,6 +27,7 @@ import { FavoritesProvider } from './customer/FavoritesContext'; import { ThemeModeProvider, useThemeMode } from './theme/ThemeContext'; import './styles.css'; import { fetchConfig } from './api'; +import BrevoTracking from './BrevoTracking'; // The brand accent is monochrome, so it inverts between themes rather than // switching to a different hue. @@ -163,6 +164,10 @@ function AppRoutes() { return ( <> + {/* Renders nothing. Mounted here because this is the innermost place + that has both the router and the auth context, which is what it + needs to report route changes for a consenting customer (#56). */} + {import.meta.env.DEV && } } /> diff --git a/frontend/tests/e2e/auth.spec.ts b/frontend/tests/e2e/auth.spec.ts index b5c7335..b503b72 100755 --- a/frontend/tests/e2e/auth.spec.ts +++ b/frontend/tests/e2e/auth.spec.ts @@ -27,8 +27,13 @@ test.describe('Customer accounts', () => { // customer actually saw. Three different wordings were in circulation // before the sign-in form was shared between the routes and the cart // prompt, and none of them matched what was stored. + // Widened in #56 to cover the Brevo tracker as well as email. The literal is + // repeated here rather than imported on purpose: importing it from the app + // would make this assert that a constant equals itself, and the drift it + // guards against is exactly the rendered label parting from the stored + // string. const consent = - 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.'; + 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs, and I agree that what I browse and buy here may be shared with our email provider to help choose what they contain. I can unsubscribe at any time.'; await expect(authModal.registerDialog).toContainText(consent); }); From 955049eac9489c1f4378984fbcca85fbc741ee25 Mon Sep 17 00:00:00 2001 From: synAdmin Date: Tue, 8 Sep 2026 11:58:20 -0500 Subject: [PATCH 2/4] fix(privacy): separate analytics consent from email consent (#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit widened the marketing consent sentence to cover the Brevo tracker, so one checkbox carried both purposes. That is the specific pattern GDPR rejects: consent has to be granular, and current EDPB guidance treats bundling tracking consent with subscription consent as invalid because the customer cannot accept one purpose and refuse the other. Quebec's Law 25 s.8.1 is stricter again — profiling technology has to be off until the person switches it on, with no pre-ticked box and no consent inherited from agreeing to something else. Building to both standards was the decision, since the storefront is publicly reachable and anyone can register. So the marketing sentence is restored to exactly what it was, which leaves every existing email consent valid and untouched, and analytics gets its own column, its own sentence, its own checkbox at registration, its own toggle in the account page and its own endpoint. A customer can now hold either, both, or neither, and withdrawing one does not disturb the other. The migration defaults analytics_consent to false, which is both the honest answer — none of the existing customers was ever asked — and what Law 25 requires. Nothing about this change opts anybody in. Two details that are compliance requirements rather than wording preferences. The sentence names Brevo instead of saying "our email provider", because informed consent means the customer can tell who receives their data and a description they cannot act on is not disclosure. And the account toggle is as prominent and as easy to switch off as it is to switch on, because withdrawal has to be as easy as consenting. The analytics endpoint is separate from the marketing one rather than a second field on it, so that a single call cannot change an answer the customer did not touch — the bundling problem moved from the form into the API. The unit tests now assert the two consents stay apart in both directions, including that the marketing sentence still says nothing about tracking, because re-bundling them would otherwise pass silently and is the mistake this project already made once. Verified: backend tsc clean, both lint suites 0 errors with no new warnings, 478 unit tests passing across 33 suites, frontend production build green. Not verified: the migration has not been run against a database, and integration and e2e need a Node this machine does not have active. None of this is legal advice and the wording is worth a lawyer's eye before it ships. Refs #56 Co-Authored-By: Claude Opus 5 --- .../1787800000000_add-analytics-consent.js | 39 ++++++++ backend/src/routes/customers.ts | 70 ++++++++++---- backend/src/utils.ts | 39 +++++--- backend/tests/unit/analyticsConsent.test.ts | 96 ++++++++++++------- frontend/src/customer/Account.tsx | 28 +++++- frontend/src/customer/AuthForm.tsx | 26 +++-- frontend/src/customer/PrivacyPolicy.tsx | 27 ++++-- frontend/src/customer/customerApi.ts | 23 ++++- frontend/tests/e2e/auth.spec.ts | 17 ++-- 9 files changed, 277 insertions(+), 88 deletions(-) create mode 100644 backend/migrations/1787800000000_add-analytics-consent.js diff --git a/backend/migrations/1787800000000_add-analytics-consent.js b/backend/migrations/1787800000000_add-analytics-consent.js new file mode 100644 index 0000000..7f7a4de --- /dev/null +++ b/backend/migrations/1787800000000_add-analytics-consent.js @@ -0,0 +1,39 @@ +exports.up = (pgm) => { + pgm.sql(` + -- Consent to the Brevo tracker, separate from marketing_consent (#56). + -- + -- Separate because GDPR requires consent to be granular: email marketing + -- and behavioural tracking are two purposes with two recipients, and + -- current EDPB guidance treats bundling tracking consent with subscription + -- consent as invalid. Quebec's Law 25 s.8.1 goes further and requires + -- profiling technology to be off until the person switches it on. + -- + -- DEFAULT FALSE is the part that must not be changed. Every existing + -- customer arrives at false, which is both the honest answer — none of them + -- were ever asked — and what Law 25 requires. A default of true would + -- silently opt in the entire customer base to something nobody agreed to. + ALTER TABLE customers + ADD COLUMN IF NOT EXISTS analytics_consent BOOLEAN NOT NULL DEFAULT FALSE; + + -- When they agreed, and to exactly what wording. Same shape and same + -- reasoning as the marketing_consent pair: the stored sentence is what + -- makes the record say what the customer actually saw, so re-wording the + -- consent later cannot retroactively broaden anyone's. + -- + -- Both nullable: a customer who has never consented has no date and no + -- text, and inventing either would be a false record of consent. + ALTER TABLE customers + ADD COLUMN IF NOT EXISTS analytics_consent_at TIMESTAMPTZ; + + ALTER TABLE customers + ADD COLUMN IF NOT EXISTS analytics_consent_text TEXT; + `); +}; + +exports.down = (pgm) => { + pgm.sql(` + ALTER TABLE customers DROP COLUMN IF EXISTS analytics_consent_text; + ALTER TABLE customers DROP COLUMN IF EXISTS analytics_consent_at; + ALTER TABLE customers DROP COLUMN IF EXISTS analytics_consent; + `); +}; diff --git a/backend/src/routes/customers.ts b/backend/src/routes/customers.ts index 170278e..47db4f5 100755 --- a/backend/src/routes/customers.ts +++ b/backend/src/routes/customers.ts @@ -8,7 +8,7 @@ import { sendMail } from '../mailer'; import { renderTemplate, greeting, formatDuration } from '../emailTemplates'; import { getSettings } from '../adminSettings'; import { loadStoredTemplate } from './adminEmailTemplates'; -import { MARKETING_CONSENT_TEXT, isValidEmail } from '../utils'; +import { ANALYTICS_CONSENT_TEXT, MARKETING_CONSENT_TEXT, isValidEmail } from '../utils'; import { ItemStatus } from '../types'; import { FAVORITE_ALERTS_CONSENT_TEXT } from '../favoriteAlerts'; import { asyncRoute } from '../asyncRoute'; @@ -109,6 +109,12 @@ interface CustomerRecord extends CustomerRow { unsubscribe_token: string; marketing_consent_at: Date | null; marketing_consent_text: string | null; + // A separate purpose from marketing, so a separate column, timestamp and + // stored wording rather than a second meaning layered onto the pair above. + // False for every customer the migration touched: none of them was asked. + analytics_consent: boolean; + analytics_consent_at: Date | null; + analytics_consent_text: string | null; favorite_alerts_at: Date | null; favorite_alerts_text: string | null; } @@ -177,19 +183,21 @@ interface CustomerOrderRow { } /** - * Whether this customer has agreed to the *current* consent wording, which is + * Whether this customer has agreed to the *current* analytics wording, which is * the only thing that authorises the Brevo tracker (#56). * - * Not the same question as `marketing_consent`. That flag says the customer - * agreed to something; `marketing_consent_text` says what. The sentence was - * widened to cover analytics, so a customer who consented to the older wording - * agreed to emails and nothing more — they keep receiving email and are not - * tracked until they re-consent to the current text through the account page. + * Reads the analytics columns and nothing else. It must never consult + * `marketing_consent`: those are two purposes with two recipients, and GDPR + * requires consent to be granular — a customer who wants the emails and not the + * tracking has to be able to have exactly that. Quebec's Law 25 s.8.1 is + * stricter again and requires this to be off until the customer switches it on, + * which is why the column defaults to false. * * Comparing the stored string is the point rather than an implementation - * detail: it is why the wording is recorded per customer at all. A boolean on - * its own could not tell these two populations apart, and assuming they are the - * same is exactly the retroactive widening this avoids. + * detail. The flag says a customer agreed to something; the text says what. If + * the sentence is ever re-worded, everyone who agreed to the previous one stops + * qualifying and is asked again, rather than being silently carried into a + * broader agreement they never saw. * * Computed here rather than stored, so it can never drift from the constant. * @@ -198,9 +206,9 @@ interface CustomerOrderRow { * nothing else, and a test should not have to invent a customer to state it. */ export function analyticsConsent( - c: Pick + c: Pick ): boolean { - return c.marketing_consent && c.marketing_consent_text === MARKETING_CONSENT_TEXT; + return c.analytics_consent && c.analytics_consent_text === ANALYTICS_CONSENT_TEXT; } /** @@ -216,8 +224,8 @@ function publicCustomer(c: CustomerRecord) { last_name: c.last_name, email_verified: c.email_verified, marketing_consent: c.marketing_consent, - // Deliberately separate from marketing_consent: the two disagree for every - // customer who consented before the wording was widened. + // Its own purpose, its own answer. A customer can have either, both, or + // neither, and the UI has to be able to show that honestly. analytics_consent: analyticsConsent(c), favorite_alerts: c.favorite_alerts, created_at: c.created_at @@ -225,7 +233,7 @@ function publicCustomer(c: CustomerRecord) { } router.post('/register', asyncRoute(async (req: Request, res: Response) => { - const { email, password, firstName, lastName, marketingConsent } = req.body; + const { email, password, firstName, lastName, marketingConsent, analyticsConsent: analyticsConsentGiven } = req.body; if (!email || !isValidEmail(String(email)) || !password || String(password).length < 8) { return res.status(400).json({ error: 'valid email and password (min 8 chars) required' }); } @@ -246,13 +254,19 @@ router.post('/register', asyncRoute(async (req: Request, res: Response) => { const passwordHash = await bcrypt.hash(password, PASSWORD_HASH_ROUNDS); const unsubscribeToken = crypto.randomBytes(16).toString('hex'); const consent = !!marketingConsent; + // Read independently of marketingConsent, and absent means false. A client + // that sends neither, or only the marketing one, registers a customer who is + // not tracked — which is the right answer for a request that never carried an + // analytics answer at all. + const analytics = !!analyticsConsentGiven; const { rows } = await pool.query( - `INSERT INTO customers (email, password_hash, first_name, last_name, marketing_consent, marketing_consent_at, marketing_consent_text, unsubscribe_token) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING *`, + `INSERT INTO customers (email, password_hash, first_name, last_name, marketing_consent, marketing_consent_at, marketing_consent_text, analytics_consent, analytics_consent_at, analytics_consent_text, unsubscribe_token) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING *`, [ normalizedEmail, passwordHash, first, last, consent, consent ? new Date() : null, consent ? MARKETING_CONSENT_TEXT : null, + analytics, analytics ? new Date() : null, analytics ? ANALYTICS_CONSENT_TEXT : null, unsubscribeToken ] ); @@ -593,6 +607,28 @@ router.post('/me/consent', requireCustomer, asyncRoute(async (req: Request, res: res.status(204).end(); })); +/** + * Analytics consent, on its own route rather than as a second field on + * `/me/consent` (#56). + * + * Separate because the two are separate purposes and must be separately + * refusable. One endpoint taking both would make it possible for a single call + * to change an answer the customer did not touch — which is the bundling + * problem again, moved from the form into the API. + * + * Withdrawal writes the reason rather than the consent sentence, so the stored + * text never claims agreement to something that was declined. Same convention + * as marketing consent above. + */ +router.post('/me/analytics-consent', requireCustomer, asyncRoute(async (req: Request, res: Response) => { + const consent = !!req.body.analyticsConsent; + await pool.query( + `UPDATE customers SET analytics_consent = $1, analytics_consent_at = now(), analytics_consent_text = $2 WHERE id = $3`, + [consent, consent ? ANALYTICS_CONSENT_TEXT : 'Withdrew analytics consent via account settings', req.customerId] + ); + res.status(204).end(); +})); + router.get('/me/orders', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query( `SELECT o.id, o.processor, o.amount_cents, o.status, o.created_at, i.name AS item_name diff --git a/backend/src/utils.ts b/backend/src/utils.ts index ab19c3e..7f7c833 100755 --- a/backend/src/utils.ts +++ b/backend/src/utils.ts @@ -72,22 +72,37 @@ export function tagColorFor(name: string): string { } /** - * Widened for #56 to cover analytics as well as email. + * Email marketing only. Deliberately says nothing about tracking. * - * The previous wording named only emails. Gating the Brevo tracker on - * `marketing_consent` while that sentence was the thing customers agreed to - * would have treated "email me about new items" as authorisation to send their - * browsing to a third party, which it plainly did not say. + * This was briefly widened during #56 to cover analytics as well, and that was + * wrong: GDPR requires consent to be granular, and current EDPB guidance treats + * bundling tracking consent with subscription consent as invalid because the + * customer cannot accept one purpose and refuse the other. Quebec's Law 25 is + * stricter still. Analytics has its own sentence and its own column below. * - * Changing the sentence does not retroactively widen anyone's consent, because - * `marketing_consent_text` records what each customer was actually shown. - * Everyone who agreed to the old wording keeps their email consent and is not - * tracked; see `analyticsConsent` in routes/customers.ts, which is what the - * tracker is gated on. That is the whole reason this string is stored per - * customer rather than assumed. + * Left exactly as it was so that every existing consent record stays valid and + * untouched — nobody has to be re-asked for something they already agreed to. */ export const MARKETING_CONSENT_TEXT = - 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs, and I agree that what I browse and buy here may be shared with our email provider to help choose what they contain. I can unsubscribe at any time.'; + 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.'; + +/** + * Consent to the Brevo tracker (#56). Separate from marketing consent, and + * separately refusable, because they are two purposes with two recipients. + * + * Names Brevo rather than saying "our email provider": informed consent means + * the customer can tell who receives their data, and a description they cannot + * act on is not disclosure. Says what is shared and why, states that it is + * optional and independent of the emails, and states that it can be turned off + * — withdrawal has to be as easy as giving it. + * + * Stored verbatim in `analytics_consent_text` for the same reason the marketing + * sentence is: a record of consent that does not say what was consented to + * cannot be audited, and re-wording this later must not silently broaden + * anybody's agreement. + */ +export const ANALYTICS_CONSENT_TEXT = + 'I agree that what I browse and buy on this site may be shared with Brevo, the service that sends our emails, so that what they contain is relevant to me. This is optional, separate from receiving the emails themselves, and I can turn it off at any time.'; /** * Strips trailing slashes so a base URL can be joined with a stored path. diff --git a/backend/tests/unit/analyticsConsent.test.ts b/backend/tests/unit/analyticsConsent.test.ts index ea2808a..17a3eab 100644 --- a/backend/tests/unit/analyticsConsent.test.ts +++ b/backend/tests/unit/analyticsConsent.test.ts @@ -1,63 +1,93 @@ import { analyticsConsent } from '../../src/routes/customers'; -import { MARKETING_CONSENT_TEXT } from '../../src/utils'; +import { ANALYTICS_CONSENT_TEXT, MARKETING_CONSENT_TEXT } from '../../src/utils'; /** - * The rule this file exists for: agreeing to the *old* consent wording does not - * authorise the Brevo tracker (#56). + * The rule this file exists for: the Brevo tracker runs only for a customer who + * agreed to the *analytics* sentence, and marketing consent has nothing to do + * with it (#56). * - * The sentence customers agree to was widened to mention analytics. Everyone - * who consented before that agreed to a sentence about email and nothing else, - * and `marketing_consent` alone cannot tell the two populations apart — which - * is exactly why the wording is stored per customer. Getting this wrong would - * silently track people who never agreed to it, and would do so invisibly, - * because the flag they set really is `true`. + * Both halves matter and both are compliance requirements rather than taste. + * GDPR requires consent to be granular — email and tracking are separate + * purposes with separate recipients, and current EDPB guidance treats bundling + * them as invalid. Quebec's Law 25 s.8.1 requires profiling to be off until the + * person switches it on, which is why the column defaults to false. + * + * The failure mode is silent: reading the wrong flag tracks people whose + * marketing consent really is `true` and who never agreed to any of this. */ describe('analyticsConsent', () => { - const OLD_WORDING = - 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.'; - - it('is true for a customer who agreed to the current wording', () => { + it('is true for a customer who agreed to the current analytics wording', () => { expect( - analyticsConsent({ marketing_consent: true, marketing_consent_text: MARKETING_CONSENT_TEXT }) + analyticsConsent({ analytics_consent: true, analytics_consent_text: ANALYTICS_CONSENT_TEXT }) ).toBe(true); }); - it('is false for a customer who agreed to the previous, email-only wording', () => { - // The case the whole mechanism exists for. They consented, and their - // consent does not cover this. + it('is false when analytics consent was never given', () => { + // Where the migration leaves every existing customer, and where Law 25 + // requires a new one to start. expect( - analyticsConsent({ marketing_consent: true, marketing_consent_text: OLD_WORDING }) + analyticsConsent({ analytics_consent: false, analytics_consent_text: null }) ).toBe(false); }); - it('is false when consent was never given, whatever text is stored', () => { + it('is false when the stored wording is not the current one', () => { + // Re-wording the sentence re-asks rather than assuming. Anyone who agreed + // to a previous version stops qualifying until they agree to this one. expect( - analyticsConsent({ marketing_consent: false, marketing_consent_text: MARKETING_CONSENT_TEXT }) + analyticsConsent({ analytics_consent: true, analytics_consent_text: 'some older sentence' }) ).toBe(false); }); - it('is false when no wording was recorded at all', () => { - // Withdrawal writes a sentinel rather than the consent text, and older rows - // may predate the column being populated. Neither is agreement. + it('is false when the flag is set but no wording was recorded', () => { expect( - analyticsConsent({ marketing_consent: true, marketing_consent_text: null }) + analyticsConsent({ analytics_consent: true, analytics_consent_text: null }) ).toBe(false); }); - it('does not accept a near-miss, so a reworded sentence re-asks rather than assumes', () => { + it('is false when withdrawal wrote its reason rather than the consent text', () => { expect( analyticsConsent({ - marketing_consent: true, - marketing_consent_text: `${MARKETING_CONSENT_TEXT} ` + analytics_consent: false, + analytics_consent_text: 'Withdrew analytics consent via account settings' }) ).toBe(false); }); - it('guards the wording itself: the current text must mention what is shared', () => { - // Not a tautology — it fails if someone narrows the sentence back to email - // while leaving the tracker gated on it, which would put this project back - // in the position #56 was filed to get it out of. - expect(MARKETING_CONSENT_TEXT).toContain('browse'); - expect(MARKETING_CONSENT_TEXT).not.toBe(OLD_WORDING); + it('does not accept a near-miss', () => { + expect( + analyticsConsent({ + analytics_consent: true, + analytics_consent_text: `${ANALYTICS_CONSENT_TEXT} ` + }) + ).toBe(false); + }); + + describe('the two consents stay separate', () => { + // Not a tautology. These fail if anyone re-bundles the purposes — either by + // folding tracking back into the marketing sentence, or by pointing this + // function at the marketing columns — which is the specific pattern GDPR + // and Law 25 both reject, and which this project shipped once already + // before it was caught. + it('marketing consent alone does not authorise tracking', () => { + expect( + analyticsConsent({ analytics_consent: false, analytics_consent_text: MARKETING_CONSENT_TEXT }) + ).toBe(false); + }); + + it('the marketing sentence says nothing about tracking', () => { + expect(MARKETING_CONSENT_TEXT).not.toContain('browse'); + expect(MARKETING_CONSENT_TEXT).not.toContain('Brevo'); + }); + + it('the analytics sentence names the recipient and says it is optional', () => { + // Informed consent means the customer can tell who receives their data; + // "our email provider" is not something they can act on. + expect(ANALYTICS_CONSENT_TEXT).toContain('Brevo'); + expect(ANALYTICS_CONSENT_TEXT).toContain('optional'); + }); + + it('the two sentences are not the same string', () => { + expect(ANALYTICS_CONSENT_TEXT).not.toBe(MARKETING_CONSENT_TEXT); + }); }); }); diff --git a/frontend/src/customer/Account.tsx b/frontend/src/customer/Account.tsx index 82929e6..092a93e 100755 --- a/frontend/src/customer/Account.tsx +++ b/frontend/src/customer/Account.tsx @@ -7,7 +7,7 @@ import message from 'antd/es/message'; import Space from 'antd/es/space'; import Divider from 'antd/es/divider'; import { useNavigate } from 'react-router-dom'; -import { updateConsent, exportMyData, deleteMyAccount, resendVerificationEmail } from './customerApi'; +import { updateConsent, updateAnalyticsConsent, exportMyData, deleteMyAccount, resendVerificationEmail } from './customerApi'; import { setFavoriteAlerts } from './favoritesApi'; import { useCustomerAuth } from './CustomerAuthContext'; import AccountDetails from './AccountDetails'; @@ -62,6 +62,14 @@ export default function Account({ onClose }: Props) { refresh(); } + // Its own handler and its own endpoint. Withdrawing this must not disturb the + // email consent, and must be exactly as easy as giving it (#56). + async function handleAnalyticsConsentToggle(checked: boolean) { + await updateAnalyticsConsent(checked); + message.success(checked ? 'Thanks — this helps us send you relevant emails' : 'Turned off — we will stop sharing your activity'); + refresh(); + } + async function handleLogout() { try { await logout(); @@ -146,6 +154,24 @@ export default function Account({ onClose }: Props) { + {/* Analytics, and a third independent consent (#56). Named as sharing + with Brevo rather than as "analytics", because the customer cannot + weigh a decision described in a word that hides who receives the + data. The subtext restates that it is optional, since this is the + control that has to make withdrawal as easy as consenting. */} +
+ + + Share what I browse and buy with Brevo, to make emails relevant + +
+ + Optional, and independent of the emails above. Turning it off stops any further + activity being shared. + +
+
+ {/* Order history is a page of its own now. The link stays here because diff --git a/frontend/src/customer/AuthForm.tsx b/frontend/src/customer/AuthForm.tsx index bdd6732..879c0d3 100644 --- a/frontend/src/customer/AuthForm.tsx +++ b/frontend/src/customer/AuthForm.tsx @@ -20,13 +20,17 @@ export type AuthMode = 'register' | 'login'; // this was shared there were three wordings in play — this one, a shorter one in // the cart prompt, and the string the server actually recorded — and none of // them matched. -// -// Widened for #56 so it covers the Brevo tracker as well as email. The tracker -// is gated on whether a customer's *stored* copy of this sentence matches the -// current one, so changing it here does not retroactively widen the consent of -// anyone who agreed to the previous wording. export const MARKETING_CONSENT_TEXT = - 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs, and I agree that what I browse and buy here may be shared with our email provider to help choose what they contain. I can unsubscribe at any time.'; + 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.'; + +// Analytics consent (#56). A second sentence and a second checkbox rather than +// wording folded into the one above, because GDPR requires consent to be +// granular: someone must be able to take the emails and refuse the tracking. +// Must stay identical to ANALYTICS_CONSENT_TEXT in backend/src/utils.ts, which +// is what gets stored verbatim — same rule, and same failure mode, as the +// marketing sentence. +export const ANALYTICS_CONSENT_TEXT = + 'I agree that what I browse and buy on this site may be shared with Brevo, the service that sends our emails, so that what they contain is relevant to me. This is optional, separate from receiving the emails themselves, and I can turn it off at any time.'; type Props = Readonly<{ mode: AuthMode; @@ -81,7 +85,7 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce layout="vertical" onFinish={(values) => submit(() => - registerCustomer(values.email, values.password, values.firstName, values.lastName, !!values.marketingConsent) + registerCustomer(values.email, values.password, values.firstName, values.lastName, !!values.marketingConsent, !!values.analyticsConsent) ) } > @@ -112,6 +116,14 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce {MARKETING_CONSENT_TEXT} + {/* Its own checkbox, and independent of the one above: someone + has to be able to take the emails and refuse the tracking, + or the consent is not granular and is not valid. Unchecked + by default and never pre-ticked — Quebec's Law 25 requires + profiling to be off until the person switches it on. */} + + {ANALYTICS_CONSENT_TEXT} + diff --git a/frontend/src/customer/PrivacyPolicy.tsx b/frontend/src/customer/PrivacyPolicy.tsx index 95b2c62..754d8f7 100755 --- a/frontend/src/customer/PrivacyPolicy.tsx +++ b/frontend/src/customer/PrivacyPolicy.tsx @@ -30,18 +30,25 @@ export default function PrivacyPolicy() { Analytics and tracking - If — and only if — you have opted in to marketing, we share what you browse and buy on - this site with Brevo, our email provider, so the emails we send you are about things you - are actually interested in. That covers the pages you visit here, items you add to your - cart or favorite, and completed orders, linked to your email address. + If — and only if — you have separately opted in to it, we share what you browse and buy on + this site with Brevo, the service that sends our emails, so that what + they contain is relevant to you. That covers the pages you visit here, items you add to + your cart or favorite, and completed orders, linked to your email address. - If you have not opted in, this does not happen: nothing is loaded, nothing is sent, and no - tracking cookie is set. The same is true if you are not signed in — we do not track - visitors who do not have an account. Withdrawing consent from your account page stops any - further activity being sent. Note that anything already shared with Brevo before you - withdrew remains with them, and a tracking cookie set earlier in your visit stays in your - browser until you close the tab or clear it. + This is a separate choice from receiving the emails themselves. You can + have the emails without it, or turn it off and keep receiving them. It is off unless you + switch it on — we never enable it by default, and never as a side effect of subscribing to + anything else. Both choices live in your account settings, and turning either off is as + easy as turning it on. + + + If you have not opted in, none of this happens: no tracking script is loaded, nothing is + sent, and no tracking cookie is set. The same is true if you are not signed in — we do not + track visitors who do not have an account. Turning it off stops any further activity being + shared. Two limits worth being plain about: anything already shared with Brevo before you + turned it off remains with them, and a tracking cookie set earlier in your visit stays in + your browser until you close the tab or clear it. Your rights diff --git a/frontend/src/customer/customerApi.ts b/frontend/src/customer/customerApi.ts index eec6e9e..0b9b7d4 100755 --- a/frontend/src/customer/customerApi.ts +++ b/frontend/src/customer/customerApi.ts @@ -43,12 +43,16 @@ export function registerCustomer( password: string, firstName: string, lastName: string, - marketingConsent: boolean + marketingConsent: boolean, + // Separate argument rather than folded into the one above: they are separate + // consents and the caller has to be able to send one true and the other + // false. The server treats an absent value as false (#56). + analyticsConsent: boolean ): Promise { return fetch('/api/customers/register', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ email, password, firstName, lastName, marketingConsent }) + body: JSON.stringify({ email, password, firstName, lastName, marketingConsent, analyticsConsent }) }).then(res => handle(res)); } @@ -93,6 +97,21 @@ export function updateConsent(marketingConsent: boolean): Promise { }).then(() => undefined); } +/** + * Its own endpoint, not a second field on updateConsent (#56). + * + * Withdrawal has to be as easy as giving consent, and it has to be possible to + * withdraw one without touching the other. A combined call would make it easy + * to send a stale value for the answer the customer did not change. + */ +export function updateAnalyticsConsent(analyticsConsent: boolean): Promise { + return fetch('/api/customers/me/analytics-consent', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ analyticsConsent }) + }).then(() => undefined); +} + export function fetchMyOrders(): Promise { return fetch('/api/customers/me/orders').then(res => handle(res)); } diff --git a/frontend/tests/e2e/auth.spec.ts b/frontend/tests/e2e/auth.spec.ts index b503b72..8529b90 100755 --- a/frontend/tests/e2e/auth.spec.ts +++ b/frontend/tests/e2e/auth.spec.ts @@ -27,14 +27,19 @@ test.describe('Customer accounts', () => { // customer actually saw. Three different wordings were in circulation // before the sign-in form was shared between the routes and the cart // prompt, and none of them matched what was stored. - // Widened in #56 to cover the Brevo tracker as well as email. The literal is - // repeated here rather than imported on purpose: importing it from the app - // would make this assert that a constant equals itself, and the drift it - // guards against is exactly the rendered label parting from the stored - // string. const consent = - 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs, and I agree that what I browse and buy here may be shared with our email provider to help choose what they contain. I can unsubscribe at any time.'; + 'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.'; await expect(authModal.registerDialog).toContainText(consent); + + // The analytics consent is a second, separate sentence and a second + // checkbox (#56). Asserted here for the same reason as the one above — it + // is stored verbatim, so the rendered label parting from the stored string + // defeats the record — and because the two being separate is the thing + // that makes the consent granular. Folding them back into one control + // would still pass the assertion above and would still be wrong. + const analytics = + 'I agree that what I browse and buy on this site may be shared with Brevo, the service that sends our emails, so that what they contain is relevant to me. This is optional, separate from receiving the emails themselves, and I can turn it off at any time.'; + await expect(authModal.registerDialog).toContainText(analytics); }); // Drives the form rather than taking the `customer` fixture: this test is From 25bec50902ce667d9486ea2f58f3a62cd85684b2 Mon Sep 17 00:00:00 2001 From: synAdmin Date: Tue, 8 Sep 2026 12:08:50 -0500 Subject: [PATCH 3/4] docs(privacy): disclose every cookie and browser-storage item (#56) Completes the consent picture the rest of this branch builds. The claim worth being able to check is that no cookie needing permission is set before it is asked for, so the policy now lists everything rather than asserting it: the rd_session sign-in cookie, which is strictly necessary and therefore exempt, the two preferences kept in localStorage and never sent anywhere, and Brevo's cookie, which cannot exist unless analytics consent was given because the script that would set it is never loaded otherwise. No cookie banner, and that is a finding rather than an omission. ePrivacy requires consent before storing anything non-essential, and this application does not store anything non-essential until the customer has asked for the feature that needs it. A banner would be asking permission for things that are either exempt or already separately consented to, which teaches people to dismiss the one consent that does matter. Described in terms of what each thing does rather than by category, since a list of cookie names tells a customer nothing they can act on. Refs #56 Co-Authored-By: Claude Opus 5 --- frontend/src/customer/PrivacyPolicy.tsx | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/frontend/src/customer/PrivacyPolicy.tsx b/frontend/src/customer/PrivacyPolicy.tsx index 754d8f7..33fd5ba 100755 --- a/frontend/src/customer/PrivacyPolicy.tsx +++ b/frontend/src/customer/PrivacyPolicy.tsx @@ -51,6 +51,31 @@ export default function PrivacyPolicy() { your browser until you close the tab or clear it. + Cookies and browser storage + + We do not show a cookie banner, because until you ask for something that needs one we do + not set anything that requires your permission. Here is everything, so you can check that + claim rather than take it on trust: + + + A sign-in cookie (rd_session). Set only when you sign in, + and only so the site knows it is still you on the next page. It cannot be read by + JavaScript, is not shared with anyone, and is not used to track you. Signing out removes + it. This is what the rules call a strictly necessary cookie: without it, signing in would + not work at all, so it does not need — and we do not ask for — separate consent. + + + Two preferences kept in your browser, not cookies and never sent to us: + whether you chose the light or dark theme, and how many items you like to see per page. + They stay on your device and are readable only by this site. Clearing your browser data + removes them. + + + Brevo's tracking cookie — only if you opted in to sharing your activity.{' '} + If you have not, the script that would set it is never loaded, so the cookie never exists. + It is not set for signed-out visitors under any circumstances. + + Your rights You may request a copy of your data ("Download my data" in your account page), or delete your From e7196f440b5a29d0e0daa42af514303ee3128082 Mon Sep 17 00:00:00 2001 From: synAdmin Date: Tue, 8 Sep 2026 12:28:17 -0500 Subject: [PATCH 4/4] test(customers): let the public shape guard see analytics_consent (#56) The register route now returns analytics_consent, and customers.integration.test.ts asserts the exact key set the public customer shape may contain. That test failed in CI, which is the guard doing its job rather than a problem with it: its whole point is that the shape cannot quietly grow, and a field appearing without someone deciding it belongs there is what it exists to catch. This field does belong there, so the expected set gains it. Three cases added while here, all of them properties the compliance work depends on and none of them observable from a unit test. Analytics consent is off for a registration that does not mention it, which is what Quebec's Law 25 s.8.1 requires and needs the column default, the register route and the stored wording to agree. Opting in to marketing alone leaves analytics off, which is the bundling GDPR treats as invalid and the mistake this branch already made once. And an analytics-only opt-in works with marketing left off, so the granularity holds in both directions rather than only the convenient one. Found by CI rather than locally: the integration suite needs a database this machine has no Docker to run, which was called out as unverified when the change went up. Typechecked, linted and the 478 unit tests still pass, but the assertion itself is only proven by the next CI run. Refs #56 Co-Authored-By: Claude Opus 5 --- .../integration/customers.integration.test.ts | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/backend/tests/integration/customers.integration.test.ts b/backend/tests/integration/customers.integration.test.ts index c0e2b5b..e99f3c5 100755 --- a/backend/tests/integration/customers.integration.test.ts +++ b/backend/tests/integration/customers.integration.test.ts @@ -81,7 +81,7 @@ describe('POST /api/customers/register', () => { }); expect(Object.keys(res.body).sort()).toEqual([ - 'created_at', 'email', 'email_verified', 'favorite_alerts', + 'analytics_consent', 'created_at', 'email', 'email_verified', 'favorite_alerts', 'first_name', 'id', 'last_name', 'marketing_consent' ]); }); @@ -106,6 +106,44 @@ describe('POST /api/customers/register', () => { expect(res.body.marketing_consent).toBe(true); }); + // Quebec's Law 25 s.8.1 requires profiling to be off until the person turns + // it on, so this is a compliance property rather than a default worth + // debating. Asserted end to end because the column default, the register + // route and the stored wording all have to agree for it to hold. + it('creates an account with analytics consent off by default', async () => { + const res = await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', + email: 'analytics-default@example.com', + password: 'supersecret123' + }); + expect(res.body.analytics_consent).toBe(false); + }); + + // The two consents are separate purposes and must be separately refusable. + // Taking the emails must not opt anybody into being tracked — that bundling + // is what GDPR treats as invalid consent, and it is the mistake this branch + // made once before it was caught. + it('opting in to marketing alone does not opt in to analytics', async () => { + const res = await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', + email: 'marketing-only@example.com', + password: 'supersecret123', + marketingConsent: true + }); + expect(res.body.marketing_consent).toBe(true); + expect(res.body.analytics_consent).toBe(false); + }); + + it('respects an explicit analytics opt-in, independently of marketing', async () => { + const res = await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', + email: 'analytics-only@example.com', + password: 'supersecret123', + analyticsConsent: true + }); + expect(res.body.analytics_consent).toBe(true); + // Refusing the emails while accepting the tracking has to be possible too, + // or the consent is not granular in both directions. + expect(res.body.marketing_consent).toBe(false); + }); + it('rejects a duplicate email', async () => { await request(app).post('/api/customers/register').send({ firstName: 'Test', lastName: 'Customer', email: 'dupe@example.com',