v3.129.0-beta.0
1316
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Paul Nothaft
|
a9e51d8fd7 |
fix(usage): reformat the consent modal so the disclosure can be read
It was seven anonymous paragraphs in one scrolling block, with the title and the buttons scrolling away with them. The scroll container is keyboard-focusable, and unstyled it drew a default focus ring, so the disclosure also looked like a giant textarea. Now: a fixed header carrying the icon, title and purpose; a scroll region with six labelled sections, each with a small heading and icon so the thing can be scanned rather than only read; and a fixed footer with the consent checkbox and the actions, which no longer scroll out of reach on a short screen. "What is never included" is set apart as a tinted block, since it is the part that answers the question an operator actually has. The focus ring is now a deliberate inset ring on a labelled region, which is correct for keyboard use instead of an accident that looked like a form field. Dark mode is fixed as part of this, and it was my own doing: the dialog used `bg-theme-surface`, which does not follow dark mode, and the section text I added carries dark: variants. Light surface plus near-white text is unreadable. The surface is class-driven now — neutral-800, which is what `.card` resolves to in dark and what the rest of the admin UI uses. Verified in both themes through the app's own theme toggle rather than by forcing the class, which is what produced the misleading half-state the first time I looked. Six section headings added in EN and DE. Refs #1110 |
||
|
Paul Nothaft
|
bb76ca5375 |
fix(usage): keep the settings tab usable on a bad collector URL, and report layouts and CSS accurately
Three items, one of which explains an error seen in the app. "The operation could not be completed" could come from a config typo. status() called collectorUrl() bare, and that throws on a bare hostname, a path, a query, or http in production. The settings page renders one generic failure when its status query errors, so a misconfigured USAGE_COLLECTOR_URL replaced the whole tab with that sentence — no cause, and no way to read the status or withdraw, because every control there sits behind that call. The URL is now reported as collector_error: 'INVALID_COLLECTOR_URL' beside the real state, the tab says what is wrong and how to fix it, and the links are only rendered when there is somewhere to point them. gallery_layouts reported grid for every preset-themed install. color_theme holds either a theme object or the NAME of a preset — the theme picker stores names, and eventTypeService seeds them (`theme_preset: 'corporateTimeline'`). Only reading value.galleryLayout made masonry, timeline, mosaic and the two gallery presets invisible. Names now resolve, and an event with no theme of its own resolves through the global one instead of being counted as grid. Only the name -> layout mapping is duplicated, not the presets; frontend/src/types/theme.types.ts stays the source of truth, and an unknown name reports `other` so a preset added later degrades to "something else" rather than quietly inflating the grid count. custom_css missed CSS applied through a template. An enabled css_templates row applied via events.css_template_id is gallery styling by the same definition as the settings fields — the Custom CSS tab is where both are authored — but neither the snapshot nor the middleware saw it, so those installs reported custom_css entirely false. Existence only; template contents are never read. Eleven tests. Reverting each fix in turn fails 3, 1 and 3 of them. Refs #1110 |
||
|
Paul Nothaft
|
22da018e1b |
fix(usage): close the remaining withdrawal races, reset per-item name consent
Follow-up review on the previous commit, including a hole in that commit's own fix. The cancellation flag became a counter. Clearing a boolean needed a write of its own, and a /disable landing between the lease and that write was erased — the same race one level down. enable() now records the counter it started with and claims only if it is unchanged, so no clearing write exists to lose. It also fixes the case a boolean could not express at all: a stale cancellation already set, and a fresh one arriving mid-activation, are indistinguishable as flags and obvious as counts. Migration 203, separate from 202 for the reason 202 was separate from 201 — knex will not re-run an applied migration. deliver() re-checks immediately before dispatch. The existing check ran before the binding lookup, which is asynchronous, so a withdrawal that COMPLETED during it still had its registration or report sent afterwards. Not an already-in-flight request — a new one started after the operator had withdrawn. The outbox writes in tick() and command() are conditional on still being active. /disable clears pending_packet without holding the lease, so an unconditional write put a report — or a feedback body and name — back into an outbox the withdrawal had just emptied, where deliver() would then leave it, since it declines to send anything but the delete. Per-item name consent resets with the item. `named` stayed checked after submitting, so the next item carried the previous name automatically, contradicting the anonymous-by-default promise the disclosure makes for each item. The remembered name stays in preferences; attaching it is decided again each time. Two of these tests were worthless when first written and are noted because the pattern keeps recurring: the pre-dispatch case passed without the guard because an empty report payload failed schema validation during signing, so nothing reached the collector for reasons unrelated to the check. With a valid payload it fails without the guard and passes with it. Same for the counter: dropping it from the claim fails two. Refs #1110 |
||
|
Paul Nothaft
|
4944b9b3b6 |
fix(usage): scope the participation notice, highlight it, and call ignoring what it is
It appears on the dashboard and settings only. It is an invitation, not an alert, so it belongs on pages an admin opens deliberately rather than on top of whatever task they are in the middle of. The activity ticker deliberately did NOT move with it. That ticker is what triggers the daily rollup — the backend has no scheduler — so tying it to the banner would have stopped reporting for an admin who works on Events and never opens the dashboard, and stopped it entirely for a participating install, where the banner never renders at all. The effect stays mounted on every admin page and only the visible aside is scoped. Two tests pin exactly that, because it is the kind of thing a later refactor would helpfully "clean up". Highlighted like the migration banner it sits under: tinted surface, border, icon, a title line above the body. It was previously the same neutral surface as the page behind it and read as filler. "Not now" is now "Ignore". The button calls dismiss(), which persists notice_dismissed on the server — the invitation never comes back. "Not now" promised otherwise. The label says what happens and a hint says where to join later. Only shown while participation is off. activation_pending, deletion_pending and identity_conflict are in-flight states the settings page explains properly; inviting someone to join in the middle of their own withdrawal would be worse than saying nothing. The first version of these tests was worthless: the negative cases asserted absence after waiting only for the status call, so the component was still rendering null for want of data and every one passed with the gates removed. They now wait for the query cache to fill. Removing the route gate fails 4; removing the status gate fails 6. Refs #1110 |
||
|
Paul Nothaft
|
83fbb63e13 |
fix(usage): protect a pending withdrawal, widen the backup signal, explain an unreadable key
Three of four findings from the follow-up review.
A withdrawal is no longer clobbered by the instance-copy check. That
update was unconditional, so an opt-out arriving while the binding
lookup was in flight was replaced by identity_conflict — and tick()
stops there, so the deletion the operator asked for was never sent. It
now carries the same whereNot('deletion_pending') guard the
collector-conflict handler beside it already had.
Scheduled database backups count as a configured backup. The middleware
records /backup/* and /database-backup/* under one capability, but
`configured` read only backup_enabled, so an install whose only backup
is the scheduled database one reported used: true, configured: false —
a contradiction in the dataset this feature exists to produce.
SIGNING_KEY_UNREADABLE gets its own message. Naming the error in the
previous commit was half the job: the settings page still showed the
generic retry/disable advice, and neither action can succeed without the
original encryption material. It now says what happened and what is
actually required, in EN and DE.
NOT fixed, and reported instead: /disable overlapping an in-flight
/enable. While activation is still doing its slow work the row still
reads `disabled`, so disable's conditional update matches nothing and
the lease conflict from its tick() is swallowed — the operator is told
participation is off while activation completes and leaves it on.
Closing it properly needs a persisted cancellation flag that enable
checks before finalising: taking the lease cannot help, since it either
conflicts immediately or would block the request for the 60s lease. That
is a schema and state-machine decision for the author, not something to
restructure underneath them.
Refs #1110
|
||
|
Paul Nothaft
|
c043897b0e |
fix(usage): name the unreadable-key failure, unpin the collector default, align the tab
Review follow-ups on #1304. SIGNING_KEY_UNREADABLE. USAGE_ENCRYPTION_KEY defaults to JWT_SECRET, so rotating JWT_SECRET — the correct response to a suspected compromise — makes the stored Ed25519 key undecryptable. That surfaced as a generic DELIVERY_FAILED which retried forever, and it silently blocks the DELETE packet too: an operator who withdraws has their local state cleared while the collector keeps its copy. decrypt() now tags its own failure and deliver() reports it under its own name, without flagging an identity conflict — an unreadable key is not evidence of a clone. The docs already warned that losing the key breaks deletion signing; they now name the trigger and the error. The collector default is no longer an inline string in the constructor. It is a declared DEFAULT_COLLECTOR_URL, since it is a deployment choice: self-hosters point USAGE_COLLECTOR_URL at their own collector and the UI already derives every link from whatever is configured. schema.cjs is deliberately untouched — it is vendored byte-identical with picpeak-usage, and its $id is a schema identity, not a delivery address. Links in the consent dialog. It named the collector inside prose but never linked it, so an operator deciding whether to opt in could not open the destination or the public schema without retyping a URL. Both are links now, built from the configured collector. UI standards. The tab hand-rolled its surfaces as `<section className="rounded-xl border border-theme …">` and imported Button from a deep path; every other settings tab uses `<Card padding="md">` from the components/common barrel. Converted, with the feedback <form> wrapped rather than replaced so its semantics survive, and headings given the same colour tokens as ImageSecurityTab. The barrel pulls ErrorBoundary -> i18n/config, so the tab's test needed the initReactI18next shim the FaceRecognitionCard test already uses. Not changed: the delete packet reusing the current sequence. The collector handles delete before any sequence check — "possession proof is sufficient for deletion, including when a restored backup has a stale sequence" (picpeak-usage server/collector.js) — so deletion is deliberately sequence-exempt and the client is correct as written. Refs #1110 |
||
|
Paul Nothaft
|
7b4a65ecc7 |
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. |
||
|
Paul Nothaft
|
1cf82746b7 |
fix(security): close two CSS url() bypasses the sanitizer dedup exposed
Both found by review against the correct base, and both are cases the
second stripRemoteCssUrls pass had been catching before this PR removed
it. Verified against the real functions before and after.
An escaped quote outside a string. `\'` is an escaped identifier
character, not a string opener, but the scanner stepped onto the
apostrophe, entered string mode and copied the rest of the stylesheet
unexamined — so `.hero{--marker:\';background:url(https://evil/p.gif)}`
kept a live remote URL. Escapes are now consumed as a unit outside
strings.
An unterminated quote. Trusting one meant a single stray apostrophe
disabled scanning for everything after it. An unclosed quote is a parse
error, so the safe reading is to emit it as an ordinary character and
keep scanning; a newline also ends a string, as it does in CSS.
The entity mismatch behind the second case. sanitize-html writes `"`
inside an attribute as `"`, so the scanner and the recipient's
browser disagreed about where strings begin: in
`style="font-family:"don't";background:url(...)"` the browser
decodes first, reads the apostrophe as ordinary text inside a real
string, and fetches the background — a tracking pixel by another name.
Style attributes are now decoded before scanning and re-encoded after,
which also stops the old code silently deleting quotes from the value.
Also detaches the image handlers before releasing the canvas source.
That one did NOT reproduce: measured in both Chromium and WebKit,
neither fires `error` when the attribute is removed after a successful
load. Applied anyway because the ordering is free and the failure it
would cause is silent — canvasFailed set, the canvas swapped for an
<img>, and the image decoded a second time, the exact opposite of what
the release is for.
Refs #1264, #1287
|
||
|
Paul Nothaft
|
b53e5d97b4 | feat: add opt-in product usage and feedback integration (#1110) | ||
|
Paul Nothaft
|
967224c030 |
fix: remove the image-fragmentation surface
Closes #1300. Fragmentation was configurable, stored per event, served to the gallery client, and consumed by nothing. It was not unbuilt scaffolding — both halves exist and are individually coherent — but they were never connected, and they disagree: the server cut a fixed 3x3 grid while the client reassembled a 4x4 one, so wiring them together as they stood would have produced scrambled images rather than protection. Removed rather than finished, because finishing it buys nothing. The client fetches the whole image and then redraws it in pieces on a canvas, so the full original has already crossed the wire before any "protection" is applied — that is obfuscation, not a control. The per-fragment canvas work also lands on mobile, which is the memory profile under investigation in #1287. Goes: secureImageService.fragmentImageBuffer and its branch, the ?fragment=N delivery path and handleFragmentedImage in secureImages, the fragmented-JSON response in protectedImages, fragmentation_level in the gallery payload, the default_fragmentation_level setting, the PUT validator, the ProtectedImage fragment renderer, and the operator control with its strings in all eight locales. No migration. `events.fragmentation_level` and the app_settings row stay — dropping a column is irreversible and the stored values are harmless once nothing reads them. If they should go, that is a deliberate data decision and its own migration. `fragmentGrid` on AuthenticatedImage and the layouts is deliberately untouched: #1299 already removes it as part of the inert prop surface, and doing it here would only collide. |
||
|
Paul Nothaft
|
49197be329 |
feat(newsletters): warn about deliverability before a large send
The composer already paces sends — the rate is clamped to what the queue can actually drain and `scheduled_at` is staggered — and the help text said so. But pacing answers the wrong risk. Spam filtering reacts to a domain's volume and reputation, not to the interval between messages, so a throttled send of several hundred near-identical mails from a domain that normally emits only gallery notifications is exactly the shape that gets junked or blocked. Nothing told the operator that. Adds a warning above the queue button once the resolved recipient count reaches 50, covering what actually goes wrong: the reputation hit, that it damages delivery of transactional email too, the SPF/DKIM/DMARC prerequisite, and the suggestion to split a first campaign. It also states the cost of the send in the operator's terms — the real duration at the chosen rate, and that gallery invitations and password resets queue behind it, because the email queue is global rather than per-campaign. 50 is deliberately low: the operators who most need this are the ones sending their first campaign. The existing rate hint now mentions looking like spam, not only being rate-limited. |
||
|
Paul Nothaft
|
fbe9757a53 |
fix(gallery): release the canvas decode when it is drawn, not at unmount
Review follow-up. The release only ran from the effect cleanup, so it fired on unmount or a src change — while the commit message and the test header both explained that grid tiles never unmount, which is the whole reason the decode piles up. For the case the change exists for, it never ran at all. drawToCanvas now reports whether it drew, and the source Image is released as soon as the pixels are on the canvas. Nothing redraws from imageRef afterwards; drawToCanvas has exactly one caller. The cleanup stays as the fallback for the paths onload cannot cover: the draw failed, or the source changed before onload fired. The new test pins release while still mounted, on the same src. It fails against the previous version. Refs #1287 |
||
|
Paul Nothaft
|
e734e41c41 |
fix(gallery): remove the inert image-protection prop surface from AuthenticatedImage
AuthenticatedImage accepted the whole image-protection prop surface and discarded it in a `void unusedProps` block. Callers computed those props from the event's protection level and passed them in good faith, so raising the level produced canvas rendering (via the layouts' own OR on `protectionLevel === 'maximum'`) and nothing else the level implies. Removes them from the interface and from every call site, so the props state what the component actually does. Two survive because they are real: `useCanvasRendering`, and `onProtectionViolation` — which #1297 listed as inert but which does fire, from the canvas context-menu handler. `useWatermark` is removed as well; #1297 did not list it (it sat outside the `unusedProps` block) but it was equally dead. Removal rather than implementation is deliberate. The implementation these props describe already exists in `ProtectedImage`, which is exported from the barrel and rendered nowhere. Wiring it in is a product decision about what protection level should mean, not a side effect of a cleanup. Analytics payloads inside the surviving onProtectionViolation handlers keep their photoId/protectionLevel fields. Refs #1297 |
||
|
Paul Nothaft
|
b1e5287351 |
fix(gallery): give the Grid layout a lazy-loading pre-load band (#1287)
Grid was the only layout passing `lazy` without an `inViewRootMargin`, so PhotoCard ran its observer at the IntersectionObserver default of `0px` with `threshold: 0.1`. A tile could not begin loading until a tenth of it was already on screen — there was no lead at all. The gallery owner's account of the symptom is that defect's exact shape: spinning the scroll wheel outran loading by roughly 50 images, then it caught up. Outrun-then-recover is what a zero-width pre-load band looks like from a chair. This is the one thing in that investigation that does not rest on the reporter's instrumented runs, which they have since withdrawn after finding their automation harness ran in a hidden pane — `innerHeight: 0`, so nothing could intersect and no tile could ever load. The missing margin is visible in the source regardless. Percent, not vh. `rootMargin` accepts only px and percentages, and a `vh` value throws SyntaxError at construction, which would have taken down every Grid gallery. Verified in Chrome: '100% 0px' → accepted '100px 0px' → accepted '100vh 0px' → SyntaxError: rootMargin must be specified in pixels or percent A percentage resolves against the root's own box, so 100% is one viewport height of lead in each direction — viewport-relative, which a fixed 100px like Justified's is not. A phone and a 4K desktop scroll past very different amounts of grid per gesture. Deliberately NOT included: a sweep for cards left un-loaded after scrolling settles. That was aimed at permanent loss from `triggerOnce`, and the owner's observation that tiles do come back on desktop argues against it. Complexity chasing a symptom nobody has reproduced outside a broken harness. Three guard tests, including one on the unit, since the failure mode of getting that wrong is a gallery that does not render at all. |
||
|
Paul Nothaft
|
be8d79e9c4 |
fix(gallery): release the canvas-mode decode, and drop a now-duplicate sanitizer
Two follow-ups to yesterday's merges. Both were already known; neither depends on the open question in #1287. 1. Canvas mode pinned every decoded image for the component's lifetime. `AuthenticatedImage` keeps a detached Image in `imageRef` so drawToCanvas can read it. The effect cleanup nulled onload/onerror and never cleared that ref, so the Image — and the decode behind it — stayed held by a live JS reference. A decoded <img> in the document is evictable under memory pressure; one held by a ref is not. That is not academic at gallery scale. The photo grid is NOT virtualised, so a 546-photo event mounts 546 of these and none ever unmount — nothing was ever released. The ref is cleared and the src dropped, so the browser can reclaim without waiting for GC. This is NOT presented as the fix for #1287. That investigation is still open: the reporter has since shown the backend idle during a stall and the renderer itself unresponsive for 45s, which rules out the theories tried so far. This is a real leak on the same path, worth fixing on its own terms while that question is settled. 2. newsletterService no longer carries its own remote-url() stripper. It was added because the shared sanitizeCSS "blocked" remote URLs with a CSS comment that parsers discard. #1290 replaced that with a lexer, so the local copy is dead weight — and two definitions of "disallowed" would drift apart. Verified the shared function covers every case the local one did, including the quoted-paren and CSS-escape forms found in review. Three tests on the release path, two of which fail without the fix: unmount clears the ref and drops the src, the blob URL is revoked, and a src change releases the previous image rather than accumulating one pinned decode per photo a recycled tile has shown. |
||
|
Paul Nothaft
|
c71ffae912 |
chore(main): release 3.123.0-beta.0 (#1294)
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Successful in 9m21s
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Successful in 9m53s
Build and Push Docker Images / smoke-aio (push) Failing after 11m17s
Build and Push Docker Images / build-ml (linux/amd64, ubuntu-latest) (push) Has been skipped
Build and Push Docker Images / build-aio (linux/amd64, ubuntu-latest) (push) Successful in 12m38s
Build and Push Docker Images / merge-backend (push) Has been cancelled
Build and Push Docker Images / build-ml (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-ml (push) Has been cancelled
Build and Push Docker Images / dockerhub-descriptions (push) Has been cancelled
Build and Push Docker Images / summary (push) Has been cancelled
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-frontend (push) Has been cancelled
Build and Push Docker Images / build-aio (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-aio (push) Has been cancelled
|
||
|
Paul Nothaft
|
fc595409b4 |
feat(crm): newsletter campaigns behind a newsletters flag (#1264)
Part B of #1264. Flag off by default, so an install that never enables it gains no route, no nav entry and no way to mass-mail. A campaign is a body plus a recipient rule. Queueing one writes ordinary email_queue rows (email_type 'newsletter', origin 'campaign', new campaign_id), so retry, rendered_html, sent_at and error_message all come from the existing processor rather than a parallel sender. Throttling staggers scheduled_at; the processor loop is untouched. Two rules the service enforces: no raw HTML is ever stored (sanitized on write and again on render, idempotently), and opt-out is checked at queue time AND again at send time. Migration 199 adds email_campaigns, email_campaign_recipients, email_queue.campaign_id, customer_accounts.marketing_opt_out(_at), and the newsletters.view / newsletters.send permissions. Three rounds of external review are folded in, including several that would otherwise have shipped broken: - Campaign rows never came due on SQLite. queueEmail writes a Date, which the sqlite3 binding stores as epoch ms; ISO text in the same column compares as TEXT against an INTEGER, and SQLite orders every INTEGER below every TEXT. The feature silently sent nothing there. - The flag had no Settings card and no sidebar entry, so it could not be enabled through the UI at all. - Consent is per ADDRESS, not per row: two accounts sharing an inbox meant unsubscribing stopped one and not the other, at both queue and send time. - The unsubscribe GET mutated consent, so a mail-security scanner walking a campaign could have unsubscribed much of the list. GET now confirms, POST acts. - The rate ceiling is clamped to the queue's real throughput (10/min), so the composer's estimate stops being wrong by up to 12x. Closes #1264 |
||
|
Paul Nothaft
|
a7d0972b13 |
fix(security): make the CSS sanitizer's remote-URL block actually block
sanitizeCSS "blocked" a remote url() by prefixing it with a /* BLOCKED URL */ COMMENT and leaving the URL in place. CSS comments are discarded during tokenization, so the declaration a browser parsed still carried the live URL — while adminCssTemplates returned sanitization_warnings claiming it had been stopped. Protection that reports success is worse than none, which is why it survived review. Scope is narrow: sanitizeCss (lowercase, the public-site path) never included the pattern and permits remote URLs by design — a test now pins that. Only sanitizeCSS (uppercase) was affected; outside this repo's newsletter branch its sole caller is adminCssTemplates.js. Migration 200 is required, not cosmetic: gallery.js serves css_templates.css_content VERBATIM as text/css and does not re-sanitize on read, so fixing the write path alone would leave every existing template serving its URL forever. Review follow-ups replaced the regex with a small three-state lexer (comment / string / identifier) over the RAW text, after five further bypasses: a ")" inside a quoted url(), CSS escapes (u\72l), the HTML comment strip JOINING tokens into a live url() after the scan, an escaped quote desynchronising the scan, and a quote inside a comment. Escapes are decoded only to decide, never to rewrite — a clean input now round-trips byte-identical, which also keeps unaffected rows out of the migration's write path. Severity is low (writing a template needs branding.edit) but the harm is a gallery visitor's IP reaching a third party from a page the operator believes carries no remote requests. |
||
|
Paul Nothaft
|
de3a7f70bf |
fix(cms): enable the Tailwind typography plugin so prose classes work (#1288)
tailwind.config.js had plugins: [] and @tailwindcss/typography was never
installed, so every prose / prose-neutral / dark:prose-invert class in the
app resolved to nothing. Preflight, which IS active, resets h1-h6 to
inherit size and weight and strips list-style from ul/ol — so an applied
<h2> rendered pixel-identical to the <p> it replaced.
The editor was never broken. The toolbar highlighted because
editor.isActive('heading') correctly returned true; only the CSS to show
it was missing. That also explains why pasting rendered rich text worked:
it carries inline styles.
Ten surfaces rely on these classes, including the PUBLIC CMS pages — so
impressum/datenschutz were serving unstyled headings to visitors too.
Review follow-ups: prose colours are mapped to the theme tokens wherever
.text-theme marks theme-owned text (a dark gallery preset sets
--color-text but no .dark class, so dark:prose-invert never engages and
headings would have gone near-black on dark); code blocks inherit rather
than being scaled twice; and H5/H6 get explicit rules, since the plugin
only styles h1-h4.
Closes #1288
|
||
|
Paul Nothaft
|
4afe7a6f08 |
fix(gallery): bound concurrent image fetches and abort them on unmount (#1287)
Hardening for the large-gallery stall. The reporter could not isolate the
cause and neither could I from static reading; these are two defects that
are wrong independently of whether they are the whole story.
Gallery grids are NOT virtualized: a 546-photo event puts 546 PhotoCards
in the DOM, each mounting its own bare fetch. Two problems there: no cap,
and a cleanup that only set a flag while the request kept running.
withImageFetchSlot now holds requests to six in flight, with the BODY read
inside the slot — fetch resolves on headers, so releasing there would have
bounded header round-trips and nothing else. Teardown aborts via
AbortController.
A request queued indefinitely is PENDING, not failed, which is why the
failure left no console error, no failed request and nothing in the
backend log.
Review follow-ups: three tiers (current lightbox slide > neighbour
prefetch > grid thumbnails), because a single FIFO put the image the user
just clicked behind hundreds of thumbnails; and a synchronous throw now
releases its slot instead of permanently draining the pool.
If it recurs, capture performance.getEntriesByType('resource') for the
stalled thumbnails — a queued request shows responseStart === 0.
Relates to #1287
|
||
|
Paul Nothaft
|
b6e40b9a2a |
feat(email): global signature footer from the business profile (#1264)
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)
|
||
|
Paul Nothaft
|
90da797e3f |
chore(main): release 3.122.7-beta.0 (#1282)
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Failing after 11s
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-aio (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-ml (linux/amd64, ubuntu-latest) (push) Has been skipped
Build and Push Docker Images / smoke-aio (push) Failing after 10s
Build and Push Docker Images / merge-backend (push) Has been cancelled
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / build-aio (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-aio (push) Has been cancelled
Build and Push Docker Images / build-ml (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-ml (push) Has been cancelled
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-frontend (push) Has been cancelled
Build and Push Docker Images / dockerhub-descriptions (push) Has been cancelled
Build and Push Docker Images / summary (push) Has been cancelled
|
||
|
Paul Nothaft
|
ea394b5ee5 |
Merge pull request #1280 from PicPeak/fix/security-scan-batch-1
fix(security): batch 1 — zxcvbn DoS, revocation forgery, unlink traversals, stored Content-Type, edge middleware |
||
|
Paul Nothaft
|
f3b062a3a7 |
docs: document the upload allow-list and the chunked-upload type rule
Adds docs/upload-file-types.md: the single Allowed File Types setting, every path it governs, the extension-to-MIME table, how to enable video, and what changed for chunked-upload/init (declared mimeType ignored, extension must be allowed, 400 File type not allowed). README links to it, and the Settings help text in EN and DE now says the list covers all upload paths and that video extensions must be added explicitly. |
||
|
Paul Nothaft
|
6350f86907 |
chore(deps): apply non-breaking npm audit fixes
backend: qs and body-parser (array-limit bypass, isBuffer DoS). frontend: axios 1.17 line (formToJSON recursion DoS, prototype-pollution gadgets, maxBodyLength bypasses), dompurify, linkify-it and the transitive set npm audit fix resolves without a major bump. Left out on purpose: sanitize-html 2.17.7 (its htmlparser2 12 tree is ESM-only, which Jest 29 cannot load, and the SVG SMIL advisory needs svg tags none of our sanitizer configs allow), and the tiptap 2->3 and react-router 6->7 majors (open redirect via <Link> needs a user-controlled navigation target, which the SPA has none of). |
||
|
Paul Nothaft
|
dc5bccba05 |
chore(main): release 3.122.6-beta.0 (#1279)
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Failing after 11s
Build and Push Docker Images / build-aio (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-ml (linux/amd64, ubuntu-latest) (push) Has been skipped
Build and Push Docker Images / smoke-aio (push) Failing after 10s
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-backend (push) Has been cancelled
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-frontend (push) Has been cancelled
Build and Push Docker Images / build-aio (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-aio (push) Has been cancelled
Build and Push Docker Images / build-ml (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-ml (push) Has been cancelled
Build and Push Docker Images / dockerhub-descriptions (push) Has been cancelled
Build and Push Docker Images / summary (push) Has been cancelled
|
||
|
Paul Nothaft
|
a87e688484 |
Merge pull request #1277 from PicPeak/fix/1275-hybrid-pointer-input-mode
fix(gallery): follow the input in use, not the device's primary pointer (#1275) |
||
|
Paul Nothaft
|
0b6b8fbdb0 |
fix(gallery): follow the input in use, not the device's primary pointer
Closes #1275. Follow-up to #1263. `matchMedia('(hover: none) and (pointer: coarse)')` answers "what is this device's primary pointer", which on anything with both inputs is the wrong question. A touchscreen laptop reports fine+hover, so a finger tap was handled as a click: the photo opened with no reveal step and the tile's own actions needed a hover a finger cannot produce. An iPad with a trackpad reports the opposite, so a mouse click was handled as a tap and opening a photo took two of them while hovering did nothing. Pointer events carry the answer per interaction. useInputMode holds one module-level mode fed by a single window-level listener pair, so every tile agrees and the listener count does not scale with the grid. The primary-pointer query stays as the opening guess -- it is right for the two single-input cases that are most of the traffic, a phone and a desktop -- and the first real interaction corrects it on a hybrid. pointermove matters as much as pointerdown: a mouse announces itself by approaching, and the mode has to be right BEFORE the click, not as a consequence of it. A pen is grouped with touch, since it taps rather than hovers on most hardware and being wrong that way costs only a reveal step. GalleryPremiumLayout's touch rules move off the media query onto a data-input-mode attribute the layout sets, for the same reason: on a touchscreen laptop the query stayed false and a finger could never reach the checkbox or like button, and on an iPad with a trackpad it stayed true and both were stuck on permanently. The #1263 guarantee is unchanged and pinned by a test that walks all three modes: a control that cannot be seen cannot be hit, whichever input is in use. Verified in the running app under Chrome touch emulation, on a device advertising a coarse primary pointer -- the iPad-with-trackpad case. A mouse merely moving switched the grid to hover semantics and revealed the overlay, and a subsequent tap switched it back; the premium layout's attribute followed, with its checkbox reachable under touch and hidden-but-inert under mouse. The mirror case (finger on a fine-primary device) cannot be staged in Chrome, which couples touch emulation to a coarse primary pointer, so it rests on the jsdom tests. 12 tests: 8 on the store, 4 more on PhotoCard. 3 of the 4 fail without the per-interaction mode; the fourth is the #1263 no-regression guard and holds on both sides by design. |
||
|
Paul Nothaft
|
30fa320dc2 |
chore(main): release 3.122.5-beta.0 (#1276)
Build and Push Docker Images / build-ml (linux/amd64, ubuntu-latest) (push) Has been skipped
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-aio (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / smoke-aio (push) Failing after 10s
Build and Push Docker Images / merge-frontend (push) Has been cancelled
Build and Push Docker Images / build-aio (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-aio (push) Has been cancelled
Build and Push Docker Images / build-ml (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-ml (push) Has been cancelled
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-backend (push) Has been cancelled
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / dockerhub-descriptions (push) Has been cancelled
Build and Push Docker Images / summary (push) Has been cancelled
|
||
|
Paul Nothaft
|
b9c29fcf9b |
Merge pull request #1274 from PicPeak/fix/1261-crm-invitation-visibility
fix(crm): tell the admin whether a customer's invitation actually went out (#1261) |
||
|
Paul Nothaft
|
ec1df704b4 |
Merge pull request #1273 from PicPeak/fix/1262-email-queue-visibility
fix(email): show a queue nobody is working instead of reporting all-clear (#1262) |
||
|
Paul Nothaft
|
db197e7685 |
Merge pull request #1272 from PicPeak/fix/1263-mobile-photo-tap-collisions
fix(gallery): stop invisible overlay controls swallowing mobile taps (#1263) |
||
|
Paul Nothaft
|
2d403f7fb2 |
fix(email): wire the settings status card, and cap-aware truncation
Codex review round 4 on #1273. Settings → Status rendered a green check for the email processor unconditionally, against an API field that was itself the literal 'active'. Both ends were lying and only one of them got fixed: adminSystem started reporting the real state in an earlier commit, but StatusTab never read it, so the second place an admin looks to find out why mail is not arriving still said everything was fine. It now shows stopped and degraded, with the reason. The truncation flag missed the case it most needed to cover. The loop broke on the 200-row report cap before the flag could be set, so 201+ overdue rows came back as exactly 200 with scanTruncated false -- a partial report presented as complete. It is now set whenever rows were left unexamined. The grace-window comment claimed the processor clears ~6000 rows inside the window. It clears on the order of 100: ten rows a pass, one pass a minute. The comment now says so, and says why the processor's own state is reported above the list rather than inferred from it -- "running, last pass sent 10" next to a backlog reads very differently from "not running" next to the same backlog. One round-4 finding is NOT fixed, deliberately, and is written up at the retry route. Clearing scheduled_at leaves created_at at the original enqueue time, so a retried old row appears in the waiting list immediately, looking overdue, until the processor sends it. Restarting that clock needs a timestamp written there and no shape works: a Date matches how queueEmail writes the column and how processEmailQueue compares it, but jest's sandbox Dates store as "[object Object]" (CLAUDE.md) so it cannot be tested; an ISO string tests fine but stores as TEXT, which SQLite then orders above the numeric bound in the processor's own pickup query, leaving the row unsendable. A requeued_at column would settle it. Cosmetic either way, and not worth risking a stuck row. 1 more test, failing before this commit. |
||
|
Paul Nothaft
|
4deac229ac |
fix(email): make waiting rows read-only, and time the grace from when due
Codex review round 3 on #1273. The first finding reverses a round-1 fix of mine, correctly. Retry no longer sends. Round 1 flagged that retry was a no-op for waiting rows and offered two remedies: give them a send-now action, or stop showing them Retry. I took the first, and round 3 showed why it is the wrong half -- processEmailQueue claims nothing before invoking the transport, so a flush overlapping the scheduled pass has both of them sending the same email. Saving 60 seconds is not worth a duplicate landing in a customer's inbox, and a claim protocol would need a status no query watches plus a reaper for rows abandoned mid-send. So retry is a reset again, as it was on main. Waiting rows now carry no actions at all, which is the other half of that round-1 remedy and closes a worse hole the shared table opened: Dismiss DELETEs the queue row. Those emails have not failed and still go out once the processor recovers, so clicking the tidy-up icon on a health warning silently cancelled a customer's mail. The section is diagnostic; what a waiting row needs is the processor fixed, which the panel above it now says. The grace window runs from when a row became DUE, not from when it was queued. A split-payment invoice created three days ago and scheduled until a minute ago has had one minute of the processor's attention, and measuring from created_at reported every scheduled mail as unworked the instant it came due -- which is most of what this panel would then have been showing. A truncated scan can no longer read as an all-clear. The scan is bounded, so a queue larger than the budget whose head is all future-scheduled can hide a due row past the last page read; the response now says so and the UI withholds the green check. The test fixtures were wrong in a way worth keeping: scheduled_at also defaults to CURRENT_TIMESTAMP, so back-dating created_at alone built rows that cannot exist in production -- old, but scheduled for the moment the fixture ran. The helper now back-dates both, as the database would have. 3 more tests; the two that pin new behaviour fail before this commit, and the reverted flush is pinned by asserting the transport is NOT invoked. |
||
|
Paul Nothaft
|
bc90b4db62 |
fix(crm): label the two invitation conflicts and stop guessing after a 5xx
Codex review round 2 on #1274. Both findings are the same shape as round 1: a message that asserts more than the response supports, and sends the admin somewhere that makes it worse. A 5xx is no longer treated as a clean failure. createInvitation inserts the customer_invitations row and only then queues the email, with no transaction around the pair, so a 500 out of the queueing step leaves an OPEN invitation behind. Telling the admin "no invitation went out, retry" there walks them into a 409 that still queues nothing. 5xx now joins the no-response case as unconfirmed; a plain 4xx keeps the clean-failure message, because that is the one shape where nothing was written. The two already-active conflicts now say so. The send-invite route returns CUSTOMER_ALREADY_ACTIVE from its own check, but createInvitation rechecks customer_accounts afterwards and threw a bare ConflictError -- code CONFLICT, indistinguishable from the pending-invitation conflict. An invitation accepted between the two checks therefore landed in the pending branch, telling the admin to cancel an invitation that acceptance had just closed. Both conflicts in the service now carry a code of their own, so the client reads the code rather than inferring from the status. 4 more tests. One round-1 test changed with the behaviour it pinned: its 500 now asserts the unconfirmed message, and a new 400 case covers the clean failure it used to stand for. |
||
|
Paul Nothaft
|
89db469f06 |
fix(email): compare queue timestamps in JS, and make retry actually send
Codex review round 1 on #1273. One of the four is a real bug on every SQLite deployment. The waiting-row query compared `created_at` against a bound ISO string. On SQLite that column does not hold a string: queueEmail writes a JS Date and the native binding stores epoch ms, and SQLite orders INTEGER before TEXT regardless of value -- so the comparison was true for EVERY row. Mail queued a second ago read as ten minutes overdue, and a scheduled_at years in the future read as already due. Confirmed directly against sqlite3: a 2026 row matches `created_at <= '2020-01-01T00:00:00.000Z'`. Binding a Date instead is not the fix, since knex hands sqlite3 a Date the same way and jest's sandbox Dates stringify to "[object Object]" (CLAUDE.md). So the engine-safe half of the predicate stays in SQL and the two time comparisons move into JS behind a toMillis() that accepts all three shapes this column really has -- Date from Postgres, ms-number from SQLite, ISO string from fixtures and older rows. The scan is capped at 1000 pending rows ordered oldest-first; everything overdue sorts into that window, and the response was already capped at 200. The existing tests missed this because they store ISO strings, which is what CLAUDE.md prescribes for jest -- so the new ones store epoch ms, the production shape, and one mixes both in a single queue. Retry was a no-op for the rows it most needed to help. It wrote pending / retry_count 0 / no schedule, which is exactly what a waiting row already is: the row came back unchanged while the toast said it had been re-queued. And since the usual reason a row is waiting is that nothing is working the queue, deferring it to the next pass is the one answer that cannot help. It now follows the reset with the same single-row flush the project cockpit uses. An idle pass no longer inherits the previous pass's totals -- the no-pending early return skipped the lastResult assignment, so System Health kept attributing an old sent/failed count to a run that did nothing. "All clear" now means the whole queue is clear, which is what the PR claimed and the code did not do. An empty waiting list is only reassuring when something is working the queue: a processor stopped a minute ago has no overdue rows yet either, and a green check there is the same false all-clear this branch exists to remove. 7 more tests. The 5 that pin new behaviour fail before this commit; the SQLite ones fail in the way the bug predicts rather than erroring. Both new tests stub the webhook transport with a spy rather than pointing it at a dead port: real connection attempts left open handles that destabilised unrelated suites in the same jest worker. |
||
|
Paul Nothaft
|
6bb12c6612 |
fix(crm): stop the invitation UI claiming more than it can know
Codex review round 1 on #1274. Three places where this branch replaced one overclaim with another. The badge tooltip said "Invitation sent". createInvitation inserts the customer_invitations row and only then queues the email, with no transaction around the pair, so an open invitation does not prove an email_queue row exists -- and even when it does, delivery is the queue processor's business minutes later. The tooltip now describes the invitation link itself and points at System health, which is the same distinction #1273 draws. Both conflicts are 409 and were being treated as one. Migration-era send-invite returns 409 with code CUSTOMER_ALREADY_ACTIVE when the customer already has a password, which happens if an open invitation for that address is accepted between createDirect and sendInvite. There is then no invitation row to cancel, so directing the admin to the Invitations tab points them at something that does not exist. The code is now read before the message is chosen. A dropped connection or a timeout rejects with no `response` at all, and the request may well have succeeded server-side. Saying "no invitation went out" there sends the admin into a retry that then 409s, which is the same trap the CUSTOMER_ALREADY_ACTIVE case sets. That branch is now explicitly unconfirmed and says where the answer is. 3 more tests, all 3 failing before this commit. |
||
|
Paul Nothaft
|
d0274886e6 |
fix(gallery): decide the overlay by pointer capability, not viewport width
Codex review round 1 on #1272. Both findings are consequences of extending the Grid/Justified tap-to-reveal model to every layout: what those two layouts got away with, because they were the only ones using it, becomes wrong once Masonry, Mosaic and Timeline inherit it. The hover variants no longer hide behind `md:`. On a fine pointer under 768px `isTouchDevice` is false, so nothing reveals the overlay, and the `md:` prefix disabled the only hover variants there were -- the controls stayed `opacity-0 pointer-events-none` with no way to reach them. Grid and Justified already behaved that way, but Masonry, Mosaic and Timeline had unprefixed `group-hover:` and revealed at any width, so this was a regression for them. Width was never the real question: what the breakpoint was standing in for is that :hover latches on a touchscreen once a tile is tapped. So the variants are emitted for pointer devices only and withheld on touch, which says that directly. detectCoarsePointer no longer ORs the touch fallbacks over matchMedia. matchMedia describes the PRIMARY pointer; `ontouchstart` and `maxTouchPoints` only say a touchscreen exists somewhere, which is equally true of a touchscreen laptop or a docked tablet being driven by its mouse. OR-ing them classified those as touch-only, so an ordinary click merely revealed the overlay and opening a photo took two clicks. The fallbacks now stand in only where matchMedia is absent, which is what the comment already claimed. 3 more tests, all 3 failing before this commit. |
||
|
Paul Nothaft
|
1b8e5f83d7 |
fix(crm): tell the admin whether a customer's invitation actually went out
Closes #1261. "Invite customer" is two calls: createDirect, then sendInvite. The mode wiring is right -- CustomerManagementPage passes mode='invite' and InlineCustomerCreate does call sendInvite -- so the reported symptom is not a missed branch. It is that nothing downstream distinguishes the outcomes. Three things could not be told apart afterwards: - The success toast claimed "portal invitation sent". sendInvite only queues an email_queue row; whether it was delivered is decided minutes later by the queue processor. The toast now says queued, and says what sends it. - When sendInvite failed, the warning read "Invitation email failed -- retry from the customer detail page", which sounds like the mail bounced. What actually remains is a PASSIVE customer with no invitation at all, so it says that instead. A 409 is now separated out: that means an invitation for the address is already open and the RE-invite was refused, so the customer is invited and telling them to retry sends them the wrong way. - The customers table rendered a customer whose invitation never went out identically to one the admin created as passive on purpose -- both showed only "Passive - admin only". Passive customers with an open invitation now show "Invitation pending", matched case-insensitively because customer_invitations lowercases the address while customer_accounts keeps what the admin typed. The invitations list was already being fetched for the tab; this only cross-references it. Active customers are left alone: they have portal access, so a stale invitation row for their address says nothing about them. 7 tests; the 5 that assert the new behaviour all fail before the change, and the 2 negative controls pass on both sides. |
||
|
Paul Nothaft
|
73d867521a |
fix(email): show a queue nobody is working instead of reporting all-clear
Closes #1262. "Gallery email queued" reads as a delivery confirmation, and System Health agreed with it: "No stuck or failed emails -- all clear", while not one email had gone out. Both statements were true and neither was the one the admin needed. Queueing writes an email_queue row at status='pending', retry_count 0 -- nothing more. /failures matched only status='failed' or pending-with-retry_count>=3, so it matched none of those rows, and there are two ordinary ways they never leave that state: - startEmailQueueProcessor() was never reached, so nothing polls the queue. - Every pass returns early. processEmailQueue bails when the transporter will not initialise, before it touches a single row, so retry_count stays 0 and no error_message is ever written. A working SMTP test button does not contradict this: that path builds its own transport. adminSystem.js made it worse by reporting `emailProcessor: { status: 'active' }` as a literal, so the one place that named the worker always said it was fine. - emailProcessor records what each pass did -- started, lastRunAt, lastResult, lastError -- and exports getQueueProcessorStatus(). The transporter bail and the queue-query failure, the two silent early returns, both write lastError. - /failures gains `waitingEmails`: pending, under the retry cap, past any scheduled_at, and queued more than 10 minutes ago. The predicate mirrors the processor's own pickup query, so a row listed there is one it should already have taken; rows over the cap stay in `stuckEmails` and are not counted twice. A future scheduled_at is left alone -- split-payment invoices and the business-hours floor park rows deliberately. - System Health leads with the processor's state (running / stopped / degraded) and lists waiting emails in their own table. The all-clear now needs both buckets empty. - adminSystem reports the real processor state instead of the literal. - The two "queued" toasts say the queue processor is what sends it and where to look if it doesn't arrive. 8 route tests, all 8 failing before the change. |
||
|
Paul Nothaft
|
c0d34796cd |
fix(gallery): stop invisible overlay controls swallowing mobile taps
Closes #1263. A tap on a photo tile did one of three things depending on where the finger landed: opened the photo, downloaded it, or liked it. The cause is that `opacity-0` hides pixels but not hit-testing. The overlay's View/Download/Like buttons and the selection checkbox were rendered at opacity 0 and left fully tappable; each one calls stopPropagation, so hitting an unseen button both fired its action and suppressed the tile's own open. On a pointer device hover reveals the controls before anyone can click them, so the gap never showed. On a touchscreen there is no hover, so in Masonry, Mosaic and Timeline the controls were invisible for good and tappable for good. Visibility and hit-testing now move together. PhotoCard computes both from one place, so every layout that uses it gets the same rule instead of passing its own opacity classes: - `touchAware` is gone. It gated the tap-to-reveal state machine, and only Grid and Justified opted in -- which is why those two behaved and the other three did not. Every PhotoCard layout is touch-aware now: first tap reveals the controls, second tap on a control acts, second tap elsewhere opens the photo. Pointer devices keep hover semantics unchanged. - The pointer reading moved from an effect into the initial state. As an effect it landed a mount-time render between the tile measurement in useLayoutEffect and the image mount that measurement gates, remounting every card once -- caught by the #1095 regression test, which is the reason that test exists. It also now degrades to ontouchstart/maxTouchPoints where matchMedia is absent, since every layout runs this path now. Two more instances of the same class, outside PhotoCard: - GalleryPremiumLayout's checkbox and like button are CSS-hidden the same way. They get pointer-events alongside opacity, and because that layout has no reveal gesture, a `(hover: none)` block shows both outright at a finger-sized target rather than leaving them unreachable. - PhotoGrid's download button called `onClick={onDownload}` with no stopPropagation, so downloading also opened the lightbox. Verified on a mobile viewport with real touch emulation: at rest the tile centre now hits the image rather than an unseen Download button, and one tap reveals the controls instead of downloading the file. 5 tests, all 5 failing before the change. |
||
|
Paul Nothaft
|
ca8050899b |
chore(main): release 3.122.4-beta.0 (#1270)
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Failing after 11s
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-aio (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-ml (linux/amd64, ubuntu-latest) (push) Has been skipped
Build and Push Docker Images / smoke-aio (push) Failing after 11s
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-backend (push) Has been cancelled
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-frontend (push) Has been cancelled
Build and Push Docker Images / build-aio (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-aio (push) Has been cancelled
Build and Push Docker Images / build-ml (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-ml (push) Has been cancelled
Build and Push Docker Images / dockerhub-descriptions (push) Has been cancelled
Build and Push Docker Images / summary (push) Has been cancelled
|
||
|
Paul Nothaft
|
f722bdaf4b |
Merge pull request #1268 from PicPeak/fix/1265-guest-identity-persistence
fix(guests): keep guest identity across a tab close (#1265) |
||
|
Paul Nothaft
|
63fa05b181 |
chore(main): release 3.122.3-beta.0 (#1269)
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-ml (linux/amd64, ubuntu-latest) (push) Has been skipped
Build and Push Docker Images / smoke-aio (push) Failing after 9s
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-aio (linux/amd64, ubuntu-latest) (push) Failing after 10s
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-backend (push) Has been cancelled
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-frontend (push) Has been cancelled
Build and Push Docker Images / build-aio (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-aio (push) Has been cancelled
Build and Push Docker Images / build-ml (linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and Push Docker Images / merge-ml (push) Has been cancelled
Build and Push Docker Images / dockerhub-descriptions (push) Has been cancelled
Build and Push Docker Images / summary (push) Has been cancelled
|
||
|
Paul Nothaft
|
28f14955e2 |
fix(gallery): clear the guest identity on gallery logout
The gallery password is one shared secret per event and does not distinguish people. With the guest identity outliving the tab, logging out and letting the next person enter that password greeted them by the previous guest's name, with "forget me" - which erases that guest's selections server-side - one click away. Logout is the leaving-this-device signal, so it now drops the local identity too. Server row untouched. |
||
|
Paul Nothaft
|
f2f40893c1 |
fix(guests): drop a stored identity when a spent invite names someone else
A guest coming back through their own already-redeemed link is the ordinary #1265 case, and the identity the device holds is theirs. The same link opened on a shared device that holds another guest's identity is not: the redemption 409s, ensureIdentity() falls through to the stored identity, and the visitor's likes are filed under the previous person. The two cases were indistinguishable client-side, so the 409/410 body now carries the invite's guest_id. On a mismatch the stored identity is cleared and the visitor is asked who they are. A response without guest_id keeps the previous behaviour. |
||
|
Paul Nothaft
|
7a1ea842e4 |
fix(guests): read identity from whichever store holds it, write it as a pair
Two defects in the storage fallback, both reproduced: The quota fallback repointed reads at sessionStorage through module state, which a reload discards. The next page load probed localStorage, passed the one-byte probe, tried to promote the pair and was refused on the same quota, swallowed that, and read an empty localStorage: the identity sat one store over, unreadable, and the guest re-registered. Reads are now read-through: primary store first, sessionStorage second, promoting into the primary only when it will take the pair and leaving it where it fits when it will not. No module state has to remember which store won. The migration wrote the token before the profile, so a store that accepted the first write and refused the second left a token with no profile: x-guest-token was sent while the provider prompted to register, producing a second row with two live tokens. Every write is now profile-first and rolls back on failure, so a store holds the whole pair or none of it. |
||
|
Paul Nothaft
|
7d51aa3db9 |
fix(guests): don't answer feedback with a stale identity mid-invite
Last open finding from codex round 3 on #1268. Invite redemption is async and the gallery stays interactive while it runs, so a like clicked in that window resolved against the persisted identity and was filed under the wrong guest permanently. ensureIdentity() now waits on the in-flight redemption and re-reads the result before falling back to the stored identity or the prompt. |
||
|
Paul Nothaft
|
e9babf65e7 |
fix(guests): rebuild consumers on identity switch; repair fallback reads
Codex review round 3 on #1268. Three of these were defects in the round 1-2 fixes themselves. Consumers holding local feedback state are now rebuilt on an identity switch. Invalidating queries was not enough: six gallery layouts seed their liked set behind a mount-only likedSeededRef ('so refetches don't clobber in-session optimistic toggles') and PhotoLightbox keeps its own copy, so a refetch left the previous guest's hearts on screen. The provider re-keys its subtree, which covers all seven without touching them. Deliberately only on a switch away from an established identity -- remounting on first sign-in would tear down the gallery under the click that triggered the prompt and drop the pending action. The storage fallback now repoints reads. storeGuestIdentity wrote to sessionStorage when localStorage rejected the real write but left resolvedStorage on localStorage, so every later read missed: x-guest-token was never sent and the identity vanished on reload. The fallback looked like it worked while achieving nothing. Clearing an identity now notifies this tab. Native storage events fire only in other documents, so the interceptor dropping a server-rejected identity left the provider still showing that guest and ensureIdentity() still handing it out. A same-tab event completes the loop. Cross-tab adoption resolves pending callers. A tab parked on the prompt awaiting ensureIdentity() while another tab registers now completes exactly as register() does, instead of hanging forever and registering a second guest if the visitor submits the still-open prompt. |
||
|
Paul Nothaft
|
f3f37a8c77 |
fix(guests): invite wins over stored identity; clear server-rejected ones
Codex review round 2 on #1268. Four findings, all reachable only because the identity now persists. An explicit ?invite= now takes precedence. The redeem effect skipped when an identity already existed, which was harmless while identity died with the tab. Persisted, it means opening guest B's invite on a browser where guest A once visited restores A, never redeems B's invite, and files B's likes under A. A ref keeps it to one redemption per token. Guest-scoped caches are invalidated when the identity changes. my-feedback, gallery-photos and photo-feedback are keyed by slug and photo id, never by guest, so they outlived an identity change and showed the previous guest's likes while requests already carried the new token. Now reachable three ways: another tab, 'Not you?', and an invite redeemed over an existing identity. An identity the server has rejected is dropped. resolveGuest nulls req.guest for a soft-deleted or merged-away row even when the JWT is validly signed and unexpired, and the route answers GUEST_IDENTITY_REQUIRED — no client-side expiry check can catch that. Self-limiting when identity died with the tab; persisted, it would fail every like for up to 30 days while the footer still showed the guest's name. The write fallback now covers the real write, not just the probe. A one-byte probe fits in a nearly-full store that still rejects a JWT plus profile, which left the context believing it was signed in with nothing persisted. |
