feat: implement item descriptions and VAT handling for invoices#65
feat: implement item descriptions and VAT handling for invoices#65
Conversation
…als calculation, and adjust related DTOs and CLI inputs.
…ount` in `InvoiceDTO`, and adjust related mappings. Closes #52
…in `CliApp` update methods.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds VAT rate and VAT amount tracking to invoices and a required item name on invoice items; updates DTOs, entity fields, service logic, CLI menus, tests, and the application entry point accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User as CLI User
participant CLI as InvoiceMenu / InvoiceItemMenu
participant Service as InvoiceService
participant Repo as InvoiceRepository
participant DB as Database
User->>CLI: provide invoice data (number, dueDate, optional vatRate, items with name/qty/price)
CLI->>Service: CreateInvoiceDTO(vatRate, items...)
Service->>Service: validate vatRate (0.0 < vatRate <= 1.0)
Service->>Repo: save Invoice entity (with vatRate, items)
Repo->>DB: persist invoice + items (name stored)
Repo-->>Service: persisted Invoice (id, stored fields)
Service->>Service: recalcTotals (subTotal, vatAmount = vatRate * subTotal, total)
Service-->>CLI: return InvoiceDTO (vatRate, vatAmount, items)
CLI-->>User: display created invoice (including VAT and item names)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/java/org/example/service/InvoiceService.java (1)
65-71: Add null validation for item name before persisting.Line 67 sets
item.setName(itemDTO.name())without validation, butInvoiceItem.nameis marked@Column(nullable = false). IfitemDTO.name()returns null, this will cause a database constraint violation at persist time.🐛 Proposed fix to validate item name
dto.items().forEach(itemDTO -> { + if (itemDTO.name() == null || itemDTO.name().isBlank()) { + throw new IllegalArgumentException("Item name is required and cannot be blank"); + } InvoiceItem item = new InvoiceItem(); item.setName(itemDTO.name()); item.setQuantity(itemDTO.quantity());src/main/java/org/example/entity/invoice/Invoice.java (2)
54-54: Fix the Lombok builder initialization.The pipeline warning indicates that
@Builderignores the= new ArrayList<>()initializing expression. This could result ininvoiceItemsbeing null when using the builder.🔧 Proposed fix
- @OneToMany(mappedBy = "invoice", cascade = CascadeType.ALL, orphanRemoval = true) - private List<InvoiceItem> invoiceItems = new ArrayList<>(); + @OneToMany(mappedBy = "invoice", cascade = CascadeType.ALL, orphanRemoval = true) + @Builder.Default + private List<InvoiceItem> invoiceItems = new ArrayList<>();
92-99: Add null/empty validation for item name.Line 95 sets the item name from
itemDTO.name()without validation. SinceInvoiceItem.namehas anullable = falseconstraint, passing a null or empty name will cause a persistence exception.🛡️ Proposed fix with validation
if (dto.items() != null) { dto.items().forEach(itemDTO -> { + if (itemDTO.name() == null || itemDTO.name().isBlank()) { + throw new ValidationException("Item name cannot be null or empty"); + } InvoiceItem item = new InvoiceItem(); item.setName(itemDTO.name()); item.setQuantity(itemDTO.quantity()); item.setUnitPrice(itemDTO.unitPrice()); invoice.addItem(item); }); }src/main/java/org/example/CliApp.java (1)
822-826: Critical: Item name lost during update.Line 824 creates an updated
InvoiceItemDTObut omits the.name()field in the builder. This causes item names to be lost when updating quantity or price, resulting in null name values that violate theInvoiceItementity'snullable = falseconstraint.🐛 Proposed fix to preserve item name
List<InvoiceItemDTO> updated = items.stream() .map(i -> i.id().equals(item.id()) - ? InvoiceItemDTO.builder().id(i.id()).quantity(quantity).unitPrice(unitPrice).build() + ? InvoiceItemDTO.builder() + .id(i.id()) + .name(i.name()) + .quantity(quantity) + .unitPrice(unitPrice) + .build() : i) .toList();
🤖 Fix all issues with AI agents
In @src/main/java/org/example/CliApp.java:
- Around line 687-688: The prompt for the item name currently assigns String
name = scanner.nextLine().trim() without validation, which allows empty/blank
values that will violate the InvoiceItem nullable=false constraint; update the
code around the scanner input (the variable name and the scanner instance) to
validate that name is not empty after trimming (e.g., loop prompting until a
non-blank value is entered), provide a user-facing error message when input is
invalid, and only proceed to construct or persist the InvoiceItem once the
validated non-empty name is obtained.
In @src/main/java/org/example/entity/invoice/InvoiceItemDTO.java:
- Around line 9-14: The DTO's name component lacks a validation annotation
causing mismatch with the InvoiceItem entity; update the InvoiceItemDTO record
declaration to annotate the name parameter with a validation constraint (e.g.,
add @NotBlank or @NotNull to the name component in public record
InvoiceItemDTO(...)) and add the corresponding import
(jakarta.validation.constraints.NotBlank or NotNull) so the DTO enforces
non-empty names consistent with the entity.
🧹 Nitpick comments (2)
src/main/java/org/example/entity/invoice/InvoiceItem.java (1)
27-28: Consider adding bean validation for defense in depth.The
nullable = falseconstraint enforces the requirement at the database level, but adding@NotBlankwould provide earlier validation and clearer error messages.♻️ Proposed enhancement with bean validation
+ import jakarta.validation.constraints.NotBlank; + + @NotBlank(message = "Item name is required") @Column(name = "name", nullable = false) private String name;src/main/java/org/example/CliApp.java (1)
646-647: Add validation for VAT amount input.The VAT amount input is parsed directly without validation. While the outer try-catch handles
NumberFormatException, there's no validation for negative values or other invalid inputs.Consider adding input validation to ensure the VAT amount is a valid non-negative decimal.
📝 Suggested improvement
System.out.print("Enter VAT amount (e.g. 250.00): "); - BigDecimal vatAmount = new BigDecimal(scanner.nextLine().trim()); + String vatInput = scanner.nextLine().trim(); + BigDecimal vatAmount = new BigDecimal(vatInput); + if (vatAmount.compareTo(BigDecimal.ZERO) < 0) { + System.out.println("✗ VAT amount cannot be negative."); + return; + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/org/example/CliApp.javasrc/main/java/org/example/entity/invoice/CreateInvoiceDTO.javasrc/main/java/org/example/entity/invoice/Invoice.javasrc/main/java/org/example/entity/invoice/InvoiceDTO.javasrc/main/java/org/example/entity/invoice/InvoiceItem.javasrc/main/java/org/example/entity/invoice/InvoiceItemDTO.javasrc/main/java/org/example/entity/invoice/UpdateInvoiceDTO.javasrc/main/java/org/example/service/InvoiceService.java
🧰 Additional context used
🪛 GitHub Actions: Java Maven Build
src/main/java/org/example/entity/invoice/Invoice.java
[warning] 54-54: @builder will ignore the initializing expression entirely. If you want the initializing expression to serve as default, add @Builder.Default.
🔇 Additional comments (10)
src/main/java/org/example/entity/invoice/UpdateInvoiceDTO.java (1)
8-14: LGTM! VAT field added correctly for partial updates.The
vatAmountfield is properly positioned and typed asBigDecimal, allowing null values to support the partial update pattern described in the PR objectives.src/main/java/org/example/service/InvoiceService.java (1)
60-60: LGTM! VAT amount update follows partial update pattern.The null check ensures
vatAmountis only updated when explicitly provided in the DTO.src/main/java/org/example/entity/invoice/InvoiceDTO.java (1)
11-22: LGTM! VAT amount properly integrated into DTO.The
vatAmountfield is correctly positioned and mapped from the entity in thefromEntitymethod.src/main/java/org/example/entity/invoice/CreateInvoiceDTO.java (1)
8-15: LGTM! VAT amount added to invoice creation flow.The
vatAmountfield is properly typed and positioned. Consider adding validation in the service layer to reject negative VAT amounts if applicable to your business rules.src/main/java/org/example/entity/invoice/InvoiceItemDTO.java (1)
15-22: LGTM!The
fromEntitymethod correctly populates all fields including the newnamefield.src/main/java/org/example/entity/invoice/Invoice.java (2)
70-78: LGTM!The
recalcTotals()method correctly handles nullvatAmountand properly computes the final amount as subtotal plus VAT.
80-90: LGTM!The
fromDTOmethod correctly initializesvatAmountand callsrecalcTotals()after populating items to ensure accurate totals.Also applies to: 102-102
src/main/java/org/example/CliApp.java (3)
696-700: LGTM!The
InvoiceItemDTObuilder correctly includes thenamefield along with quantity and unit price.
766-780: LGTM!The method correctly prompts for the item name and includes it in the
InvoiceItemDTObuilder.
783-783: No action needed. TheUpdateInvoiceDTOconstructor signature matches the call pattern:(invoiceId: UUID, dueDate: LocalDateTime, vatAmount: BigDecimal, items: List<InvoiceItemDTO>, status: InvoiceStatus). Passing null values for partial updates is acceptable and follows the intended design.
…oice` and `InvoiceItem` entities, and refine totals calculation logic.
- Add a new field of type float vatRate that will hold the vatRate of the invoice - Keep vatAmount as BigDecimal that will hold the actual currency amount of the totatl vat - Remove DTO display from CLI and prettyprint Invoices and InvoiceItems listings
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/example/entity/invoice/Invoice.java (1)
56-56: Address pipeline warning: Add @Builder.Default annotation.The pipeline warns that
@Builderignores thenew ArrayList<>()initialization. Without@Builder.Default, the builder will usenullforinvoiceItemsinstead of an empty list, potentially causing NPEs.🔧 Proposed fix
@OneToMany(mappedBy = "invoice", cascade = CascadeType.ALL, orphanRemoval = true) + @Builder.Default private List<InvoiceItem> invoiceItems = new ArrayList<>();Based on learnings and pipeline failure logs.
src/main/java/org/example/CliApp.java (1)
853-859: Critical: Preserve item name during update.Line 855 rebuilds the InvoiceItemDTO with only
id,quantity, andunitPrice, losing the item'snamefield. When this updated list is sent to the service, all item names will be lost or set to null.🐛 Proposed fix to preserve item name
List<InvoiceItemDTO> updated = items.stream() .map(i -> i.id().equals(item.id()) - ? InvoiceItemDTO.builder().id(i.id()).quantity(quantity).unitPrice(unitPrice).build() + ? InvoiceItemDTO.builder() + .id(i.id()) + .name(i.name()) + .quantity(quantity) + .unitPrice(unitPrice) + .build() : i) .toList();
🤖 Fix all issues with AI agents
In @src/main/java/org/example/CliApp.java:
- Around line 775-789: The loop that prints InvoiceItemDTO items may NPE when
calling item.name().isBlank(); update the printing logic inside the for-loop
(the block iterating invoice.items()) to null-safe-check the name before calling
isBlank — e.g., compute a safeName by checking item.name() == null ||
item.name().isBlank() and use "(no name)" when true, otherwise use item.name();
also apply the same null-safe pattern to the analogous logic in listInvoices()
to avoid duplicate NPEs.
- Around line 598-620: The code prints invoice items using item.name().isBlank()
which can throw NPE if name() is null; update the printing logic in the invoice
items loop (in the block iterating InvoiceItemDTO items) to treat null names as
blank before calling isBlank(), e.g., compute a safeName via a null check (or
use Objects.toString(item.name(), "") ) and then test safeName.isBlank(), and
use "(no name)" when safeName is null/blank.
- Around line 667-669: Replace the Float parsing for VAT with BigDecimal: change
the vatRate variable to BigDecimal, keep the empty-input -> null behavior, and
wrap the parse in a try-catch to handle invalid input (catch
NumberFormatException); if parsing fails, inform the user or re-prompt (or set
vatRate to null) and avoid letting the exception propagate. Use new
BigDecimal(vatInput) (after trimming) for conversion and include any necessary
scale/rounding adjustments where downstream code expects them.
In @src/main/java/org/example/entity/invoice/CreateInvoiceDTO.java:
- Line 12: The vatRate field in CreateInvoiceDTO is currently a Float which can
cause rounding errors; change its type to java.math.BigDecimal (replace Float
vatRate with BigDecimal vatRate) and update all related DTOs and domain classes:
adjust Invoice.java (field declaration and any arithmetic to use BigDecimal
operations), InvoiceDTO.java and UpdateInvoiceDTO.java (field types), and update
parsing logic in CliApp.java to construct BigDecimal from String (e.g., new
BigDecimal(input) or using parse with proper scale/MathContext) ensuring all
arithmetic uses BigDecimal methods (add, multiply, setScale) to preserve
financial precision.
In @src/main/java/org/example/entity/invoice/Invoice.java:
- Line 42: The vatRate field in class Invoice is declared as Float which causes
precision loss before any BigDecimal conversion; change the field declaration
vatRate from Float to BigDecimal and update all usages (getters/setters,
constructors, equals/hashCode, toString) as well as any DTOs, mappers, and
service methods that set or read Invoice.vatRate to accept/return BigDecimal;
ensure any parsing or JSON (de)serialization uses BigDecimal (e.g., adjust
Jackson annotations or DTO types) and update calculations that previously called
new BigDecimal(vatRate) to use the BigDecimal directly.
- Around line 73-80: The VAT calculation currently converts a Float vatRate to
BigDecimal (causing precision loss) and uses a ternary that still creates a
BigDecimal for nulls; change the logic to use a BigDecimal vatRate (or, if you
cannot change the field type, convert from Float via
BigDecimal.valueOf(vatRate.doubleValue()) only when non-null) and compute vat as
Optional.ofNullable(vatRate).orElse(BigDecimal.ZERO).multiply(subTotal) (apply
an explicit scale/rounding mode if needed), then set this.vatAmount = vat and
this.amount = subTotal.add(vat); update references to vatRate, vatAmount, amount
in class Invoice accordingly.
In @src/main/java/org/example/service/InvoiceService.java:
- Line 67: The call to item.setName(itemDTO.name()) can pass null and violate
the non-null DB constraint on InvoiceItem.name; update the code around
item.setName to guard against null (e.g., supply a default empty string when
itemDTO.name() is null) or enforce non-null at the DTO level by adding @NotNull
to InvoiceItemDTO.name so the setter always receives a non-null value.
🧹 Nitpick comments (1)
src/main/java/org/example/CliApp.java (1)
707-722: LGTM! Item name collection implemented correctly.The method now properly collects item names and includes them in the InvoiceItemDTO. The flow is clear and user-friendly.
Consider adding validation to prevent blank item names:
String name = scanner.nextLine().trim(); if (name.isEmpty()) { System.out.println("Item name cannot be empty."); continue; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/org/example/CliApp.javasrc/main/java/org/example/entity/invoice/CreateInvoiceDTO.javasrc/main/java/org/example/entity/invoice/Invoice.javasrc/main/java/org/example/entity/invoice/InvoiceDTO.javasrc/main/java/org/example/entity/invoice/UpdateInvoiceDTO.javasrc/main/java/org/example/service/InvoiceService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/example/entity/invoice/InvoiceDTO.java
- src/main/java/org/example/entity/invoice/UpdateInvoiceDTO.java
🧰 Additional context used
🪛 GitHub Actions: Java Maven Build
src/main/java/org/example/entity/invoice/Invoice.java
[warning] 56-56: @builder will ignore the initializing expression entirely. If you want the initializing expression to serve as default, add @Builder.Default. If it is not supposed to be settable during building, make the field final.
🔇 Additional comments (5)
src/main/java/org/example/service/InvoiceService.java (1)
60-60: LGTM with caveat on Float type.The VAT rate update logic correctly checks for null before applying. However, note that using Float for VAT rate (as flagged in CreateInvoiceDTO) will propagate precision issues throughout the calculation chain.
src/main/java/org/example/entity/invoice/Invoice.java (1)
97-97: Verify null handling for item.setName().Similar to InvoiceService, this sets the item name without checking if
itemDTO.name()is null. Ensure this aligns with the InvoiceItem entity's constraints.This concern can be verified using the same script provided in the InvoiceService.java review.
src/main/java/org/example/CliApp.java (3)
895-899: LGTM!The removal logic correctly filters out the selected item while preserving the rest of the items unchanged.
941-946: LGTM! Improved invoice selection UI.The enhanced formatting with separators and invoice details makes the selection interface clearer and more user-friendly.
797-814: LGTM! The UpdateInvoiceDTO parameter order is correct, and the item name collection is properly implemented. All code changes look good.
| invoice.clearItems(); | ||
| dto.items().forEach(itemDTO -> { | ||
| InvoiceItem item = new InvoiceItem(); | ||
| item.setName(itemDTO.name()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find InvoiceItem.java file first
fd -name "InvoiceItem.java" --type f
# Once found, examine the entity definition and name field
if [ -f "src/main/java/org/example/model/InvoiceItem.java" ]; then
cat -n "src/main/java/org/example/model/InvoiceItem.java"
fiRepository: ithsjava25/project-jpa-grupp-3-d
Length of output: 306
🏁 Script executed:
#!/bin/bash
# Find InvoiceItem.java file
fd -name "InvoiceItem.java" --type f
# Read the file with line numbers
find . -name "InvoiceItem.java" -type f -exec cat -n {} \;Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 1283
🏁 Script executed:
#!/bin/bash
# Find ItemDTO file
find . -name "*ItemDTO*" -type f -exec cat -n {} \;
# Also check InvoiceService.java around line 67
find . -name "InvoiceService.java" -type f -exec sed -n '50,85p' {} +Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 2180
🏁 Script executed:
#!/bin/bash
# Find UpdateInvoiceDTO definition
find . -name "*UpdateInvoiceDTO*" -type f -exec cat -n {} \;
# Also search for validation annotations on the items field
rg -B5 -A5 "items()" --type=javaRepository: ithsjava25/project-jpa-grupp-3-d
Length of output: 20715
Add null check before setting item name.
The code calls item.setName(itemDTO.name()) without null validation. Since InvoiceItem.name has @Column(nullable = false), passing a null value will violate the database constraint and cause a runtime error. Add a null check or provide a default value before setting the name:
item.setName(itemDTO.name() != null ? itemDTO.name() : "");Alternatively, add @NotNull validation to InvoiceItemDTO.name to enforce constraints at the DTO level.
🤖 Prompt for AI Agents
In @src/main/java/org/example/service/InvoiceService.java at line 67, The call
to item.setName(itemDTO.name()) can pass null and violate the non-null DB
constraint on InvoiceItem.name; update the code around item.setName to guard
against null (e.g., supply a default empty string when itemDTO.name() is null)
or enforce non-null at the DTO level by adding @NotNull to InvoiceItemDTO.name
so the setter always receives a non-null value.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/org/example/InvoiceServiceTest.java (1)
119-129: Add assertions to verify VAT calculations are applied correctly.This test updates an invoice with a vatRate of 0.25f and items totaling 1000.00 (2 × 500.00), but doesn't verify the VAT amount or final total. The
Invoice.recalcTotals()method calculates vatAmount as vatRate × subtotal and sets amount as subtotal + vatAmount, andInvoiceService.updateInvoice()calls this method before returning the result.Add assertions to verify:
- VAT amount is correctly calculated (250.00 for 25% of 1000.00)
- Final amount includes subtotal + VAT (1250.00)
- The returned InvoiceDTO contains the expected vatAmount value
🧹 Nitpick comments (3)
src/test/java/org/example/InvoiceServiceTest.java (3)
51-51: Consider adding test coverage for edge cases of vatRate.All test cases use the same hardcoded vatRate value (0.25f). While this is sufficient for basic functionality testing, consider adding tests that cover edge cases such as:
- Zero VAT rate (0.0f)
- Maximum reasonable VAT rate (e.g., 1.0f for 100%)
- Boundary values
This would improve confidence in the VAT handling logic across different scenarios.
Also applies to: 155-155, 167-167, 207-207
119-119: Use more descriptive test data for item name.The InvoiceItemDTO uses the literal string
"name"as the name parameter. Consider using a more descriptive value like"Test Item","Consulting Service", or"Product A"to make the test data clearer and more realistic.♻️ Proposed refactor
- UpdateInvoiceDTO updateDto = new UpdateInvoiceDTO(invoiceId, LocalDateTime.now().plusDays(30), 0.25f, List.of(new InvoiceItemDTO(null, "name", 2, new BigDecimal("500.00"))), InvoiceStatus.SENT); + UpdateInvoiceDTO updateDto = new UpdateInvoiceDTO(invoiceId, LocalDateTime.now().plusDays(30), 0.25f, List.of(new InvoiceItemDTO(null, "Consulting Service", 2, new BigDecimal("500.00"))), InvoiceStatus.SENT);
212-227: Consider initializing VAT fields in the test helper for consistency.The
createFullInvoicehelper doesn't initializevatRateorvatAmount. While theInvoice.recalcTotals()method safely handles null values (treatingvatRateas zero when null), initializing these fields—similar to howInvoice.fromDTO()explicitly setsvatRate—would improve clarity and consistency across test setup methods.Suggested approach: Initialize
vatRateto0fandvatAmounttoBigDecimal.ZEROin the helper.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/main/java/org/example/CliApp.java (5)
594-625: Fixprintf("%.2f", BigDecimal)(runtimeIllegalFormatConversionException) and guarditem.name()nulls.
Ifamount() / vatAmount() / unitPrice()areBigDecimal(as implied by the PR),%.2fwill crash at runtime; alsoitem.name().isBlank()will NPE ifnameis null (see update flow).Proposed fix
@@ import java.math.BigDecimal; +import java.math.RoundingMode; @@ private void listInvoices() { @@ for (InvoiceDTO invoice : invoices) { System.out.println("Invoice: " + invoice.number()); System.out.println("Status : " + invoice.status()); - System.out.printf("Amount : %.2f%n", invoice.amount()); - System.out.printf("VAT : %.2f%n", invoice.vatAmount()); + System.out.println("Amount : " + formatMoney(invoice.amount())); + System.out.println("VAT : " + formatMoney(invoice.vatAmount())); System.out.println("Due : " + invoice.dueDate().toLocalDate()); @@ for (InvoiceItemDTO item : invoice.items()) { - System.out.printf(" - %s: %d x %.2f%n", - item.name().isBlank() ? "(no name)" : item.name(), - item.quantity(), - item.unitPrice()); + System.out.printf(" - %s: %d x %s%n", + displayName(item.name()), + item.quantity(), + formatMoney(item.unitPrice())); } } System.out.println("--------------------------------------------------"); } @@ } + + private static String displayName(String name) { + return (name == null || name.isBlank()) ? "(no name)" : name; + } + + private static String formatMoney(BigDecimal value) { + if (value == null) return "0.00"; + return value.setScale(2, RoundingMode.HALF_UP).toPlainString(); + }
710-733: Handle invalid unit price input here (otherwisecreateInvoice()blames VAT).
new BigDecimal(scanner.nextLine().trim())can throwNumberFormatException; right now that gets caught increateInvoice()and prints a VAT error.Proposed fix
@@ System.out.print("Unit price: "); - BigDecimal unitPrice = new BigDecimal(scanner.nextLine().trim()); + BigDecimal unitPrice; + try { + unitPrice = new BigDecimal(scanner.nextLine().trim()); + } catch (NumberFormatException e) { + System.out.println("✗ Invalid unit price format."); + continue; + }
779-798: Same formatting + null-name issues aslistInvoices(); reuse the helper methods.
831-877: Updating an item dropsname(and may introducename == null, causing NPE in listings).
You rebuild the DTO with onlyid/quantity/unitPrice; ifnameis required/persisted, this will either wipe it or set it null.Proposed fix (preserve name)
@@ List<InvoiceItemDTO> updated = items.stream() .map(i -> i.id().equals(item.id()) - ? InvoiceItemDTO.builder().id(i.id()).quantity(quantity).unitPrice(unitPrice).build() + ? InvoiceItemDTO.builder() + .id(i.id()) + .name(i.name()) + .quantity(quantity) + .unitPrice(unitPrice) + .build() : i) .toList();
627-708: BroadenNumberFormatExceptioncatch message—not VAT-specific.The VAT field is correctly implemented:
CreateInvoiceDTOexpectsvatRate(notvatAmount), and the service calculatesvatAmountinternally fromvatRate * subTotalinrecalcTotals(). The CLI prompt for "VAT rate" aligns with this design.However, the
NumberFormatExceptioncatch is overly specific. It prints a VAT-focused message, but the exception can be triggered by either invalid VAT input or invalid unit prices fromreadInvoiceItems(). Use a generic message instead:catch (NumberFormatException e) { - System.out.println("✗ Invalid VAT rate format. Please use decimal format (e.g., 0.25)."); + System.out.println("✗ Invalid decimal format (VAT or unit price)."); }
🤖 Fix all issues with AI agents
In @src/main/java/org/example/entity/invoice/Invoice.java:
- Around line 72-81: The recalcTotals() method assumes vatRate is a decimal
(e.g., 0.25) but there is no documentation or validation; add JavaDoc to the
vatRate field clarifying "VAT rate as a decimal (e.g., 0.25 for 25%) and
expected range 0.0–1.0", and enforce validation either in the
Invoice.fromDTO(...) factory or a setVatRate(...) setter (validate vatRate !=
null then ensure 0.0 <= vatRate <= 1.0, otherwise throw
IllegalArgumentException) so callers cannot pass negative values or whole
percentages like 25.
- Around line 42-44: Add explicit @Column annotations to the Invoice entity's
vatRate and vatAmount fields to match the financial precision used elsewhere:
annotate vatRate (field name vatRate) with precision=5, scale=4 and annotate
vatAmount (field name vatAmount) with precision=19, scale=2 so VAT rate storage
supports four decimal places and VAT amounts use the same precision/scale as
amount; update the import if necessary and ensure the annotations are placed
directly above the corresponding fields in the Invoice class.
🧹 Nitpick comments (2)
src/main/java/org/example/CliApp.java (2)
800-829: Item add flow looks consistent with the newnamefield.
Only suggestion: validatename/quantity/unitPricebefore sending the DTO (to avoid service-layer roundtrips).
879-915: Remove-item flow looks fine; just ensure the update DTO semantics are “replace items” (not “patch lists”).
Given the PR mentions partial-update patterns, it’s worth confirming this call won’t unintentionally overwrite other invoice fields.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/org/example/CliApp.javasrc/main/java/org/example/entity/invoice/CreateInvoiceDTO.javasrc/main/java/org/example/entity/invoice/Invoice.javasrc/main/java/org/example/entity/invoice/InvoiceDTO.javasrc/main/java/org/example/entity/invoice/UpdateInvoiceDTO.javasrc/test/java/org/example/InvoiceServiceTest.java
🔇 Additional comments (8)
src/main/java/org/example/entity/invoice/UpdateInvoiceDTO.java (1)
3-14: LGTM! Clean DTO update.The addition of
vatRateto the DTO is straightforward and aligns with the PR's objective to support manual VAT handling.src/main/java/org/example/entity/invoice/CreateInvoiceDTO.java (1)
3-15: LGTM! Consistent DTO extension.The addition of
vatRatefollows the same pattern as UpdateInvoiceDTO and supports the VAT handling feature.src/main/java/org/example/entity/invoice/InvoiceDTO.java (1)
17-18: LGTM! Proper DTO-to-entity mapping.The DTO correctly includes both
vatRateand the computedvatAmount, andfromEntityproperly populates them from the Invoice entity.Also applies to: 35-36
src/main/java/org/example/entity/invoice/Invoice.java (2)
83-93: LGTM! Proper builder usage for vatRate.The
vatRateis correctly wired from the DTO through the builder pattern. Consider adding validation forvatRateformat as noted in the separate comment on therecalcTotalsmethod.
95-103: LGTM! Item name properly populated.The addition of
item.setName(itemDTO.name())correctly implements the item description feature from the PR objectives.src/test/java/org/example/InvoiceServiceTest.java (2)
51-51: Tests properly updated for new DTO signatures.All test cases correctly include the new
vatRateparameter usingnew BigDecimal("0.25"), confirming the expected decimal format (0.25 = 25% VAT). The test at line 119 also properly includes the newnamefield inInvoiceItemDTO.Also applies to: 119-119, 155-155, 167-167, 179-179, 207-207
51-51: AI summary contains incorrect field naming.The AI-generated summary for this file incorrectly refers to the new BigDecimal parameter as "extraCharge" when it's actually
vatRate. This inconsistency might confuse reviewers.Actual field:
vatRate(as seen in CreateInvoiceDTO.java and UpdateInvoiceDTO.java)
Summary claims: "extraCharge (BigDecimal)"Also applies to: 119-119, 155-155, 167-167, 179-179, 207-207
src/main/java/org/example/CliApp.java (1)
939-965: Invoice selection output improvements look good.
| private BigDecimal vatRate; | ||
|
|
||
| private BigDecimal vatAmount; |
There was a problem hiding this comment.
Add @column annotations with precision and scale for financial fields.
The vatRate and vatAmount fields lack @Column annotations with precision and scale attributes. Unlike the amount field (line 40), which properly specifies precision = 19, scale = 2, these VAT-related fields rely on database defaults, which can lead to:
- Inconsistent precision across different database vendors
- Potential rounding errors in VAT calculations
- Loss of precision during persistence
🔧 Proposed fix with proper column definitions
- private BigDecimal vatRate;
-
- private BigDecimal vatAmount;
+ @Column(name = "vat_rate", precision = 5, scale = 4)
+ private BigDecimal vatRate;
+
+ @Column(name = "vat_amount", precision = 19, scale = 2)
+ private BigDecimal vatAmount;Rationale:
vatRate: precision=5, scale=4 allows rates like 0.2500 (25%) up to 9.9999 (999.99%)vatAmount: precision=19, scale=2 matches theamountfield for consistency
📝 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.
| private BigDecimal vatRate; | |
| private BigDecimal vatAmount; | |
| @Column(name = "vat_rate", precision = 5, scale = 4) | |
| private BigDecimal vatRate; | |
| @Column(name = "vat_amount", precision = 19, scale = 2) | |
| private BigDecimal vatAmount; |
🤖 Prompt for AI Agents
In @src/main/java/org/example/entity/invoice/Invoice.java around lines 42 - 44,
Add explicit @Column annotations to the Invoice entity's vatRate and vatAmount
fields to match the financial precision used elsewhere: annotate vatRate (field
name vatRate) with precision=5, scale=4 and annotate vatAmount (field name
vatAmount) with precision=19, scale=2 so VAT rate storage supports four decimal
places and VAT amounts use the same precision/scale as amount; update the import
if necessary and ensure the annotations are placed directly above the
corresponding fields in the Invoice class.
| public void recalcTotals() { | ||
| amount = invoiceItems.stream() | ||
| .map(i -> i.getUnitPrice().multiply(BigDecimal.valueOf(i.getQuantity()))) | ||
| BigDecimal subTotal = invoiceItems.stream() | ||
| .map(item -> item.getUnitPrice().multiply(BigDecimal.valueOf(item.getQuantity()))) | ||
| .reduce(BigDecimal.ZERO, BigDecimal::add); | ||
|
|
||
| BigDecimal vat = (vatRate != null ? vatRate : BigDecimal.ZERO) | ||
| .multiply(subTotal); | ||
| this.vatAmount = vat; | ||
| this.amount = subTotal.add(vat); | ||
| } |
There was a problem hiding this comment.
Document and validate vatRate format expectations.
The VAT calculation assumes vatRate is in decimal form (e.g., 0.25 for 25% VAT), as evidenced by the direct multiplication at line 78. However:
- No documentation: The expected format for
vatRateisn't documented in the entity or DTOs - No validation: Nothing prevents invalid values (e.g., negative rates, rates > 1.0 for typical VAT scenarios, or someone mistakenly entering 25 instead of 0.25)
- Potential user confusion: Without clear guidance, API consumers might enter 25 instead of 0.25
Recommendations:
- Add JavaDoc to clarify the format:
/**
* VAT rate as a decimal (e.g., 0.25 for 25% VAT).
* Should be between 0.0 and 1.0 for typical use cases.
*/
@Column(name = "vat_rate", precision = 5, scale = 4)
private BigDecimal vatRate;- Consider adding validation in
fromDTOor a setter:
if (vatRate != null && (vatRate.compareTo(BigDecimal.ZERO) < 0 || vatRate.compareTo(BigDecimal.ONE) > 1)) {
throw new IllegalArgumentException("VAT rate must be between 0.0 and 1.0");
}🤖 Prompt for AI Agents
In @src/main/java/org/example/entity/invoice/Invoice.java around lines 72 - 81,
The recalcTotals() method assumes vatRate is a decimal (e.g., 0.25) but there is
no documentation or validation; add JavaDoc to the vatRate field clarifying "VAT
rate as a decimal (e.g., 0.25 for 25%) and expected range 0.0–1.0", and enforce
validation either in the Invoice.fromDTO(...) factory or a setVatRate(...)
setter (validate vatRate != null then ensure 0.0 <= vatRate <= 1.0, otherwise
throw IllegalArgumentException) so callers cannot pass negative values or whole
percentages like 25.
…into feature/Invoice-&-InvoiceItems # Conflicts: # src/main/java/org/example/entity/invoice/Invoice.java
…Jakarta Validation dependencies in `pom.xml`.
…efine `name` checks in CLI, and improve error handling for invalid inputs.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/example/CliApp.java (1)
858-868: Critical: Item name is lost during update.Line 866 creates an
InvoiceItemDTOwith onlyid,quantity, andunitPrice, omitting thenamefield. This will cause the item name to be set to null during the update, resulting in data loss. The original item's name should be preserved.🐛 Preserve item name during update
List<InvoiceItemDTO> updated = items.stream() .map(i -> i.id().equals(item.id()) - ? InvoiceItemDTO.builder().id(i.id()).quantity(quantity).unitPrice(unitPrice).build() + ? InvoiceItemDTO.builder().id(i.id()).name(i.name()).quantity(quantity).unitPrice(unitPrice).build() : i) .toList();
🤖 Fix all issues with AI agents
In @pom.xml:
- Around line 91-100: Update the hibernate-validator dependency to a 9.x release
to move off limited-support 8.x; locate the <dependency> block with groupId
"org.hibernate.validator" and artifactId "hibernate-validator" and change its
<version> from "8.0.1.Final" to a 9.x release such as "9.1.0.Final" (or
"9.0.1.Final") while leaving the jakarta.validation-api dependency unchanged.
In @src/main/java/org/example/CliApp.java:
- Around line 786-800: The loop printing InvoiceItemDTO items calls
item.name().isBlank() without checking for null, risking a NullPointerException;
update the printing logic in the loop that iterates invoice.items() to first
check if item.name() == null (or use Objects.toString/item.name() safely) and
treat null the same as blank by printing "(no name)"; modify the loop around the
InvoiceItemDTO variable `item` to use a null-safe name check before calling
isBlank().
- Around line 668-670: The code parses VAT input with vatInput and constructs
vatRate using new BigDecimal(vatInput) without catching NumberFormatException;
update the input handling around scanner.nextLine()/vatInput so non-numeric
entries are validated or wrapped in a try/catch, e.g., attempt to parse vatInput
into a BigDecimal inside a try block and on NumberFormatException prompt the
user again or set vatRate to null, ensuring any thrown NumberFormatException
from new BigDecimal(...) is handled alongside the existing
IllegalArgumentException handling.
In @src/main/java/org/example/entity/invoice/InvoiceItemDTO.java:
- Around line 12-13: The InvoiceItemDTO field 'name' is annotated with @NotBlank
but the project never invokes Jakarta Validation, so either remove @NotBlank
from InvoiceItemDTO or add enforcement where DTOs are created (e.g., CLI input
handling) by obtaining a Validator via ValidatorFactory.getValidator() and
validating the InvoiceItemDTO instance and rejecting/inputting again on
violations; locate the InvoiceItemDTO class and the CLI input code that
constructs it, then choose to (A) delete the @NotBlank annotation from
InvoiceItemDTO if you don't want validation, or (B) call
ValidatorFactory.getValidator().validate(invoiceItemDto) after building
InvoiceItemDTO and handle ConstraintViolations to enforce the "Name cannot be
empty" rule.
🧹 Nitpick comments (1)
src/main/java/org/example/entity/invoice/Invoice.java (1)
101-109: Consider adding defensive validation for item name.Line 104 sets the item name from
itemDTO.name()without checking for null or blank values. WhileInvoiceItemDTOhas a@NotBlankannotation, Bean Validation constraints are not automatically enforced in plain JPA applications without explicit validator invocation.🛡️ Add defensive check
if (dto.items() != null) { dto.items().forEach(itemDTO -> { + if (itemDTO.name() == null || itemDTO.name().isBlank()) { + throw new IllegalArgumentException("Item name cannot be empty"); + } InvoiceItem item = new InvoiceItem(); item.setName(itemDTO.name()); item.setQuantity(itemDTO.quantity()); item.setUnitPrice(itemDTO.unitPrice()); invoice.addItem(item); }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
pom.xmlsrc/main/java/org/example/CliApp.javasrc/main/java/org/example/entity/invoice/Invoice.javasrc/main/java/org/example/entity/invoice/InvoiceItem.javasrc/main/java/org/example/entity/invoice/InvoiceItemDTO.javasrc/main/java/org/example/service/InvoiceService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/example/service/InvoiceService.java
- src/main/java/org/example/entity/invoice/InvoiceItem.java
🔇 Additional comments (6)
src/main/java/org/example/entity/invoice/Invoice.java (3)
42-43: LGTM!The
vatRatecolumn definition withprecision = 5, scale = 4appropriately supports rates between 0.0 and 1.0 with four decimal places of precision (e.g., 0.2525 for 25.25%).
73-82: LGTM!The
recalcTotals()implementation correctly computes the invoice totals:
- Subtotal from item quantities and prices
- VAT amount from the rate (with appropriate null handling)
- Final amount as subtotal plus VAT
85-89: LGTM!The VAT rate validation correctly enforces the range constraint (0.0 to 1.0) with a clear error message, and appropriately allows null values for invoices without VAT.
src/main/java/org/example/CliApp.java (3)
716-721: LGTM!The item name input validation correctly ensures names are not empty and provides clear feedback, allowing users to retry when validation fails.
602-621: LGTM!The enhanced invoice listing correctly handles null names with the null-safe check on line 615, providing a clear fallback display for missing item names.
825-825: UpdateInvoiceDTO constructor signature is correct and properly handles partial updates.The constructor signature matches the three calls at lines 825, 870, and 910. The
UpdateInvoiceDTOrecord defines fields:invoiceId,dueDate,vatRate,items, andstatus. The calls correctly passnullfordueDate,vatRate, andstatusto update only the items list. TheInvoiceService.updateInvoice()method properly implements the partial update pattern by checking each field withif (dto.field() != null)before applying updates, treatingnullas "no change."
| <dependency> | ||
| <groupId>jakarta.validation</groupId> | ||
| <artifactId>jakarta.validation-api</artifactId> | ||
| <version>3.0.2</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.hibernate.validator</groupId> | ||
| <artifactId>hibernate-validator</artifactId> | ||
| <version>8.0.1.Final</version> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Jakarta Validation API Versions ==="
curl -s https://repo1.maven.org/maven2/jakarta/validation/jakarta.validation-api/maven-metadata.xml | grep -oP '(?<=<version>)[^<]+' | tail -10
echo -e "\n=== Hibernate Validator Versions ==="
curl -s https://repo1.maven.org/maven2/org/hibernate/validator/hibernate-validator/maven-metadata.xml | grep -oP '(?<=<version>)[^<]+' | tail -10Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 343
🌐 Web query:
Jakarta Validation API 3.0.2 security vulnerability CVE
💡 Result:
I couldn't find any CVE that names jakarta.validation-api version 3.0.2 itself as vulnerable. Relevant public CVEs affect related projects/implementations (not the API artifact):
- CVE-2020-10693 — message interpolation flaw in Hibernate Validator (affects some Hibernate Validator versions). [1]
- CVE-2019-10219 — SafeHtml sanitization/XSS issue in Hibernate Validator. [2]
- CVE-2025-35036 — expression-language interpolation / arbitrary code risk in Hibernate Validator (reported 2025). [3]
- CVE-2021-28170 — bug in Jakarta Expression Language (jakarta.el) allowing invalid EL expressions to be evaluated. [4]
If you want, I can: (a) check whether a specific Maven/GAV that pulls jakarta.validation-api:3.0.2 is transitively affected by any of the above, or (b) scan a project dependency tree for vulnerabilities. Which would you prefer?
🌐 Web query:
Hibernate Validator 8.0.1.Final security vulnerability CVE 2024 2025
💡 Result:
Short answer: There is no widely‑published upstream CVE that specifically names org.hibernate.validator:hibernate-validator 8.0.1.Final as vulnerable in 2024–2025. However:
- Red Hat/EAP packaged variants had a related SAML/signature issue (CVE‑2024‑8698) affecting eap8-hibernate-validator-cdi; Red Hat published fixes. [1]
- Separate, higher‑impact CVEs from 2025 (e.g. CVE‑2025‑35036) affect older Hibernate Validator series (pre‑6.2.0 / pre‑7.0.0) due to Expression Language interpolation, not 8.0.1.Final upstream. [2][3]
- Hibernate Validator 8.0 is in limited support; newer 8.x releases (e.g. 8.0.3.Final) and 9.x are available — upgrading to a supported/updated release or using vendor patches (for Red Hat/EAP builds) is recommended. [4]
Sources:
- Snyk / Red Hat advisory for CVE‑2024‑8698 (eap8‑hibernate‑validator‑cdi). [https://security.snyk.io/vuln/SNYK-RHEL8-EAP8HIBERNATEVALIDATORCDI-8342924] [1]
- NVD / CVE‑2025‑35036 (Expression Language interpolation; affects pre‑6.2.0 / pre‑7.0.0). [https://nvd.nist.gov/vuln/detail/CVE-2025-35036] [2]
- GitHub Advisory entry for the same 2025 advisory. [https://github.com/advisories/GHSA-7v6m-28jr-rg84] [3]
- Hibernate Validator 8.0 release/maintenance page (shows limited‑support status and newer 8.x releases). [https://hibernate.org/validator/releases/8.0/] [4]
If you want, I can:
- check your specific distribution/vendor build (Red Hat, WildFly, Payara) for vendor advisories for 8.0.1.Final, or
- scan a pom.gradle to see if you use a packaged variant that is affected.
Upgrade Hibernate Validator to 9.x series; Jakarta Validation API version is acceptable.
Jakarta Validation API 3.0.2 is stable and reasonably current (3.1.1 is the latest stable release). However, Hibernate Validator 8.0.1.Final is in limited-support status with version 9.1.0.Final available. Upgrade Hibernate Validator to 9.1.0.Final or 9.0.1.Final to ensure continued support and compatibility with current Jakarta Validation standards.
🤖 Prompt for AI Agents
In @pom.xml around lines 91 - 100, Update the hibernate-validator dependency to
a 9.x release to move off limited-support 8.x; locate the <dependency> block
with groupId "org.hibernate.validator" and artifactId "hibernate-validator" and
change its <version> from "8.0.1.Final" to a 9.x release such as "9.1.0.Final"
(or "9.0.1.Final") while leaving the jakarta.validation-api dependency
unchanged.
| System.out.print("Enter VAT rate (e.g. 0.25, 0.12): "); | ||
| String vatInput = scanner.nextLine().trim(); | ||
| BigDecimal vatRate = vatInput.isEmpty() ? null : new BigDecimal(vatInput); |
There was a problem hiding this comment.
VAT rate parsing lacks error handling.
If the user enters non-numeric input for the VAT rate, new BigDecimal(vatInput) will throw a NumberFormatException that isn't caught by the existing exception handlers (lines 692-706). While IllegalArgumentException is caught at line 692, it doesn't cover parsing errors.
🔧 Improve error handling
System.out.print("Enter VAT rate (e.g. 0.25, 0.12): ");
String vatInput = scanner.nextLine().trim();
- BigDecimal vatRate = vatInput.isEmpty() ? null : new BigDecimal(vatInput);
+ BigDecimal vatRate = null;
+ if (!vatInput.isEmpty()) {
+ try {
+ vatRate = new BigDecimal(vatInput);
+ } catch (NumberFormatException e) {
+ System.out.println("✗ Invalid VAT rate format.");
+ return;
+ }
+ }📝 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.
| System.out.print("Enter VAT rate (e.g. 0.25, 0.12): "); | |
| String vatInput = scanner.nextLine().trim(); | |
| BigDecimal vatRate = vatInput.isEmpty() ? null : new BigDecimal(vatInput); | |
| System.out.print("Enter VAT rate (e.g. 0.25, 0.12): "); | |
| String vatInput = scanner.nextLine().trim(); | |
| BigDecimal vatRate = null; | |
| if (!vatInput.isEmpty()) { | |
| try { | |
| vatRate = new BigDecimal(vatInput); | |
| } catch (NumberFormatException e) { | |
| System.out.println("✗ Invalid VAT rate format."); | |
| return; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @src/main/java/org/example/CliApp.java around lines 668 - 670, The code
parses VAT input with vatInput and constructs vatRate using new
BigDecimal(vatInput) without catching NumberFormatException; update the input
handling around scanner.nextLine()/vatInput so non-numeric entries are validated
or wrapped in a try/catch, e.g., attempt to parse vatInput into a BigDecimal
inside a try block and on NumberFormatException prompt the user again or set
vatRate to null, ensuring any thrown NumberFormatException from new
BigDecimal(...) is handled alongside the existing IllegalArgumentException
handling.
| @NotBlank(message = "Name cannot be empty") | ||
| String name, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Jakarta Validator usage in service layer and entity lifecycle
echo "=== Checking for explicit Validator usage ==="
rg -n --type=java 'Validator\.validate|ValidatorFactory|@Valid' -C 2
echo -e "\n=== Checking for JPA validation callbacks ==="
rg -n --type=java '@PrePersist|@PreUpdate' -C 3
echo -e "\n=== Checking for Spring @Validated or validation configuration ==="
rg -n --type=java '@Validated|ValidationConfiguration|ValidatorBean' -C 2
echo -e "\n=== Checking for JAX-RS @Valid ==="
rg -n --type=java 'javax.validation|jakarta.validation' -C 2Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 697
🏁 Script executed:
#!/bin/bash
# Find where InvoiceItemDTO is used
echo "=== Searching for InvoiceItemDTO usage ==="
rg -n 'InvoiceItemDTO' --type=java
echo -e "\n=== Looking for REST controllers with @PostMapping or @Valid ==="
rg -n --type=java '@RestController|@PostMapping|@Valid' -C 1
echo -e "\n=== Checking if Spring Boot is used ==="
git ls-files | grep -E '(pom.xml|build.gradle|application.properties|application.yml)' | head -5Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 2883
🏁 Script executed:
#!/bin/bash
echo "=== Checking pom.xml for validation dependencies ==="
cat pom.xml | grep -A 20 -B 5 'validation\|jakarta'
echo -e "\n=== Full dependency section ==="
cat pom.xml | grep -A 100 '<dependencies>'Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 4681
Remove @NotBlank or implement validation enforcement.
The @NotBlank annotation is present but not enforced—no validation code calls ValidatorFactory.getValidator() or uses JPA callbacks. Although Jakarta Validation and Hibernate Validator libraries are available as dependencies, they are unused throughout the codebase. Either remove the annotation if validation is not needed, or implement explicit validation in the CLI input handling to enforce the constraint.
🤖 Prompt for AI Agents
In @src/main/java/org/example/entity/invoice/InvoiceItemDTO.java around lines 12
- 13, The InvoiceItemDTO field 'name' is annotated with @NotBlank but the
project never invokes Jakarta Validation, so either remove @NotBlank from
InvoiceItemDTO or add enforcement where DTOs are created (e.g., CLI input
handling) by obtaining a Validator via ValidatorFactory.getValidator() and
validating the InvoiceItemDTO instance and rejecting/inputting again on
violations; locate the InvoiceItemDTO class and the CLI input code that
constructs it, then choose to (A) delete the @NotBlank annotation from
InvoiceItemDTO if you don't want validation, or (B) call
ValidatorFactory.getValidator().validate(invoiceItemDto) after building
InvoiceItemDTO and handle ConstraintViolations to enforce the "Name cannot be
empty" rule.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/example/CliApp.java (2)
826-841: Missing validation for empty item name.Unlike
readInvoiceItems()(lines 736-739) which validates that item names cannot be empty,addInvoiceItem()allows empty names. This inconsistency could lead to items being created with blank names.🐛 Proposed fix
System.out.print("Item name: "); String name = scanner.nextLine().trim(); + if (name.isEmpty()) { + System.out.println("✗ Item name cannot be empty."); + return; + } System.out.print("Quantity: ");
882-888: Item name is lost when updating invoice items.The updated item is built without setting the
namefield (line 884). This causes data loss - the item's name will be nullified after any update to quantity or unit price.🐛 Proposed fix to preserve item name
List<InvoiceItemDTO> updated = items.stream() .map(i -> i.id().equals(item.id()) - ? InvoiceItemDTO.builder().id(i.id()).quantity(quantity).unitPrice(unitPrice).build() + ? InvoiceItemDTO.builder().id(i.id()).name(i.name()).quantity(quantity).unitPrice(unitPrice).build() : i) .toList();
🤖 Fix all issues with AI agents
In @src/main/java/org/example/CliApp.java:
- Around line 811-816: The loop printing invoice items can throw a
NullPointerException because it calls item.name().isBlank() without null
checking; update the loop that iterates invoice.items() (and any formatting call
using InvoiceItemDTO.name()) to guard against null by checking item.name() ==
null || item.name().isBlank() (or use Objects.toString(item.name(), "") and then
isBlank()), and use "(no name)" when the name is null or blank so behavior
matches the earlier listInvoices handling.
🧹 Nitpick comments (2)
src/main/java/org/example/CliApp.java (2)
686-688: VAT input error handling could be more specific.Invalid VAT input (e.g., "abc") throws
NumberFormatException, which is caught byIllegalArgumentException. The generic error message may confuse users about which input was malformed.💡 Suggested improvement
System.out.print("Enter VAT rate (e.g. 0.25, 0.12): "); String vatInput = scanner.nextLine().trim(); - BigDecimal vatRate = vatInput.isEmpty() ? null : new BigDecimal(vatInput); + BigDecimal vatRate = null; + if (!vatInput.isEmpty()) { + try { + vatRate = new BigDecimal(vatInput); + } catch (NumberFormatException e) { + System.out.println("✗ Invalid VAT rate format. Please enter a decimal number."); + return; + } + }
709-724: Inconsistent formatting in catch blocks.The catch block formatting is inconsistent - some use
}catch(line 710) while others use} catchwith a space (implied standard). This is a minor style issue.✨ Consistent formatting
- }catch (IllegalArgumentException e) { + } catch (IllegalArgumentException e) { System.out.println("✗ Input error: " + e.getMessage()); - } - catch (DateTimeParseException e) { + } catch (DateTimeParseException e) { System.out.println("✗ Invalid date format. Please use yyyy-MM-dd."); - } - catch (BusinessRuleException e) { + } catch (BusinessRuleException e) { System.out.println("✗ Business Rule Violation: " + e.getMessage()); - } - catch (EntityNotFoundException e) { + } catch (EntityNotFoundException e) { System.out.println("✗ Creation failed: " + e.getMessage()); - } - catch (ValidationException e) { + } catch (ValidationException e) { System.out.println("✗ Validation error: " + e.getMessage()); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/example/CliApp.javasrc/main/java/org/example/service/InvoiceService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/example/service/InvoiceService.java
🔇 Additional comments (3)
src/main/java/org/example/CliApp.java (3)
727-754: LGTM!Good validation for empty item names with user feedback, and proper use of the builder pattern.
923-929: LGTM!The item removal logic correctly filters out the selected item and uses the partial update pattern.
968-975: LGTM!Enhanced invoice selection display with clearer formatting and visual separators.
| for (InvoiceItemDTO item : invoice.items()) { | ||
| System.out.printf(" - %s: %d x %.2f%n", | ||
| item.name().isBlank() ? "(no name)" : item.name(), | ||
| item.quantity(), | ||
| item.unitPrice()); | ||
| } |
There was a problem hiding this comment.
Potential NullPointerException on item.name().isBlank().
Line 813 calls item.name().isBlank() without a null check, unlike line 633 in listInvoices which properly handles null: item.name() == null || item.name().isBlank(). This will throw NPE if an item has a null name.
🐛 Proposed fix
for (InvoiceItemDTO item : invoice.items()) {
System.out.printf(" - %s: %d x %.2f%n",
- item.name().isBlank() ? "(no name)" : item.name(),
+ item.name() == null || item.name().isBlank() ? "(no name)" : item.name(),
item.quantity(),
item.unitPrice());
}📝 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.
| for (InvoiceItemDTO item : invoice.items()) { | |
| System.out.printf(" - %s: %d x %.2f%n", | |
| item.name().isBlank() ? "(no name)" : item.name(), | |
| item.quantity(), | |
| item.unitPrice()); | |
| } | |
| for (InvoiceItemDTO item : invoice.items()) { | |
| System.out.printf(" - %s: %d x %.2f%n", | |
| item.name() == null || item.name().isBlank() ? "(no name)" : item.name(), | |
| item.quantity(), | |
| item.unitPrice()); | |
| } |
🤖 Prompt for AI Agents
In @src/main/java/org/example/CliApp.java around lines 811 - 816, The loop
printing invoice items can throw a NullPointerException because it calls
item.name().isBlank() without null checking; update the loop that iterates
invoice.items() (and any formatting call using InvoiceItemDTO.name()) to guard
against null by checking item.name() == null || item.name().isBlank() (or use
Objects.toString(item.name(), "") and then isBlank()), and use "(no name)" when
the name is null or blank so behavior matches the earlier listInvoices handling.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/org/example/entity/invoice/UpdateInvoiceDTO.java:
- Around line 19-22: The @DecimalMin on the vatRate field in UpdateInvoiceDTO is
currently set to inclusive = false which incorrectly disallows a 0.0 VAT that
the business logic (Invoice class VAT check) permits; change the @DecimalMin on
the BigDecimal vatRate in UpdateInvoiceDTO to inclusive = true and update the
message to indicate the range allows 0.0 through 1.0 (e.g., "VAT rate must be
between 0.0 and 1.0"), and make the same change to the vatRate field in
CreateInvoiceDTO so DTO validation matches Invoice validation.
🧹 Nitpick comments (2)
src/main/java/org/example/entity/invoice/CreateInvoiceDTO.java (1)
28-31: Validation range assumes fraction (0–1); ensure callers don’t pass “25” for 25%If user input is likely “25%”, you’ll either need mapping (25 → 0.25) or change constraints to
0–100. Right now tests use0.25, but the CLI/user-facing contract needs to be explicit.src/test/java/org/example/InvoiceServiceTest.java (1)
51-53: Tests: make VAT semantics + timestamps less brittle
- You’re now passing
new BigDecimal("0.25"); please ensure this matches the intended contract (fraction vs percent) and add at least one assertion around totals/VAT behavior if this PR changes invoice math.LocalDateTime.now()can cause flaky tests around time boundaries; consider a fixed timestamp in tests.Also applies to: 120-121, 156-158, 169-171, 182-183, 210-212
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/org/example/entity/invoice/CreateInvoiceDTO.javasrc/main/java/org/example/entity/invoice/InvoiceItemDTO.javasrc/main/java/org/example/entity/invoice/UpdateInvoiceDTO.javasrc/test/java/org/example/InvoiceServiceTest.java
🔇 Additional comments (4)
src/main/java/org/example/entity/invoice/CreateInvoiceDTO.java (1)
28-31: vatRate is intentionally optional—no action neededThe current implementation correctly allows
nullforvatRate. Both theInvoiceentity column (nonullable = false) and therecalcTotals()method (which handles null by defaulting toBigDecimal.ZERO) confirm this is by design. The absence of@NotNullmatches the pattern used elsewhere for optional fields likedueDateinUpdateInvoiceDTO.src/main/java/org/example/entity/invoice/InvoiceItemDTO.java (1)
13-33: The entity-level constraint already prevents NULL names; no legacy data concern exists hereThe
InvoiceItementity already has@Column(name = "name", nullable = false), which enforces the constraint at the database level. Since Hibernate auto-generates the schema based on entity annotations (viaschema-generation.database.action = "update"), NULL names cannot be inserted. The@NotBlankvalidation onInvoiceItemDTOis aligned with this constraint, andfromEntity()is safe—it will never receive NULL values from the database. No migration or backfill is required.src/test/java/org/example/InvoiceServiceTest.java (1)
13-23: No action needed on dependency version. JUnit Jupiter 6.0.1 is a real artifact available on Maven Central with proper distributions (jar, sources, javadoc). The version is supported and correctly specified.Likely an incorrect or invalid review comment.
src/main/java/org/example/entity/invoice/UpdateInvoiceDTO.java (1)
19-22: No change needed—vatRate field design is correct and intentionalThe DTO properly uses
vatRateas a fraction (0–1, e.g., 0.25 for 25%), which is consistent with theInvoiceentity, all related DTOs, service mappings, and validation messages. The entity also includes a separatevatAmountfield that is automatically calculated asvatRate × subTotal. The naming and units are clear and correctly implemented across the codebase.
| @DecimalMin(value = "0.0", inclusive = false, message = "VAT rate must be greater than 0") | ||
| @DecimalMax(value = "1.0", inclusive = true, message = "VAT rate must not exceed 1.0") | ||
| BigDecimal vatRate, | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate and read the UpdateInvoiceDTO file
find . -name "UpdateInvoiceDTO.java" -type fRepository: ithsjava25/project-jpa-grupp-3-d
Length of output: 140
🏁 Script executed:
# Search for usage of UpdateInvoiceDTO to understand the context
rg -t java "UpdateInvoiceDTO" --max-count 20Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 1219
🏁 Script executed:
# Check for any tests related to VAT validation
rg -t java "vatRate|VAT" --max-count 30 | head -50Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 1902
🏁 Script executed:
cat -n ./src/main/java/org/example/entity/invoice/UpdateInvoiceDTO.javaRepository: ithsjava25/project-jpa-grupp-3-d
Length of output: 1024
🏁 Script executed:
# Read Invoice.java to see the server-side validation logic
cat -n ./src/main/java/org/example/entity/invoice/Invoice.java | grep -A 5 -B 5 "vatRate"Repository: ithsjava25/project-jpa-grupp-3-d
Length of output: 1919
@DecimalMin constraint incorrectly blocks 0% VAT, contradicting server-side validation
The @DecimalMin(inclusive = false) constraint rejects 0.0, but Invoice.java line 86 explicitly allows VAT rates >= 0.0. This creates an inconsistency where the DTO validation prevents a scenario the business logic supports. The null-handling semantics are correct (null = "leave unchanged"), but 0% VAT should be a valid value. Update the constraint to inclusive = true and adjust the message to match the intended range.
Required fix
- @DecimalMin(value = "0.0", inclusive = false, message = "VAT rate must be greater than 0")
+ @DecimalMin(value = "0.0", inclusive = true, message = "VAT rate must be at least 0")
@DecimalMax(value = "1.0", inclusive = true, message = "VAT rate must not exceed 1.0")
BigDecimal vatRate,Note: CreateInvoiceDTO.java has the same constraint issue and should also be corrected.
📝 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.
| @DecimalMin(value = "0.0", inclusive = false, message = "VAT rate must be greater than 0") | |
| @DecimalMax(value = "1.0", inclusive = true, message = "VAT rate must not exceed 1.0") | |
| BigDecimal vatRate, | |
| @DecimalMin(value = "0.0", inclusive = true, message = "VAT rate must be at least 0") | |
| @DecimalMax(value = "1.0", inclusive = true, message = "VAT rate must not exceed 1.0") | |
| BigDecimal vatRate, |
🤖 Prompt for AI Agents
In @src/main/java/org/example/entity/invoice/UpdateInvoiceDTO.java around lines
19 - 22, The @DecimalMin on the vatRate field in UpdateInvoiceDTO is currently
set to inclusive = false which incorrectly disallows a 0.0 VAT that the business
logic (Invoice class VAT check) permits; change the @DecimalMin on the
BigDecimal vatRate in UpdateInvoiceDTO to inclusive = true and update the
message to indicate the range allows 0.0 through 1.0 (e.g., "VAT rate must be
between 0.0 and 1.0"), and make the same change to the vatRate field in
CreateInvoiceDTO so DTO validation matches Invoice validation.
2 participants
This PR introduces two major enhancements to the Invoice Management System: descriptive invoice items and manual VAT handling. It also refactors the update logic to support a partial update pattern.
Key Changes:
Descriptive Invoice Items:
Added name (String) to the InvoiceItem entity and corresponding DTOs.
Updated CLI to allow users to input a description (e.g., "Consulting Services") instead of only numeric values.
VAT Support:
Added vatAmount (BigDecimal) to Invoice to allow manual entry of VAT on a per-invoice basis.
Updated CreateInvoiceDTO and UpdateInvoiceDTO to carry VAT data.
Business Logic:
Enhanced recalcTotals() in the Invoice entity to automatically calculate the final amount as Subtotal + vatAmount.
Implemented a Partial Update pattern in InvoiceService. By allowing null values in the UpdateInvoiceDTO, we can now update specific parts of an invoice (like adding an item) without accidentally overwriting existing data like the due date or VAT.
CLI Improvements:
Synchronized all invoice management menus (add, update, remove items) with the new DTO structures.
Technical Notes:
The system now ensures that even when items are cleared and refreshed during an update, the name property is persisted.
The recalcTotals() method acts as the "Single Source of Truth" for the financial integrity of the invoice.
Closes #52
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.