From c0cb013f54d6fd430b739e6616323c8741031838 Mon Sep 17 00:00:00 2001 From: Aaron Po Date: Tue, 13 Oct 2026 01:23:29 -0400 Subject: [PATCH] Show a user's avatar on the public profile (#333) --- docs/web/database.md | 6 +- docs/web/environment-variables.md | 12 +- docs/web/testing.md | 3 +- web/.env.example | 6 +- .../Features/PublicUserProfile.feature | 62 ++++++ .../Mocks/MockFileStorageProvider.cs | 55 +++++ .../API/API.Specs/Steps/ApiGeneralSteps.cs | 46 +++++ web/backend/API/API.Specs/Steps/UserSteps.cs | 189 +++++++++++++++++- web/backend/API/API.Specs/TestApiFactory.cs | 10 + .../Configuration/ConfigurationKeys.cs | 8 + .../Domain.Entities/Entities/UserAccount.cs | 8 + .../Services/S3FileStorageProvider.cs | 36 +++- .../Queries/GetAllUsersHandlerTests.cs | 77 ++++++- .../GetPublicUserProfileByIdHandlerTests.cs | 98 ++++++++- .../Features.Users/Dtos/UserDtoMapper.cs | 22 +- .../Features/Features.Users/Dtos/UserDtos.cs | 1 + .../Queries/GetAllUsers/GetAllUsersHandler.cs | 9 +- .../GetPublicUserProfileByIdHandler.cs | 9 +- .../Repository/UserListRepository.cs | 83 ++++++-- web/docker-compose.db.yaml | 1 + web/docker-compose.dev.yaml | 2 + web/docker-compose.min.yaml | 1 + web/docker-compose.prod.seed.yaml | 1 + web/docker-compose.prod.yaml | 1 + web/docker-compose.test.yaml | 1 + .../features/users/USER_PROFILE_HANDOFF.md | 34 ++-- .../profile/ProfileAvatar.stories.tsx | 51 +++++ .../components/profile/ProfileAvatar.tsx | 42 ++++ .../features/users/routes/user-profile.tsx | 11 +- .../app/features/users/users.server.ts | 3 + .../users/utils/filler-user-profile.ts | 2 +- 31 files changed, 812 insertions(+), 78 deletions(-) create mode 100644 web/backend/API/API.Specs/Mocks/MockFileStorageProvider.cs create mode 100644 web/frontend/app/features/users/components/profile/ProfileAvatar.stories.tsx create mode 100644 web/frontend/app/features/users/components/profile/ProfileAvatar.tsx diff --git a/docs/web/database.md b/docs/web/database.md index 70204a8d..2ffb894f 100644 --- a/docs/web/database.md +++ b/docs/web/database.md @@ -76,14 +76,14 @@ updates (`UPDATE ... WHERE ... AND RowVersion = @RowVersion`). | `UserAccount` | `Domain.Entities.UserAccount`, `Features.Users` | `Username` and `Email` are unique | | `UserCredential` | `Domain.Entities.UserCredential`, `Features.Users` | Argon2id password hash; `IsRevoked`/`RevokedAt` track credential rotation, not JWTs | | `UserVerification` | `Domain.Entities.UserVerification`, `Features.Users` | Written when a user confirms their email | -| `UserProfile` | `Domain.Entities.UserProfile`, `Features.Users` | 1:1 with `UserAccount`; holds the user's biography; written via `IUserProfileRepository`, read via `IUserListRepository.GetByIdAsync`'s left join | -| `UserAvatar` | `Domain.Entities.UserAvatar`, `Features.Users` | Links a `UserProfile` to a `Photo` as its avatar; written via `IUserProfileRepository.SaveAvatarAsync` | +| `UserProfile` | `Domain.Entities.UserProfile`, `Features.Users` | 1:1 with `UserAccount`; holds the user's biography; written via `IUserProfileRepository`, read via `IUserListRepository`'s left joins in both `GetByIdAsync` and `GetAllAsync` | +| `UserAvatar` | `Domain.Entities.UserAvatar`, `Features.Users` | Links a `UserProfile` to a `Photo` as its avatar; written via `IUserProfileRepository.SaveAvatarAsync`, read via `IUserListRepository`'s `ValidTo IS NULL` join for the active avatar | | `BreweryPost` | `Domain.Entities.BreweryPost`, `Features.Breweries` | A user-submitted brewery listing | | `BreweryPostLocation` | `Domain.Entities.BreweryPostLocation`, `Features.Breweries` | 1:1 with `BreweryPost`; `Coordinates` is a `GEOGRAPHY` column, read via a plain Dapper query that selects `CONVERT(varbinary(max), Coordinates)`, since Dapper can't deserialize SQL Server UDTs directly | | `City` | `Domain.Entities.City`, `Features.Locations` | Referenced by `BreweryPostLocation.CityID`; `CityName` and `StateProvinceID` together are unique; written via `ILocationRepository`, read via `ILocationRepository` and `IBreweryRepository`'s location joins, currently written only by `Database.Seed` | | `StateProvince` | `Domain.Entities.StateProvince`, `Features.Locations` | Referenced by `City.StateProvinceID`; created on demand by `ILocationRepository.GetOrCreateCityIdAsync`, read via `IBreweryRepository`'s location joins | | `Country` | `Domain.Entities.Country`, `Features.Locations` | Referenced by `StateProvince.CountryID`; created on demand by `ILocationRepository.GetOrCreateCityIdAsync`, read via `IBreweryRepository`'s location joins | -| `Photo` | `Domain.Entities.Photo`, `Features.PhotoUpload` | Written via `IPhotoUploadRepository.CreateAsync`, sent as an `UploadPhotoCommand` from other features' handlers; not yet seeded by `Database.Seed` | +| `Photo` | `Domain.Entities.Photo`, `Features.PhotoUpload` | Written via `IPhotoUploadRepository.CreateAsync`, sent as an `UploadPhotoCommand` from other features' handlers; `Hyperlink` holds the object key within the bucket, which readers sign into a time-limited URL via `IFileStorageProvider.GetPresignedUrl`; the active avatar's row is also read via `IUserListRepository`'s `Media.Photo` join; not yet seeded by `Database.Seed` | ### Tables defined in the schema but not yet wired to a feature slice diff --git a/docs/web/environment-variables.md b/docs/web/environment-variables.md index 656d0cc1..54035a7d 100644 --- a/docs/web/environment-variables.md +++ b/docs/web/environment-variables.md @@ -260,17 +260,25 @@ and retrieving photos via an S3-compatible object store. ```bash SEAWEEDFS_SERVICE_URL=http://seaweedfs:8333 # Required, no default +SEAWEEDFS_PUBLIC_URL=http://localhost:8333 # Optional, defaults to SEAWEEDFS_SERVICE_URL SEAWEEDFS_ACCESS_KEY_ID=admin # Required, no default SEAWEEDFS_SECRET_ACCESS_KEY=secret # Required, no default SEAWEEDFS_BUCKET=my-bucket # Required, no default ``` - **Implementation**: `S3FileStorageProvider` throws on construction if any of - these four values is missing + the four required values is missing - **Local dev**: point at the `seaweedfs` Docker service (S3 API on port 8333) - **Production**: point at any S3-compatible endpoint (AWS S3, a hosted SeaweedFS cluster, and so on); `ForcePathStyle` is always enabled, which SeaweedFS requires and AWS S3 also accepts +- **`SEAWEEDFS_PUBLIC_URL`**: the endpoint download URLs are signed against, + used by `GetPresignedUrl` and reaching the API response as + `PublicUserProfileDto.AvatarUrl`. A presigned URL is signed over its host, so + this has to be the address the browser uses (`http://localhost:8333` with the + dev compose file, which publishes the S3 API port), separate from the + in-network `SEAWEEDFS_SERVICE_URL` the API uploads through. The two can be the + same value wherever one address serves both ## Docker-specific variables @@ -341,6 +349,7 @@ from the template in place; edit them manually if you go that route. | `SMTP_FROM_EMAIL` | ✓ | | ✓ | Yes | Email sender address | | `SMTP_FROM_NAME` | ✓ | | ✓ | No | Defaults to `The Biergarten` | | `SEAWEEDFS_SERVICE_URL` | ✓ | | ✓ | Yes | S3-compatible endpoint URL | +| `SEAWEEDFS_PUBLIC_URL` | ✓ | | ✓ | No | Endpoint download URLs are signed against; defaults to `SEAWEEDFS_SERVICE_URL` | | `SEAWEEDFS_ACCESS_KEY_ID` | ✓ | | ✓ | Yes | S3 access key | | `SEAWEEDFS_SECRET_ACCESS_KEY` | ✓ | | ✓ | Yes | S3 secret key | | `SEAWEEDFS_BUCKET` | ✓ | | ✓ | Yes | S3 bucket name | @@ -415,6 +424,7 @@ SMTP_FROM_NAME=The Biergarten App # SeaweedFS (S3-compatible storage) SEAWEEDFS_SERVICE_URL=http://seaweedfs:8333 +SEAWEEDFS_PUBLIC_URL=http://localhost:8333 SEAWEEDFS_ACCESS_KEY_ID= SEAWEEDFS_SECRET_ACCESS_KEY= SEAWEEDFS_BUCKET=my-bucket diff --git a/docs/web/testing.md b/docs/web/testing.md index 7ba616e8..b84a6a3c 100644 --- a/docs/web/testing.md +++ b/docs/web/testing.md @@ -257,7 +257,8 @@ API.Specs/ │ └── ApiGeneralSteps.cs # Shared/general step definitions ├── Mocks/ │ ├── MockEmailDispatcher.cs # Substitutes Features.Emails' IEmailDispatcher -│ └── MockEmailProvider.cs # Substitutes Features.Emails' IEmailProvider +│ ├── MockEmailProvider.cs # Substitutes Features.Emails' IEmailProvider +│ └── MockFileStorageProvider.cs # Substitutes Features.ImageUploads' IFileStorageProvider └── TestApiFactory.cs # Test server setup (swaps in the mocks above) ``` diff --git a/web/.env.example b/web/.env.example index bbad7355..01ed405b 100644 --- a/web/.env.example +++ b/web/.env.example @@ -68,8 +68,12 @@ WEBSITE_BASE_URL=http://localhost:3000 # ====================== # SeaweedFS (S3) Configuration # ====================== -# Credentials and default bucket for the local SeaweedFS S3-compatible storage +# Credentials and default bucket for the local SeaweedFS S3-compatible storage. +# SEAWEEDFS_SERVICE_URL is the in-network endpoint the API uploads through; +# SEAWEEDFS_PUBLIC_URL is the endpoint download URLs are signed against, so it has to be +# reachable from the browser. It defaults to SEAWEEDFS_SERVICE_URL when left blank. SEAWEEDFS_SERVICE_URL=http://seaweedfs:8333 +SEAWEEDFS_PUBLIC_URL=http://localhost:8333 SEAWEEDFS_ACCESS_KEY_ID=admin SEAWEEDFS_SECRET_ACCESS_KEY=secret SEAWEEDFS_BUCKET=my-bucket diff --git a/web/backend/API/API.Specs/Features/PublicUserProfile.feature b/web/backend/API/API.Specs/Features/PublicUserProfile.feature index 6469dcfc..39f1beb6 100644 --- a/web/backend/API/API.Specs/Features/PublicUserProfile.feature +++ b/web/backend/API/API.Specs/Features/PublicUserProfile.feature @@ -13,9 +13,61 @@ Scenario: Public profile exposes only allowlisted fields Then the response has HTTP status 200 And the public profile response should match the registered account And the response JSON should have "biography" equal "" + And the response JSON should have "avatarUrl" equal "" And the response JSON should not contain "email" And the response JSON should not contain "dateOfBirth" +Scenario: Public profile exposes the uploaded avatar + Given I have registered a new account + And I have a valid access token for my account + And I have uploaded an avatar + When I retrieve the public profile by ID + Then the response has HTTP status 200 + And the public profile response should carry my current avatar + +Scenario: Public profile exposes the current avatar after it is replaced + Given I have registered a new account + And I have a valid access token for my account + And I have uploaded an avatar + And I have uploaded a new avatar, replacing the previous one + When I retrieve the public profile by ID + Then the response has HTTP status 200 + And the public profile response should carry my current avatar + And the public profile response should not carry my previous avatar + +Scenario: Fetching the avatar without authentication succeeds + Given I have registered a new account + And I have a valid access token for my account + And I have uploaded an avatar + When I retrieve the public profile by ID without authentication + Then the response has HTTP status 200 + And the public profile response should carry my current avatar + +Scenario: A different user can read someone else's avatar + Given I have registered a new account + And I have a valid access token for my account + And I have uploaded an avatar + And I am logged in as a different user + When I retrieve the public profile by ID + Then the response has HTTP status 200 + And the public profile response should carry my current avatar + +Scenario: Listing user accounts exposes each account's avatar + Given I have registered a new account + And I have a valid access token for my account + And I have uploaded an avatar + When I list user accounts + Then the response has HTTP status 200 + And my listed account should carry my current avatar + +Scenario: Listing user accounts exposes each account's biography + Given I have registered a new account + And I have a valid access token for my account + And I have set my biography to "Drinks lagers." + When I list user accounts + Then the response has HTTP status 200 + And my listed account should carry my biography + Scenario: Public profile for a non-existent account returns not found Given I have registered a new account And I have a valid access token for my account @@ -26,6 +78,7 @@ Scenario: Fetching the public profile without authentication succeeds Given I have registered a new account When I retrieve the public profile by ID without authentication Then the response has HTTP status 200 + And the response JSON should have "avatarUrl" equal "" And the response JSON should not contain "email" And the response JSON should not contain "dateOfBirth" @@ -50,6 +103,7 @@ Scenario: Listing user accounts as an authenticated user excludes private fields And I have a valid access token for my account When I list user accounts Then the response has HTTP status 200 + And the response JSON should contain "avatarUrl" And the response JSON should not contain "email" And the response JSON should not contain "dateOfBirth" @@ -60,6 +114,14 @@ Scenario: The caller can fetch their own account Then the response has HTTP status 200 And the public profile response should match the registered account +Scenario: The caller's own account omits the avatar graph + Given I have registered a new account + And I have a valid access token for my account + And I have uploaded an avatar + When I retrieve my own account + Then the response has HTTP status 200 + And the response JSON should not contain "userAvatar" + Scenario: The account endpoint serves the caller named by the access token Given I have registered a new account And I am logged in as a different user diff --git a/web/backend/API/API.Specs/Mocks/MockFileStorageProvider.cs b/web/backend/API/API.Specs/Mocks/MockFileStorageProvider.cs new file mode 100644 index 00000000..71a38dcb --- /dev/null +++ b/web/backend/API/API.Specs/Mocks/MockFileStorageProvider.cs @@ -0,0 +1,55 @@ +using Features.ImageUploads.Services; + +namespace API.Specs.Mocks; + +/// +/// Test double for that records uploads in memory, so specs can +/// exercise photo-backed features without object storage. +/// +public class MockFileStorageProvider : IFileStorageProvider +{ + private readonly List _keys = []; + private readonly Dictionary _objects = new(); + + public IReadOnlyCollection UploadedKeys => _keys.ToList(); + + /// The key of the most recent upload. + /// Thrown when nothing has been uploaded yet. + public string LastUploadedKey => + _keys.Count > 0 + ? _keys[^1] + : throw new InvalidOperationException("no file has been uploaded"); + + public Task UploadAsync( + string key, + Stream content, + string contentType, + CancellationToken cancellationToken = default + ) + { + using MemoryStream buffer = new(); + content.CopyTo(buffer); + + if (!_objects.ContainsKey(key)) + _keys.Add(key); + + _objects[key] = buffer.ToArray(); + return Task.CompletedTask; + } + + public Task DeleteAsync(string key, CancellationToken cancellationToken = default) + { + _objects.Remove(key); + _keys.Remove(key); + return Task.CompletedTask; + } + + /// + /// Stands in for a signed download URL: an absolute URL on a host the object store would serve, + /// carrying the key in its path, so specs can assert the shape callers actually receive. + /// + public string GetPresignedUrl(string key, TimeSpan expiresIn) + { + return $"https://storage.test/{key}?X-Amz-Expires={(int)expiresIn.TotalSeconds}"; + } +} diff --git a/web/backend/API/API.Specs/Steps/ApiGeneralSteps.cs b/web/backend/API/API.Specs/Steps/ApiGeneralSteps.cs index 211f9355..ea462f57 100644 --- a/web/backend/API/API.Specs/Steps/ApiGeneralSteps.cs +++ b/web/backend/API/API.Specs/Steps/ApiGeneralSteps.cs @@ -183,6 +183,25 @@ public class ApiGeneralSteps(ScenarioContext scenario) AssertFieldAbsent(payloadElem, field); } + [Then("the response JSON should contain {string}")] + public void ThenTheResponseJsonShouldContainString(string field) + { + scenario.TryGetValue(ResponseBodyKey, out string? responseBody).Should().BeTrue(); + + using JsonDocument doc = JsonDocument.Parse(responseBody!); + JsonElement root = doc.RootElement; + + if (root.ValueKind == JsonValueKind.Object && + !root.TryGetProperty(field, out _) && + root.TryGetProperty("payload", out JsonElement payloadElem)) + { + AssertFieldPresent(payloadElem, field); + return; + } + + AssertFieldPresent(root, field); + } + /// /// Recurses into arrays so a listing endpoint's response can be asserted the same way as a /// single-resource response. @@ -203,4 +222,31 @@ public class ApiGeneralSteps(ScenarioContext scenario) break; } } + + /// + /// Recurses into arrays so every item of a listing endpoint's response must carry the field. + /// + private static void AssertFieldPresent(JsonElement element, string field) + { + switch (element.ValueKind) + { + case JsonValueKind.Object: + element + .TryGetProperty(field, out _) + .Should() + .BeTrue("Expected field '{0}' to be present in the response", field); + break; + case JsonValueKind.Array: + element + .EnumerateArray() + .Should() + .NotBeEmpty( + "Expected at least one item to assert field '{0}' against", + field + ); + foreach (JsonElement item in element.EnumerateArray()) + AssertFieldPresent(item, field); + break; + } + } } diff --git a/web/backend/API/API.Specs/Steps/UserSteps.cs b/web/backend/API/API.Specs/Steps/UserSteps.cs index d1957205..e5d72313 100644 --- a/web/backend/API/API.Specs/Steps/UserSteps.cs +++ b/web/backend/API/API.Specs/Steps/UserSteps.cs @@ -1,5 +1,12 @@ using System.Text.Json; +using API.Specs.Mocks; +using Features.ImageUploads.Services; +using Features.Users.Commands.Profile.UploadAvatar; +using Features.Users.Repository; using FluentAssertions; +using MediatR; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Reqnroll; namespace API.Specs.Steps; @@ -10,6 +17,11 @@ public class UserSteps(ScenarioContext scenario) : ApiStepsBase(scenario) private const string AccessTokenKey = "accessToken"; private const string RegisteredUserIdKey = "registeredUserId"; private const string RegisteredUsernameKey = "registeredUsername"; + private const string CurrentAvatarKeyKey = "currentAvatarKey"; + private const string PreviousAvatarKeyKey = "previousAvatarKey"; + private const string BiographyKey = "biography"; + + private static readonly byte[] PngSignature = [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]; private async Task SendAsync(HttpRequestMessage requestMessage) { @@ -36,22 +48,193 @@ public class UserSteps(ScenarioContext scenario) : ApiStepsBase(scenario) : throw new InvalidOperationException("registered user ID not found in scenario"); } + /// + /// Builds the smallest upload the photo validator accepts: a PNG signature padded out to the + /// 1 KiB minimum. + /// + private static IFormFile CreateAvatarFile() + { + byte[] content = new byte[2048]; + PngSignature.CopyTo(content, 0); + MemoryStream stream = new(content); + + return new FormFile(stream, 0, content.Length, "file", "avatar.png") + { + Headers = new HeaderDictionary { ["Content-Type"] = "image/png" }, + }; + } + + /// + /// Uploads an avatar and records the object key the storage provider actually received, rather + /// than re-deriving the key template the handler uses. + /// + private async Task UploadAvatarAsync() + { + Guid userId = GetRegisteredUserId(); + + using IServiceScope scope = GetFactory().Services.CreateScope(); + IMediator mediator = scope.ServiceProvider.GetRequiredService(); + await mediator.Send(new UploadAvatarCommand(userId, CreateAvatarFile())); + + if (Scenario.TryGetValue(CurrentAvatarKeyKey, out string? existing)) + Scenario[PreviousAvatarKeyKey] = existing; + + Scenario[CurrentAvatarKeyKey] = GetStorageProvider().LastUploadedKey; + } + + private MockFileStorageProvider GetStorageProvider() + { + return (MockFileStorageProvider) + GetFactory().Services.GetRequiredService(); + } + + private string GetCurrentAvatarKey() + { + return Scenario.TryGetValue(CurrentAvatarKeyKey, out string? key) && key is not null + ? key + : throw new InvalidOperationException("no avatar has been uploaded in this scenario"); + } + + private string GetPreviousAvatarKey() + { + return Scenario.TryGetValue(PreviousAvatarKeyKey, out string? key) && key is not null + ? key + : throw new InvalidOperationException( + "only one avatar has been uploaded in this scenario" + ); + } + + /// + /// Asserts that is something a browser can fetch -- an absolute + /// HTTP(S) URL -- pointing at the object stored under . A bare + /// storage key would resolve against the page's own origin, so the shape is the contract here, + /// not just the value. + /// + private static Uri AssertFetchableUrl(string? avatarUrl, string expectedKey) + { + avatarUrl.Should().NotBeNullOrWhiteSpace(); + + Uri.TryCreate(avatarUrl, UriKind.Absolute, out Uri? uri) + .Should() + .BeTrue("avatarUrl must be an absolute URL, but was '{0}'", avatarUrl); + uri!.Scheme.Should().BeOneOf("http", "https"); + uri.AbsolutePath.Should().EndWith(expectedKey, "the URL must address the uploaded object"); + + return uri; + } + + private string GetAvatarUrlFromResponse() + { + Scenario.TryGetValue(ResponseBodyKey, out string? responseBody).Should().BeTrue(); + + using JsonDocument doc = JsonDocument.Parse(responseBody!); + return doc.RootElement.GetProperty("avatarUrl").GetString()!; + } + + [Given("I have uploaded an avatar")] + public async Task GivenIHaveUploadedAnAvatar() + { + await UploadAvatarAsync(); + } + + [Given("I have uploaded a new avatar, replacing the previous one")] + public async Task GivenIHaveUploadedANewAvatarReplacingThePreviousOne() + { + await UploadAvatarAsync(); + } + + /// + /// Writes the biography straight through the repository: registration creates the profile row + /// with an empty biography, and no endpoint edits it yet. + /// + [Given("I have set my biography to {string}")] + public async Task GivenIHaveSetMyBiographyTo(string biography) + { + using IServiceScope scope = GetFactory().Services.CreateScope(); + IUserProfileRepository profiles = + scope.ServiceProvider.GetRequiredService(); + + await profiles.UpdateBiographyAsync( + GetRegisteredUserId(), + biography, + CancellationToken.None + ); + + Scenario[BiographyKey] = biography; + } + + [Then("the public profile response should carry my current avatar")] + public void ThenThePublicProfileResponseShouldCarryMyCurrentAvatar() + { + AssertFetchableUrl(GetAvatarUrlFromResponse(), GetCurrentAvatarKey()); + } + + [Then("the public profile response should not carry my previous avatar")] + public void ThenThePublicProfileResponseShouldNotCarryMyPreviousAvatar() + { + Uri uri = AssertFetchableUrl(GetAvatarUrlFromResponse(), GetCurrentAvatarKey()); + uri.AbsolutePath.Should().NotContain(GetPreviousAvatarKey()); + } + + [Then("my listed account should carry my current avatar")] + public void ThenMyListedAccountShouldCarryMyCurrentAvatar() + { + AssertFetchableUrl( + GetMyListedAccount().GetProperty("avatarUrl").GetString(), + GetCurrentAvatarKey() + ); + } + + [Then("my listed account should carry my biography")] + public void ThenMyListedAccountShouldCarryMyBiography() + { + string expected = + Scenario.TryGetValue(BiographyKey, out string? biography) && biography is not null + ? biography + : throw new InvalidOperationException("no biography was set in this scenario"); + + GetMyListedAccount().GetProperty("biography").GetString().Should().Be(expected); + } + + /// Finds the registered account in a listing response. + private JsonElement GetMyListedAccount() + { + Scenario.TryGetValue(ResponseBodyKey, out string? responseBody).Should().BeTrue(); + + using JsonDocument doc = JsonDocument.Parse(responseBody!); + doc.RootElement.ValueKind.Should().Be(JsonValueKind.Array); + + return doc + .RootElement.EnumerateArray() + .Should() + .ContainSingle(item => + item.GetProperty("userAccountId").GetGuid() == GetRegisteredUserId() + ) + .Subject.Clone(); + } + [When("I retrieve the public profile by ID")] public async Task WhenIRetrieveThePublicProfileById() { - await SendAsync(NewAuthenticatedRequest(HttpMethod.Get, $"/api/user/{GetRegisteredUserId()}/profile")); + await SendAsync( + NewAuthenticatedRequest(HttpMethod.Get, $"/api/user/{GetRegisteredUserId()}/profile") + ); } [When("I retrieve the public profile by ID without authentication")] public async Task WhenIRetrieveThePublicProfileByIdWithoutAuthentication() { - await SendAsync(new HttpRequestMessage(HttpMethod.Get, $"/api/user/{GetRegisteredUserId()}/profile")); + await SendAsync( + new HttpRequestMessage(HttpMethod.Get, $"/api/user/{GetRegisteredUserId()}/profile") + ); } [When("I retrieve the public profile by a non-existent ID")] public async Task WhenIRetrieveThePublicProfileByANonExistentId() { - await SendAsync(NewAuthenticatedRequest(HttpMethod.Get, $"/api/user/{Guid.NewGuid()}/profile")); + await SendAsync( + NewAuthenticatedRequest(HttpMethod.Get, $"/api/user/{Guid.NewGuid()}/profile") + ); } [When("I retrieve my own account without authentication")] diff --git a/web/backend/API/API.Specs/TestApiFactory.cs b/web/backend/API/API.Specs/TestApiFactory.cs index e6b5c1e0..c9e57a00 100644 --- a/web/backend/API/API.Specs/TestApiFactory.cs +++ b/web/backend/API/API.Specs/TestApiFactory.cs @@ -1,6 +1,7 @@ using API.Core; using API.Specs.Mocks; using Features.Emails.Services; +using Features.ImageUploads.Services; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; @@ -32,6 +33,15 @@ internal class TestApiFactory : WebApplicationFactory services.Remove(emailDispatcherDescriptor); services.AddScoped(); + + ServiceDescriptor? fileStorageDescriptor = services.SingleOrDefault(d => + d.ServiceType == typeof(IFileStorageProvider) + ); + + if (fileStorageDescriptor != null) + services.Remove(fileStorageDescriptor); + + services.AddSingleton(); }); } } diff --git a/web/backend/Configuration/ConfigurationKeys.cs b/web/backend/Configuration/ConfigurationKeys.cs index 06763a01..a01927e8 100644 --- a/web/backend/Configuration/ConfigurationKeys.cs +++ b/web/backend/Configuration/ConfigurationKeys.cs @@ -23,6 +23,14 @@ public static class ConfigurationKeys public const string SmtpFromName = "SMTP_FROM_NAME"; public const string SeaweedFsServiceUrl = "SEAWEEDFS_SERVICE_URL"; + + /// + /// The browser-reachable base URL of the object store, used to sign download URLs. Falls back to + /// when unset: SigV4 covers the host, so a URL signed against + /// the container-internal endpoint is only usable by callers that can resolve that host. + /// + public const string SeaweedFsPublicUrl = "SEAWEEDFS_PUBLIC_URL"; + public const string SeaweedFsAccessKeyId = "SEAWEEDFS_ACCESS_KEY_ID"; public const string SeaweedFsSecretAccessKey = "SEAWEEDFS_SECRET_ACCESS_KEY"; public const string SeaweedFsBucket = "SEAWEEDFS_BUCKET"; diff --git a/web/backend/Domain/Domain.Entities/Entities/UserAccount.cs b/web/backend/Domain/Domain.Entities/Entities/UserAccount.cs index 8a974e94..648bbaa4 100644 --- a/web/backend/Domain/Domain.Entities/Entities/UserAccount.cs +++ b/web/backend/Domain/Domain.Entities/Entities/UserAccount.cs @@ -1,3 +1,5 @@ +using System.Text.Json.Serialization; + namespace Domain.Entities; public sealed class UserAccount @@ -20,4 +22,10 @@ public sealed class UserAccount // Navigation properties public UserCredential? UserCredential { get; set; } public UserProfile? UserProfile { get; set; } + + // GET /api/user/authenticated serializes this entity directly, and the avatar graph carries the + // storage key and uploader of the underlying Photo. Clients read the avatar from + // PublicUserProfileDto.AvatarUrl, which is a signed URL, so keep this one out of the response. + [JsonIgnore] + public UserAvatar? UserAvatar { get; set; } } diff --git a/web/backend/Features/Features.PhotoUpload/Services/S3FileStorageProvider.cs b/web/backend/Features/Features.PhotoUpload/Services/S3FileStorageProvider.cs index 236d851a..c52b1ad7 100644 --- a/web/backend/Features/Features.PhotoUpload/Services/S3FileStorageProvider.cs +++ b/web/backend/Features/Features.PhotoUpload/Services/S3FileStorageProvider.cs @@ -13,11 +13,14 @@ public class S3FileStorageProvider : IFileStorageProvider, IDisposable { private readonly string _bucket; private readonly IAmazonS3 _client; + private readonly IAmazonS3 _presignClient; + private readonly Protocol _presignProtocol; /// /// Initializes a new instance of the class, reading storage /// configuration (SEAWEEDFS_SERVICE_URL, SEAWEEDFS_ACCESS_KEY_ID, - /// SEAWEEDFS_SECRET_ACCESS_KEY, SEAWEEDFS_BUCKET) from . + /// SEAWEEDFS_SECRET_ACCESS_KEY, SEAWEEDFS_BUCKET, and the optional + /// SEAWEEDFS_PUBLIC_URL) from . /// /// /// Thrown when any of the required configuration values is not set. @@ -45,6 +48,31 @@ public class S3FileStorageProvider : IFileStorageProvider, IDisposable AmazonS3Config config = new() { ServiceURL = serviceUrl, ForcePathStyle = true }; _client = new AmazonS3Client(accessKeyId, secretAccessKey, config); + + // A presigned URL is signed over its host, so one signed against the container-internal + // endpoint is only usable from inside the network. Sign downloads against the public + // endpoint instead, so a browser can fetch the object. + string? configuredPublicUrl = configuration[ConfigurationKeys.SeaweedFsPublicUrl]; + string publicUrl = string.IsNullOrWhiteSpace(configuredPublicUrl) + ? serviceUrl + : configuredPublicUrl; + + _presignClient = + publicUrl == serviceUrl + ? _client + : new AmazonS3Client( + accessKeyId, + secretAccessKey, + new AmazonS3Config { ServiceURL = publicUrl, ForcePathStyle = true } + ); + + // GetPreSignedUrlRequest.Protocol defaults to HTTPS and ignores the ServiceURL scheme, so + // take the scheme from the endpoint the URL is signed against. + _presignProtocol = + Uri.TryCreate(publicUrl, UriKind.Absolute, out Uri? publicUri) + && publicUri.Scheme == Uri.UriSchemeHttp + ? Protocol.HTTP + : Protocol.HTTPS; } /// @@ -82,14 +110,18 @@ public class S3FileStorageProvider : IFileStorageProvider, IDisposable BucketName = _bucket, Key = key, Expires = DateTime.UtcNow.Add(expiresIn), + Protocol = _presignProtocol, }; - return _client.GetPreSignedURL(request); + return _presignClient.GetPreSignedURL(request); } /// public void Dispose() { + if (!ReferenceEquals(_presignClient, _client)) + _presignClient.Dispose(); + _client.Dispose(); GC.SuppressFinalize(this); } diff --git a/web/backend/Features/Features.Users.Tests/Queries/GetAllUsersHandlerTests.cs b/web/backend/Features/Features.Users.Tests/Queries/GetAllUsersHandlerTests.cs index c7f8bd44..75240529 100644 --- a/web/backend/Features/Features.Users.Tests/Queries/GetAllUsersHandlerTests.cs +++ b/web/backend/Features/Features.Users.Tests/Queries/GetAllUsersHandlerTests.cs @@ -1,4 +1,5 @@ using Domain.Entities; +using Features.ImageUploads.Services; using Features.Users.Dtos; using Features.Users.Queries.GetAllUsers; using Features.Users.Repository; @@ -9,27 +10,36 @@ namespace Features.Users.Tests.Queries; public class GetAllUsersHandlerTests { + private readonly Mock _repoMock = new(); + private readonly Mock _storageMock = new(); + private readonly GetAllUsersHandler _handler; + + public GetAllUsersHandlerTests() + { + _storageMock + .Setup(s => s.GetPresignedUrl(It.IsAny(), It.IsAny())) + .Returns((string key, TimeSpan _) => $"https://storage.test/{key}?signed"); + + _handler = new GetAllUsersHandler(_repoMock.Object, _storageMock.Object); + } + [Fact] public async Task Handle_PassesLimitAndOffset_ToRepository() { - Mock repoMock = new(); - GetAllUsersHandler handler = new(repoMock.Object); - repoMock.Setup(r => r.GetAllAsync(10, 5)).ReturnsAsync(Array.Empty()); + _repoMock.Setup(r => r.GetAllAsync(10, 5)).ReturnsAsync(Array.Empty()); - IEnumerable result = await handler.Handle( + IEnumerable result = await _handler.Handle( new GetAllUsersQuery(10, 5), CancellationToken.None ); result.Should().BeEmpty(); - repoMock.Verify(r => r.GetAllAsync(10, 5), Times.Once); + _repoMock.Verify(r => r.GetAllAsync(10, 5), Times.Once); } [Fact] public async Task Handle_MapsUsersToPublicProfileDtos_AndExcludesPrivateFields() { - Mock repoMock = new(); - GetAllUsersHandler handler = new(repoMock.Object); UserAccount user = new() { UserAccountId = Guid.NewGuid(), @@ -38,13 +48,60 @@ public class GetAllUsersHandlerTests LastName = "One", CreatedAt = DateTime.UtcNow, }; - repoMock.Setup(r => r.GetAllAsync(null, null)).ReturnsAsync([user]); + _repoMock.Setup(r => r.GetAllAsync(null, null)).ReturnsAsync([user]); - IEnumerable result = await handler.Handle( + IEnumerable result = await _handler.Handle( new GetAllUsersQuery(null, null), CancellationToken.None ); - result.Should().ContainSingle().Which.Should().BeEquivalentTo(user.ToPublicProfileDto()); + result + .Should() + .ContainSingle() + .Which.Should() + .BeEquivalentTo(user.ToPublicProfileDto(_storageMock.Object)); + } + + [Fact] + public async Task Handle_MapsEachUsersOwnAvatarUrl() + { + string avatarKey = $"avatars/{Guid.NewGuid()}/{Guid.NewGuid()}"; + UserAccount withAvatar = new() + { + UserAccountId = Guid.NewGuid(), + Username = "has-avatar", + FirstName = "Has", + LastName = "Avatar", + CreatedAt = DateTime.UtcNow, + UserProfile = new UserProfile { Biography = "Drinks lagers." }, + UserAvatar = new UserAvatar { Photo = new Photo { Hyperlink = avatarKey } }, + }; + UserAccount withoutAvatar = new() + { + UserAccountId = Guid.NewGuid(), + Username = "no-avatar", + FirstName = "No", + LastName = "Avatar", + CreatedAt = DateTime.UtcNow, + }; + _repoMock.Setup(r => r.GetAllAsync(null, null)).ReturnsAsync([withAvatar, withoutAvatar]); + + IEnumerable result = await _handler.Handle( + new GetAllUsersQuery(null, null), + CancellationToken.None + ); + + result + .Select(dto => dto.AvatarUrl) + .Should() + .Equal($"https://storage.test/{avatarKey}?signed", string.Empty); + _storageMock.Verify( + s => s.GetPresignedUrl(avatarKey, UserDtoMapper.AvatarUrlLifetime), + Times.Once + ); + _storageMock.Verify( + s => s.GetPresignedUrl(It.IsAny(), It.IsAny()), + Times.Once + ); } } diff --git a/web/backend/Features/Features.Users.Tests/Queries/GetPublicUserProfileByIdHandlerTests.cs b/web/backend/Features/Features.Users.Tests/Queries/GetPublicUserProfileByIdHandlerTests.cs index 73d57615..028dd59b 100644 --- a/web/backend/Features/Features.Users.Tests/Queries/GetPublicUserProfileByIdHandlerTests.cs +++ b/web/backend/Features/Features.Users.Tests/Queries/GetPublicUserProfileByIdHandlerTests.cs @@ -1,5 +1,6 @@ using Domain.Entities; using Domain.Exceptions; +using Features.ImageUploads.Services; using Features.Users.Dtos; using Features.Users.Queries.GetPublicUserProfileById; using Features.Users.Repository; @@ -12,15 +13,21 @@ public class GetPublicUserProfileByIdHandlerTests { private readonly GetPublicUserProfileByIdHandler _handler; private readonly Mock _repoMock = new(); + private readonly Mock _storageMock = new(); public GetPublicUserProfileByIdHandlerTests() { - _handler = new GetPublicUserProfileByIdHandler(_repoMock.Object); + _storageMock + .Setup(s => s.GetPresignedUrl(It.IsAny(), It.IsAny())) + .Returns((string key, TimeSpan _) => $"https://storage.test/{key}?signed"); + + _handler = new GetPublicUserProfileByIdHandler(_repoMock.Object, _storageMock.Object); } [Fact] public async Task Handle_ReturnsPublicProfile_WhenFound() { + string avatarKey = $"avatars/{Guid.NewGuid()}/{Guid.NewGuid()}"; UserAccount user = new() { UserAccountId = Guid.NewGuid(), @@ -31,6 +38,7 @@ public class GetPublicUserProfileByIdHandlerTests DateOfBirth = new DateTime(1990, 1, 1), CreatedAt = new DateTime(2024, 1, 1), UserProfile = new UserProfile { Biography = "Drinks lagers." }, + UserAvatar = new UserAvatar { Photo = new Photo { Hyperlink = avatarKey } }, }; _repoMock.Setup(r => r.GetByIdAsync(user.UserAccountId)).ReturnsAsync(user); @@ -48,13 +56,48 @@ public class GetPublicUserProfileByIdHandlerTests user.FirstName, user.LastName, "Drinks lagers.", + $"https://storage.test/{avatarKey}?signed", user.CreatedAt ) ); } + /// + /// Photo.Hyperlink holds an object key, so the DTO has to carry a signed URL rather than + /// the stored value: a bare key resolves against the calling page's own origin. + /// [Fact] - public async Task Handle_ReturnsEmptyBiography_WhenProfileMissing() + public async Task Handle_SignsTheAvatarKey_RatherThanReturningIt() + { + string avatarKey = $"avatars/{Guid.NewGuid()}/{Guid.NewGuid()}"; + UserAccount user = new() + { + UserAccountId = Guid.NewGuid(), + Username = "test", + FirstName = "Jane", + LastName = "Doe", + CreatedAt = new DateTime(2024, 1, 1), + UserAvatar = new UserAvatar { Photo = new Photo { Hyperlink = avatarKey } }, + }; + _repoMock.Setup(r => r.GetByIdAsync(user.UserAccountId)).ReturnsAsync(user); + + PublicUserProfileDto result = await _handler.Handle( + new GetPublicUserProfileByIdQuery(user.UserAccountId), + CancellationToken.None + ); + + result.AvatarUrl.Should().NotBe(avatarKey); + Uri.TryCreate(result.AvatarUrl, UriKind.Absolute, out Uri? uri).Should().BeTrue(); + uri!.Scheme.Should().BeOneOf("http", "https"); + uri.AbsolutePath.Should().EndWith(avatarKey); + _storageMock.Verify( + s => s.GetPresignedUrl(avatarKey, UserDtoMapper.AvatarUrlLifetime), + Times.Once + ); + } + + [Fact] + public async Task Handle_ReturnsEmptyBiographyAndAvatarUrl_WhenProfileMissing() { UserAccount user = new() { @@ -72,6 +115,57 @@ public class GetPublicUserProfileByIdHandlerTests ); result.Biography.Should().BeEmpty(); + result.AvatarUrl.Should().BeEmpty(); + } + + [Fact] + public async Task Handle_ReturnsEmptyAvatarUrl_WhenProfileHasNoAvatar() + { + UserAccount user = new() + { + UserAccountId = Guid.NewGuid(), + Username = "test", + FirstName = "Jane", + LastName = "Doe", + CreatedAt = new DateTime(2024, 1, 1), + UserProfile = new UserProfile { Biography = "Drinks lagers." }, + }; + _repoMock.Setup(r => r.GetByIdAsync(user.UserAccountId)).ReturnsAsync(user); + + PublicUserProfileDto result = await _handler.Handle( + new GetPublicUserProfileByIdQuery(user.UserAccountId), + CancellationToken.None + ); + + result.Biography.Should().Be("Drinks lagers."); + result.AvatarUrl.Should().BeEmpty(); + } + + [Fact] + public async Task Handle_ReturnsEmptyAvatarUrl_WhenAvatarHasNoPhoto() + { + UserAccount user = new() + { + UserAccountId = Guid.NewGuid(), + Username = "test", + FirstName = "Jane", + LastName = "Doe", + CreatedAt = new DateTime(2024, 1, 1), + UserProfile = new UserProfile { Biography = "Drinks lagers." }, + UserAvatar = new UserAvatar(), + }; + _repoMock.Setup(r => r.GetByIdAsync(user.UserAccountId)).ReturnsAsync(user); + + PublicUserProfileDto result = await _handler.Handle( + new GetPublicUserProfileByIdQuery(user.UserAccountId), + CancellationToken.None + ); + + result.AvatarUrl.Should().BeEmpty(); + _storageMock.Verify( + s => s.GetPresignedUrl(It.IsAny(), It.IsAny()), + Times.Never + ); } [Fact] diff --git a/web/backend/Features/Features.Users/Dtos/UserDtoMapper.cs b/web/backend/Features/Features.Users/Dtos/UserDtoMapper.cs index 47277365..a56d6566 100644 --- a/web/backend/Features/Features.Users/Dtos/UserDtoMapper.cs +++ b/web/backend/Features/Features.Users/Dtos/UserDtoMapper.cs @@ -1,19 +1,39 @@ using Domain.Entities; +using Features.ImageUploads.Services; namespace Features.Users.Dtos; /// Converts user domain entities into API response models. public static class UserDtoMapper { + /// + /// How long a presigned avatar URL stays valid. Long enough to outlive the page that embeds it, + /// short enough that a leaked URL stops working. + /// + public static readonly TimeSpan AvatarUrlLifetime = TimeSpan.FromHours(1); + /// Converts a user account into its public-profile representation. - public static PublicUserProfileDto ToPublicProfileDto(this UserAccount user) + /// The account to convert. + /// + /// Signs the download URL for the account's avatar. Photo.Hyperlink holds an object key + /// rather than a URL, so it has to be signed before a client can fetch it. + /// + public static PublicUserProfileDto ToPublicProfileDto( + this UserAccount user, + IFileStorageProvider fileStorageProvider + ) { + string? avatarKey = user.UserAvatar?.Photo?.Hyperlink; + return new PublicUserProfileDto( UserAccountId : user.UserAccountId, Username : user.Username, FirstName : user.FirstName, LastName : user.LastName, Biography : user.UserProfile?.Biography ?? string.Empty, + AvatarUrl : string.IsNullOrWhiteSpace(avatarKey) + ? string.Empty + : fileStorageProvider.GetPresignedUrl(avatarKey, AvatarUrlLifetime), CreatedAt : user.CreatedAt ); } diff --git a/web/backend/Features/Features.Users/Dtos/UserDtos.cs b/web/backend/Features/Features.Users/Dtos/UserDtos.cs index 2aee8af4..39ec4a9a 100644 --- a/web/backend/Features/Features.Users/Dtos/UserDtos.cs +++ b/web/backend/Features/Features.Users/Dtos/UserDtos.cs @@ -7,5 +7,6 @@ public record PublicUserProfileDto( string FirstName, string LastName, string Biography, + string AvatarUrl, DateTime CreatedAt ); diff --git a/web/backend/Features/Features.Users/Queries/GetAllUsers/GetAllUsersHandler.cs b/web/backend/Features/Features.Users/Queries/GetAllUsers/GetAllUsersHandler.cs index 9ccabf84..9722bf5f 100644 --- a/web/backend/Features/Features.Users/Queries/GetAllUsers/GetAllUsersHandler.cs +++ b/web/backend/Features/Features.Users/Queries/GetAllUsers/GetAllUsersHandler.cs @@ -1,12 +1,15 @@ using Domain.Entities; +using Features.ImageUploads.Services; using Features.Users.Dtos; using Features.Users.Repository; using MediatR; namespace Features.Users.Queries.GetAllUsers; -public class GetAllUsersHandler(IUserListRepository repository) - : IRequestHandler> +public class GetAllUsersHandler( + IUserListRepository repository, + IFileStorageProvider fileStorageProvider +) : IRequestHandler> { public async Task> Handle( GetAllUsersQuery request, @@ -14,6 +17,6 @@ public class GetAllUsersHandler(IUserListRepository repository) ) { IEnumerable users = await repository.GetAllAsync(request.Limit, request.Offset); - return users.Select(user => user.ToPublicProfileDto()); + return users.Select(user => user.ToPublicProfileDto(fileStorageProvider)); } } diff --git a/web/backend/Features/Features.Users/Queries/GetPublicUserProfileById/GetPublicUserProfileByIdHandler.cs b/web/backend/Features/Features.Users/Queries/GetPublicUserProfileById/GetPublicUserProfileByIdHandler.cs index 47965586..0769b37e 100644 --- a/web/backend/Features/Features.Users/Queries/GetPublicUserProfileById/GetPublicUserProfileByIdHandler.cs +++ b/web/backend/Features/Features.Users/Queries/GetPublicUserProfileById/GetPublicUserProfileByIdHandler.cs @@ -1,13 +1,16 @@ using Domain.Entities; using Domain.Exceptions; +using Features.ImageUploads.Services; using Features.Users.Dtos; using Features.Users.Repository; using MediatR; namespace Features.Users.Queries.GetPublicUserProfileById; -public class GetPublicUserProfileByIdHandler(IUserListRepository repository) - : IRequestHandler +public class GetPublicUserProfileByIdHandler( + IUserListRepository repository, + IFileStorageProvider fileStorageProvider +) : IRequestHandler { /// Thrown when no user account exists with the given ID. public async Task Handle( @@ -18,6 +21,6 @@ public class GetPublicUserProfileByIdHandler(IUserListRepository repository) UserAccount? user = await repository.GetByIdAsync(request.UserAccountId); if (user is null) throw new NotFoundException($"User with ID {request.UserAccountId} not found"); - return user.ToPublicProfileDto(); + return user.ToPublicProfileDto(fileStorageProvider); } } diff --git a/web/backend/Features/Features.Users/Repository/UserListRepository.cs b/web/backend/Features/Features.Users/Repository/UserListRepository.cs index f5dc8543..91781dc7 100644 --- a/web/backend/Features/Features.Users/Repository/UserListRepository.cs +++ b/web/backend/Features/Features.Users/Repository/UserListRepository.cs @@ -15,7 +15,10 @@ public class UserListRepository(ISqlConnectionFactory connectionFactory) /// /// /// Left-joins Social.UserProfile, so is - /// null for an account whose profile has yet to be created. + /// null for an account whose profile has yet to be created, and left-joins the + /// account's active Social.UserAvatar (ValidTo IS NULL) with its + /// Media.Photo, so is null for an account + /// with no avatar. /// public async Task GetByIdAsync(Guid id) { @@ -23,51 +26,89 @@ public class UserListRepository(ISqlConnectionFactory connectionFactory) IEnumerable results = await connection.QueryAsync< UserAccount, UserProfile, + Photo, + UserAvatar, UserAccount >( """ SELECT - ua.UserAccountID, - ua.Username, - ua.FirstName, ua.LastName, ua.Email, ua.CreatedAt, ua.UpdatedAt, - ua.DateOfBirth, ua.RowVersion, - up.UserProfileID, up.UserAccountID, up.Biography, up.RowVersion + ua.UserAccountID, ua.Username, ua.FirstName, ua.LastName, + ua.Email, ua.CreatedAt, ua.UpdatedAt, ua.DateOfBirth, + ua.RowVersion, up.UserProfileID, up.UserAccountID, + up.Biography, up.RowVersion, p.PhotoID, p.UploadedByID, + p.Hyperlink, p.UploadedAt, p.RowVersion, av.UserAvatarID, + av.UserProfileID, av.PhotoID, av.ValidFrom, av.ValidTo, + av.RowVersion FROM Auth.UserAccount ua - LEFT JOIN Social.UserProfile up ON ua.UserAccountID = up.UserAccountID + LEFT JOIN Social.UserProfile up + ON ua.UserAccountID = up.UserAccountID + LEFT JOIN Social.UserAvatar av + ON up.UserProfileID = av.UserProfileID + AND av.ValidTo IS NULL + LEFT JOIN Media.Photo p ON av.PhotoID = p.PhotoID WHERE ua.UserAccountID = @UserAccountId """, MapUserRow, new { UserAccountId = id }, - splitOn: "UserProfileID" + splitOn: AvatarSplitOn ); return results.SingleOrDefault(); } - private static UserAccount MapUserRow(UserAccount user, UserProfile? profile) - { - user.UserProfile = profile; - return user; - } - /// /// /// Always applies pagination via OFFSET/FETCH, ordered by creation date descending. /// A null / is treated as "no limit"/"no offset". - /// Does not select Email or DateOfBirth: this listing is consumed only as public - /// profiles, so those fields are left at their default rather than read from the database. + /// Joins the same profile and active-avatar rows as , both of which are + /// part of a public profile. Does not select Email or DateOfBirth: this listing is + /// consumed only as public profiles, so those fields are left at their default rather than read + /// from the database. /// public async Task> GetAllAsync(int? limit, int? offset) { await using DbConnection connection = await CreateConnection(); - return await connection.QueryAsync( + return await connection.QueryAsync< + UserAccount, + UserProfile, + Photo, + UserAvatar, + UserAccount + >( """ - SELECT UserAccountID, Username, FirstName, LastName, CreatedAt, UpdatedAt, RowVersion - FROM Auth.UserAccount - ORDER BY CreatedAt DESC + SELECT + ua.UserAccountID, ua.Username, ua.FirstName, ua.LastName, + ua.CreatedAt, ua.UpdatedAt, ua.RowVersion, + up.UserProfileID, up.UserAccountID, up.Biography, up.RowVersion, + p.PhotoID, p.UploadedByID, p.Hyperlink, p.UploadedAt, p.RowVersion, + av.UserAvatarID, av.UserProfileID, av.PhotoID, av.ValidFrom, av.ValidTo, + av.RowVersion + FROM Auth.UserAccount ua + LEFT JOIN Social.UserProfile up ON ua.UserAccountID = up.UserAccountID + LEFT JOIN Social.UserAvatar av + ON up.UserProfileID = av.UserProfileID AND av.ValidTo IS NULL + LEFT JOIN Media.Photo p ON av.PhotoID = p.PhotoID + ORDER BY ua.CreatedAt DESC OFFSET @Offset ROWS FETCH NEXT @Limit ROWS ONLY """, - new { Offset = offset ?? 0, Limit = limit ?? int.MaxValue } + MapUserRow, + new { Offset = offset ?? 0, Limit = limit ?? int.MaxValue }, + splitOn: AvatarSplitOn ); } + + private const string AvatarSplitOn = "UserProfileID,PhotoID,UserAvatarID"; + + private static UserAccount MapUserRow( + UserAccount user, + UserProfile? profile, + Photo? photo, + UserAvatar? avatar + ) + { + user.UserProfile = profile; + avatar?.Photo = photo; + user.UserAvatar = avatar; + return user; + } } diff --git a/web/docker-compose.db.yaml b/web/docker-compose.db.yaml index e678d997..bea0ae8c 100644 --- a/web/docker-compose.db.yaml +++ b/web/docker-compose.db.yaml @@ -70,6 +70,7 @@ services: DB_USER: "${DB_USER}" DB_PASSWORD: "${DB_PASSWORD}" SEAWEEDFS_SERVICE_URL: "${SEAWEEDFS_SERVICE_URL}" + SEAWEEDFS_PUBLIC_URL: "${SEAWEEDFS_PUBLIC_URL}" SEAWEEDFS_ACCESS_KEY_ID: "${SEAWEEDFS_ACCESS_KEY_ID}" SEAWEEDFS_SECRET_ACCESS_KEY: "${SEAWEEDFS_SECRET_ACCESS_KEY}" SEAWEEDFS_BUCKET: "${SEAWEEDFS_BUCKET}" diff --git a/web/docker-compose.dev.yaml b/web/docker-compose.dev.yaml index afdfa5d2..9a94b516 100644 --- a/web/docker-compose.dev.yaml +++ b/web/docker-compose.dev.yaml @@ -72,6 +72,7 @@ services: DB_USER: "${DB_USER}" DB_PASSWORD: "${DB_PASSWORD}" SEAWEEDFS_SERVICE_URL: "${SEAWEEDFS_SERVICE_URL}" + SEAWEEDFS_PUBLIC_URL: "${SEAWEEDFS_PUBLIC_URL}" SEAWEEDFS_ACCESS_KEY_ID: "${SEAWEEDFS_ACCESS_KEY_ID}" SEAWEEDFS_SECRET_ACCESS_KEY: "${SEAWEEDFS_SECRET_ACCESS_KEY}" SEAWEEDFS_BUCKET: "${SEAWEEDFS_BUCKET}" @@ -110,6 +111,7 @@ services: CONFIRMATION_TOKEN_SECRET: "${CONFIRMATION_TOKEN_SECRET}" WEBSITE_BASE_URL: "${WEBSITE_BASE_URL}" SEAWEEDFS_SERVICE_URL: "${SEAWEEDFS_SERVICE_URL}" + SEAWEEDFS_PUBLIC_URL: "${SEAWEEDFS_PUBLIC_URL}" SEAWEEDFS_ACCESS_KEY_ID: "${SEAWEEDFS_ACCESS_KEY_ID}" SEAWEEDFS_SECRET_ACCESS_KEY: "${SEAWEEDFS_SECRET_ACCESS_KEY}" SEAWEEDFS_BUCKET: "${SEAWEEDFS_BUCKET}" diff --git a/web/docker-compose.min.yaml b/web/docker-compose.min.yaml index d2c9ae7f..d77c930c 100644 --- a/web/docker-compose.min.yaml +++ b/web/docker-compose.min.yaml @@ -84,6 +84,7 @@ services: DB_USER: "${DB_USER}" DB_PASSWORD: "${DB_PASSWORD}" SEAWEEDFS_SERVICE_URL: "${SEAWEEDFS_SERVICE_URL}" + SEAWEEDFS_PUBLIC_URL: "${SEAWEEDFS_PUBLIC_URL}" SEAWEEDFS_ACCESS_KEY_ID: "${SEAWEEDFS_ACCESS_KEY_ID}" SEAWEEDFS_SECRET_ACCESS_KEY: "${SEAWEEDFS_SECRET_ACCESS_KEY}" SEAWEEDFS_BUCKET: "${SEAWEEDFS_BUCKET}" diff --git a/web/docker-compose.prod.seed.yaml b/web/docker-compose.prod.seed.yaml index bc46f2af..1dbf7314 100644 --- a/web/docker-compose.prod.seed.yaml +++ b/web/docker-compose.prod.seed.yaml @@ -19,6 +19,7 @@ services: DB_USER: "${DB_USER}" DB_PASSWORD: "${DB_PASSWORD}" SEAWEEDFS_SERVICE_URL: "${SEAWEEDFS_SERVICE_URL}" + SEAWEEDFS_PUBLIC_URL: "${SEAWEEDFS_PUBLIC_URL}" SEAWEEDFS_ACCESS_KEY_ID: "${SEAWEEDFS_ACCESS_KEY_ID}" SEAWEEDFS_SECRET_ACCESS_KEY: "${SEAWEEDFS_SECRET_ACCESS_KEY}" SEAWEEDFS_BUCKET: "${SEAWEEDFS_BUCKET}" diff --git a/web/docker-compose.prod.yaml b/web/docker-compose.prod.yaml index 4b99d938..b35f9033 100644 --- a/web/docker-compose.prod.yaml +++ b/web/docker-compose.prod.yaml @@ -51,6 +51,7 @@ services: CONFIRMATION_TOKEN_SECRET: "${CONFIRMATION_TOKEN_SECRET}" WEBSITE_BASE_URL: "${WEBSITE_BASE_URL}" SEAWEEDFS_SERVICE_URL: "${SEAWEEDFS_SERVICE_URL}" + SEAWEEDFS_PUBLIC_URL: "${SEAWEEDFS_PUBLIC_URL}" SEAWEEDFS_ACCESS_KEY_ID: "${SEAWEEDFS_ACCESS_KEY_ID}" SEAWEEDFS_SECRET_ACCESS_KEY: "${SEAWEEDFS_SECRET_ACCESS_KEY}" SEAWEEDFS_BUCKET: "${SEAWEEDFS_BUCKET}" diff --git a/web/docker-compose.test.yaml b/web/docker-compose.test.yaml index 45405a94..ca079b25 100644 --- a/web/docker-compose.test.yaml +++ b/web/docker-compose.test.yaml @@ -70,6 +70,7 @@ services: DB_USER: "${DB_USER}" DB_PASSWORD: "${DB_PASSWORD}" SEAWEEDFS_SERVICE_URL: "${SEAWEEDFS_SERVICE_URL}" + SEAWEEDFS_PUBLIC_URL: "${SEAWEEDFS_PUBLIC_URL}" SEAWEEDFS_ACCESS_KEY_ID: "${SEAWEEDFS_ACCESS_KEY_ID}" SEAWEEDFS_SECRET_ACCESS_KEY: "${SEAWEEDFS_SECRET_ACCESS_KEY}" SEAWEEDFS_BUCKET: "${SEAWEEDFS_BUCKET}" diff --git a/web/frontend/app/features/users/USER_PROFILE_HANDOFF.md b/web/frontend/app/features/users/USER_PROFILE_HANDOFF.md index 24fa5a83..53cb347e 100644 --- a/web/frontend/app/features/users/USER_PROFILE_HANDOFF.md +++ b/web/frontend/app/features/users/USER_PROFILE_HANDOFF.md @@ -6,21 +6,21 @@ Covers `/users/:id` (`routes/user-profile.tsx`). Mirrors the format of ## Status legend - ✅ **Available**: real data, wired up. -- ⚠️ **Partial**: schema/repository exists but no public read endpoint. +- ⚠️ **Partial**: reads real data, but a related gap remains (see the Gap column). - ❌ **Missing**: no backend support; page shows filler or local-only state. ## Header & sidebar -| Field | Status | Source | Gap | -| ---------------------------------------- | ------ | --------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Username, name, joined date | ✅ | `GET /api/user/{id}/profile` (`PublicUserProfileDto`) | | -| `isOwnProfile` | ✅ | Compares the session's `userAccountId` to the route param | | -| Bio | ✅ | `GET /api/user/{id}/profile` (`PublicUserProfileDto`) | Reads `Social.UserProfile.Biography` via the left join in `UserListRepository.GetByIdAsync`; an account with no profile row yet returns an empty string. | -| Location | ❌ | `FILLER_PROFILE_META.location` | No location field on `UserAccount` or `UserProfile`. | -| Avatar photo | ⚠️ | Initials placeholder (`avatar-placeholder`) | `Social.UserAvatar` + `UploadAvatarHandler` exist but there's no GET endpoint to read a user's stored avatar back. | -| Cover photo | ❌ | Flat `bg-base-300` block | No cover-photo concept anywhere in the schema. | -| Following / Followers counts | ❌ | `FILLER_PROFILE_META.following` / `.followers` | `Social.UserFollow` table exists but is unused by any query/controller — needs count queries (`WHERE UserAccountID = ?` / `WHERE FollowingID = ?`). | -| Breweries visited / Ratings given counts | ❌ | `FILLER_PROFILE_META.breweriesVisited` / `.ratingsGiven` | Blocked on the same like/rating gaps called out in `BREWERY_HANDOFF.md`. | +| Field | Status | Source | Gap | +| ---------------------------------------- | ------ | --------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Username, name, joined date | ✅ | `GET /api/user/{id}/profile` (`PublicUserProfileDto`) | | +| `isOwnProfile` | ✅ | Compares the session's `userAccountId` to the route param | | +| Bio | ✅ | `GET /api/user/{id}/profile` (`PublicUserProfileDto`) | Reads `Social.UserProfile.Biography` via the left join in `UserListRepository.GetByIdAsync`; an account with no profile row yet returns an empty string. | +| Location | ❌ | `FILLER_PROFILE_META.location` | No location field on `UserAccount` or `UserProfile`. | +| Avatar photo | ⚠️ | `GET /api/user/{id}/profile` (`PublicUserProfileDto`) | Reads the current `Social.UserAvatar` photo via the left joins in `UserListRepository.GetByIdAsync` and signs it into `avatarUrl`; an account with no avatar returns an empty string and `ProfileAvatar` shows initials. `UploadAvatarCommand` has no HTTP route yet, so only accounts created by `Database.Seed` have one — see (4) below. | +| Cover photo | ❌ | Flat `bg-base-300` block | No cover-photo concept anywhere in the schema. | +| Following / Followers counts | ❌ | `FILLER_PROFILE_META.following` / `.followers` | `Social.UserFollow` table exists but is unused by any query/controller — needs count queries (`WHERE UserAccountID = ?` / `WHERE FollowingID = ?`). | +| Breweries visited / Ratings given counts | ❌ | `FILLER_PROFILE_META.breweriesVisited` / `.ratingsGiven` | Blocked on the same like/rating gaps called out in `BREWERY_HANDOFF.md`. | ## Follow action @@ -36,7 +36,7 @@ Covers `/users/:id` (`routes/user-profile.tsx`). Mirrors the format of | Commenter name links (brewery detail comments) | ⚠️ | `CommentItem` links only when `comment.userAccountId` is set | `FILLER_COMMENTS` has no backing accounts, so those rows render as plain text; a comment added during the session by a signed-in user does carry a real GUID and links correctly. | | Following-list person links (profile page) | ⚠️ | `FollowingTab` links only when `f.userAccountId` is set | Same gap as above — `FILLER_FOLLOWING`'s two person rows have no backing accounts, so they render as plain text until real follow data exists. | -Both components already support linking (see `CommentItem.stories.tsx`'s `WithLinkedAccount` story); what's missing is real account IDs on the filler rows, which can't be faked without recreating the 404 this was fixed for. This resolves itself once (4) below closes and comments/follows are loader-backed. +Both components already support linking (see `CommentItem.stories.tsx`'s `WithLinkedAccount` story); what's missing is real account IDs on the filler rows, which can't be faked without recreating the 404 this was fixed for. This resolves itself once (3) below closes and comments/follows are loader-backed. ## Activity / Following / Liked tabs @@ -49,8 +49,8 @@ Both components already support linking (see `CommentItem.stories.tsx`'s `WithLi ## Summary of backend work -1. A public query + controller route to read a user's stored `Social.UserAvatar`. -2. A location field on `UserAccount` or `UserProfile`, and a cover-photo storage concept (mirrors the avatar gap). -3. `Social.UserFollow` command/query/controller: follow, unfollow, "is following" check, and follower/following counts. -4. An activity/timeline query unioning ratings, likes, and comments for a user, ordered by time — depends on the like/rating/comment tables called out in `BREWERY_HANDOFF.md` existing first. -5. Once (3) and the like/rating gaps close, swap `FILLER_PROFILE_META` / `FILLER_ACTIVITY` / `FILLER_FOLLOWING` / `FILLER_LIKED` (`utils/filler-user-profile.ts`) and the local `useState` in `routes/user-profile.tsx` for loader data and `authorizedRequest`-based mutations (see `auth.server.ts`). Swapping in real (possibly empty) arrays is also what makes the Activity/Following `EmptyState`s reachable from the actual route — today they're only exercised by the `Empty` Storybook stories, since the filler arrays are never empty. +1. A location field on `UserAccount` or `UserProfile`, and a cover-photo storage concept. +2. `Social.UserFollow` command/query/controller: follow, unfollow, "is following" check, and follower/following counts. +3. An activity/timeline query unioning ratings, likes, and comments for a user, ordered by time — depends on the like/rating/comment tables called out in `BREWERY_HANDOFF.md` existing first. +4. An avatar-upload endpoint: `UploadAvatarCommand`/`UploadAvatarHandler` exist and `Database.Seed` sends them, but no controller action does, so a signed-in user has no way to set their own avatar. Needs a `multipart/form-data` route on the authenticated caller's own account (`IFormFile`, same validation the seeder's uploads pass through). +5. Once (2) and the like/rating gaps close, swap `FILLER_PROFILE_META` / `FILLER_ACTIVITY` / `FILLER_FOLLOWING` / `FILLER_LIKED` (`utils/filler-user-profile.ts`) and the local `useState` in `routes/user-profile.tsx` for loader data and `authorizedRequest`-based mutations (see `auth.server.ts`). Swapping in real (possibly empty) arrays is also what makes the Activity/Following `EmptyState`s reachable from the actual route — today they're only exercised by the `Empty` Storybook stories, since the filler arrays are never empty. diff --git a/web/frontend/app/features/users/components/profile/ProfileAvatar.stories.tsx b/web/frontend/app/features/users/components/profile/ProfileAvatar.stories.tsx new file mode 100644 index 00000000..fb077adb --- /dev/null +++ b/web/frontend/app/features/users/components/profile/ProfileAvatar.stories.tsx @@ -0,0 +1,51 @@ +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, within } from 'storybook/test'; +import ProfileAvatar from './ProfileAvatar'; + +const profileAvatarDescription = `Avatar shown at the top of the public profile sidebar. It renders the photo from the public profile payload's \`avatarUrl\`, and shows the first two letters of the display name when that field is empty or the image fails to load.`; + +const meta = { + title: 'Users/ProfileAvatar', + component: ProfileAvatar, + tags: ['autodocs'], + args: { + displayName: 'Maren Iverson', + avatarUrl: 'https://picsum.photos/id/1027/240/240', + }, + parameters: { + layout: 'centered', + docs: { + description: { + component: profileAvatarDescription, + }, + }, + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const WithPhoto: Story = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const image = canvas.getByRole('img', { name: "Maren Iverson's avatar" }); + await expect(image).toHaveAttribute('src', 'https://picsum.photos/id/1027/240/240'); + }, +}; + +export const WithoutPhoto: Story = { + args: { avatarUrl: '' }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByText('MA')).toBeInTheDocument(); + await expect(canvas.queryByRole('img')).not.toBeInTheDocument(); + }, +}; + +export const BrokenPhotoUrl: Story = { + args: { avatarUrl: 'https://example.invalid/missing-avatar.jpg' }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(await canvas.findByText('MA')).toBeInTheDocument(); + }, +}; diff --git a/web/frontend/app/features/users/components/profile/ProfileAvatar.tsx b/web/frontend/app/features/users/components/profile/ProfileAvatar.tsx new file mode 100644 index 00000000..00bdd598 --- /dev/null +++ b/web/frontend/app/features/users/components/profile/ProfileAvatar.tsx @@ -0,0 +1,42 @@ +import { useState } from 'react'; + +interface ProfileAvatarProps { + avatarUrl: string; + displayName: string; +} + +/** + * Renders the user's current avatar photo, falling back to their initials when the public + * profile payload carries an empty `avatarUrl` or the hosted image fails to load. + */ +const ProfileAvatar = ({ avatarUrl, displayName }: ProfileAvatarProps) => { + const [didImageFail, setDidImageFail] = useState(false); + const showPhoto = avatarUrl.length > 0 && !didImageFail; + + return ( +
+ {showPhoto ? ( +
+ {`${displayName}'s setDidImageFail(true)} + /> +
+ ) : ( +
+ + {displayName.slice(0, 2).toUpperCase()} + +
+ )} +
+ ); +}; + +export default ProfileAvatar; diff --git a/web/frontend/app/features/users/routes/user-profile.tsx b/web/frontend/app/features/users/routes/user-profile.tsx index 6ec2d507..f4de9901 100644 --- a/web/frontend/app/features/users/routes/user-profile.tsx +++ b/web/frontend/app/features/users/routes/user-profile.tsx @@ -3,6 +3,7 @@ import { data } from 'react-router'; import RouteErrorState from '../../../components/ui/error/RouteErrorState'; import { getOptionalAuth } from '../../auth/auth.server'; import { getPublicUserProfile, type PublicUserProfile } from '../users.server'; +import ProfileAvatar from '../components/profile/ProfileAvatar'; import ProfileSidebarCard from '../components/profile/ProfileSidebarCard'; import ProfileTabs, { type ProfileTab } from '../components/profile/ProfileTabs'; import ActivityTab from '../components/profile/tabs/ActivityTab'; @@ -80,15 +81,7 @@ const ProfileContent = ({ profile, isOwnProfile }: ProfileContentProps) => {
-
-
-
- - {displayName.slice(0, 2).toUpperCase()} - -
-
-
+