Conversation
- Improved logic in `UpdateTransactionService` to handle previous balances and prevent negative results after updates. - Adjusted error messaging in `DeleteTransactionCallbackHandler` for clarity. - Updated `DeleteTransactionByIdService` to refine transaction deletion permission checks. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
…figurations - Implemented build cache in Dockerfile for Maven dependencies and package phases. - Enhanced `ENTRYPOINT` in Dockerfile with memory optimization (`-XX:MaxRAMPercentage=75.0`). - Updated `.dockerignore` to exclude additional unnecessary files and folders (e.g., `.github`, `.env`). - Improved GitHub Actions workflows with test report publishing and Docker caching for faster builds. - Removed exposed ports from `compose.yml` for Postgres and Redis while streamlining health checks. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
…tency - Introduced `EditTransactionFlowHandler` and `EditTransactionCallbackHandler` for transaction editing in Telegram bot. - Added support for dynamic field editing (amount, description, date, type) with validation. - Updated date formatting to `DD/MM/YYYY` for better locale alignment across handlers. - Adjusted `CreateTransactionFlowHandler` and `SelectTransactionCallbackHandler` to integrate updated date format. - Enhanced `MenuUtils` with centralized date format utility and improved transaction formatting. - Expanded `FlowContext` to manage transaction editing state and active fields. - Refined `ChatState` and `BotAction` enums to support transaction editing workflows. Signed-off-by: Willian Silva <williansilva@alu.ufc.br>
There was a problem hiding this comment.
Pull request overview
This pull request adds transaction editing functionality to the Telegram bot, improves transaction validation logic when updating transactions, standardizes date formatting, and optimizes Docker build configuration. The implementation allows users to edit individual transaction fields (amount, description, date, and type) through a conversational flow in the Telegram bot.
Changes:
- Added Telegram bot handlers for editing transactions with field-specific prompts and validation
- Fixed transaction update validation to properly recalculate wallet balance by reversing the old transaction before applying the new one
- Standardized date format to DD/MM/YYYY throughout the Telegram bot interface
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| EditTransactionCallbackHandler.java | New handler to display transaction edit menu with field selection options |
| EditTransactionFieldCallbackHandler.java | New handler to prompt user for specific field value based on selected field |
| EditTransactionFlowHandler.java | New flow handler to process user input and update transaction field |
| UpdateTransactionService.java | Fixed balance validation logic to properly handle transaction updates by reversing old transaction impact |
| DeleteTransactionByIdService.java | Fixed error message to refer to "Transação" instead of "carteira" |
| BotAction.java | Added EDIT_TRANSACTION_FIELD action for field-specific editing |
| ChatState.java | Added WAITING_EDIT_TX_VALUE state for edit transaction flow |
| FlowContext.java | Added editingField property to track which field is being edited |
| MenuUtils.java | Added getFormat() method and dateTimeFormatter for consistent date formatting |
| CreateTransactionFlowHandler.java | Updated date format in prompts and parsing to use DD/MM/YYYY |
| SelectTransactionCallbackHandler.java | Updated date display to use consistent format |
| DeleteTransactionCallbackHandler.java | Improved error message and navigation when transaction/wallet cannot be identified |
| TransactionController.java | Reformatted annotation for better readability (style only) |
| Dockerfile | Added Maven cache mounting and MaxRAMPercentage JVM flag |
| .dockerignore | Expanded to exclude more files for faster builds |
| compose.yml | Removed port mappings for postgres and redis, reformatted healthcheck |
| maven.yml | Split build and test steps, added test report artifact upload |
| docker-image.yml | Added GitHub Actions cache for Docker builds |
Comments suppressed due to low confidence (1)
zaldo-api/src/main/java/br/com/github/williiansilva51/zaldo/application/service/transaction/UpdateTransactionService.java:54
- The UpdateTransactionService does not verify that the user attempting to update the transaction is authorized to do so. Unlike DeleteTransactionByIdService which checks wallet ownership, this service allows any user to update any transaction. Consider adding authorization validation similar to the delete operation to ensure users can only update transactions in their own wallets.
public Transaction execute(Long id, Transaction transaction) {
Transaction existingTransaction = transactionRepositoryPort
.findById(id)
.orElseThrow(() -> new ResourceNotFoundException("Transação não encontrada para atualização: " + id));
BigDecimal oldAmount = existingTransaction.getAmount();
boolean oldIsIncome = existingTransaction.isIncome();
existingTransaction.update(transaction);
if (!existingTransaction.isPositive()) {
throw new IllegalArgumentException("A transação não pode ter valor negativo ou zero após atualização.");
}
Long walletId = existingTransaction.getWallet().getId();
String userId = existingTransaction.getWallet().getUser().getId();
BigDecimal currentBalance = getBalanceByWalletAndUserUseCase.execute(walletId, userId);
BigDecimal balanceWithoutOld = oldIsIncome ?
currentBalance.subtract(oldAmount) :
currentBalance.add(oldAmount);
BigDecimal finalBalance = existingTransaction.isIncome()
? balanceWithoutOld.add(existingTransaction.getAmount())
: balanceWithoutOld.subtract(existingTransaction.getAmount());
if (finalBalance.compareTo(BigDecimal.ZERO) < 0) {
throw new BusinessRuleException("Operação cancelada: o saldo final seria negativo (" + finalBalance + ")");
}
return transactionRepositoryPort.save(existingTransaction);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (Exception e) { | ||
| return SendMessage.builder() | ||
| .chatId(chatId) | ||
| .text("❌ Valor inválido. Verifique o formato e tente novamente.\nErro: " + e.getMessage()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
The catch block catches all exceptions and displays the exception message directly to the user. This could expose sensitive internal error details or stack traces. Consider catching specific exceptions (NumberFormatException, DateTimeParseException, BusinessRuleException) separately and providing user-friendly error messages for each case, while logging the full exception details for debugging.
| .chatId(chatId) | ||
| .messageId(messageId) | ||
| .text("⚠\uFE0F Erro: Não foi possível identificar a carteira. Tente listar novamente.") | ||
| .text("⚠\uFE0F Erro: Não foi possível identificar a carteira. Tente listar novamente ou a transação.") |
There was a problem hiding this comment.
The error message says "Não foi possível identificar a carteira" (could not identify the wallet) but the condition checks for both transactionId and walletId being null. When transactionId is null, the message should mention the transaction instead. Consider revising the message to be more accurate, such as "Não foi possível identificar a transação ou a carteira."
| .text("⚠\uFE0F Erro: Não foi possível identificar a carteira. Tente listar novamente ou a transação.") | |
| .text("⚠\uFE0F Erro: Não foi possível identificar a transação ou a carteira. Tente listar novamente.") |
| if (text.equalsIgnoreCase("receita")) transaction.setType(TransactionType.INCOME); | ||
| else if (text.equalsIgnoreCase("despesa")) transaction.setType(TransactionType.EXPENSE); | ||
| else throw new IllegalArgumentException("Tipo inválido"); | ||
| } |
There was a problem hiding this comment.
The switch statement doesn't have a default case that throws an exception. When an unexpected fieldBeingEdited value is passed, the method returns a Transaction object with all fields set to null, which could lead to subtle bugs. Consider adding a default case that throws an IllegalArgumentException for unknown field types.
| } | |
| } | |
| default -> throw new IllegalArgumentException("Campo sendo editado desconhecido: " + fieldBeingEdited); |
| String promptMessage = switch (fieldToEdit) { | ||
| case "AMOUNT" -> "💰 Digite o <b>NOVO VALOR</b> (Ex: 50.50):"; | ||
| case "DESCRIPTION" -> "📝 Digite a <b>NOVA DESCRIÇÃO</b>:"; | ||
| case "DATE" -> "📅 Digite a <b>NOVA DATA</b> (DD/MM/AAAA):"; |
There was a problem hiding this comment.
The date format shown in the prompt message is "DD/MM/AAAA" (with AAAA meaning year in Portuguese), but it should be "DD/MM/YYYY" to be consistent with the format used throughout the application (line 119 in CreateTransactionFlowHandler and line 137 in the same file). While AAAA is acceptable in Portuguese, consistency is important for user experience.
| case "DATE" -> "📅 Digite a <b>NOVA DATA</b> (DD/MM/AAAA):"; | |
| case "DATE" -> "📅 Digite a <b>NOVA DATA</b> (DD/MM/YYYY):"; |
| FlowContext context = sessionManager.get(chatId); | ||
|
|
||
| Long currentTransactionId = context.getTempTransactionId(); | ||
| assert currentTransactionId != null; |
There was a problem hiding this comment.
Using assert for production code validation is problematic because assertions can be disabled at runtime with the -da JVM flag. This means the null check may not execute in production, potentially causing a NullPointerException. Replace the assert statement with a proper null check that throws an appropriate exception or returns an error message to the user.
| assert currentTransactionId != null; | |
| if (currentTransactionId == null) { | |
| throw new IllegalStateException("Temporary transaction id must not be null when editing a transaction"); | |
| } |
This pull request introduces improvements to the transaction editing flow in the Telegram bot, enhances Docker build efficiency, and refines input handling and validation. The main changes include the addition of handlers for editing transaction fields, improved validation logic for transaction updates, and optimizations to Docker caching and build configuration.
Telegram Bot Transaction Editing Enhancements:
EditTransactionCallbackHandlerandEditTransactionFieldCallbackHandlerto support editing individual transaction fields via Telegram bot, including dynamic prompts for each field. [1] [2]BotActionandChatStateenums to support new editing actions and states for transaction editing flows. [1] [2]Transaction Validation Improvements:
UpdateTransactionServiceto properly recalculate wallet balance when editing transactions, preventing negative balances and ensuring correct validation.DeleteTransactionByIdServicefor unauthorized transaction deletion to refer to "Transação" instead of "carteira".Telegram Bot UI and Input Handling:
DD/MM/YYYYin transaction creation and editing flows, including improved parsing and error messages. [1] [2]SelectTransactionCallbackHandler. [1] [2]DeleteTransactionCallbackHandlerfor cases where wallet or transaction cannot be identified.Docker Build and Caching Optimization:
.github/workflows/docker-image.ymland optimized Maven dependency caching inzaldo-api/Dockerfilefor faster builds. [1] [2].dockerignorefor better build context exclusion. [1] [2]Compose and Infrastructure Adjustments:
compose.ymlby removing explicit port mappings forpostgresandredis, and standardized healthcheck formatting. [1] [2]These changes collectively improve the user experience for editing transactions in the Telegram bot, ensure robust validation, and optimize build and deployment workflows.