Feature/referee match assignment#42
Feature/referee match assignment#42pol-rivero merged 14 commits intoUdL-EPS-SoftArch-Igualada:mainfrom
Conversation
…d TeamMemberTest.java
…on, locking and tests
|
Thank you for your PR @polMarsol! Now, you should wait for the automated code analysis by CodeRabbit, Copilot and SonarQube. Once you are confident that you have fixed all the detected issues and this PR is ready to be merged, add a comment with exactly one word: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request introduces a complete match referee assignment feature with validation. It adds a REST controller, service layer with six validation checks, custom exception handling, domain model extensions (Match state and referee association), and comprehensive unit tests covering success and all error scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pom.xml (1)
54-127:⚠️ Potential issue | 🟠 MajorDependency updates in
pom.xmlviolate repository policy.This patch modifies/adds multiple project dependencies in
pom.xml, which is currently disallowed by the repo rules. Please revert these dependency changes (or move them to a separately approved dependency-update PR).As per coding guidelines, "Disallow modifications to the project dependencies".
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (20)
pom.xmlsrc/main/java/cat/udl/eps/softarch/demo/api/dto/AssignRefereeRequest.javasrc/main/java/cat/udl/eps/softarch/demo/api/dto/AssignRefereeResponse.javasrc/main/java/cat/udl/eps/softarch/demo/api/dto/AssignmentErrorResponse.javasrc/main/java/cat/udl/eps/softarch/demo/controller/MatchAssignmentController.javasrc/main/java/cat/udl/eps/softarch/demo/domain/Match.javasrc/main/java/cat/udl/eps/softarch/demo/domain/MatchState.javasrc/main/java/cat/udl/eps/softarch/demo/domain/Referee.javasrc/main/java/cat/udl/eps/softarch/demo/domain/Team.javasrc/main/java/cat/udl/eps/softarch/demo/exception/AssignmentErrorCode.javasrc/main/java/cat/udl/eps/softarch/demo/exception/AssignmentValidationException.javasrc/main/java/cat/udl/eps/softarch/demo/exception/MatchAssignmentExceptionHandler.javasrc/main/java/cat/udl/eps/softarch/demo/repository/MatchRepository.javasrc/main/java/cat/udl/eps/softarch/demo/repository/RefereeRepository.javasrc/main/java/cat/udl/eps/softarch/demo/repository/VolunteerRepository.javasrc/main/java/cat/udl/eps/softarch/demo/service/MatchAssignmentService.javasrc/test/java/cat/udl/eps/softarch/demo/TeamMemberTest.javasrc/test/java/cat/udl/eps/softarch/demo/TeamTest.javasrc/test/java/cat/udl/eps/softarch/demo/controller/MatchAssignmentControllerTest.javasrc/test/java/cat/udl/eps/softarch/demo/service/MatchAssignmentServiceTest.java
src/main/java/cat/udl/eps/softarch/demo/repository/MatchRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/demo/repository/RefereeRepository.java
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/demo/service/MatchAssignmentService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This pull request implements the match referee assignment feature as specified in issue #33. The implementation adds functionality to assign referees to matches with comprehensive validation including role verification, availability conflict detection, and match state validation. However, the PR includes unrelated changes to Team functionality and violates critical project guidelines regarding testing practices and dependency management.
Changes:
- Implements match referee assignment API with comprehensive validation (match existence, referee existence, role validation, availability conflicts, match state, and existing referee checks)
- Adds domain entities (
Match,Referee,MatchState) and repositories with pessimistic locking for concurrency control - Includes unit and integration tests but lacks required Cucumber BDD tests
- Contains unrelated Team validation changes and tests written in Catalan
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
MatchAssignmentService.java |
Core service implementing referee assignment logic with validation chain |
MatchAssignmentController.java |
REST controller exposing POST /match-assignments endpoint with authentication |
MatchRepository.java |
Repository with pessimistic locking and overlap detection query |
VolunteerRepository.java |
Added findByIdForUpdate method with pessimistic locking |
RefereeRepository.java |
New repository for Referee entity management |
Match.java |
New domain entity for competition matches |
Referee.java |
New domain entity extending Volunteer |
MatchState.java |
Enum defining match states (SCHEDULED, FINISHED) |
AssignRefereeRequest.java |
Request DTO with validation |
AssignRefereeResponse.java |
Response DTO for successful assignments |
AssignmentErrorResponse.java |
Error response DTO |
AssignmentErrorCode.java |
Enum defining all assignment error codes |
AssignmentValidationException.java |
Custom exception for assignment validation failures |
MatchAssignmentExceptionHandler.java |
Controller advice for exception handling and HTTP status mapping |
MatchAssignmentServiceTest.java |
Unit tests covering all validation scenarios |
MatchAssignmentControllerTest.java |
Integration tests for API endpoints |
Team.java |
Adds @Size validation annotation (unrelated to feature) |
TeamTest.java |
Unit tests for Team functionality with Catalan comments (unrelated to feature) |
TeamMemberTest.java |
Unit tests for TeamMember with Catalan comments (unrelated to feature) |
pom.xml |
Formatting changes and adds junit-jupiter-api dependency (violates guidelines) |
src/main/java/cat/udl/eps/softarch/demo/service/MatchAssignmentService.java
Outdated
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/demo/exception/MatchAssignmentExceptionHandler.java
Outdated
Show resolved
Hide resolved
…g, hidden internals, and test/code cleanup
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/cat/udl/eps/softarch/demo/domain/Team.java (1)
71-74:⚠️ Potential issue | 🟡 Minor
@Sizeon lazy@OneToMany– performance andLazyInitializationExceptionrisk remains.When Bean Validation is triggered (e.g., via
@Validin a controller or a Hibernatepre-updatelistener), Hibernate must load the entirememberscollection to evaluate@Size. If that validation runs outside an active session (detached entity scenario), aLazyInitializationExceptionis thrown. Ensure all call-sites that trigger Bean Validation on aTeaminstance remain within transaction boundaries.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
src/main/java/cat/udl/eps/softarch/demo/domain/Match.javasrc/main/java/cat/udl/eps/softarch/demo/domain/Team.javasrc/main/java/cat/udl/eps/softarch/demo/repository/MatchRepository.javasrc/main/java/cat/udl/eps/softarch/demo/repository/RefereeRepository.javasrc/main/java/cat/udl/eps/softarch/demo/service/MatchAssignmentService.javasrc/test/java/cat/udl/eps/softarch/demo/TeamMemberTest.javasrc/test/java/cat/udl/eps/softarch/demo/TeamTest.javasrc/test/java/cat/udl/eps/softarch/demo/service/MatchAssignmentServiceTest.java
src/main/java/cat/udl/eps/softarch/demo/service/MatchAssignmentService.java
Outdated
Show resolved
Hide resolved
… save in assignment service
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/main/java/cat/udl/eps/softarch/demo/domain/Match.javasrc/main/java/cat/udl/eps/softarch/demo/service/MatchAssignmentService.javasrc/test/java/cat/udl/eps/softarch/demo/service/MatchAssignmentServiceTest.java
src/main/java/cat/udl/eps/softarch/demo/service/MatchAssignmentService.java
Outdated
Show resolved
Hide resolved
src/test/java/cat/udl/eps/softarch/demo/service/MatchAssignmentServiceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/cat/udl/eps/softarch/demo/service/MatchAssignmentServiceTest.java
Outdated
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/demo/repository/RefereeRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/demo/repository/MatchRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/demo/exception/MatchAssignmentExceptionHandler.java
Outdated
Show resolved
Hide resolved
src/test/java/cat/udl/eps/softarch/demo/service/MatchAssignmentServiceTest.java
Outdated
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/demo/repository/MatchRepository.java
Outdated
Show resolved
Hide resolved
…isibility/behavior
|
done |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/main/java/cat/udl/eps/softarch/demo/domain/Match.javasrc/main/java/cat/udl/eps/softarch/demo/service/MatchAssignmentService.javasrc/test/java/cat/udl/eps/softarch/demo/TeamTest.javasrc/test/java/cat/udl/eps/softarch/demo/service/MatchAssignmentServiceTest.javasrc/test/java/cat/udl/eps/softarch/demo/steps/MatchAssignmentStepDefs.javasrc/test/resources/features/MatchAssignment.feature
src/test/java/cat/udl/eps/softarch/demo/steps/MatchAssignmentStepDefs.java
Show resolved
Hide resolved
…ptions in match assignment scenarios
…on and conflict checks
5119a3c to
9fd115e
Compare
…ol/first-lego-league-backend into feature/referee-match-assignment
src/main/java/cat/udl/eps/softarch/fll/service/MatchAssignmentService.java
Outdated
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/fll/service/MatchAssignmentService.java
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/fll/service/MatchAssignmentService.java
Outdated
Show resolved
Hide resolved
src/main/java/cat/udl/eps/softarch/fll/exception/MatchAssignmentExceptionHandler.java
Outdated
Show resolved
Hide resolved
src/test/java/cat/udl/eps/softarch/fll/controller/MatchAssignmentControllerTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
src/main/java/cat/udl/eps/softarch/fll/controller/MatchAssignmentController.javasrc/main/java/cat/udl/eps/softarch/fll/controller/dto/MatchAssignmentRequest.javasrc/main/java/cat/udl/eps/softarch/fll/controller/dto/MatchAssignmentResponse.javasrc/main/java/cat/udl/eps/softarch/fll/domain/Match.javasrc/main/java/cat/udl/eps/softarch/fll/domain/MatchState.javasrc/main/java/cat/udl/eps/softarch/fll/exception/MatchAssignmentErrorCode.javasrc/main/java/cat/udl/eps/softarch/fll/exception/MatchAssignmentException.javasrc/main/java/cat/udl/eps/softarch/fll/exception/MatchAssignmentExceptionHandler.javasrc/main/java/cat/udl/eps/softarch/fll/repository/MatchRepository.javasrc/main/java/cat/udl/eps/softarch/fll/service/MatchAssignmentService.javasrc/test/java/cat/udl/eps/softarch/fll/controller/MatchAssignmentControllerTest.javasrc/test/java/cat/udl/eps/softarch/fll/service/MatchAssignmentServiceTest.java
src/main/java/cat/udl/eps/softarch/fll/service/MatchAssignmentService.java
Outdated
Show resolved
Hide resolved
src/test/java/cat/udl/eps/softarch/fll/controller/MatchAssignmentControllerTest.java
Show resolved
Hide resolved
src/test/java/cat/udl/eps/softarch/fll/service/MatchAssignmentServiceTest.java
Show resolved
Hide resolved
…ignment implementation
… cucumber coverage
src/main/java/cat/udl/eps/softarch/fll/repository/MatchRepository.java
Outdated
Show resolved
Hide resolved
… deprecations, cucumber coverage)
|
|
@pol-rivero @copilot code review[agent] I think it's done! |
|
LGTM 👍 By the way, next time remember that you can mark a PR as ready by creating a comment with just the word |



Closes #33