Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis pull request adds unit tests for BookingService and CarService, extends the Addon entity with an additional constructor, introduces a Debit payment method option, and applies minor formatting adjustments across configuration files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✏️ Tip: You can disable this entire section by setting 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: 3
🤖 Fix all issues with AI agents
In `@src/main/java/org/example/entity/Addon.java`:
- Around line 28-31: The constructor Addon(String name, String description,
double price) currently ignores the description parameter; add a private String
description field to the Addon entity, annotate it for persistence (e.g.,
`@Column`), set this.description = description in the constructor, and add the
corresponding getter/setter so the value is stored and retrievable; ensure any
JPA mappings/DTOs and tests that rely on Addon now include the new field.
In `@src/main/java/org/example/entity/PaymentMethod.java`:
- Line 4: Update the stale enum documentation comment in the Payment class (the
comment around line 26 in Payment.java) to list the current PaymentMethod values
"CARD, CASH, INVOICE" instead of the removed "CREDIT_CARD, DEBIT_CARD, CASH,
INVOICE"; also verify the entity uses `@Enumerated`(EnumType.STRING) and either
add a short TODO/javadoc note in the same file indicating that DB values must be
migrated if this is an existing project or state in the PR description that no
migration is needed for a new project so reviewers know persisted
"CREDIT_CARD"/"DEBIT_CARD" values must be handled.
In `@src/test/java/org/example/CarServiceTest.java`:
- Line 14: Tests fail because CarService constructs a real CarRepository (which
calls JpaUtil.getEntityManager()) and requires a DB; refactor CarService to
accept a CarRepository via constructor injection (add a constructor
CarService(CarRepository) and stop new-ing CarRepository inside CarService) and
update CarServiceTest to pass a mocked CarRepository (or use a mocking framework
to create a stub) so unit tests don’t hit JPA, alternatively convert
CarServiceTest into an integration test by annotating the test with
`@SpringBootTest` (or appropriate Spring/JPA test annotations), provide an
in-memory datasource, and move/rename the test accordingly; target symbols:
CarService, CarRepository, JpaUtil.getEntityManager(), CarServiceTest.
🧹 Nitpick comments (5)
faktura.txt (1)
1-3: Consider whether this generated file should be version-controlled.This appears to be generated invoice output with test data (customer "jejejej"). Committing generated output files can cause unnecessary merge conflicts and repository clutter. Consider adding this file to
.gitignoreif it's generated during runtime or testing.src/test/java/org/example/CarServiceTest.java (2)
1-1: Minor: Remove leading empty line.Suggested fix
- package org.example;
22-26: TheisInstanceOf(List.class)assertion is redundant.Since
getAllCars()returnsList<Car>, the compile-time type guarantees it's aList. This test doesn't verify any meaningful behavior. Consider testing actual business logic instead (e.g., expected data, filtering, or empty list scenarios).src/test/java/org/example/BookingServiceTest.java (2)
15-17: Consider using dependency injection for better test isolation.The direct instantiation of
BookingServiceworks forcalculatePrice()since it doesn't touchBookingRepositoryorEmailService. However, for testing other methods (e.g.,cancelBooking), you'll need mocks to avoid database/email side effects.
24-24: Consider adding a test for null addons handling.The service's
calculatePrice()will throwNullPointerExceptionifaddonsis null due toaddons.stream(). Consider either:
- Adding a test that documents this expected behavior, or
- Adding null-safety in the service and a corresponding test.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docker-compose.ymlfaktura.txtsql/seeds.sqlsrc/main/java/org/example/controller/PaymentController.javasrc/main/java/org/example/entity/Addon.javasrc/main/java/org/example/entity/PaymentMethod.javasrc/test/java/org/example/AppIT.javasrc/test/java/org/example/BookingServiceTest.javasrc/test/java/org/example/CarServiceTest.java
💤 Files with no reviewable changes (1)
- docker-compose.yml
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/org/example/BookingServiceTest.java (1)
src/main/java/org/example/service/BookingService.java (1)
BookingService(14-101)
src/test/java/org/example/CarServiceTest.java (1)
src/main/java/org/example/service/CarService.java (1)
CarService(7-14)
🔇 Additional comments (6)
sql/seeds.sql (1)
1-1: LGTM!Proper SQL comment formatting for the description line.
src/test/java/org/example/AppIT.java (1)
7-7: LGTM!Trivial formatting change - extra blank line added after imports. No functional impact.
src/test/java/org/example/BookingServiceTest.java (4)
19-30: LGTM!Test correctly validates hourly pricing calculation: 2 hours × 500 kr/hour = 1000 kr.
32-43: LGTM!Test correctly validates daily pricing calculation: 2 days × 2000 kr/day = 4000 kr.
45-59: LGTM!Test correctly validates addon pricing: (2 hours × 500 kr) + 100 kr addon = 1100 kr. Good coverage for the addon calculation path.
61-72: LGTM!Test correctly validates the minimum 1-hour charge rule when the booking duration is less than an hour.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.