diff --git a/Tharga.Team.MongoDB/TeamServiceRepositoryBase.cs b/Tharga.Team.MongoDB/TeamServiceRepositoryBase.cs index 4940636..5713d90 100644 --- a/Tharga.Team.MongoDB/TeamServiceRepositoryBase.cs +++ b/Tharga.Team.MongoDB/TeamServiceRepositoryBase.cs @@ -132,6 +132,7 @@ protected override Task SetTeamMemberNameAsync(string teamKey, string userKey, s protected override IAsyncEnumerable GetTeamsAsync(IUser user) { + if (user == null) return AsyncEnumerable.Empty(); return _teamRepository.GetTeamsByUserAsync(user.Key); } diff --git a/Tharga.Team.Service.Tests/UnauthenticatedTeamServiceTests.cs b/Tharga.Team.Service.Tests/UnauthenticatedTeamServiceTests.cs new file mode 100644 index 0000000..7e0f5f2 --- /dev/null +++ b/Tharga.Team.Service.Tests/UnauthenticatedTeamServiceTests.cs @@ -0,0 +1,69 @@ +namespace Tharga.Team.Service.Tests; + +/// +/// Verifies that handles a null current user gracefully: +/// read paths return empty, side-effect paths throw . +/// +public class UnauthenticatedTeamServiceTests +{ + private readonly TestTeamService _sut; + + public UnauthenticatedTeamServiceTests() + { + var userService = Substitute.For(); + userService.GetCurrentUserAsync().Returns((IUser)null); + + _sut = new TestTeamService(userService); + _sut.AddTeam("team-1", "Test Team", + new TestMember { Key = "user-1", AccessLevel = AccessLevel.Owner, State = MembershipState.Member }, + new TestMember { Key = "user-2", AccessLevel = AccessLevel.User, State = MembershipState.Member }); + } + + [Fact] + public async Task GetTeamsAsync_Unauthenticated_ReturnsEmpty() + { + var teams = new List(); + await foreach (var t in _sut.GetTeamsAsync()) teams.Add(t); + + Assert.Empty(teams); + } + + [Fact] + public async Task GetTeamsAsync_Generic_Unauthenticated_ReturnsEmpty() + { + var teams = new List>(); + await foreach (var t in _sut.GetTeamsAsync()) teams.Add(t); + + Assert.Empty(teams); + } + + [Fact] + public async Task CreateTeamAsync_Unauthenticated_Throws() + { + await Assert.ThrowsAsync( + () => _sut.CreateTeamAsync("New Team")); + } + + [Fact] + public async Task RemoveMemberAsync_Unauthenticated_Throws() + { + // user-2 is a non-owner; attempting to remove them as an unauthenticated caller + // should hit the RequireCurrentUserAsync guard before any DB write. + await Assert.ThrowsAsync( + () => _sut.RemoveMemberAsync("team-1", "user-2")); + } + + [Fact] + public async Task SetMemberLastSeenAsync_Unauthenticated_DoesNotThrow() + { + var ex = await Record.ExceptionAsync(() => _sut.SetMemberLastSeenAsync("team-1")); + Assert.Null(ex); + } + + [Fact] + public async Task TransferOwnershipAsync_Unauthenticated_Throws() + { + await Assert.ThrowsAsync( + () => _sut.TransferOwnershipAsync("team-1", "user-1")); + } +} diff --git a/Tharga.Team/TeamServiceBase.cs b/Tharga.Team/TeamServiceBase.cs index d5d390c..b3b86a5 100644 --- a/Tharga.Team/TeamServiceBase.cs +++ b/Tharga.Team/TeamServiceBase.cs @@ -36,6 +36,7 @@ protected TeamServiceBase(IUserService userService) public async IAsyncEnumerable GetTeamsAsync() { var user = await GetCurrentUserAsync(); + if (user == null) yield break; await foreach (var team in GetTeamsAsync(user)) { @@ -46,6 +47,7 @@ public async IAsyncEnumerable GetTeamsAsync() public async IAsyncEnumerable> GetTeamsAsync() where TMember : ITeamMember { var user = await GetCurrentUserAsync(); + if (user == null) yield break; await foreach (var team in GetTeamsAsync(user)) { @@ -61,7 +63,7 @@ public async Task> GetTeamAsync(string teamKey) where TM public async Task CreateTeamAsync(string name) { - var user = await GetCurrentUserAsync(); + var user = await RequireCurrentUserAsync(); var displayName = ResolveDisplayName(user); name ??= $"{displayName}'s team"; @@ -124,7 +126,7 @@ public async Task RemoveMemberAsync(string teamKey, string userKey) if (member.AccessLevel == AccessLevel.Owner) throw new InvalidOperationException("The owner cannot leave the team. Transfer ownership first."); - var user = await GetCurrentUserAsync(); + var user = await RequireCurrentUserAsync(); if (member.Key == user.Key && member.AccessLevel == AccessLevel.Administrator) { var otherAdminsOrOwners = members.Count(x => @@ -211,13 +213,14 @@ protected virtual Task GetInvitedMemberNameAsync(string teamKey, string public async Task SetMemberLastSeenAsync(string teamKey) { var user = await GetCurrentUserAsync(); + if (user == null) return; await SetTeamMemberLastSeenAsync(teamKey, user.Key); _teamMemberCache.TryRemove($"{teamKey}.{user.Key}", out _); } public async Task TransferOwnershipAsync(string teamKey, string newOwnerUserKey) where TMember : ITeamMember { - var user = await GetCurrentUserAsync(); + var user = await RequireCurrentUserAsync(); var team = await GetTeamAsync(teamKey); var currentOwner = team.Members.SingleOrDefault(x => x.Key == user.Key); if (currentOwner == null || currentOwner.AccessLevel != AccessLevel.Owner) @@ -262,7 +265,7 @@ private async Task GetRandomUnsusedTeamKey() private async Task AssureAccessLevel(string teamKey, AccessLevel accessLevel) where TMember : ITeamMember { - var user = await GetCurrentUserAsync(); + var user = await RequireCurrentUserAsync(); var team = await GetTeamAsync(teamKey); var member = team.Members.Single(x => x.Key == user.Key); if (member.State != MembershipState.Member) throw new InvalidOperationException("User is not a member."); @@ -275,6 +278,13 @@ private async Task GetCurrentUserAsync() return user; } + private async Task RequireCurrentUserAsync() + { + var user = await GetCurrentUserAsync(); + if (user == null) throw new UnauthorizedAccessException("Authentication required."); + return user; + } + public static string ResolveDisplayName(IUser user) { if (user == null) return "Unknown"; diff --git a/plan/feature.md b/plan/feature.md deleted file mode 100644 index 0252696..0000000 --- a/plan/feature.md +++ /dev/null @@ -1,118 +0,0 @@ -# Feature: Stable Member.Key + name promotion + user self-edit - -Filed in `Requests.md` 2026-05-10 ("TeamComponent inline name-edit broken for invited members"). Scope expanded during planning to include (a) name promotion on invitation accept, and (b) user self-edit of their own member row that propagates to `User.Name`. - -## Problem - -Invited (not-yet-accepted) team members are commonly created with `Member.Key = null` (the canonical pattern in the sample's `CreateTeamMember`). Two downstream effects: - -1. **UI** — the inline name-edit gate uses `_editingMemberKey == context.Key`. When `_editingMemberKey` is reset to `null` after Cancel/Save, `null == null` matches indefinitely → edit mode sticks; opening one invited row's editor puts every invited row into edit mode at once. -2. **Server** — `TeamRepository.SetMemberNameAsync` does `team.Members.Single(x => x.Key == userKey)` and `.Where(x => x.Key != userKey)`. Multiple invited members (all `Key=null`) cause `.Single` to throw and `.Where` to strip every null-keyed sibling on save. - -Separately, the admin-entered invitation name (`Member.Name`) is the only place a new user's display name is captured before the user signs in. Today, on accept, that name stays as a per-team override on `Member.Name` and is never promoted to `User.Name`. If the IdP doesn't supply a name, the user shows up as "Unknown" everywhere except the inviting team. - -A third gap: today only callers with `team:manage` can edit the inline name. A user has no way to update their own display name from the team UI — they'd need a separate "edit profile" page that doesn't exist. Editing your own name should be a self-service action that updates the global `User.Name` (the truth) and clears the per-team override in the team where the edit happened. - -## Goal - -1. Eliminate the null-`Member.Key` anomaly: invited members get a stable, generated key from creation. -2. On accept, promote the admin-entered `Member.Name` into `User.Name` if the user has no name yet, then clear the per-team override. The IdP claim wins when present; admin's invitation name is the fallback; per-team override goes away once the user has a real identity. -3. Allow users to edit their own member row. Self-edit always overwrites `User.Name` and clears `Member.Name` in the current team. Other teams retain whatever override their admins set. - -## Scope - -In-scope: - -1. **Auto-generate `Member.Key` on invitation** — `TeamServiceRepositoryBase.AddTeamMemberAsync`: if `memberModel.Key` is null/empty, assign `Guid.NewGuid().ToString()`. Symmetric to existing `Invitation` and `State` defaults in the same method. -2. **Promote Member.Name → User.Name on accept (only-if-empty rule)** — at acceptance, if `User.Name` is currently null/empty, seed it with the captured `Member.Name`. Always clear `Member.Name` on accept regardless. -3. **User self-edit own row** — show the inline name-edit pencil on the current user's own member row regardless of `team:manage` (so any user can edit their own name). On save: always overwrite `User.Name`, always clear `Member.Name` in the current team. Admin-edit on other rows is unchanged (writes per-team override). -4. **UI defensive gate (legacy data)** — `TeamComponent.razor`: change `_editingMemberKey == context.Key` → `!string.IsNullOrEmpty(_editingMemberKey) && _editingMemberKey == context.Key`. Belt-and-suspenders for any team document that still has null-keyed members from before this fix. - -Confirmed during planning, no change needed: - -- `TeamRepository.SetInvitationResponseAsync` already swaps `Key = userKey` on accept (and on reject, with state=Rejected). - -Out of scope: - -- Reset-button polish for invited rows. (User declined.) -- Migration of existing data with `Key=null`. (UI gate prevents the symptom; existing nulls heal naturally as invitations flow through accept/reject.) -- Server-side `.Single` resilience inside `TeamRepository.SetMemberNameAsync`. (Once `Member.Key` is unique by construction the existing `.Single` is correct.) -- Per-team alias for self (i.e., a user picking different names per team). The chosen design treats self-edit as global identity. -- Separate "Edit profile" page outside `TeamComponent`. The team grid is the only edit surface delivered here. - -## Approach - -### A. Auto-generate Member.Key (low-risk) - -In `Tharga.Team.MongoDB/TeamServiceRepositoryBase.cs`, inside `AddTeamMemberAsync`, after `CreateTeamMember` and adjacent to the existing `Invitation` / `State` auto-generation: - -```csharp -if (string.IsNullOrEmpty(memberModel.Key)) -{ - memberModel = memberModel with { Key = Guid.NewGuid().ToString() }; -} -``` - -### B. Name promotion on accept (cross-repo bridge) - -The plumbing must reach across the team-repo and user-repo boundaries. Chosen path: route via `IUserService` so `TeamServiceRepositoryBase`'s constructor signature does not change. - -1. **`Tharga.Team.MongoDB/IUserRepository.cs`** — add two methods: - - `Task GetByKeyAsync(string userKey)` — lookup-by-Key (today only `GetAsync(identity)` exists). - - `Task UpdateAsync(TUserEntity user)` — replace the whole user document. - -2. **`Tharga.Team.MongoDB/UserRepository.cs`** — implement both via `_collection.GetOneAsync(x => x.Key == userKey)` and `_collection.ReplaceOneAsync` respectively. - -3. **`Tharga.Team/IUserService.cs`** — add two methods (different semantics): - - `Task SeedUserNameAsync(string userKey, string name)` — only-if-empty. Used by `TeamServiceBase` on invitation-accept. - - `Task SetUserNameAsync(string userKey, string name)` — always overwrites. Used by user self-edit. - -4. **`Tharga.Team/UserServiceBase.cs`** — provide virtual `Task.CompletedTask` no-op defaults for both so consumers extending the base without overriding aren't broken. - -5. **`Tharga.Team.MongoDB/UserServiceRepositoryBase.cs`** — override both: - - Shared helper: `var user = await _userRepository.GetByKeyAsync(userKey);` if null → log warning + return. - - `SeedUserNameAsync`: if `!string.IsNullOrEmpty(user.Name)` → return (only-if-empty). Else write + cache invalidate. - - `SetUserNameAsync`: always write (`user with { Name = name }` → `UpdateAsync`) + cache invalidate. - -6. **`Tharga.Team/TeamServiceBase.cs`** — orchestrate in `SetInvitationResponseAsync`: - - On `accept == true`: pre-fetch the team, find the member with `Invitation.InviteKey == inviteKey`, capture `member.Name` as `seedName`. - - Call existing `SetTeamMemberInvitationResponseAsync(teamKey, userKey, inviteKey, true)` — already swaps `Key = userKey` and clears `Invitation`. **Update** the underlying `TeamRepository.SetInvitationResponseAsync` to also set `Name = null` on accept (1-line addition to the existing `with` block). - - If `seedName` is non-empty, `await _userService.SeedUserNameAsync(userKey, seedName)`. - -### C. User self-edit own row (UI + orchestration) - -In `Tharga.Team.Blazor/Features/Team/TeamComponent.razor`: - -1. **Pencil visibility** — change the gate from `EnableMemberNameEdit && _canManage` to `EnableMemberNameEdit && (_canManage || context.Key == _user.Key)` so a non-admin user sees the pencil on their own row only. -2. **Save behaviour** — `SaveMemberName` branches: - - If `member.Key == _user.Key` (self-edit): `await UserService.SetUserNameAsync(_user.Key, newName); await TeamManagementService.SetMemberNameAsync(team.Key, _user.Key, null);` Then reload + fire `OnMemberNameChanged` with the (cleared) override and the new identity. - - Else (admin-edit on another row): unchanged — `await TeamManagementService.SetMemberNameAsync(...)` writes the per-team override. -3. **Reset button** — hide for self-edit (`context.Key == _user.Key && !_canManage`); for self there is no override-vs-default distinction. - -### D. UI defensive gate - -In `Tharga.Team.Blazor/Features/Team/TeamComponent.razor`, line ~65: - -```razor -@if (EnableMemberNameEdit && (_canManage || context.Key == _user.Key) && !string.IsNullOrEmpty(_editingMemberKey) && _editingMemberKey == context.Key) -``` - -(Note: the visibility changes from C.1 and the null guard are merged into a single condition.) - -## Acceptance criteria - -- New invited members in MongoDB carry a non-null `Member.Key` (manual verify on sample app). -- Inline name-edit on an invited member returns the row to read-only display after save (manual verify). -- Multiple invited members do not all enter edit mode together when one is opened (manual verify). -- On invitation accept: `User.Name` is seeded from `Member.Name` if previously empty; never overwritten if present. `Member.Name` is cleared on accept regardless. (Manual verify with a fresh user; unit tested for the orchestration logic.) -- A non-admin user can edit their own row in `` and the change updates `User.Name` globally + clears `Member.Name` in the current team. Other teams' admin-set overrides for that user are unaffected. (Manual verify across two teams; unit tested.) -- `dotnet build -c Release` clean. -- `dotnet test -c Release` — all existing tests still pass; new orchestration tests added in `Tharga.Team.Service.Tests` for: invitation-accept seed flow (called when prior name was set, NOT called when empty, NOT called on reject), user self-edit branch (calls `SetUserNameAsync` + clears Member.Name) vs admin-edit branch (calls only `SetMemberNameAsync`). -- Bundled with the close-out commit from #61. - -## Done condition - -User confirms manually on the sample app: -- A new invitation creates a member with non-null `Key` and non-cleared `Name` while invited. -- After accept (with a fresh user whose IdP returns no name), `User.Name` carries the admin-entered invitation name and `Member.Name` is null. -- After accept (with a user whose IdP already returns a name), `User.Name` is unchanged and `Member.Name` is null. diff --git a/plan/plan.md b/plan/plan.md deleted file mode 100644 index a66691e..0000000 --- a/plan/plan.md +++ /dev/null @@ -1,44 +0,0 @@ -# Plan: Stable Member.Key + name promotion + user self-edit - -Branch: `feature/invited-member-edit-fix` (already carries the close-out commit `69c8226 feat: teammember-duplicate-fix complete` — bundled per user preference to avoid an extra PR). - -## Steps - -- [x] **1. Confirm plan with user.** Done. -- [x] **2. Auto-generate `Member.Key` in `AddTeamMemberAsync`.** `TeamServiceRepositoryBase.cs` — symmetric block alongside existing Invitation/State defaults. -- [x] **3. Extend `IUserRepository`.** Added `GetByKeyAsync(string userKey)` and `SetNameAsync(string userKey, string name)`. Note: shipped a focused `SetNameAsync` (partial MongoDB update via `UpdateDefinitionBuilder.Set`) instead of a generic `UpdateAsync(TUserEntity)` because `IUser.Name` is read-only on the interface and `with { Name = … }` doesn't compile against the constraint `where TUserEntity : EntityBase, IUser`. The focused-method form is also more efficient (no full-document round-trip). -- [x] **4. Extend `IUserService`.** Added `SeedUserNameAsync` (only-if-empty) and `SetUserNameAsync` (always-overwrites) with `Task.CompletedTask` virtual defaults on `UserServiceBase`, real implementations in `UserServiceRepositoryBase` (GetByKey → check rule → SetNameAsync → InvalidateUserCache). Added `protected void InvalidateUserCache(string identity)` on `UserServiceBase` for the cache invalidation. -- [x] **5. Clear `Member.Name` on accept.** Added `Name = null,` to the `member with { ... }` accept block in `TeamRepository.SetInvitationResponseAsync`. -- [x] **6. Orchestrate seeding in `TeamServiceBase.SetInvitationResponseAsync`.** Pre-capture via new virtual hook `GetInvitedMemberNameAsync(teamKey, inviteKey)` (default returns null on `TeamServiceBase`; overridden in `TeamServiceRepositoryBase` using its typed access to `team.Members`). After response call, if seedName non-empty, `await _userService.SeedUserNameAsync(userKey, seedName)`. -- [x] **7. Self-edit branch in `TeamComponent.razor`.** `SaveMemberName` branches on `member.Key == _user.Key`. Self: `UserService.SetUserNameAsync` + `SetMemberNameAsync(..., null)`. Other: existing per-team override path. Empty self-edit input is treated as a no-op cancel. -- [x] **8. UI gates in `TeamComponent.razor`.** New `canEditThisRow` / `isSelfEdit` locals at the top of the Name template. Edit-mode visibility now `canEditThisRow && !string.IsNullOrEmpty(_editingMemberKey) && _editingMemberKey == context.Key`. Pencil visible whenever `canEditThisRow`. Reset button hidden when `isSelfEdit`. -- [x] **9. Tests.** Added in `Tharga.Team.Service.Tests`: - - `SetInvitationResponseSeedTests` — 4 tests: accept with name calls Seed once, accept with empty/whitespace name doesn't call Seed (×2), reject doesn't call Seed. - - `UserServiceBaseDefaultsTests` — 2 tests: both new virtual defaults are no-ops. - - UI self-edit branch not unit-tested (no Razor harness). Manual verification will cover it. - No tests for `IUserRepository.SetNameAsync` / `GetByKeyAsync` — no MongoDB integration test project in repo. -- [x] **10. Build and run all tests.** Build clean (0 warnings, 0 errors). 274 / 274 tests passing (was 268, +6 new). 145 Service + 37 Mcp + 92 Blazor. -- [~] **11. Manual verification on sample.** Run the sample app: - - Invite a new user with a name. Inspect MongoDB → confirm `Members[].Key` is a non-null GUID and `Members[].Name` is the admin-entered value. - - Edit the invited member's name in the UI, save. Confirm row returns to read-only display showing the new name. - - Invite a second user; open one's edit pencil — confirm the other invited row stays read-only. - - Accept the invitation as the invitee (using a fresh user whose IdP-returned name is empty or present). Confirm: - - `Member.Key` was swapped to `User.Key`. - - `Member.Name` is now null. - - `User.Name` was seeded from `Member.Name` only if it was empty before. - - **Self-edit:** as a non-admin member of a team, click the pencil on your own row, change the name, save. Confirm: - - `User.Name` updated globally. - - `Member.Name` is null in this team. - - In a *different* team where the same user has an admin-set override, that override is unchanged. -- [ ] **12. Commit.** Conventional commit `fix: stable Member.Key for invited members + Member.Name promotion (accept + self-edit)`. Includes `plan/`. -- [ ] **13. Hand back to user for testing.** -- [ ] **14. (After user OK) Open PR.** Push and open PR against `master`. Description doubles as release notes (per `mission.md`); cover the close-out + all three areas (null-Key fix, accept-time promotion, self-edit). -- [ ] **15. (After PR merged) Close out.** Archive `plan/feature.md` to `$DOC_ROOT/Tharga/plans/Toolkit/Platform/done/invited-member-edit-fix.md`, delete `plan/`, final commit `feat: invited-member-edit-fix complete`. - -## Notes - -- No NuGet upgrades needed (`dotnet outdated` reports nothing). -- The branch already carries the close-out commit for the previous feature (`69c8226`). The single PR will include: `plan/` deletion (close-out of #61) + new `plan/` + this fix's code changes. Net result on master: clean. -- **API surface change** (consumer-visible): `IUserRepository` gains `GetByKeyAsync` + `UpdateAsync`. `IUserService` gains `SeedUserNameAsync` + `SetUserNameAsync`. All four are non-breaking for consumers extending `UserServiceRepositoryBase` / using `IUserRepository` as DI-only (typical pattern). External implementers of those interfaces directly would see breakage — none known in this codebase. Suggest a minor version bump on next release. -- **Decided against** adding `TUserEntity` as a third type-parameter to `TeamServiceRepositoryBase`. Would have been a breaking change for every consumer extending it (`Tharga.Platform.Sample/Framework/Team/TeamService.cs`, plus external). Routing through `IUserService` keeps `TeamServiceRepositoryBase`'s ctor stable. \ No newline at end of file