Skip to content

Add app-wide login and profile-photo avatars (BL-16514)#8046

Merged
StephenMcConnel merged 5 commits into
masterfrom
BL-16514LoginAndAvatars
Jul 9, 2026
Merged

Add app-wide login and profile-photo avatars (BL-16514)#8046
StephenMcConnel merged 5 commits into
masterfrom
BL-16514LoginAndAvatars

Conversation

@hatton

@hatton hatton commented Jul 8, 2026

Copy link
Copy Markdown
Member

[Claude Opus 4.8]

What this does

Makes Bloom Library login an app-wide concern (instead of living only on the Publish-to-Web screen) and shows the signed-in user's avatar in the workspace top bar.

Login

  • A new account API — account/status, account/login, account/logout — exposes and controls the Bloom Library login state (backed by BloomLibraryBookApiClient). State changes are broadcast over an account websocket so every screen stays in sync; the front end consumes this through the useLoginState hook.
  • AccountMenu in the top bar shows a Sign in button when signed out, and when signed in shows the user's avatar; clicking it opens a menu with the email and a Sign out item.
  • The login mechanism itself is unchanged: Bloom opens the website's login page in a browser, which posts the result back to external/login.

Avatars

  • Avatars are served by a new person-keyed cache on Bloom's local server: GET /bloom/api/avatar?email=<email>. BloomAvatar points its image at this endpoint, so the account menu and Team Collection member avatars both go through it.
  • The cache is keyed by the MD5 of the normalized email (the Gravatar scheme). For each person it resolves an image source in priority order:
    1. the person's Google/Firebase profile photo, if known;
    2. otherwise Gravatar;
    3. otherwise it returns 404, and react-avatar shows generated initials.
  • The website now includes the Google/Firebase photoUrl in the external/login callback (companion PR: Send Google/Firebase profile picture to Bloom editor on login (BL-16516) BloomLibrary2#615). Bloom stores it keyed by the user's email. This is currently the only way a "known" provider photo gets populated, so team members who have never logged in on this machine resolve via Gravatar.
  • The cache stores the actual image bytes on disk, so avatars survive a restart and work offline. It attempts a fresh fetch once per Bloom run and falls back to the cached copy when the fetch fails — so a user who changes their Gravatar or Google photo sees the new one after restarting Bloom.

Compatibility

The photoUrl field on the external/login callback is optional. Bloom and the website deploy independently in either order: an older website just omits the field (Bloom falls back to Gravatar), and an older Bloom ignores it. No coordinated deploy is required.

Deployment status (as of 2026-07-08): the companion website PR (BloomBooks/BloomLibrary2#615) is not yet deployed to sandbox/production, so signing in currently shows the Gravatar fallback — this is expected, not a desktop bug. The Google/Firebase photo will appear once the website change ships. For robustness across the independent deploys, this desktop side accepts either photoUrl or Firebase's native photoURL (capital URL) casing.

Tests

New C# tests cover the avatar cache (source precedence, once-per-run refresh, offline fallback to cached bytes, and persistence/clearing of the known-photo map) and the avatar / external/login endpoints.

Ref: BL-16514

Devin review


This change is Reviewable

Move Bloom Library login state out of the Publish screen into an app-wide
account API (account/status|login|logout) that broadcasts changes over a
websocket, and add a top-bar account menu that shows the signed-in user's
avatar and a sign-out item.

Avatars are served by a new person-keyed cache on Bloom's local server
(GET /bloom/api/avatar?email=). Keyed by the MD5 of the email (Gravatar's
scheme), it caches the image bytes on disk so avatars survive a restart and
work offline. Source precedence per person: the user's Google/Firebase
profile photo (now sent by the website in the login callback, when present),
otherwise Gravatar, otherwise a 404 so the UI shows generated initials. It
refetches once per Bloom run, falling back to the cached copy if the fetch
fails.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR promotes Bloom Library login from a Publish-screen-local concern to an app-wide one, adds the signed-in user's avatar to the workspace top bar, and backs all avatar display with a disk-persisted, offline-capable local cache keyed by the MD5 of each person's email.

  • A new AccountApi exposes account/status, account/login, and account/logout; state changes propagate via a loginStateChanged websocket event consumed by the new useLoginState hook, which is now shared between AccountMenu and LibraryPublishSteps so both UIs always agree.
  • AvatarCache resolves a source per person (Google/Firebase photo if known, otherwise Gravatar), caches the raw bytes on disk so avatars survive restarts and work offline, and attempts one fresh fetch per Bloom run — with the _refreshedThisRun guard noted in the prior review still present (only successful fetches write to _index, so the short-circuit does not fire for users with no Gravatar account).
  • The external/login callback now accepts an optional photoUrl/photoURL field and registers it in the cache before broadcasting the login state, so the Google photo is available the moment the front end requests the avatar.

Important Files Changed

Filename Overview
src/BloomExe/web/AvatarCache.cs New person-keyed avatar byte cache. Core caching logic is well-designed (write-outside-lock, virtual FetchAsync for testing), but the "once per run" short-circuit is bypassed on failed fetches because _index is only written on success — an issue flagged in the previous review that remains unresolved.
src/BloomExe/web/controllers/AccountApi.cs New app-wide account API (status/login/logout). Clean event-driven design: subscribes to LoginDataChanged, broadcasts loginStateChanged over websocket. Correctly captures email before Logout clears it in HandleLogout. Implements IDisposable to unsubscribe from the singleton event.
src/BloomExe/web/controllers/ExternalApi.cs Accepts optional photoUrl/photoURL (both casings) from the browser login callback and registers it in AvatarCache before SetLoginData fires LoginDataChanged. Correctly tolerates absence of the field for older website builds. Old LoginSuccessful event removed and replaced by the new event-driven path.
src/BloomExe/web/controllers/AvatarApi.cs Thin async endpoint (requiresSync:false) that delegates to AvatarCache and returns 404 when nothing is available so react-avatar falls back to generated initials. Clean and straightforward.
src/BloomExe/WebLibraryIntegration/BloomLibraryBookApiClient.cs Adds LoginDataChanged event, TryRestoreSavedLogin (destination-matched settings restore), and fixes Logout to clear all persisted session tokens. AttemptSignInAgainForCommandLine now delegates to TryRestoreSavedLogin, removing code duplication.
src/BloomExe/ProjectContext.cs Registers AccountApi, AvatarApi, and AvatarCache. Uses an explicit factory (new AvatarCache()) instead of RegisterType to avoid Autofac mis-injecting an unrelated registered string as the cache folder — the DI regression test guards this fix.
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/AccountMenu.tsx New top-bar account control. Shows "Sign in" button when logged out; when logged in shows avatar + dropdown with email and sign-out item. Correctly uses useLoginState hook for shared app-wide state.
src/BloomBrowserUI/react_components/bloomAvatar.tsx BloomAvatar now sources images from the local server (avatar?email=…) rather than calling Gravatar directly. The per-render Cache allocation is correctly promoted to a module-level constant. Falls back to react-avatar's name-based initials on 404.
src/BloomBrowserUI/react_components/useLoginState.ts New shared hook for app-wide login state using useWatchApiObject. Provides email (undefined when signed out), signIn, and signOut — used by both AccountMenu and LibraryPublishSteps so they always agree.
src/BloomTests/web/AvatarCacheTests.cs Comprehensive unit tests for AvatarCache covering source precedence, once-per-run refresh (success path), offline fallback, persistence, and the DI injection regression guard. The once-per-run guarantee for failed fetches (no Gravatar, no known photo) is tested only with a single call; a second call in the same run would reveal the existing bug.
src/BloomTests/web/AvatarApiTests.cs Integration tests covering the avatar HTTP endpoint (200 with bytes, 404 when nothing) and ExternalApi's photo-URL wiring (both photoUrl and photoURL casings, and the absence of the field). Good use of Monitor.Enter to serialize against the shared test port.
src/BloomTests/WebLibraryIntegration/BloomLibraryBookApiClientLoginTests.cs Tests for TryRestoreSavedLogin (no token, destination mismatch, matching destination), Logout (clears all saved settings), and LoginDataChanged event firing. Settings are saved and restored around each test.
src/BloomExe/web/controllers/LibraryPublishApi.cs Removes the now-redundant screen-local login/logout handlers and the ExternalApi.LoginSuccessful subscription; these responsibilities have moved to AccountApi and the LoginDataChanged event.
src/BloomBrowserUI/publish/LibraryPublish/LibraryPublishSteps.tsx Replaces the screen-local WebSocket subscription and post("libraryPublish/login
DistFiles/localization/en/Bloom.xlf Adds three new AccountMenu strings (Account, Sign in, Sign out) with translate="no", consistent with the project-wide convention used throughout this file.
src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs Removes the now-unused LogIn/LogOut forwarding methods; callers have been redirected to AccountApi.
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsx Layout updated from column to row flex to accommodate the AccountMenu alongside the existing language/help/zoom controls.

Reviews (5): Last reviewed commit: "Avoid nested ternary; document the prefe..." | Re-trigger Greptile

Comment thread src/BloomExe/WebLibraryIntegration/BookUpload.cs Outdated
Comment thread src/BloomExe/WebLibraryIntegration/BookUpload.cs Outdated
@hatton

hatton commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-08 19:00 UTC up to commit ae181659b. Result: 0 bugs, 1 Investigate flag ("Debug builds now upload to production" — posted inline, corroborates Greptile's P1), and 5 informational items (skipped as low-signal: logout broadcast-before-clear ordering, unconditional registration/userInfo fetch, undisposed MemoryStream in AvatarApi, ValidateTokenAsync no-op stub, and useLoginState return recreated each render).

hatton and others added 3 commits July 8, 2026 13:06
…ton (BL-16514)

- BookUpload.UseSandboxByDefault: revert the temporary DEBUG override so debug
  builds default to the sandbox again (flagged by all three reviewers as an
  accidental production-upload risk).
- Register AvatarCache as SingleInstance instead of InstancePerLifetimeScope, so
  its shared in-memory known-photo map and disk cache can't be duplicated by a
  child-scope resolve.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…6514)

Autofac was injecting the registered editableCollectionDirectory string into
AvatarCache's optional `cacheFolder` constructor parameter, so the avatar cache
wrote its files into the user's open collection folder instead of the app-data
AvatarCache folder -- polluting the collection and tying the persisted
known-photo map to whichever collection happened to be open. Register AvatarCache
via a `new AvatarCache()` factory so the parameterless (app-data) path is used.

Also accept Firebase's native `photoURL` casing as well as `photoUrl` in the
external/login POST, since the website (a separate repo that deploys
independently) may forward `user.photoURL` verbatim.

Adds a photoURL-casing test and a DI-registration regression test (the latter
with a sanity assertion proving the Autofac optional-parameter gotcha is real).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Devin flagged that the HttpResponseMessage returned by s_httpClient.GetAsync was
never disposed on any path, leaking its pooled network connection. Since we fetch
one avatar per person, a Team Collection with many members could eventually starve
the shared HttpClient's connection pool. Wrap it in a using declaration.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hatton

hatton commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-08 (23:03 UTC) up to commit 983e84a2b. Result: 0 bugs (the earlier HttpResponseMessage-disposal bug is now fixed — Devin marks it Resolved), 2 Investigate flags: ValidateTokenAsync stub (intentional & documented — surfaced to the developer for awareness) and "Debug builds upload to production" (false positive — that block is identical to master and returns sandbox under #if DEBUG; the temporary override was reverted in 695d4f6; thread resolved), and ~15 informational items (skipped as low-signal).

@hatton

hatton commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Preflight decision record — two Devin/local-review items reviewed and deliberately left as-is:

  1. ValidateTokenAsync is a stub (restored logins aren't re-validated). Intentional and already documented in-code: api.bloomlibrary.org has no "who am I" endpoint yet, and being offline must not force a logout. A revoked-while-closed token would appear signed-in only until its first rejected request. Accepted as a known limitation; will implement real validation when a users/me endpoint exists.

  2. The DI regression test constructs a real AvatarCache against the app-data folder. Deliberate: it verifies the production DI wiring resolves the app-data path rather than an injected string, and it only asserts on the resulting path (can't false-pass). The lone side effect is creating an empty SIL/Bloom/AvatarCache folder; adding production indirection purely for test tidiness wasn't judged worthwhile.

Replace the stacked ternary for photoUrl/photoURL casing with an
if/else-if chain per review feedback, and add the no-nested-ternary
rule to AGENTS.md so it's documented as a codebase convention.

@hatton hatton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hatton reviewed 6 files and all commit messages.
Reviewable status: 6 of 18 files reviewed, 1 unresolved discussion.

@hatton hatton marked this pull request as ready for review July 9, 2026 16:32

@StephenMcConnel StephenMcConnel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@StephenMcConnel reviewed 19 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on hatton).

@StephenMcConnel StephenMcConnel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StephenMcConnel resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on hatton).

@StephenMcConnel StephenMcConnel merged commit ad2ffc8 into master Jul 9, 2026
4 checks passed
@StephenMcConnel StephenMcConnel deleted the BL-16514LoginAndAvatars branch July 9, 2026 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants