Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move logged-in user endpoints to /users/me and apply code cleanup #682

Merged
merged 4 commits into from
Jan 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions application/account-management/Api/Endpoints/UserEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ public void MapEndpoints(IEndpointRouteBuilder routes)
=> await mediator.Send(new GetUserSummaryQuery())
).Produces<GetUserSummaryResponse>();

group.MapGet("/{id}", async Task<ApiResult<UserResponse>> ([AsParameters] GetUserQuery query, IMediator mediator)
=> await mediator.Send(query)
).Produces<UserResponse>();

group.MapPost("/", async Task<ApiResult> (CreateUserCommand command, IMediator mediator)
=> (await mediator.Send(command)).AddResourceUri(RoutesPrefix)
);
Expand All @@ -43,19 +39,23 @@ public void MapEndpoints(IEndpointRouteBuilder routes)
);

// The following endpoints are for the current user only
group.MapPut("/", async Task<ApiResult> (UpdateUserCommand command, IMediator mediator)
group.MapGet("/me", async Task<ApiResult<UserResponse>> ([AsParameters] GetUserQuery query, IMediator mediator)
=> await mediator.Send(query)
).Produces<UserResponse>();

group.MapPut("/me", async Task<ApiResult> (UpdateCurrentUserCommand command, IMediator mediator)
=> (await mediator.Send(command)).AddRefreshAuthenticationTokens()
);

group.MapPost("/update-avatar", async Task<ApiResult> (IFormFile file, IMediator mediator)
group.MapPost("/me/update-avatar", async Task<ApiResult> (IFormFile file, IMediator mediator)
=> await mediator.Send(new UpdateAvatarCommand(file.OpenReadStream(), file.ContentType))
).DisableAntiforgery(); // Disable anti-forgery until we implement it

group.MapDelete("/remove-avatar", async Task<ApiResult> (IMediator mediator)
group.MapDelete("/me/remove-avatar", async Task<ApiResult> (IMediator mediator)
=> await mediator.Send(new RemoveAvatarCommand())
);

group.MapPut("/change-locale", async Task<ApiResult> (ChangeLocaleCommand command, IMediator mediator)
group.MapPut("/me/change-locale", async Task<ApiResult> (ChangeLocaleCommand command, IMediator mediator)
=> (await mediator.Send(command)).AddRefreshAuthenticationTokens()
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
public LoginId Id { get; init; } = null!;
}

public sealed class CompleteLoginHandler(

Check warning on line 21 in application/account-management/Core/Features/Authentication/Commands/CompleteLogin.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Constructor has 8 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)

Check warning on line 21 in application/account-management/Core/Features/Authentication/Commands/CompleteLogin.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Constructor has 8 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)

Check warning on line 21 in application/account-management/Core/Features/Authentication/Commands/CompleteLogin.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Constructor has 8 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)

Check warning on line 21 in application/account-management/Core/Features/Authentication/Commands/CompleteLogin.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Constructor has 8 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)
IUserRepository userRepository,
ILoginRepository loginRepository,
OneTimePasswordHelper oneTimePasswordHelper,
Expand All @@ -33,10 +33,7 @@
{
var login = await loginRepository.GetByIdAsync(command.Id, cancellationToken);

if (login is null)
{
return Result.NotFound($"Login with id '{command.Id}' not found.");
}
if (login is null) return Result.NotFound($"Login with id '{command.Id}' not found.");

if (login.Completed)
{
Expand Down Expand Up @@ -79,7 +76,7 @@
var gravatar = await gravatarClient.GetGravatar(user.Id, user.Email, cancellationToken);
if (gravatar is not null)
{
if (await avatarUpdater.UpdateAvatar(user, true, gravatar.ContentType, gravatar.Stream, cancellationToken))

Check warning on line 79 in application/account-management/Core/Features/Authentication/Commands/CompleteLogin.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Merge this if statement with the enclosing one. (https://rules.sonarsource.com/csharp/RSPEC-1066)

Check warning on line 79 in application/account-management/Core/Features/Authentication/Commands/CompleteLogin.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Merge this if statement with the enclosing one. (https://rules.sonarsource.com/csharp/RSPEC-1066)

Check warning on line 79 in application/account-management/Core/Features/Authentication/Commands/CompleteLogin.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Merge this if statement with the enclosing one. (https://rules.sonarsource.com/csharp/RSPEC-1066)

Check warning on line 79 in application/account-management/Core/Features/Authentication/Commands/CompleteLogin.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Merge this if statement with the enclosing one. (https://rules.sonarsource.com/csharp/RSPEC-1066)
{
events.CollectEvent(new GravatarUpdated(gravatar.Stream.Length));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@ ILogger<ResendLoginCodeCommandHandler> logger
public async Task<Result<ResendLoginCodeResponse>> Handle(ResendLoginCodeCommand command, CancellationToken cancellationToken)
{
var login = await loginRepository.GetByIdAsync(command.Id, cancellationToken);
if (login is null)
{
return Result<ResendLoginCodeResponse>.NotFound($"Login with id '{command.Id}' not found.");
}
if (login is null) return Result<ResendLoginCodeResponse>.NotFound($"Login with id '{command.Id}' not found.");

if (login.Completed)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ public async Task<Result> Handle(CompleteSignupCommand command, CancellationToke
{
var signup = await signupRepository.GetByIdAsync(command.Id, cancellationToken);

if (signup is null)
{
return Result.NotFound($"Signup with id '{command.Id}' not found.");
}
if (signup is null) return Result.NotFound($"Signup with id '{command.Id}' not found.");

if (signup.Completed)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ ILogger<ResendSignupCodeCommandHandler> logger
public async Task<Result<ResendSignupCodeResponse>> Handle(ResendSignupCodeCommand command, CancellationToken cancellationToken)
{
var signup = await signupRepository.GetByIdAsync(command.Id, cancellationToken);
if (signup is null)
{
return Result<ResendSignupCodeResponse>.NotFound($"Signup with id '{command.Id}' not found.");
}
if (signup is null) return Result<ResendSignupCodeResponse>.NotFound($"Signup with id '{command.Id}' not found.");

if (signup.Completed)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ public sealed class ChangeLocaleHandler(IUserRepository userRepository, ITelemet
public async Task<Result> Handle(ChangeLocaleCommand command, CancellationToken cancellationToken)
{
var user = await userRepository.GetLoggedInUserAsync(cancellationToken);
if (user is null) return Result.BadRequest("User not found.");

var fromLocale = user.Locale;
user.ChangeLocale(command.Locale);
userRepository.Update(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ public sealed class ChangeUserRoleHandler(IUserRepository userRepository, IExecu
{
public async Task<Result> Handle(ChangeUserRoleCommand command, CancellationToken cancellationToken)
{
if (executionContext.UserInfo.Id == command.Id)
{
return Result.Forbidden("You cannot change your own user role.");
}
if (executionContext.UserInfo.Id == command.Id) return Result.Forbidden("You cannot change your own user role.");

if (executionContext.UserInfo.Role != UserRole.Owner.ToString())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ public sealed class DeleteUserHandler(IUserRepository userRepository, IExecution
{
public async Task<Result> Handle(DeleteUserCommand command, CancellationToken cancellationToken)
{
if (executionContext.UserInfo.Id == command.Id)
{
return Result.Forbidden("You cannot delete yourself.");
}
if (executionContext.UserInfo.Id == command.Id) return Result.Forbidden("You cannot delete yourself.");

if (executionContext.UserInfo.Role != UserRole.Owner.ToString())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ public sealed class RemoveAvatarCommandHandler(IUserRepository userRepository, I
public async Task<Result> Handle(RemoveAvatarCommand command, CancellationToken cancellationToken)
{
var user = await userRepository.GetLoggedInUserAsync(cancellationToken);
if (user is null) return Result.BadRequest("User not found.");

user.RemoveAvatar();
userRepository.Update(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,12 @@ public UpdateAvatarValidator()
}
}

public sealed class UpdateAvatarHandler(
IUserRepository userRepository,
AvatarUpdater avatarUpdater,
ITelemetryEventsCollector events
) : IRequestHandler<UpdateAvatarCommand, Result>
public sealed class UpdateAvatarHandler(IUserRepository userRepository, AvatarUpdater avatarUpdater, ITelemetryEventsCollector events)
: IRequestHandler<UpdateAvatarCommand, Result>
{
public async Task<Result> Handle(UpdateAvatarCommand command, CancellationToken cancellationToken)
{
var user = await userRepository.GetLoggedInUserAsync(cancellationToken);
if (user is null)
{
return Result.BadRequest("User not found.");
}

if (await avatarUpdater.UpdateAvatar(user, false, command.ContentType, command.FileSteam, cancellationToken))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
namespace PlatformPlatform.AccountManagement.Features.Users.Commands;

[PublicAPI]
public sealed record UpdateUserCommand : ICommand, IRequest<Result>
public sealed record UpdateCurrentUserCommand : ICommand, IRequest<Result>
{
public required string Email { get; init; }

Expand All @@ -19,9 +19,9 @@ public sealed record UpdateUserCommand : ICommand, IRequest<Result>
public required string Title { get; init; }
}

public sealed class UpdateUserValidator : AbstractValidator<UpdateUserCommand>
public sealed class UpdateCurrentUserValidator : AbstractValidator<UpdateCurrentUserCommand>
{
public UpdateUserValidator()
public UpdateCurrentUserValidator()
{
RuleFor(x => x.Email).NotEmpty().SetValidator(new SharedValidations.Email());
RuleFor(x => x.FirstName).NotEmpty().MaximumLength(30).WithMessage("First name must be no longer than 30 characters.");
Expand All @@ -30,13 +30,12 @@ public UpdateUserValidator()
}
}

public sealed class UpdateUserHandler(IUserRepository userRepository, ITelemetryEventsCollector events)
: IRequestHandler<UpdateUserCommand, Result>
public sealed class UpdateCurrentUserHandler(IUserRepository userRepository, ITelemetryEventsCollector events)
: IRequestHandler<UpdateCurrentUserCommand, Result>
{
public async Task<Result> Handle(UpdateUserCommand command, CancellationToken cancellationToken)
public async Task<Result> Handle(UpdateCurrentUserCommand command, CancellationToken cancellationToken)
{
var user = await userRepository.GetLoggedInUserAsync(cancellationToken);
if (user is null) return Result.BadRequest("User not found.");

user.UpdateEmail(command.Email);
user.Update(command.FirstName, command.LastName, command.Title);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
{
Task<User?> GetByIdUnfilteredAsync(UserId id, CancellationToken cancellationToken);

Task<User?> GetLoggedInUserAsync(CancellationToken cancellationToken);
Task<User> GetLoggedInUserAsync(CancellationToken cancellationToken);

Task<User?> GetUserByEmailUnfilteredAsync(string email, CancellationToken cancellationToken);

Expand All @@ -20,7 +20,7 @@

Task<(int TotalUsers, int ActiveUsers, int PendingUsers)> GetUserSummaryAsync(CancellationToken cancellationToken);

Task<(User[] Users, int TotalItems, int TotalPages)> Search(

Check warning on line 23 in application/account-management/Core/Features/Users/Domain/UserRepository.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Method has 10 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)

Check warning on line 23 in application/account-management/Core/Features/Users/Domain/UserRepository.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Method has 10 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)

Check warning on line 23 in application/account-management/Core/Features/Users/Domain/UserRepository.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Method has 10 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)

Check warning on line 23 in application/account-management/Core/Features/Users/Domain/UserRepository.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Method has 10 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)
string? search,
UserRole? userRole,
UserStatus? userStatus,
Expand Down Expand Up @@ -48,10 +48,11 @@
.SingleOrDefaultAsync(u => u.Id == id, cancellationToken);
}

public async Task<User?> GetLoggedInUserAsync(CancellationToken cancellationToken)
public async Task<User> GetLoggedInUserAsync(CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(executionContext.UserInfo.Id);
return await GetByIdAsync(executionContext.UserInfo.Id, cancellationToken);
return await GetByIdAsync(executionContext.UserInfo.Id, cancellationToken) ??
throw new InvalidOperationException("Logged in user not found.");
}

/// <summary>
Expand Down Expand Up @@ -93,7 +94,7 @@
return (summary.TotalUsers, summary.ActiveUsers, summary.PendingUsers);
}

public async Task<(User[] Users, int TotalItems, int TotalPages)> Search(

Check warning on line 97 in application/account-management/Core/Features/Users/Domain/UserRepository.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 97 in application/account-management/Core/Features/Users/Domain/UserRepository.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 97 in application/account-management/Core/Features/Users/Domain/UserRepository.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 97 in application/account-management/Core/Features/Users/Domain/UserRepository.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
string? search,
UserRole? userRole,
UserStatus? userStatus,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
namespace PlatformPlatform.AccountManagement.Features.Users.Queries;

[PublicAPI]
public sealed record GetUserQuery(UserId Id) : IRequest<Result<UserResponse>>;
public sealed record GetUserQuery : IRequest<Result<UserResponse>>;

[PublicAPI]
public sealed record UserResponse(
Expand All @@ -27,7 +27,7 @@ public sealed class GetUserHandler(IUserRepository userRepository)
{
public async Task<Result<UserResponse>> Handle(GetUserQuery query, CancellationToken cancellationToken)
{
var user = await userRepository.GetByIdAsync(query.Id, cancellationToken);
return user?.Adapt<UserResponse>() ?? Result<UserResponse>.NotFound($"User with id '{query.Id}' not found.");
var user = await userRepository.GetLoggedInUserAsync(cancellationToken);
return user.Adapt<UserResponse>();
}
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,18 @@
using System.Net;
using FluentAssertions;
using NJsonSchema;
using PlatformPlatform.AccountManagement.Database;
using PlatformPlatform.SharedKernel.Domain;
using PlatformPlatform.SharedKernel.Tests;
using Xunit;

namespace PlatformPlatform.AccountManagement.Tests.Users;

public sealed class GetUserTests : EndpointBaseTest<AccountManagementDbContext>
public sealed class GetCurrentUserTests : EndpointBaseTest<AccountManagementDbContext>
{
[Fact]
public async Task GetUser_WhenUserExists_ShouldReturnUserWithValidContract()
public async Task GetLoggedInUser_WhenUserExists_ShouldReturnUserWithValidContract()
{
// Arrange
var existingUserId = DatabaseSeeder.User1.Id;

// Act
var response = await AuthenticatedHttpClient.GetAsync($"/api/account-management/users/{existingUserId}");
var response = await AuthenticatedHttpClient.GetAsync("/api/account-management/users/me");

// Assert
response.ShouldBeSuccessfulGetRequest();
Expand Down Expand Up @@ -47,30 +42,4 @@ public async Task GetUser_WhenUserExists_ShouldReturnUserWithValidContract()
var responseBody = await response.Content.ReadAsStringAsync();
schema.Validate(responseBody).Should().BeEmpty();
}

[Fact]
public async Task GetUser_WhenUserDoesNotExist_ShouldReturnNotFound()
{
// Arrange
var unknownUserId = UserId.NewId();

// Act
var response = await AuthenticatedHttpClient.GetAsync($"/api/account-management/users/{unknownUserId}");

// Assert
await response.ShouldHaveErrorStatusCode(HttpStatusCode.NotFound, $"User with id '{unknownUserId}' not found.");
}

[Fact]
public async Task GetUser_WhenInvalidUserId_ShouldReturnBadRequest()
{
// Arrange
var invalidUserId = Faker.Random.AlphaNumeric(31);

// Act
var response = await AuthenticatedHttpClient.GetAsync($"/api/account-management/users/{invalidUserId}");

// Assert
await response.ShouldHaveErrorStatusCode(HttpStatusCode.BadRequest, $"""Failed to bind parameter "UserId Id" from "{invalidUserId}".""");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

namespace PlatformPlatform.AccountManagement.Tests.Users;

public sealed class UpdateUserTests : EndpointBaseTest<AccountManagementDbContext>
public sealed class UpdateCurrentUserTests : EndpointBaseTest<AccountManagementDbContext>
{
[Fact]
public async Task UpdateUser_WhenValid_ShouldUpdateUser()
public async Task UpdateCurrentUser_WhenValid_ShouldUpdateUser()
{
// Arrange
var command = new UpdateUserCommand
var command = new UpdateCurrentUserCommand
{
Email = Faker.Internet.Email(),
FirstName = Faker.Name.FirstName(),
Expand All @@ -23,17 +23,17 @@ public async Task UpdateUser_WhenValid_ShouldUpdateUser()
};

// Act
var response = await AuthenticatedHttpClient.PutAsJsonAsync("/api/account-management/users", command);
var response = await AuthenticatedHttpClient.PutAsJsonAsync("/api/account-management/users/me", command);

// Assert
response.ShouldHaveEmptyHeaderAndLocationOnSuccess();
}

[Fact]
public async Task UpdateUser_WhenInvalid_ShouldReturnBadRequest()
public async Task UpdateCurrentUser_WhenInvalid_ShouldReturnBadRequest()
{
// Arrange
var command = new UpdateUserCommand
var command = new UpdateCurrentUserCommand
{
Email = Faker.InvalidEmail(),
FirstName = Faker.Random.String(31),
Expand All @@ -42,7 +42,7 @@ public async Task UpdateUser_WhenInvalid_ShouldReturnBadRequest()
};

// Act
var response = await AuthenticatedHttpClient.PutAsJsonAsync("/api/account-management/users", command);
var response = await AuthenticatedHttpClient.PutAsJsonAsync("/api/account-management/users/me", command);

// Assert
var expectedErrors = new[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function FeatureSection4() {
/>
<FeatureBlock
title="Enterprise grade"
content="Multi tenancy to fit any organisation structure in B2B and B2C businesses including audit logs."
content="Multi tenancy to fit any organization structure in B2B and B2C businesses including audit logs."
/>
<FeatureBlock
title="Production ready"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ export default function UserProfileModal({ isOpen, onOpenChange, userId }: Reado
setIsSaving(false);

api
.get("/api/account-management/users/{id}", { params: { path: { id: userId } } })
.get("/api/account-management/users/me")
.then((response) => setData(response))
.catch((error) => setError(error))
.finally(() => setLoading(false));
}
}, [isOpen, userId]);
}, [isOpen]);

// Close dialog and cleanup
const closeDialog = useCallback(() => {
Expand All @@ -68,7 +68,7 @@ export default function UserProfileModal({ isOpen, onOpenChange, userId }: Reado

// Handle form submission
const [{ success, errors, title, message }, action, isPending] = useActionState(
api.actionPut("/api/account-management/users"),
api.actionPut("/api/account-management/users/me"),
{ success: null }
);

Expand All @@ -78,9 +78,9 @@ export default function UserProfileModal({ isOpen, onOpenChange, userId }: Reado

try {
if (selectedAvatarFile) {
await api.uploadFile("/api/account-management/users/update-avatar", selectedAvatarFile);
await api.uploadFile("/api/account-management/users/me/update-avatar", selectedAvatarFile);
} else if (removeAvatarFlag) {
await api.delete("/api/account-management/users/remove-avatar");
await api.delete("/api/account-management/users/me/remove-avatar");
setRemoveAvatarFlag(false);
}

Expand All @@ -97,7 +97,7 @@ export default function UserProfileModal({ isOpen, onOpenChange, userId }: Reado
// Add a small delay to ensure all requests have completed
setTimeout(async () => {
try {
const response = await api.get("/api/account-management/users/{id}", { params: { path: { id: userId } } });
const response = await api.get("/api/account-management/users/me");
updateUserInfo({
firstName: response.firstName,
lastName: response.lastName,
Expand All @@ -112,7 +112,7 @@ export default function UserProfileModal({ isOpen, onOpenChange, userId }: Reado
}
}, 100);
}
}, [success, closeDialog, userId, updateUserInfo, isSaving]);
}, [success, closeDialog, updateUserInfo, isSaving]);

// Handle file selection
const onFileSelect = (files: FileList | null) => {
Expand Down
Loading