Adds user profile and password update features#81
Merged
Conversation
Introduces PATCH endpoints for managing user basic information and changing user passwords. Enhances API security and request processing by reordering HTTP middleware for HTTPS redirection, authentication, and authorization. Updates application services and repositories to support these new functionalities, including a refined return type for user info updates. Includes comprehensive unit and integration tests, along with general dependency upgrades and a migration to the .slnx solution format.
jldsilva
added
.NET
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAtualiza várias dependências NuGet; altera a assinatura de Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
InvoiceReminder.UnitTests.API/Endpoints/UserPasswordEndpointsTests.cs (1)
215-288: Considere extrair helpers para reduzir duplicação nos testes.Há repetição de criação de
HttpRequestMessage, configuração deAuthorizatione mock deAuthorizeAsync; um helper local deixaria os testes mais enxutos e fáceis de manter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@InvoiceReminder.UnitTests.API/Endpoints/UserPasswordEndpointsTests.cs` around lines 215 - 288, Extract and reuse helpers to remove duplication across the ChangeUserPassword tests: add a helper like CreatePatchRequest(string basepath, string? bearer = null) to build and optionally set Authorization header, and add SetupAuthorizationSuccess() and SetupAuthorizationFailure() that configure _authorizationService.AuthorizeAsync(...) to return AuthorizationResult.Success()/Failed(); then update the tests (ChangeUserPassword_WhenUserIsAuthenticated_ShouldReturnOk, ChangeUserPassword_WhenUserIsNotAuthenticated_ShouldReturnUnauthorized, ChangeUserPassword_WhenUserIsAuthenticatedButServiceFails_ShouldReturnInternalServerError) to call CreatePatchRequest and the appropriate SetupAuthorization helper instead of repeating request construction and _authorizationService.AuthorizeAsync(...) stubs, and reuse a helper to set the JSON content (e.g., SetJsonContent(HttpRequestMessage, object)).InvoiceReminder.UnitTests.API/Endpoints/UserEndpointsTests.cs (1)
517-591: Testes de endpoint MapPatch bem estruturados.Os testes cobrem os cenários essenciais de autenticação e falha de serviço. Considere adicionar um teste para o cenário "User not Found" similar aos outros endpoints GET que possuem esse caso (ex:
GetUserById_WhenUserIsAuthenticatedButNotExists_ShouldReturnNotFound).💡 Sugestão de teste adicional para cobertura completa
[TestMethod] public async Task UpdateBasicUserInfo_WhenUserIsAuthenticatedButNotExists_ShouldReturnNotFound() { // Arrange var request = new HttpRequestMessage(HttpMethod.Patch, basepath); request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", "test_token"); var userViewModel = _userViewModelFaker.Generate(); var expectedResult = Result<UserViewModel>.Failure("User not Found!"); _ = _userAppService.UpdateBasicUserInfoAsync(Arg.Any<UserViewModel>(), Arg.Any<CancellationToken>()) .Returns(expectedResult); _ = _authorizationService.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Any<object>(), Arg.Any<IEnumerable<IAuthorizationRequirement>>()) .Returns(Task.FromResult(AuthorizationResult.Success())); // Act request.Content = JsonContent.Create(userViewModel); var response = await _client.SendAsync(request, TestContext.CancellationToken); // Assert _ = _userAppService.Received(1).UpdateBasicUserInfoAsync(Arg.Any<UserViewModel>(), Arg.Any<CancellationToken>()); response.StatusCode.ShouldBe(HttpStatusCode.NotFound); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@InvoiceReminder.UnitTests.API/Endpoints/UserEndpointsTests.cs` around lines 517 - 591, Add a new unit test named UpdateBasicUserInfo_WhenUserIsAuthenticatedButNotExists_ShouldReturnNotFound that mirrors the existing patch tests: arrange a HttpRequestMessage with Authorization header, generate a UserViewModel via _userViewModelFaker, have _userAppService.UpdateBasicUserInfoAsync return Result<UserViewModel>.Failure("User not Found!"), mock _authorizationService.AuthorizeAsync to return AuthorizationResult.Success(), send the request to the same basepath with JsonContent.Create(userViewModel), assert that _userAppService.Received(1).UpdateBasicUserInfoAsync(...) was called and that response.StatusCode is HttpStatusCode.NotFound; reuse existing helpers and naming consistent with other tests.InvoiceReminder.Application/AppServices/UserAppService.cs (1)
90-92: Considere retornar dados atualizados do banco de dados em vez do viewModel de entrada.O método retorna o
viewModelde entrada ao invés de buscar a entidade atualizada do repositório. Se o banco de dados modificar campos automaticamente (comoUpdatedAt), o cliente receberá dados desatualizados.♻️ Sugestão de refatoração para retornar dados atualizados
var entity = viewModel.Adapt<User>(); var result = await _repository.UpdateBasicUserInfoAsync(entity, cancellationToken); - return result - ? Result<UserViewModel>.Success(viewModel) - : Result<UserViewModel>.Failure("User not Found!"); + if (!result) + { + return Result<UserViewModel>.Failure("User not Found!"); + } + + var updatedEntity = await _repository.GetByIdAsync(entity.Id, cancellationToken); + return Result<UserViewModel>.Success(updatedEntity.Adapt<UserViewModel>());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@InvoiceReminder.Application/AppServices/UserAppService.cs` around lines 90 - 92, The current return uses the input viewModel, which can be stale; after a successful update (the bool result), load the updated entity from the repository (e.g., call the repository method used to fetch users such as GetById/GetAsync) and map that entity to a fresh UserViewModel before returning; update the success branch that currently returns viewModel to instead retrieve the updated User entity, map it to UserViewModel (using the existing mapper or factory), and return Result<UserViewModel>.Success(updatedViewModel) so fields like UpdatedAt reflect database values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@InvoiceReminder.API/Program.cs`:
- Around line 50-53: Move the global exception handler so it wraps
authentication/authorization: call app.UseExceptionHandler(...) before
app.UseAuthentication() and app.UseAuthorization() (ideally near the start of
the pipeline immediately after app.UseHttpsRedirection()/app.UseCors()),
ensuring any exceptions thrown in the Authentication/Authorization middleware
are captured by the global handler; update the ordering of
app.UseExceptionHandler(), app.UseAuthentication(), and app.UseAuthorization()
accordingly.
In
`@InvoiceReminder.IntegrationTests/Data/Repository/UserPasswordRepositoryIntegrationTests.cs`:
- Around line 263-264: The test compares updatedUserPassword.CreatedAt to
originalCreatedAt with an overly strict <1ms tolerance (variable
timeDifference); relax this to a reasonable explicit margin to avoid flakiness
(e.g., 20–100ms) by changing the assertion to check timeDifference against that
larger value or using a time-tolerance assertion helper for CreatedAt vs
originalCreatedAt in UserPasswordRepositoryIntegrationTests.cs.
---
Nitpick comments:
In `@InvoiceReminder.Application/AppServices/UserAppService.cs`:
- Around line 90-92: The current return uses the input viewModel, which can be
stale; after a successful update (the bool result), load the updated entity from
the repository (e.g., call the repository method used to fetch users such as
GetById/GetAsync) and map that entity to a fresh UserViewModel before returning;
update the success branch that currently returns viewModel to instead retrieve
the updated User entity, map it to UserViewModel (using the existing mapper or
factory), and return Result<UserViewModel>.Success(updatedViewModel) so fields
like UpdatedAt reflect database values.
In `@InvoiceReminder.UnitTests.API/Endpoints/UserEndpointsTests.cs`:
- Around line 517-591: Add a new unit test named
UpdateBasicUserInfo_WhenUserIsAuthenticatedButNotExists_ShouldReturnNotFound
that mirrors the existing patch tests: arrange a HttpRequestMessage with
Authorization header, generate a UserViewModel via _userViewModelFaker, have
_userAppService.UpdateBasicUserInfoAsync return
Result<UserViewModel>.Failure("User not Found!"), mock
_authorizationService.AuthorizeAsync to return AuthorizationResult.Success(),
send the request to the same basepath with JsonContent.Create(userViewModel),
assert that _userAppService.Received(1).UpdateBasicUserInfoAsync(...) was called
and that response.StatusCode is HttpStatusCode.NotFound; reuse existing helpers
and naming consistent with other tests.
In `@InvoiceReminder.UnitTests.API/Endpoints/UserPasswordEndpointsTests.cs`:
- Around line 215-288: Extract and reuse helpers to remove duplication across
the ChangeUserPassword tests: add a helper like CreatePatchRequest(string
basepath, string? bearer = null) to build and optionally set Authorization
header, and add SetupAuthorizationSuccess() and SetupAuthorizationFailure() that
configure _authorizationService.AuthorizeAsync(...) to return
AuthorizationResult.Success()/Failed(); then update the tests
(ChangeUserPassword_WhenUserIsAuthenticated_ShouldReturnOk,
ChangeUserPassword_WhenUserIsNotAuthenticated_ShouldReturnUnauthorized,
ChangeUserPassword_WhenUserIsAuthenticatedButServiceFails_ShouldReturnInternalServerError)
to call CreatePatchRequest and the appropriate SetupAuthorization helper instead
of repeating request construction and _authorizationService.AuthorizeAsync(...)
stubs, and reuse a helper to set the JSON content (e.g.,
SetJsonContent(HttpRequestMessage, object)).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
Directory.Packages.propsInvoiceReminder.API/Program.csInvoiceReminder.Application/AppServices/UserAppService.csInvoiceReminder.Application/Interfaces/IUserAppService.csInvoiceReminder.ExternalServices/Gmail/GmailServiceWrapper.csInvoiceReminder.ExternalServices/Gmail/GoogleOAuthService.csInvoiceReminder.IntegrationTests/Data/Repository/UserPasswordRepositoryIntegrationTests.csInvoiceReminder.IntegrationTests/Data/Repository/UserRepositoryIntegrationTests.csInvoiceReminder.UnitTests.API/Endpoints/UserEndpointsTests.csInvoiceReminder.UnitTests.API/Endpoints/UserPasswordEndpointsTests.csInvoiceReminder.UnitTests.Application/AppServices/UserAppServiceTests.csInvoiceReminder.UnitTests.Application/AppServices/UserPasswordAppServiceTests.csInvoiceReminder.slnInvoiceReminder.slnx
💤 Files with no reviewable changes (1)
- InvoiceReminder.sln
Adjusts the order of API exception and status code middleware for more robust error handling, ensuring these catch errors from all subsequent middleware. Relaxes an integration test assertion for timestamp differences to improve reliability and reduce test flakiness.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@InvoiceReminder.IntegrationTests/Data/Repository/UserPasswordRepositoryIntegrationTests.cs`:
- Around line 193-215: O teste está lendo a mesma entidade rastreada pelo
CoreDbContext (via CreateAndSaveUserPasswordAsync) e por isso a chamada a
_repository.GetByUserIdAsync pode retornar o objeto em memória em vez do valor
persistido, causando falso positivo; após chamar
_repository.ChangePasswordAsync(userPassword, ...) carregue o UserPassword a
partir de um novo contexto/repositório (ou desanexe a entidade rastreada) antes
de verificar valores — por exemplo recrie a instância do repositório ou do
DbContext e então chamar _repository.GetByUserIdAsync(userPassword.UserId, ...)
para validar PasswordHash, PasswordSalt e UpdatedAt (funções relevantes:
CreateAndSaveUserPasswordAsync, _repository.ChangePasswordAsync,
_repository.GetByUserIdAsync).
- Around line 269-293: Atualize o teste
ChangePasswordAsync_Should_Update_Multiple_Passwords_Independently para também
verificar que PasswordSalt é atualizado somente para o usuário alterado: após
chamar _repository.ChangePasswordAsync(...) e buscar updated1/updated2 via
_repository.GetByUserIdAsync(...), adicione asserções equivalentes às de
PasswordHash garantindo updated1.PasswordSalt foi alterado em relação ao
original (original salt armazenado antes da alteração) e que
updated2.PasswordSalt permaneceu igual ao original do segundo usuário; mantenha
as outras verificações (result.ShouldBeTrue, PasswordHash asserções) intactas.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
InvoiceReminder.API/Program.csInvoiceReminder.IntegrationTests/Data/Repository/UserPasswordRepositoryIntegrationTests.cs
Adds `ChangeTracker.Clear()` to integration tests to ensure data is retrieved fresh from the database after updates. Introduces assertions for `PasswordSalt` to verify that `UpdateAsync` correctly modifies both password hash and salt, improving test correctness and coverage.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@InvoiceReminder.IntegrationTests/Data/Repository/UserPasswordRepositoryIntegrationTests.cs`:
- Around line 207-214: Adicione uma asserção explícita de não-nulo antes de
acessar propriedades de updatedUserPassword: após chamar
UserPasswordRepositoryIntegrationTests->_repository.GetByUserIdAsync(userPassword.UserId,
TestContext.CancellationToken) chame updatedUserPassword.ShouldNotBeNull() (ou
equivalente) antes do bloco ShouldSatisfyAllConditions para garantir que a
variável não seja nula antes de verificar PasswordHash, PasswordSalt e
UpdatedAt.
- Around line 240-267: The test
ChangePasswordAsync_Should_Update_Only_PasswordHash_And_PasswordSalt currently
asserts Id, UserId and CreatedAt but never asserts the actual PasswordHash and
PasswordSalt changed; update the assertions after retrieving updatedUserPassword
(from GetByUserIdAsync) to explicitly assert updatedUserPassword.PasswordHash
equals the new userPassword.PasswordHash and updatedUserPassword.PasswordSalt
equals the new userPassword.PasswordSalt, while keeping the existing checks for
Id, UserId and the CreatedAt time difference to ensure only those fields were
changed.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
InvoiceReminder.IntegrationTests/Data/Repository/UserPasswordRepositoryIntegrationTests.cs
Adds assertions to verify `PasswordHash`, `PasswordSalt`, and `UpdatedAt` fields are correctly modified during a user password update. Includes null checks for retrieved entities, enhancing test robustness and preventing potential null reference issues in subsequent assertions.
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces PATCH endpoints for managing user basic information and changing user passwords. Enhances API security and request processing by reordering HTTP middleware for HTTPS redirection, authentication, and authorization.
Updates application services and repositories to support these new functionalities, including a refined return type for user info updates. Includes comprehensive unit and integration tests, along with general dependency upgrades and a migration to the .slnx solution format.
Summary by CodeRabbit
Notas de Lançamento