Skip to content

Phase 3: Functional fixes — amount validation, BigDecimal money, checkUserExists - #49

Open
devin-ai-integration[bot] wants to merge 1 commit into
devin/1783889217-phase2-secrets-loggingfrom
devin/1783889324-phase3-functional-fixes
Open

devin-ai-integration[bot] wants to merge 1 commit into
devin/1783889217-phase2-secrets-loggingfrom
devin/1783889324-phase3-functional-fixes

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 12, 2026

Copy link
Copy Markdown

Summary

Phase 3 of the modernization plan: version-independent functional bug fixes. Stacked on the Phase 2 PR (#48) (which is itself on Phase 1). Backend suite grows 21 → 35 tests, all green.

1. checkUserExists email bug

-if (checkUsernameExists(username) || checkEmailExists(username)) {
+if (checkUsernameExists(username) || checkEmailExists(email)) {

Previously a signup with a new username but an already-registered email slipped through (it checked the username against the email index twice).

2. Money handling → BigDecimal

PrimaryTransaction.amount / SavingsTransaction.amount changed from double to BigDecimal, and AccountService.deposit/withdraw now take BigDecimal instead of double. This removes binary floating-point drift on currency and the Double.parseDouble(...) calls in the service/controller layers.

3. Amount validation + sufficient funds (no unhandled NumberFormatException)

New AmountUtil centralizes parsing/validation, plus an InsufficientFundsException:

BigDecimal parsePositiveAmount(String)   // throws IllegalArgumentException on null/non-numeric/<= 0
void requirePositive(BigDecimal)
void requireSufficientFunds(BigDecimal balance, BigDecimal amount) // throws InsufficientFundsException
  • AccountController deposit/withdraw parse via AmountUtil and, on bad input, redirect to …?error instead of throwing a raw NumberFormatException (500).
  • AccountServiceImpl / TransactionServiceImpl validate positivity and check funds before subtracting, for deposits, withdrawals, between-account transfers, and transfers to a recipient.

4. Consistent transfer category labels

Between-accounts transfers were inconsistent: Primary→Savings recorded type "Account" while Savings→Primary recorded "Transfer". Both now use "Account".

Tests (+14)

  • AmountUtilTest: valid/negative/zero/non-numeric/null parsing, over-draw vs exact-balance funds check.
  • AccountServiceIntegrationTest: non-positive deposit, zero withdraw, insufficient-funds withdraw.
  • TransactionServiceIntegrationTest: negative / non-numeric / insufficient-funds between-accounts transfer.
  • UserServiceIntegrationTest: checkUserExists detects duplicate email under a new username.

Verified: mvn clean test35 tests, 0 failures.

Note: TransferController's recipient/toSomeoneElse endpoints now surface IllegalArgumentException/InsufficientFundsException on bad input; user-facing error handling there is folded into the Phase 4 controller work (state-changing GET→POST/DELETE + ownership checks).

Link to Devin session: https://app.devin.ai/sessions/82be2e41fda54343a20a9b73e01c0631
Requested by: @gaelkekatos-jpg


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)

…er labels (Phase 3)

- checkUserExists now checks checkEmailExists(email) instead of the username
- Transaction amounts stored/handled as BigDecimal (PrimaryTransaction,
  SavingsTransaction) to avoid floating-point currency drift
- AccountService deposit/withdraw take BigDecimal; add AmountUtil helper that
  parses/validates positive amounts and checks sufficient funds
- Guard deposit/withdraw/transfer against non-positive, non-numeric, and
  insufficient-funds inputs (no unhandled NumberFormatException)
- Make between-accounts transfer category label consistent (Account) for both
  Primary->Savings and Savings->Primary
- Add tests for invalid/negative/zero amounts, insufficient funds, and the
  checkUserExists email fix

Co-Authored-By: Gael Kekatos <gael.kekatos@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants