fix(backend): resolve shift signup by looking up Member via ExternalUserId
All checks were successful
CI Pipeline / Backend Build & Test (pull_request) Successful in 52s
CI Pipeline / Frontend Lint, Test & Build (pull_request) Successful in 29s
CI Pipeline / Infrastructure Validation (pull_request) Successful in 5s

The signup/cancel endpoints were passing the Keycloak sub claim (external UUID)
directly as MemberId, but ShiftSignup.MemberId references the internal Member.Id.
Now ShiftService resolves ExternalUserId to the internal Member.Id before creating
the signup record. Integration tests updated to seed proper Member entities.
This commit is contained in:
WorkClub Automation
2026-03-09 13:24:50 +01:00
parent 1117cf2004
commit a8730245b2
3 changed files with 104 additions and 38 deletions

View File

@@ -118,17 +118,17 @@ public static class ShiftEndpoints
ShiftService shiftService,
HttpContext httpContext)
{
var userIdClaim = httpContext.User.FindFirst("sub")?.Value;
if (string.IsNullOrEmpty(userIdClaim) || !Guid.TryParse(userIdClaim, out var memberId))
var externalUserId = httpContext.User.FindFirst("sub")?.Value;
if (string.IsNullOrEmpty(externalUserId))
{
return TypedResults.UnprocessableEntity("Invalid user ID");
}
var (success, error, isConflict) = await shiftService.SignUpForShiftAsync(id, memberId);
var (success, error, isConflict) = await shiftService.SignUpForShiftAsync(id, externalUserId);
if (!success)
{
if (error == "Shift not found")
if (error == "Shift not found" || error == "Member not found")
return TypedResults.NotFound();
if (error == "Cannot sign up for past shifts")
@@ -146,17 +146,17 @@ public static class ShiftEndpoints
ShiftService shiftService,
HttpContext httpContext)
{
var userIdClaim = httpContext.User.FindFirst("sub")?.Value;
if (string.IsNullOrEmpty(userIdClaim) || !Guid.TryParse(userIdClaim, out var memberId))
var externalUserId = httpContext.User.FindFirst("sub")?.Value;
if (string.IsNullOrEmpty(externalUserId))
{
return TypedResults.UnprocessableEntity("Invalid user ID");
}
var (success, error) = await shiftService.CancelSignupAsync(id, memberId);
var (success, error) = await shiftService.CancelSignupAsync(id, externalUserId);
if (!success)
{
if (error == "Sign-up not found")
if (error == "Sign-up not found" || error == "Member not found")
return TypedResults.NotFound();
return TypedResults.UnprocessableEntity(error!);

View File

@@ -207,10 +207,18 @@ public class ShiftService
return true;
}
public async Task<(bool success, string? error, bool isConflict)> SignUpForShiftAsync(Guid shiftId, Guid memberId)
public async Task<(bool success, string? error, bool isConflict)> SignUpForShiftAsync(Guid shiftId, string externalUserId)
{
var tenantId = _tenantProvider.GetTenantId();
var member = await _context.Members
.FirstOrDefaultAsync(m => m.ExternalUserId == externalUserId && m.TenantId == tenantId);
if (member == null)
return (false, "Member not found", false);
var memberId = member.Id;
var shift = await _context.Shifts.FindAsync(shiftId);
if (shift == null)
@@ -265,10 +273,18 @@ public class ShiftService
return (false, "Shift capacity changed during sign-up", true);
}
public async Task<(bool success, string? error)> CancelSignupAsync(Guid shiftId, Guid memberId)
public async Task<(bool success, string? error)> CancelSignupAsync(Guid shiftId, string externalUserId)
{
var tenantId = _tenantProvider.GetTenantId();
var member = await _context.Members
.FirstOrDefaultAsync(m => m.ExternalUserId == externalUserId && m.TenantId == tenantId);
if (member == null)
return (false, "Member not found");
var signup = await _context.ShiftSignups
.FirstOrDefaultAsync(ss => ss.ShiftId == shiftId && ss.MemberId == memberId);
.FirstOrDefaultAsync(ss => ss.ShiftId == shiftId && ss.MemberId == member.Id);
if (signup == null)
{

View File

@@ -3,6 +3,7 @@ using System.Net.Http.Json;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using WorkClub.Domain.Entities;
using WorkClub.Domain.Enums;
using WorkClub.Infrastructure.Data;
using WorkClub.Tests.Integration.Infrastructure;
using Xunit;
@@ -23,9 +24,60 @@ public class ShiftCrudTests : IntegrationTestBase
// Clean up existing test data
context.ShiftSignups.RemoveRange(context.ShiftSignups);
context.Shifts.RemoveRange(context.Shifts);
context.Members.RemoveRange(context.Members);
context.Clubs.RemoveRange(context.Clubs);
await context.SaveChangesAsync();
}
private async Task<(Guid clubId, Guid memberId, string externalUserId)> SeedMemberAsync(
string tenantId,
string email,
string? externalUserId = null,
ClubRole role = ClubRole.Member)
{
externalUserId ??= Guid.NewGuid().ToString();
var clubId = Guid.NewGuid();
var memberId = Guid.NewGuid();
var now = DateTimeOffset.UtcNow;
using var scope = Factory.Services.CreateScope();
var context = scope.ServiceProvider.GetRequiredService<AppDbContext>();
var existingClub = await context.Clubs.FirstOrDefaultAsync(c => c.TenantId == tenantId);
if (existingClub != null)
{
clubId = existingClub.Id;
}
else
{
context.Clubs.Add(new Club
{
Id = clubId,
TenantId = tenantId,
Name = "Test Club",
SportType = SportType.Tennis,
CreatedAt = now,
UpdatedAt = now
});
}
context.Members.Add(new Member
{
Id = memberId,
TenantId = tenantId,
ExternalUserId = externalUserId,
DisplayName = email.Split('@')[0],
Email = email,
Role = role,
ClubId = clubId,
CreatedAt = now,
UpdatedAt = now
});
await context.SaveChangesAsync();
return (clubId, memberId, externalUserId);
}
[Fact]
public async Task CreateShift_AsManager_ReturnsCreated()
{
@@ -343,8 +395,8 @@ public class ShiftCrudTests : IntegrationTestBase
public async Task SignUpForShift_WithCapacity_ReturnsOk()
{
// Arrange
var (clubId, memberId, externalUserId) = await SeedMemberAsync("tenant1", "member@test.com");
var shiftId = Guid.NewGuid();
var clubId = Guid.NewGuid();
var createdBy = Guid.NewGuid();
var now = DateTimeOffset.UtcNow;
@@ -370,7 +422,7 @@ public class ShiftCrudTests : IntegrationTestBase
}
SetTenant("tenant1");
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" });
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, externalUserId);
// Act
var response = await Client.PostAsync($"/api/shifts/{shiftId}/signup", null);
@@ -384,6 +436,7 @@ public class ShiftCrudTests : IntegrationTestBase
var context = scope.ServiceProvider.GetRequiredService<AppDbContext>();
var signups = await context.ShiftSignups.Where(ss => ss.ShiftId == shiftId).ToListAsync();
Assert.Single(signups);
Assert.Equal(memberId, signups[0].MemberId);
}
}
@@ -391,11 +444,14 @@ public class ShiftCrudTests : IntegrationTestBase
public async Task SignUpForShift_WhenFull_ReturnsConflict()
{
// Arrange
var (clubId, _, externalUserId) = await SeedMemberAsync("tenant1", "member@test.com");
var shiftId = Guid.NewGuid();
var clubId = Guid.NewGuid();
var createdBy = Guid.NewGuid();
var now = DateTimeOffset.UtcNow;
// Seed a different member to fill the single slot
var (_, fillerMemberId, _) = await SeedMemberAsync("tenant1", "filler@test.com");
using (var scope = Factory.Services.CreateScope())
{
var context = scope.ServiceProvider.GetRequiredService<AppDbContext>();
@@ -420,7 +476,7 @@ public class ShiftCrudTests : IntegrationTestBase
Id = Guid.NewGuid(),
TenantId = "tenant1",
ShiftId = shiftId,
MemberId = Guid.NewGuid(),
MemberId = fillerMemberId,
SignedUpAt = now
});
@@ -428,7 +484,7 @@ public class ShiftCrudTests : IntegrationTestBase
}
SetTenant("tenant1");
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" });
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, externalUserId);
// Act
var response = await Client.PostAsync($"/api/shifts/{shiftId}/signup", null);
@@ -441,8 +497,8 @@ public class ShiftCrudTests : IntegrationTestBase
public async Task SignUpForShift_ForPastShift_ReturnsUnprocessableEntity()
{
// Arrange
var (clubId, _, externalUserId) = await SeedMemberAsync("tenant1", "member@test.com");
var shiftId = Guid.NewGuid();
var clubId = Guid.NewGuid();
var createdBy = Guid.NewGuid();
var now = DateTimeOffset.UtcNow;
@@ -455,7 +511,7 @@ public class ShiftCrudTests : IntegrationTestBase
Id = shiftId,
TenantId = "tenant1",
Title = "Past Shift",
StartTime = now.AddHours(-2), // Past shift
StartTime = now.AddHours(-2),
EndTime = now.AddHours(-1),
Capacity = 5,
ClubId = clubId,
@@ -468,7 +524,7 @@ public class ShiftCrudTests : IntegrationTestBase
}
SetTenant("tenant1");
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" });
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, externalUserId);
// Act
var response = await Client.PostAsync($"/api/shifts/{shiftId}/signup", null);
@@ -481,10 +537,9 @@ public class ShiftCrudTests : IntegrationTestBase
public async Task SignUpForShift_Duplicate_ReturnsConflict()
{
// Arrange
var (clubId, memberId, externalUserId) = await SeedMemberAsync("tenant1", "member@test.com");
var shiftId = Guid.NewGuid();
var clubId = Guid.NewGuid();
var createdBy = Guid.NewGuid();
var memberId = Guid.Parse("00000000-0000-0000-0000-000000000001"); // Fixed member ID
var now = DateTimeOffset.UtcNow;
using (var scope = Factory.Services.CreateScope())
@@ -505,7 +560,6 @@ public class ShiftCrudTests : IntegrationTestBase
UpdatedAt = now
});
// Add existing signup
context.ShiftSignups.Add(new ShiftSignup
{
Id = Guid.NewGuid(),
@@ -519,7 +573,7 @@ public class ShiftCrudTests : IntegrationTestBase
}
SetTenant("tenant1");
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, memberId.ToString());
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, externalUserId);
// Act
var response = await Client.PostAsync($"/api/shifts/{shiftId}/signup", null);
@@ -532,10 +586,9 @@ public class ShiftCrudTests : IntegrationTestBase
public async Task CancelSignup_BeforeShift_ReturnsOk()
{
// Arrange
var (clubId, memberId, externalUserId) = await SeedMemberAsync("tenant1", "member@test.com");
var shiftId = Guid.NewGuid();
var clubId = Guid.NewGuid();
var createdBy = Guid.NewGuid();
var memberId = Guid.Parse("00000000-0000-0000-0000-000000000001");
var now = DateTimeOffset.UtcNow;
using (var scope = Factory.Services.CreateScope())
@@ -569,7 +622,7 @@ public class ShiftCrudTests : IntegrationTestBase
}
SetTenant("tenant1");
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, memberId.ToString());
AuthenticateAs("member@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, externalUserId);
// Act
var response = await Client.DeleteAsync($"/api/shifts/{shiftId}/signup");
@@ -577,7 +630,6 @@ public class ShiftCrudTests : IntegrationTestBase
// Assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
// Verify signup was deleted
using (var scope = Factory.Services.CreateScope())
{
var context = scope.ServiceProvider.GetRequiredService<AppDbContext>();
@@ -590,8 +642,11 @@ public class ShiftCrudTests : IntegrationTestBase
public async Task SignUpForShift_ConcurrentLastSlot_HandlesRaceCondition()
{
// Arrange
var (clubId, fillerMemberId, _) = await SeedMemberAsync("tenant1", "filler@test.com");
var (_, _, externalUserId1) = await SeedMemberAsync("tenant1", "member1@test.com");
var (_, _, externalUserId2) = await SeedMemberAsync("tenant1", "member2@test.com");
var shiftId = Guid.NewGuid();
var clubId = Guid.NewGuid();
var createdBy = Guid.NewGuid();
var now = DateTimeOffset.UtcNow;
@@ -613,13 +668,12 @@ public class ShiftCrudTests : IntegrationTestBase
UpdatedAt = now
});
// Add one signup (leaving one slot)
context.ShiftSignups.Add(new ShiftSignup
{
Id = Guid.NewGuid(),
TenantId = "tenant1",
ShiftId = shiftId,
MemberId = Guid.NewGuid(),
MemberId = fillerMemberId,
SignedUpAt = now
});
@@ -628,24 +682,20 @@ public class ShiftCrudTests : IntegrationTestBase
SetTenant("tenant1");
// Act - Simulate two concurrent requests
var member1 = Guid.NewGuid();
var member2 = Guid.NewGuid();
AuthenticateAs("member1@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, member1.ToString());
// Act
AuthenticateAs("member1@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, externalUserId1);
var response1Task = Client.PostAsync($"/api/shifts/{shiftId}/signup", null);
AuthenticateAs("member2@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, member2.ToString());
AuthenticateAs("member2@test.com", new Dictionary<string, string> { ["tenant1"] = "Member" }, externalUserId2);
var response2Task = Client.PostAsync($"/api/shifts/{shiftId}/signup", null);
var responses = await Task.WhenAll(response1Task, response2Task);
// Assert - One should succeed (200), one should fail (409)
// Assert
var statuses = responses.Select(r => r.StatusCode).OrderBy(s => s).ToList();
Assert.Contains(HttpStatusCode.OK, statuses);
Assert.Contains(HttpStatusCode.Conflict, statuses);
// Verify only 2 total signups exist (capacity limit enforced)
using (var scope = Factory.Services.CreateScope())
{
var context = scope.ServiceProvider.GetRequiredService<AppDbContext>();