Add SQL for Invoice by Pipatq · Pull Request #21 · f-midterm/project-ffinal

Pipatq · 2025-11-14T18:22:08Z

No description provided.

Copilot

Pull Request Overview

This PR implements a comprehensive invoice management system that replaces individual payment creation with a master invoice approach. The system generates invoices with a structured format (INV-YYYYMMDD-XXX) that contain multiple payment line items (rent, electricity, water) grouped under a single invoice number.

Introduces Invoice entity with one-to-many relationship to Payment entities
Adds InvoiceService for creating invoices with automatic invoice number generation
Implements PDF generation functionality for lease agreements/invoices
Updates frontend to use the new invoice creation API instead of individual payment creation

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`backend/init.sql`	Adds `invoices` table schema with proper constraints, indexes, and foreign keys; modifies `payments` table to link to invoices
`backend/src/main/java/apartment/example/backend/entity/Invoice.java`	New entity representing master invoices with bidirectional relationship to payments and lifecycle callbacks
`backend/src/main/java/apartment/example/backend/entity/Payment.java`	Adds optional invoice reference to link payments as line items under invoices
`backend/src/main/java/apartment/example/backend/repository/InvoiceRepository.java`	New repository with queries for invoice lookup, counting, and duplicate checking
`backend/src/main/java/apartment/example/backend/service/InvoiceService.java`	Core business logic for invoice creation with payment line items and unique invoice number generation
`backend/src/main/java/apartment/example/backend/service/PaymentService.java`	Exposes receipt number generation method for use by InvoiceService
`backend/src/main/java/apartment/example/backend/service/PdfService.java`	New service for generating PDF invoices using iText library with payment details
`backend/src/main/java/apartment/example/backend/controller/InvoiceController.java`	REST endpoints for invoice creation and retrieval operations
`backend/src/main/java/apartment/example/backend/controller/LeaseController.java`	Adds PDF generation endpoint for lease agreements
`backend/build.gradle`	Adds iText PDF library dependency for document generation
`frontend/src/api/services/invoices.service.js`	New service module with API calls for invoice operations
`frontend/src/pages/admin/unit/send_bill/page.jsx`	Refactored to use invoice creation API instead of creating individual payments; displays generated invoice number

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-14T18:27:43Z

backend/src/main/java/apartment/example/backend/controller/InvoiceController.java

+
+            return ResponseEntity.ok(invoice);
+        } catch (RuntimeException e) {
+            return ResponseEntity.badRequest().body(null);


Inconsistent error handling. The catch blocks for RuntimeException return ResponseEntity.badRequest().body(null) (line 92), which returns a 400 Bad Request with a null body. This provides no information about what went wrong. Consider returning a proper error message or using a standard error response format to help clients understand the failure.

Suggested change

return ResponseEntity.badRequest().body(null);

Map<String, String> errorBody = Map.of("error", e.getMessage());

return ResponseEntity.badRequest().body(errorBody);

Copilot · 2025-11-14T18:27:43Z

backend/init.sql

 DROP TABLE IF EXISTS payments;
+DROP TABLE IF EXISTS invoices;


The SQL DROP TABLE order is incorrect. The invoices table is being dropped before the payments table, but payments has a foreign key reference to invoices. This will cause a foreign key constraint error during table drops. The payments table should be dropped before invoices.

Suggested change

DROP TABLE IF EXISTS payments;

DROP TABLE IF EXISTS invoices;

DROP TABLE IF EXISTS invoices;

DROP TABLE IF EXISTS payments;

Copilot · 2025-11-14T18:27:43Z

backend/src/main/java/apartment/example/backend/service/InvoiceService.java

+    private String generateInvoiceNumber(LocalDate invoiceDate) {
+        // Format date as YYYYMMDD
+        String dateStr = invoiceDate.format(DateTimeFormatter.ofPattern("yyyyMMdd"));
+
+        // Get count of invoices for this date
+        long count = invoiceRepository.countByInvoiceDate(invoiceDate);
+
+        // Generate sequential number
+        long sequenceNumber = count + 1;
+
+        // Format: INV-YYYYMMDD-XXX
+        String invoiceNumber = String.format("INV-%s-%d", dateStr, sequenceNumber);
+
+        // Check if exists (race condition protection)
+        while (invoiceRepository.existsByInvoiceNumber(invoiceNumber)) {
+            sequenceNumber++;
+            invoiceNumber = String.format("INV-%s-%d", dateStr, sequenceNumber);
+        }
+
+        return invoiceNumber;
+    }


Potential race condition in invoice number generation. While there's a check for existing invoice numbers using existsByInvoiceNumber, there's a window between counting invoices and saving where another thread could create an invoice with the same number. Consider using database-level unique constraints (already present) with retry logic, or using a database sequence/lock to ensure uniqueness.

Copilot · 2025-11-14T18:27:44Z

backend/src/main/java/apartment/example/backend/service/PdfService.java

+            List<Invoice> invoices = invoiceRepository.findByLeaseId(lease.getId());
+            String invoiceNumber = invoices.isEmpty() ? 
+                String.format("INV-%05d", lease.getId()) : 
+                invoices.get(invoices.size() - 1).getInvoiceNumber(); // Get latest invoice
+
+            LocalDate invoiceDate = invoices.isEmpty() ? 
+                LocalDate.now() : 
+                invoices.get(invoices.size() - 1).getInvoiceDate();
+
+            LocalDate dueDate = invoices.isEmpty() ? 
+                lease.getEndDate() : 
+                invoices.get(invoices.size() - 1).getDueDate();


Logic error when retrieving the latest invoice. The code uses invoices.get(invoices.size() - 1) which gets the last element in the list, but the list order depends on the database query order which may not be guaranteed. Although there's a method findByInvoiceDateOrderByCreatedAtDesc in the repository, this code uses findByLeaseId which doesn't specify ordering. Use the ordered query method or add explicit ordering to ensure you're getting the latest invoice.

Suggested change

List<Invoice> invoices = invoiceRepository.findByLeaseId(lease.getId());

String invoiceNumber = invoices.isEmpty() ?

String.format("INV-%05d", lease.getId()) :

invoices.get(invoices.size() - 1).getInvoiceNumber(); // Get latest invoice

LocalDate invoiceDate = invoices.isEmpty() ?

LocalDate.now() :

invoices.get(invoices.size() - 1).getInvoiceDate();

LocalDate dueDate = invoices.isEmpty() ?

lease.getEndDate() :

invoices.get(invoices.size() - 1).getDueDate();

List<Invoice> invoices = invoiceRepository.findByLeaseIdOrderByCreatedAtDesc(lease.getId());

String invoiceNumber = invoices.isEmpty() ?

String.format("INV-%05d", lease.getId()) :

invoices.get(0).getInvoiceNumber(); // Get latest invoice

LocalDate invoiceDate = invoices.isEmpty() ?

LocalDate.now() :

invoices.get(0).getInvoiceDate();

LocalDate dueDate = invoices.isEmpty() ?

lease.getEndDate() :

invoices.get(0).getDueDate();

Copilot · 2025-11-14T18:27:44Z

backend/src/main/java/apartment/example/backend/controller/InvoiceController.java

+    public static class CreateInvoiceRequest {
+        private Long leaseId;
+        private LocalDate invoiceDate;
+        private LocalDate dueDate;
+        private BigDecimal rentAmount;
+        private BigDecimal electricityAmount;
+        private BigDecimal waterAmount;
+        private String notes;


Missing validation for required fields in CreateInvoiceRequest. The fields leaseId, invoiceDate, and dueDate are required for invoice creation but there's no validation to ensure they are not null before processing. This could lead to NullPointerException when these fields are accessed in the service layer.

Copilot · 2025-11-14T18:27:44Z

backend/src/main/java/apartment/example/backend/service/PaymentService.java

    }

-    private String generateReceiptNumber(PaymentType paymentType) {
+    public String generateReceiptNumber(PaymentType paymentType) {


[nitpick] The visibility change of generateReceiptNumber from private to public is correct for allowing InvoiceService to use it. However, this exposes the method to all callers. Consider whether this method should be part of a shared utility or if it's appropriate to make it public. If it's only meant for internal service-to-service calls, document this in a comment or consider using package-private visibility.

Suggested change

public String generateReceiptNumber(PaymentType paymentType) {

/**

* Generates a receipt number for a payment.

* Intended for internal service-to-service use only.

* Package-private to restrict access to this package.

*/

String generateReceiptNumber(PaymentType paymentType) {

Copilot · 2025-11-14T18:27:44Z

backend/src/main/java/apartment/example/backend/service/InvoiceService.java

+    public static class PaymentItem {
+        private PaymentType paymentType;
+        private BigDecimal amount;
+        private String description;
+
+        public PaymentItem() {
+        }
+
+        public PaymentItem(PaymentType paymentType, BigDecimal amount, String description) {
+            this.paymentType = paymentType;
+            this.amount = amount;
+            this.description = description;
+        }
+
+        public PaymentType getPaymentType() {
+            return paymentType;
+        }
+
+        public void setPaymentType(PaymentType paymentType) {
+            this.paymentType = paymentType;
+        }
+
+        public BigDecimal getAmount() {
+            return amount;
+        }
+
+        public void setAmount(BigDecimal amount) {
+            this.amount = amount;
+        }
+
+        public String getDescription() {
+            return description;
+        }
+
+        public void setDescription(String description) {
+            this.description = description;
+        }
+    }


[nitpick] The InvoiceService.PaymentItem inner class lacks proper encapsulation and validation. While it has getters and setters, there's no validation to ensure that amount is positive or that paymentType is not null. Consider adding validation in the setters or making this an immutable class with validation in the constructor.

Copilot · 2025-11-14T18:27:45Z

backend/src/main/java/apartment/example/backend/service/PdfService.java

+            // Get payments for this lease to get payment details
+            List<Payment> payments = paymentRepository.findByLeaseId(lease.getId());


[nitpick] Potential memory issue with fetching all payments. The code uses paymentRepository.findByLeaseId(lease.getId()) which could return a large number of payment records if the lease has been active for a long time. For production use, consider adding pagination or filtering to only fetch recent/relevant payments, or optimize the query to only retrieve necessary fields for the PDF generation.

Suggested change

// Get payments for this lease to get payment details

List<Payment> payments = paymentRepository.findByLeaseId(lease.getId());

// Get the most recent payment for this lease to get payment details

Payment latestPayment = paymentRepository.findTopByLeaseIdOrderByPaymentDateDesc(lease.getId());

Copilot · 2025-11-14T18:27:45Z

backend/init.sql

+    INDEX idx_due_date (due_date),
+    INDEX idx_status (status),
+    CONSTRAINT chk_invoice_total_amount CHECK (total_amount > 0),
+    CONSTRAINT chk_invoice_dates CHECK (due_date >= invoice_date)


[nitpick] The constraint chk_invoice_dates uses >= which allows invoice date and due date to be the same. While this might be intentional, it's unusual in business contexts where typically a grace period is expected between invoice issuance and payment due date. Consider if this should be > instead to enforce at least one day difference.

Suggested change

CONSTRAINT chk_invoice_dates CHECK (due_date >= invoice_date)

CONSTRAINT chk_invoice_dates CHECK (due_date > invoice_date)

Copilot · 2025-11-14T18:27:45Z

frontend/src/pages/admin/unit/send_bill/page.jsx

            // Success - navigate back
-            console.log('✅ Bills sent successfully!');
-            alert('✅ ส่งบิลให้ผู้เช่าเรียบร้อยแล้ว');
+            alert(`✅ ส่งบิลเรียบร้อยแล้ว\nเลขที่ใบแจ้งหนี้: ${invoice.invoiceNumber}`);


Missing error handling for the case where the invoice response doesn't contain an invoiceNumber field. If the API returns an invoice without this field, the code will attempt to access invoice.invoiceNumber which could be undefined, leading to runtime errors in the alert message.

Add SQL for Invoice

a8ae869

Pipatq requested a review from Copilot November 14, 2025 18:22

Copilot started reviewing on behalf of Pipatq November 14, 2025 18:22 View session

Copilot finished reviewing on behalf of Pipatq November 14, 2025 18:24

Copilot AI reviewed Nov 14, 2025

View reviewed changes

Base automatically changed from develop/frontend/admin/maintenance-requests to feature/handle/jwt November 15, 2025 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SQL for Invoice#21

Add SQL for Invoice#21
Pipatq wants to merge 1 commit intofeature/handle/jwtfrom
backend/api/pdfservice

Pipatq commented Nov 14, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	return ResponseEntity.badRequest().body(null);
	Map<String, String> errorBody = Map.of("error", e.getMessage());
	return ResponseEntity.badRequest().body(errorBody);

		DROP TABLE IF EXISTS payments;
		DROP TABLE IF EXISTS invoices;

-    public String generateReceiptNumber(PaymentType paymentType) {
+    /**
+     * Generates a receipt number for a payment.
+     * Intended for internal service-to-service use only.
+     * Package-private to restrict access to this package.
+     */
+    String generateReceiptNumber(PaymentType paymentType) {

		// Get payments for this lease to get payment details
		List<Payment> payments = paymentRepository.findByLeaseId(lease.getId());

	CONSTRAINT chk_invoice_dates CHECK (due_date >= invoice_date)
	CONSTRAINT chk_invoice_dates CHECK (due_date > invoice_date)

Conversation

Pipatq commented Nov 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant