Feature/34 finance service balance crud#97
Conversation
📝 WalkthroughWalkthroughIntroduces a complete transaction and balance CRUD implementation for the finance service. Adds JPA entities for members, directors, teams, trainers, and trainees with their repositories. Adds Finance Service CRUD Implementation
Sequence Diagram(s)sequenceDiagram
participant Client
participant FinanceController
participant TransactionService
participant DirectorRepository
participant TeamRepository
participant TransactionRepository
Client->>FinanceController: POST /finance/transactions (JWT)
FinanceController->>FinanceController: extractRequesterId(JWT subject → UUID)
FinanceController->>FinanceController: extractIsAdmin(ROLE_admin in authorities)
FinanceController->>TransactionService: createTransaction(body, requesterId, isAdmin)
alt not admin
TransactionService->>DirectorRepository: findSportNamesByMemberId(requesterId)
TransactionService->>TeamRepository: findTraineesBySportName(sport)
Note over TransactionService: ForbiddenException if not director/trainer of target member
end
TransactionService->>TransactionRepository: save(TransactionEntity)
TransactionService-->>FinanceController: Transaction
FinanceController-->>Client: 201 Created + Transaction JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
raphael-frank
left a comment
There was a problem hiding this comment.
I added a few test for more coverage of the services. Rest lgtm!
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java (1)
202-207: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrop the extra team lookup on trainer-scoped checks.
findTeamIdByMemberId()already gives you the team id, andfindTraineesByTeamId(teamId)naturally returns an empty list when the team is gone. ThefindById()calls here double the repository round-trips on every trainer authorization/balance path.Suggested refactor
for (UUID teamId : trainerRepository.findTeamIdByMemberId(requesterId)) { - teamRepository.findById(teamId).ifPresent(team -> - teamRepository.findTraineesByTeamId(team.getId()).stream() - .map(t -> t.getId().getMemberId()) - .forEach(ids::add)); + teamRepository.findTraineesByTeamId(teamId).stream() + .map(t -> t.getId().getMemberId()) + .forEach(ids::add); } @@ for (UUID teamId : trainerRepository.findTeamIdByMemberId(requesterId)) { - Optional<TeamEntity> team = teamRepository.findById(teamId); - if (team.isPresent()) { - boolean found = teamRepository.findTraineesByTeamId(team.get().getId()).stream() - .map(TraineeEntity::getId) - .map(TraineeEntity.Id::getMemberId) - .anyMatch(memberId::equals); - if (found) { - return true; - } - } + boolean found = teamRepository.findTraineesByTeamId(teamId).stream() + .map(TraineeEntity::getId) + .map(TraineeEntity.Id::getMemberId) + .anyMatch(memberId::equals); + if (found) { + return true; + } }Also applies to: 224-234
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java` around lines 202 - 207, The trainer-scoped member collection in TransactionService is doing an unnecessary extra teamRepository.findById() lookup even though trainerRepository.findTeamIdByMemberId(requesterId) already returns team IDs and teamRepository.findTraineesByTeamId(teamId) can safely return empty when missing. Refactor the loop in the authorization/balance path to iterate the team IDs directly and fetch trainees for each teamId without the intermediate presence check, and apply the same cleanup to the related logic noted in the other affected block.services/spring-finance/src/test/java/tum/devoops/financeservice/FinanceControllerTest.java (1)
71-123: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd 400/404 payload assertions for the API error contract.
These tests only verify status codes. The linked requirement also calls for
application/jsonresponses with{"error": ...}bodies on 400/404, and there are no not-found-path assertions here, so theGlobalExceptionHandlercontract can regress unnoticed.Also applies to: 127-178, 182-295
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/spring-finance/src/test/java/tum/devoops/financeservice/FinanceControllerTest.java` around lines 71 - 123, The FinanceControllerTest cases only assert HTTP status and miss the API error contract for JSON error bodies. Update the relevant tests around createTransaction and the other controller scenarios to also verify application/json responses with an error field for 400 and 404 cases, using FinanceControllerTest and the GlobalExceptionHandler contract as anchors. Add missing not-found-path coverage so regressions in the error payload shape are caught, not just the status codes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.java`:
- Around line 32-34: The JPQL in the trainee repository methods is selecting the
collection-valued association directly, which is invalid. Update both
findTraineesBySportName and findTraineesByTeamId to join TeamEntity.trainees
with an alias and select that joined TraineeEntity alias instead of using SELECT
t.trainees, so the queries parse and execute correctly for the
TransactionService flow.
In
`@services/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.java`:
- Around line 13-17: The TeamRepository queries are selecting the collection
property directly via TeamEntity aliases, which is invalid JPQL/HQL. Update
findTraineesBySportName and findTraineesByTeamId to use an explicit join from
TeamEntity to the trainees association and return the joined TraineeEntity alias
instead of SELECT t.trainees so Spring Data can validate the queries
successfully.
In
`@services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java`:
- Around line 42-46: The member UUID parsing in TransactionService.handle the
`transactionCreate.getMember()` value is only catching IllegalArgumentException,
so a missing member can still crash with a NullPointerException. Update the UUID
validation around `UUID.fromString(transactionCreate.getMember())` to explicitly
reject null/blank member values before parsing, and route both missing and
malformed input through the same BadRequestException path so the API returns the
expected 400 JSON error.
---
Nitpick comments:
In
`@services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java`:
- Around line 202-207: The trainer-scoped member collection in
TransactionService is doing an unnecessary extra teamRepository.findById()
lookup even though trainerRepository.findTeamIdByMemberId(requesterId) already
returns team IDs and teamRepository.findTraineesByTeamId(teamId) can safely
return empty when missing. Refactor the loop in the authorization/balance path
to iterate the team IDs directly and fetch trainees for each teamId without the
intermediate presence check, and apply the same cleanup to the related logic
noted in the other affected block.
In
`@services/spring-finance/src/test/java/tum/devoops/financeservice/FinanceControllerTest.java`:
- Around line 71-123: The FinanceControllerTest cases only assert HTTP status
and miss the API error contract for JSON error bodies. Update the relevant tests
around createTransaction and the other controller scenarios to also verify
application/json responses with an error field for 400 and 404 cases, using
FinanceControllerTest and the GlobalExceptionHandler contract as anchors. Add
missing not-found-path coverage so regressions in the error payload shape are
caught, not just the status codes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 31d0a1dd-3853-4792-907b-3db23c6b83a4
📒 Files selected for processing (24)
services/spring-finance/src/main/java/tum/devoops/financeservice/config/SecurityConfig.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/controller/FinanceController.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/controller/HelloController.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/converter/TransactionConverter.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/entity/DirectorEntity.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/entity/MemberEntity.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/entity/TraineeEntity.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/entity/TrainerEntity.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/entity/TransactionEntity.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/exception/BadRequestException.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/exception/ForbiddenException.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/exception/GlobalExceptionHandler.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/exception/NotFoundException.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/repository/DirectorRepository.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/repository/MemberRepository.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/repository/TraineeRepository.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/repository/TrainerRepository.javaservices/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.javaservices/spring-finance/src/test/java/tum/devoops/financeservice/FinanceControllerTest.javaservices/spring-finance/src/test/java/tum/devoops/financeservice/HelloControllerTest.javaservices/spring-finance/src/test/java/tum/devoops/financeservice/TransactionServiceApplicationTests.javaservices/spring-finance/src/test/java/tum/devoops/financeservice/TransactionServiceTest.java
| @OneToMany | ||
| @JoinColumn(name = "team_id", referencedColumnName = "id", insertable = false, updatable = false) | ||
| private List<TraineeEntity> trainees; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does JPQL allow selecting a collection-valued path expression like SELECT t.trainees FROM TeamEntity t, or must it use JOIN t.trainees trainee and select the joined alias?
💡 Result:
JPQL does not allow selecting a collection-valued path expression directly in the SELECT clause [1][2][3]. A query such as SELECT t.trainees FROM TeamEntity t is invalid because the SELECT clause must contain only single-valued expressions [2][3][4]. To achieve this, you must use a JOIN in the FROM clause to define an identification variable for the elements within the collection, and then select that alias [1][2][4]. The correct approach is: SELECT trainee FROM TeamEntity t JOIN t.trainees trainee Alternatively, using the IN syntax: SELECT trainee FROM TeamEntity t, IN (t.trainees) trainee In both cases, you are selecting the single-valued alias (trainee) that represents an individual element of the collection, which is permitted by the JPQL specification [2][4][5].
Citations:
- 1: https://www.objectdb.com/java/jpa/query/jpql/select
- 2: https://javaee.github.io/tutorial/persistence-querylanguage006.html
- 3: https://docs.oracle.com/html/E13946_03/ejb3_langref.html
- 4: https://jakartaee.github.io/jakartaee-documentation/jakartaee-tutorial/9.1/persist/persistence-querylanguage/persistence-querylanguage.html
- 5: https://docs.oracle.com/cd/E19879-01/819-3669/6n5sg7c79/index.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files 'services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.java' \
'services/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.java' \
'services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java'
printf '\n== TeamEntity outline ==\n'
ast-grep outline services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.java --view expanded
printf '\n== TeamRepository outline ==\n'
ast-grep outline services/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.java --view expanded
printf '\n== TransactionService outline (relevant symbols) ==\n'
ast-grep outline services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java --view expanded | sed -n '1,220p'Repository: AET-DevOps26/team-devoops
Length of output: 2036
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== TeamRepository query lines ==\n'
nl -ba services/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.java | sed -n '1,120p'
printf '\n== TransactionService relevant lines ==\n'
nl -ba services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java | sed -n '180,260p'
printf '\n== TeamEntity relevant lines ==\n'
nl -ba services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.java | sed -n '1,120p'Repository: AET-DevOps26/team-devoops
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== TeamRepository relevant lines ==\n'
awk 'NR>=1 && NR<=80 {printf "%4d: %s\n", NR, $0}' \
services/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.java
printf '\n== TransactionService relevant lines ==\n'
awk 'NR>=190 && NR<=245 {printf "%4d: %s\n", NR, $0}' \
services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java
printf '\n== TeamEntity relevant lines ==\n'
awk 'NR>=1 && NR<=80 {printf "%4d: %s\n", NR, $0}' \
services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.javaRepository: AET-DevOps26/team-devoops
Length of output: 4622
Use a JOIN in the trainee repository queries
SELECT t.trainees is not valid JPQL for a collection-valued association. Both findTraineesBySportName and findTraineesByTeamId should join t.trainees and select the joined alias, otherwise the authorization flow in TransactionService can fail when these queries are parsed or executed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.java`
around lines 32 - 34, The JPQL in the trainee repository methods is selecting
the collection-valued association directly, which is invalid. Update both
findTraineesBySportName and findTraineesByTeamId to join TeamEntity.trainees
with an alias and select that joined TraineeEntity alias instead of using SELECT
t.trainees, so the queries parse and execute correctly for the
TransactionService flow.
| @Query("SELECT t.trainees FROM TeamEntity t WHERE t.sportName = :sportName") | ||
| List<TraineeEntity> findTraineesBySportName(@Param("sportName") String sportName); | ||
|
|
||
| @Query("SELECT t.trainees FROM TeamEntity t WHERE t.id = :teamId") | ||
| List<TraineeEntity> findTraineesByTeamId(@Param("teamId") UUID teamId); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'services/spring-finance/src/main/java/**/TeamRepository.java' \
'services/spring-finance/src/main/java/**/TeamEntity.java' \
'services/spring-finance/src/main/java/**/TraineeEntity.java' \
'services/spring-finance/src/main/java/**/TransactionService.java'
printf '\n--- TeamRepository outline ---\n'
ast-grep outline services/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.java --view expanded || true
printf '\n--- TeamEntity outline ---\n'
ast-grep outline services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.java --view expanded || true
printf '\n--- TraineeEntity outline ---\n'
ast-grep outline services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TraineeEntity.java --view expanded || true
printf '\n--- Relevant repository lines ---\n'
sed -n '1,120p' services/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.java
printf '\n--- TeamEntity lines containing trainees ---\n'
rg -n -C 4 'trainees|sportName|id' services/spring-finance/src/main/java/tum/devoops/financeservice/entity/TeamEntity.java
printf '\n--- TransactionService references ---\n'
rg -n -C 3 'findTraineesBySportName|findTraineesByTeamId|TeamRepository' services/spring-finance/src/main/java/tum/devoops/financeserviceRepository: AET-DevOps26/team-devoops
Length of output: 10408
🌐 Web query:
JPQL select collection-valued path expression select t.trainees Hibernate HQL valid or invalid
💡 Result:
In both JPQL and HQL, a SELECT clause cannot directly specify a collection-valued path expression [1][2][3]. Therefore, SELECT t.trainees (where trainees is a collection) is invalid [1][2]. To select elements from a collection, you must define an identification variable for the collection in the FROM clause and then select that variable [1][2]. For example, to make the query valid, you would use a syntax similar to this: SELECT t FROM Teacher teacher, IN (teacher.trainees) t Alternatively, if you are using Hibernate (HQL), you can also use join syntax: SELECT t FROM Teacher teacher JOIN teacher.trainees t In summary: 1. Direct collection-valued path expressions in the SELECT clause are illegal because the clause must evaluate to single-valued expressions [1][2][3]. 2. The correct approach is to iterate over the collection using an IN expression or a JOIN in the FROM clause to expose individual elements as identification variables, which can then be selected [1][2].
Citations:
- 1: https://javaee.github.io/tutorial/persistence-querylanguage006.html
- 2: https://www.objectdb.com/java/jpa/query/jpql/select
- 3: https://docs.oracle.com/html/E13946_04/ejb3_langref.html
Use a join when selecting trainees Line 13 and Line 16 select the collection directly with SELECT t.trainees, which JPQL/HQL does not allow. Select the joined alias instead; otherwise this repository query can fail when Spring Data validates it and block the dependent authorization flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/spring-finance/src/main/java/tum/devoops/financeservice/repository/TeamRepository.java`
around lines 13 - 17, The TeamRepository queries are selecting the collection
property directly via TeamEntity aliases, which is invalid JPQL/HQL. Update
findTraineesBySportName and findTraineesByTeamId to use an explicit join from
TeamEntity to the trainees association and return the joined TraineeEntity alias
instead of SELECT t.trainees so Spring Data can validate the queries
successfully.
| UUID memberId; | ||
| try { | ||
| memberId = UUID.fromString(transactionCreate.getMember()); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new BadRequestException("Invalid memberId format."); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle a missing member field before parsing the UUID.
Line 44 can throw NullPointerException when transactionCreate.getMember() is absent. The catch only handles IllegalArgumentException, so this path returns a 500 instead of the required 400 JSON error.
Suggested fix
public Transaction createTransaction(TransactionCreate transactionCreate, UUID requesterId, boolean isAdmin) {
+ if (transactionCreate.getMember() == null) {
+ throw new BadRequestException("Member is required.");
+ }
+
UUID memberId;
try {
memberId = UUID.fromString(transactionCreate.getMember());
} catch (IllegalArgumentException e) {
throw new BadRequestException("Invalid memberId format.");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UUID memberId; | |
| try { | |
| memberId = UUID.fromString(transactionCreate.getMember()); | |
| } catch (IllegalArgumentException e) { | |
| throw new BadRequestException("Invalid memberId format."); | |
| if (transactionCreate.getMember() == null) { | |
| throw new BadRequestException("Member is required."); | |
| } | |
| UUID memberId; | |
| try { | |
| memberId = UUID.fromString(transactionCreate.getMember()); | |
| } catch (IllegalArgumentException e) { | |
| throw new BadRequestException("Invalid memberId format."); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/spring-finance/src/main/java/tum/devoops/financeservice/service/TransactionService.java`
around lines 42 - 46, The member UUID parsing in TransactionService.handle the
`transactionCreate.getMember()` value is only catching IllegalArgumentException,
so a missing member can still crash with a NullPointerException. Update the UUID
validation around `UUID.fromString(transactionCreate.getMember())` to explicitly
reject null/blank member values before parsing, and route both missing and
malformed input through the same BadRequestException path so the API returns the
expected 400 JSON error.
Resolve TransactionEntity conflict by keeping the nullable creator_id (this branch's ON DELETE SET NULL change in V3) over main's not-null column. Adapt main's finance implementation (#97) to this branch's regenerated Reference-based finance contract: Transaction.member/creator and Balance.member are now Reference{id,name} instead of String. Resolve member display names via the member schema (firstName/lastName added to the read-only finance MemberEntity), mirroring the feedback/event services. Creator references are null-safe to match the nullable creator_id. Finance tests updated to assert against the Reference shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implemented Transactions for the finance service.
Closes #34
Summary by CodeRabbit
New Features
Bug Fixes
Tests