Conversation
…or password encoding - Fixed grammatical errors in `@DisplayName` annotations in `UpdateUserServiceTest`, `ListUsersServiceTest`, and `CreateUserServiceTest`. - Added missing mock setup for `passwordEncoder.encode` in `UpdateUserServiceTest` for improved test reliability. - Updated Maven workflow by removing the `push` trigger to streamline CI execution. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
- Introduced `existsById` method in `UserRepositoryPort` and `UserPersistenceAdapter` for user existence validation. - Implemented user existence check in `FindWalletByUserIdService` to improve error handling. - Added unit tests for wallet services: `FindWalletByIdService`, `CreateWalletService`, and `FindWalletByUserIdService`. - Refactored `CreateWalletService` constructor to inject wallet limit configuration. - Enhanced error messaging for scenarios like wallet name duplication, user not found, and max wallet limit. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
…lletService` tests - Introduced wallet name duplication check in `UpdateWalletService` to enforce unique wallet names per user. - Added exception handling for database integrity violations during wallet updates. - Replaced `FindWalletByIdService` with `FindWalletByIdUseCase` for better modularity. - Enhanced test coverage for `UpdateWalletService` with scenarios including validation, exception handling, and edge cases. - Added tests for `DeleteWalletByIdService` and `GetBalanceByWalletAndUserService` to ensure consistent functionality. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
…ion editing - Enhanced `EditTransactionFlowHandler` to handle `IllegalArgumentException` and `BusinessRuleException` with user-friendly error messages. - Updated error messaging in multiple handlers, adding null checks and validation for temporary transaction IDs. - Standardized date format in `EditTransactionFieldCallbackHandler` to `DD/MM/YYYY`. - Refined transaction editing flows by introducing fallback cases for unexpected fields. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
- Introduced `ConfigCallbackHandler` for handling configuration menu interactions. - Added `createConfigKeyboard` to `MenuUtils` for generating configuration-related keyboards, including login and delete-user options. - Updated `BotAction` enum with new actions (`CONFIG`, `CONFIRM_DELETE_USER`) for improved flow management. - Refined main menu keyboard by separating configuration and web access actions. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
…b Actions - Modified `Run tests` step to save test output to `test-output.txt`. - Added a new `Add Test Summary` step to include test result summaries in GitHub Actions using `GITHUB_STEP_SUMMARY`. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
Removed duplicate step for adding test summary in workflow.
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive test coverage for wallet services and improves validation, error handling, and user experience across the application. The changes include new test files for all wallet service operations, enhancements to the Telegram bot interface with a new configuration menu, and various code quality improvements.
Changes:
- Added complete test coverage for wallet services (Create, Update, Delete, Find, GetBalance)
- Enhanced validation in wallet operations with duplicate name checking and user existence validation
- Introduced a new configuration menu in the Telegram bot separating settings from the main menu
- Refactored CreateWalletService to use constructor injection for the maximum wallet limit
- Minor fixes including typo corrections, improved error messages, and test reliability improvements
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| UpdateWalletServiceTest.java | New comprehensive test file covering update scenarios, authorization, and duplicate name validation |
| GetBalanceByWalletAndUserServiceTest.java | New test file for wallet balance retrieval functionality |
| FindWalletByUserIdTest.java | New test file covering paginated wallet listing with user validation |
| FindWalletByIdServiceTest.java | New test file for single wallet retrieval |
| DeleteWalletByIdServiceTest.java | New test file covering wallet deletion with authorization checks |
| CreateWalletServiceTest.java | New comprehensive test file for wallet creation scenarios |
| UpdateUserServiceTest.java | Fixed typo in test description and added missing password encoder mock |
| ListUsersServiceTest.java | Fixed grammatical error in test description |
| CreateUserServiceTest.java | Fixed typo in test description |
| UserPersistenceAdapter.java | Added existsById implementation to support user validation |
| MenuUtils.java | Updated main menu button text and added createConfigKeyboard method |
| EditTransactionFlowHandler.java | Enhanced error handling for invalid fields and business rule violations |
| SelectWalletCallbackHandler.java | Removed currency symbol from balance display for consistency |
| ConfigCallbackHandler.java | New handler for configuration menu displaying login and delete user options |
| EditTransactionFieldCallbackHandler.java | Fixed date format prompt from DD/MM/AAAA to DD/MM/YYYY |
| EditTransactionCallbackHandler.java | Replaced assertion with proper null check and exception |
| DeleteTransactionCallbackHandler.java | Improved error message clarity |
| BotAction.java | Added CONFIG, CONFIRM_DELETE_USER, and DELETE_USER actions |
| UpdateWalletService.java | Added duplicate name validation and database integrity violation handling |
| FindWalletByUserIdService.java | Added user existence validation before fetching wallets |
| CreateWalletService.java | Refactored to use constructor injection for maximum wallet limit |
| UserRepositoryPort.java | Added existsById method declaration |
| maven.yml | Enhanced CI workflow to capture and display test summaries |
Comments suppressed due to low confidence (1)
.github/workflows/maven.yml:44
- There are two steps with the same name "Add Test Summary" that perform similar but slightly different actions. The first step (lines 32-36) does not handle the case where grep fails, while the second step (lines 38-44) does. This duplication is unnecessary and can cause confusion. Consider consolidating these into a single step that includes proper error handling.
- name: Add Test Summary
if: always()
run: |
echo "## 🧪 Resultado dos Testes" >> $GITHUB_STEP_SUMMARY
echo "" >> $GITHUB_STEP_SUMMARY
grep "Tests run:" test-output.txt >> $GITHUB_STEP_SUMMARY || echo "Nenhum resumo encontrado." >> $GITHUB_STEP_SUMMARY
echo "" >> $GITHUB_STEP_SUMMARY
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| verify(findWalletByIdUseCase).execute(walletId); | ||
| verify(walletRepositoryPort).save(any(Wallet.class)); | ||
|
|
There was a problem hiding this comment.
The test is missing verifyNoMoreInteractions call at the end, which is inconsistent with the pattern used in other test methods in this file. For consistency and to ensure no unexpected interactions occur, add verifyNoMoreInteractions(findWalletByIdUseCase, walletRepositoryPort) after line 146.
| verifyNoMoreInteractions(findWalletByIdUseCase, walletRepositoryPort); |
|
|
||
| verify(walletRepositoryPort).save(any(Wallet.class)); |
There was a problem hiding this comment.
The test is missing verifyNoMoreInteractions call and does not verify all expected interactions. The test should verify that userRepositoryPort.findById, walletRepositoryPort.countByUserId, and walletRepositoryPort.existsByUserIdAndName were called, then add verifyNoMoreInteractions(userRepositoryPort, walletRepositoryPort) to ensure no unexpected interactions occurred.
| verify(walletRepositoryPort).save(any(Wallet.class)); | |
| verify(userRepositoryPort).findById(user.getId()); | |
| verify(walletRepositoryPort).countByUserId(user.getId()); | |
| verify(walletRepositoryPort).existsByUserIdAndName(user.getId(), wallet.getName()); | |
| verify(walletRepositoryPort).save(any(Wallet.class)); | |
| verifyNoMoreInteractions(userRepositoryPort, walletRepositoryPort); |
| private GetBalanceByWalletAndUserService getBalanceByWalletAndUserService; | ||
|
|
||
| @Test | ||
| @DisplayName("Deve mostrar o saldo da carteira do usuário do forma correta") |
There was a problem hiding this comment.
There is a grammatical error in the test description. "do forma" should be "de forma".
| @DisplayName("Deve mostrar o saldo da carteira do usuário do forma correta") | |
| @DisplayName("Deve mostrar o saldo da carteira do usuário de forma correta") |
| updateWalletService.execute(walletId, userId, updateRequest); | ||
|
|
||
| verify(walletRepositoryPort, never()).existsByUserIdAndName(anyString(), anyString()); | ||
| verify(walletRepositoryPort).save(existingWallet); |
There was a problem hiding this comment.
The test is missing verifyNoMoreInteractions call at the end, which is inconsistent with the pattern used in other test methods in this file. For consistency and to ensure no unexpected interactions occur, add verifyNoMoreInteractions(findWalletByIdUseCase, walletRepositoryPort) after line 131.
| verify(walletRepositoryPort).save(existingWallet); | |
| verify(walletRepositoryPort).save(existingWallet); | |
| verifyNoMoreInteractions(findWalletByIdUseCase, walletRepositoryPort); |
|
|
||
| - name: Publish test report | ||
| - name: Add Test Summary | ||
| if: always() |
There was a problem hiding this comment.
The test-output.txt file is created in the zaldo-api directory (due to working-directory being set), but the subsequent steps that reference it don't specify a working directory. This means they will look for test-output.txt in the root directory, not in zaldo-api/, which will cause the grep command to fail. Either add working-directory: zaldo-api to the test summary steps or adjust the path to zaldo-api/test-output.txt.
| if: always() | |
| if: always() | |
| working-directory: zaldo-api |
This pull request introduces several improvements and fixes across the API and Telegram bot integration, focusing on user experience, validation, and code clarity. The main highlights include enhanced validation for wallet and transaction operations, a new configuration menu in the Telegram bot, and improved error handling. Minor changes address formatting, typo corrections, and test reliability.
Validation and Error Handling Enhancements:
FindWalletByUserIdService, throwing aResourceNotFoundExceptionif the user does not exist before fetching wallets.UpdateWalletServiceto check for duplicate wallet names and handle database integrity violations with specific exceptions.Telegram Bot Improvements:
ConfigCallbackHandlerand related menu options, separating configuration actions (login, delete user) from the main menu for better UX. [1] [2] [3] [4]Code and Test Quality:
CreateWalletServiceto use constructor injection for maximum wallet limit, improving testability and clarity. [1] [2]CI Workflow: