Develop #965
Develop #965
Conversation
ISTE-229: Payment in chronological order based on demand generation d…
WalkthroughThe pull request introduces enhancements across several classes related to water connection services. Key modifications include the addition of a new configuration property for report ordering, improvements in SQL query construction for flexible searches, and refined payment processing logic in ledger reports. These changes collectively aim to enhance the functionality and configurability of the water connection service. Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (8)
municipal-services/ws-services/src/main/java/org/egov/waterconnection/config/WSConfiguration.java (1)
315-317: Consider renaming the field for consistency and fixing a typo.The addition of this configuration property is good, as it allows for flexible report ordering. However, there are a couple of points to consider:
- The field name
isReportRequiredInChronnologicalOrdercontains a typo. "Chronnological" should be "Chronological".- The naming convention for boolean fields in this class typically starts with "is" (e.g.,
isLocalizationStateLevel), but the property name in the@Valueannotation doesn't follow this pattern.Consider applying the following changes:
- @Value("${report.in.chronological.order}") - private boolean isReportRequiredInChronnologicalOrder; + @Value("${is.report.required.in.chronological.order}") + private boolean isReportRequiredInChronologicalOrder;This change will:
- Fix the typo in the field name.
- Align the property name in the
@Valueannotation with the existing naming convention in the class.municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/rowmapper/LedgerReportRowMapper.java (6)
7-7: Remove unused import statementThe import
io.swagger.models.auth.Inon line 4 appears to be unused. Removing unnecessary imports improves code cleanliness and reduces potential confusion.Apply this diff to remove the unused import:
- import io.swagger.models.auth.In;
90-90: Remove unnecessary logging statementThe log statement on line 90 appears to be for debugging purposes and may not be needed in production code.
Apply this diff to remove the log statement:
- log.info("Arrers are "+ledgerReport.getDemand().getArrears()+" and monthandYear"+ ledgerReport.getDemand().getMonthAndYear());
160-163: Add braces for single-line conditional statementsFor better readability and to avoid potential errors during future modifications, it's recommended to use braces
{}even for single-lineif-elsestatements.Apply this diff:
if(config.isReportRequiredInChronnologicalOrder()) - addPaymentToLedgerChronlogicalOrder(monthlyRecordsList); + { + addPaymentToLedgerChronologicalOrder(monthlyRecordsList); + } else - addPaymentToLedger(monthlyRecordsList); + { + addPaymentToLedger(monthlyRecordsList); + }
252-263: Use lambda expressions for comparatorJava 8 and above supports lambda expressions, which can simplify the comparator code and improve readability.
Apply this diff to refactor the comparator using a lambda expression:
- monthlyRecordList.sort(new Comparator<Map<String, Object>>() { - @Override - public int compare(Map<String, Object> o1, Map<String, Object> o2) { - String monthAndYear1 = (String) o1.keySet().iterator().next(); - String monthAndYear2 = (String) o2.keySet().iterator().next(); - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("MMMM yyyy", Locale.ENGLISH); - YearMonth yearMonth1 = YearMonth.parse(monthAndYear1, formatter); - YearMonth yearMonth2 = YearMonth.parse(monthAndYear2, formatter); - return yearMonth1.compareTo(yearMonth2); - } - }); + monthlyRecordList.sort((o1, o2) -> { + String monthAndYear1 = o1.keySet().iterator().next(); + String monthAndYear2 = o2.keySet().iterator().next(); + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("MMMM yyyy", Locale.ENGLISH); + YearMonth yearMonth1 = YearMonth.parse(monthAndYear1, formatter); + YearMonth yearMonth2 = YearMonth.parse(monthAndYear2, formatter); + return yearMonth1.compareTo(yearMonth2); + });
401-426: Add method documentation and adhere to JavaDoc standardsThe comments for the
getDemandGenerationDateOfNextMonthmethod are close to JavaDoc format but missing the@paramand@returnannotations. For consistency and better tooling support, consider updating the comments to follow JavaDoc standards.Apply this diff:
/** - * This method retrieves the demand generation date for the next month in the ledger report. - * If the next month is beyond the list size, it returns 0 to indicate no demand for a future month. - * - * @param monthlyRecordList The list of monthly ledger records. - * @param currentIndex The index of the current month. - * @return The demand generation date of the next month or 0 if there is no next month. - */ + * Retrieves the demand generation date for the next month in the ledger report. + * If the next month is beyond the list size, it returns 0 to indicate no demand for a future month. + * + * @param monthlyRecordList The list of monthly ledger records + * @param currentIndex The index of the current month + * @return The demand generation date of the next month or 0 if there is no next month + */
431-434: Use constants for tax head codes in queriesHardcoding tax head codes like
'WS_ADVANCE_CARRYFORWARD'can lead to errors if the codes change. Consider defining constants or using an enum for tax head codes to improve maintainability.Apply this diff:
+ private static final String TAX_HEAD_WS_ADVANCE_CARRYFORWARD = "WS_ADVANCE_CARRYFORWARD"; ... taxAmountQuery.append(" AND taxheadcode != '" + TAX_HEAD_WS_ADVANCE_CARRYFORWARD + "'");municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/builder/WsQueryBuilder.java (1)
Line range hint
922-923: Prevent SQL Injection by using parameterized queriesIn the methods
getQueryForWCCountbyDemandDateandgetQueryForWCCountForPreviousreadingdate, thetenantIdis concatenated directly into the SQL query:query.append("WHERE d.tenantid = '" +criteria.getTenantId()+"' "); // ... query.append(" AND d.tenantid='"+criteria.getTenantId()+"') ");Concatenating user input directly into SQL queries can lead to SQL injection vulnerabilities. To enhance security, use parameterized queries by replacing the concatenation with placeholders and adding the parameters to the
preparedStatementlist.Apply this diff to fix the issue:
- query.append("WHERE d.tenantid = '" + criteria.getTenantId() + "' "); + query.append("WHERE d.tenantid = ? "); + preparedStatement.add(criteria.getTenantId()); // ... - " and d.tenantid='" + criteria.getTenantId() + "') "); + " and d.tenantid=?) "); + preparedStatement.add(criteria.getTenantId());Ensure to update all occurrences where variables are concatenated into SQL strings within these methods and elsewhere in the code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/config/WSConfiguration.java (1 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/builder/WsQueryBuilder.java (1 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/rowmapper/LedgerReportRowMapper.java (7 hunks)
- municipal-services/ws-services/src/main/resources/application.properties (1 hunks)
🔇 Additional comments (2)
municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/rowmapper/LedgerReportRowMapper.java (2)
58-59: Proper dependency injection ofWSConfigurationInjecting
WSConfigurationusing@Autowiredis appropriate and follows best practices for managing configurations.
106-108: Ensure correct calculation of tax amounts withpaymentExistsflagWith the introduction of the
paymentExistsparameter ingetMonthlyTaxAmount, verify that the method correctly calculates the tax amount based on whether payments have been made. This change alters the query behavior and could affect the accuracy of the tax calculations.
|
|
||
| #ledger-report-format | ||
| report.in.chronological.order=true |
There was a problem hiding this comment.
💡 Codebase verification
Unutilized property report.in.chronological.order detected.
The property report.in.chronological.order=true is defined in application.properties and injected into WSConfiguration.java. However, it is not utilized anywhere within the codebase, which may indicate redundancy or an incomplete implementation.
Please consider the following actions:
- Remove the unused property if it is not required to avoid unnecessary configuration.
- Verify the intended use of this property to ensure that its implementation aligns with the project's objectives and to complete any pending functionality.
🔗 Analysis chain
Confirm the impact of enabling chronological ordering for ledger reports.
The addition of report.in.chronological.order=true is noted. This change appears to enable chronological ordering for ledger reports, which could affect how reports are generated and displayed in the application.
Please address the following points:
- Confirm that this change is intentional and aligns with the PR objectives.
- Verify the expected behavior of reports with this setting enabled.
- Update any relevant documentation or user guides to reflect this change in report ordering.
- Ensure thorough testing of affected reports to validate the impact of this change.
To help verify the impact of this change, you can run the following script to search for usage of this property in the codebase:
This will help identify where the property is being used and potentially affected areas of the code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of the new property in the codebase
rg --type java --type kotlin "report\.in\.chronological\.order"
Length of output: 315
Script:
#!/bin/bash
# Search for all usages of the 'report.in.chronological.order' property in Java and Kotlin files
rg --type java --type kotlin "report\.in\.chronological\.order"
Length of output: 315
Script:
#!/bin/bash
# Find the variable name injected with 'report.in.chronological.order' in WSConfiguration.java
grep -E '@Value\("\${report\.in\.chronological\.order}"\)' municipal-services/ws-services/src/main/java/org/egov/waterconnection/config/WSConfiguration.java
# Assuming the variable name is 'reportInChronologicalOrder', search for its usages within the file
rg 'reportInChronologicalOrder' municipal-services/ws-services/src/main/java/org/egov/waterconnection/config/WSConfiguration.java
Length of output: 332
|
|
||
| ledgerReport.setTotalBalanceLeftInMonth(ledgerReport.getDemand().getTotal_due_amount().subtract(totalPaymentInMonth)); | ||
| ledgerReport.setTotalPaymentInMonth(totalPaymentInMonth); | ||
| } | ||
| // Keep track of the last valid demand (non-zero demandGenerationDate) | ||
| if (ledgerReport.getDemand().getDemandGenerationDate() != 0) { | ||
| log.info("Last Valid Demand monnth:"+ledgerReport.getDemand()); | ||
| lastValidDemandReport = ledgerReport; | ||
| } | ||
| if (!paymentMatched ) { | ||
| // Add a default PaymentLedgerReport if no payments matched | ||
| log.info("If not matched:"+ledgerReport.getDemand().getMonthAndYear()); | ||
| PaymentLedgerReport defaultPaymentLedgerReport = new PaymentLedgerReport(); | ||
| defaultPaymentLedgerReport.setCollectionDate(null); | ||
| defaultPaymentLedgerReport.setReceiptNo("N/A"); | ||
| defaultPaymentLedgerReport.setPaid(BigDecimal.ZERO); | ||
| defaultPaymentLedgerReport.setBalanceLeft(ledgerReport.getDemand().getTotal_due_amount()); | ||
|
|
||
| if (ledgerReport.getPayment() == null) { | ||
| ledgerReport.setPayment(new ArrayList<>()); | ||
| } | ||
| ledgerReport.getPayment().add(defaultPaymentLedgerReport); | ||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
||
| // Handle payments for months with no valid demands | ||
| if (lastValidDemandReport != null) { | ||
| log.info("Last month logic:"+lastValidDemandReport); | ||
| // Assign payments to the last valid demand if found | ||
| List<Payment> payments = addPaymentDetails(lastValidDemandReport.getDemand().getConnectionNo()); | ||
| BigDecimal totalPaymentInMonthlastValidMonth = BigDecimal.ZERO; | ||
| BigDecimal totalBalanceLeftInMonthLatValidMonth=BigDecimal.ZERO; | ||
| lastValidDemandReport.getPayment().clear(); | ||
| boolean ifLastDemandHavePayments= false; | ||
|
|
||
| if (payments != null && !payments.isEmpty()) { | ||
|
|
||
| for (Payment payment : payments) { | ||
| Long transactionDateLong = payment.getTransactionDate(); | ||
| if (transactionDateLong >= lastValidDemandReport.getDemand().getDemandGenerationDate()) { | ||
| PaymentLedgerReport paymentLedgerReport = new PaymentLedgerReport(); | ||
| paymentLedgerReport.setCollectionDate(transactionDateLong); | ||
| paymentLedgerReport.setReceiptNo(payment.getPaymentDetails().get(0).getReceiptNumber()); | ||
| paymentLedgerReport.setPaid(payment.getTotalAmountPaid()); | ||
| paymentLedgerReport.setBalanceLeft(payment.getTotalDue().subtract(paymentLedgerReport.getPaid())); | ||
|
|
||
| lastValidDemandReport.getPayment().add(paymentLedgerReport); | ||
| totalPaymentInMonthlastValidMonth = totalPaymentInMonthlastValidMonth.add(payment.getTotalAmountPaid()); | ||
| totalBalanceLeftInMonthLatValidMonth = totalBalanceLeftInMonthLatValidMonth.add(payment.getTotalDue().subtract(payment.getTotalAmountPaid())); | ||
| ifLastDemandHavePayments=true; | ||
| } | ||
| } | ||
| lastValidDemandReport.setTotalBalanceLeftInMonth(lastValidDemandReport.getDemand().getTotal_due_amount().subtract(totalPaymentInMonthlastValidMonth)); | ||
| lastValidDemandReport.setTotalPaymentInMonth(totalPaymentInMonthlastValidMonth); | ||
|
|
||
| } | ||
| if(!ifLastDemandHavePayments){ | ||
| // Add a default PaymentLedgerReport if no payments matched | ||
| log.info("If not matched:"+lastValidDemandReport.getDemand().getMonthAndYear()); | ||
| PaymentLedgerReport defaultPaymentLedgerReport = new PaymentLedgerReport(); | ||
| defaultPaymentLedgerReport.setCollectionDate(null); | ||
| defaultPaymentLedgerReport.setReceiptNo("N/A"); | ||
| defaultPaymentLedgerReport.setPaid(BigDecimal.ZERO); | ||
| defaultPaymentLedgerReport.setBalanceLeft(lastValidDemandReport.getDemand().getTotal_due_amount()); | ||
|
|
||
| if (lastValidDemandReport.getPayment() == null) { | ||
| lastValidDemandReport.setPayment(new ArrayList<>()); | ||
| } | ||
| lastValidDemandReport.getPayment().add(defaultPaymentLedgerReport); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * This method retrieves the demand generation date for the next month in the ledger report. | ||
| * If the next month is beyond the list size, it returns 0 to indicate no demand for a future month. | ||
| * | ||
| * @param monthlyRecordList The list of monthly ledger records. | ||
| * @param currentIndex The index of the current month. | ||
| * @return The demand generation date of the next month or 0 if there is no next month. | ||
| */ | ||
| private Long getDemandGenerationDateOfNextMonth(List<Map<String, Object>> monthlyRecordList, int currentIndex) { | ||
| // Check if there is a next month in the list | ||
| if (currentIndex + 1 < monthlyRecordList.size()) { | ||
| // Get the next month's record | ||
| Map<String, Object> nextMonthRecord = monthlyRecordList.get(currentIndex + 1); | ||
|
|
||
| // Extract the LedgerReport object for the next month | ||
| LedgerReport nextMonthLedgerReport = (LedgerReport) nextMonthRecord.values().iterator().next(); | ||
|
|
||
| // Return the demand generation date of the next month | ||
| if (nextMonthLedgerReport != null && nextMonthLedgerReport.getDemand() != null) { | ||
| return nextMonthLedgerReport.getDemand().getDemandGenerationDate(); | ||
| } | ||
| } | ||
| // Return 0 if there is no next month or no valid demand for the next month | ||
| return 0L; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor the addPaymentToLedgerChronologicalOrder method for maintainability
The addPaymentToLedgerChronologicalOrder method is lengthy and complex, which can make it difficult to understand and maintain. Consider refactoring it into smaller, well-named helper methods to enhance readability.
| @@ -150,7 +157,11 @@ public List<Map<String, Object>> extractData(ResultSet resultSet) throws SQLExce | |||
| } | |||
| log.info("ledger report list" + monthlyRecordsList); | |||
| if (!monthlyRecordsList.isEmpty()) { | |||
| addPaymentToLedger(monthlyRecordsList); | |||
| if(config.isReportRequiredInChronnologicalOrder()) | |||
| addPaymentToLedgerChronlogicalOrder(monthlyRecordsList); | |||
There was a problem hiding this comment.
Correct typo in method name addPaymentToLedgerChronlogicalOrder
There is a typo in the method name addPaymentToLedgerChronlogicalOrder. The correct spelling is addPaymentToLedgerChronologicalOrder. This typo occurs in both the method call and its definition.
Apply this diff to correct the method name:
- addPaymentToLedgerChronlogicalOrder(monthlyRecordsList);
+ addPaymentToLedgerChronologicalOrder(monthlyRecordsList);Also, update the method definition accordingly:
- private void addPaymentToLedgerChronlogicalOrder(List<Map<String, Object>> monthlyRecordList) {
+ private void addPaymentToLedgerChronologicalOrder(List<Map<String, Object>> monthlyRecordList) {📝 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.
| addPaymentToLedgerChronlogicalOrder(monthlyRecordsList); | |
| addPaymentToLedgerChronologicalOrder(monthlyRecordsList); |
| @@ -4,6 +4,7 @@ | |||
| import io.swagger.models.auth.In; | |||
| import lombok.Setter; | |||
| import lombok.extern.slf4j.Slf4j; | |||
| import org.egov.waterconnection.config.WSConfiguration; | |||
There was a problem hiding this comment.
Add missing import for Locale
The class uses Locale.ENGLISH in date formatting but does not import java.util.Locale. This could lead to a Compilation Error.
Apply this diff to add the necessary import:
+ import java.util.Locale;📝 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.
| import org.egov.waterconnection.config.WSConfiguration; | |
| import org.egov.waterconnection.config.WSConfiguration; | |
| import java.util.Locale; |
| @@ -199,7 +210,7 @@ private void addPaymentToLedger(List<Map<String, Object>> monthlyRecordList) { | |||
| Long transactionDateLong = payment.getTransactionDate(); | |||
| LocalDate transactionDate = Instant.ofEpochMilli(transactionDateLong).atZone(ZoneId.systemDefault()).toLocalDate(); | |||
| String transactionMonthAndYear = transactionDate.format(DateTimeFormatter.ofPattern("MMMM yyyy")); | |||
| if (ledgerReport.getDemand().getMonthAndYear().equals(transactionMonthAndYear)) { | |||
| if (ledgerReport.getDemand().getDemandGenerationDate().compareTo(transactionDateLong)<0 ) { | |||
There was a problem hiding this comment.
Potential logic error in payment matching condition
The condition ledgerReport.getDemand().getDemandGenerationDate().compareTo(transactionDateLong) < 0 on line 213 may incorrectly filter payments. Typically, you may want to include payments made on the same day (<= 0) or ensure the logic aligns with business requirements.
Apply this diff if the intention is to include payments made on the same day:
- if (ledgerReport.getDemand().getDemandGenerationDate().compareTo(transactionDateLong) < 0 ) {
+ if (ledgerReport.getDemand().getDemandGenerationDate().compareTo(transactionDateLong) <= 0 ) {📝 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.
| if (ledgerReport.getDemand().getDemandGenerationDate().compareTo(transactionDateLong)<0 ) { | |
| if (ledgerReport.getDemand().getDemandGenerationDate().compareTo(transactionDateLong) <= 0 ) { |
|
|
||
| LedgerReport ledgerReport = ledgerReports.get(monthAndYear); | ||
| ledgerReport.getDemand().setArrears(taxAmountResult.subtract(totalAmountPaidResult)); |
There was a problem hiding this comment.
Handle potential NullPointerException when setting arrears
The method taxAmountResult.subtract(totalAmountPaidResult) could throw a NullPointerException if either taxAmountResult or totalAmountPaidResult is null. Ensure both variables are non-null before performing the subtraction.
Apply this diff to add null checks:
- ledgerReport.getDemand().setArrears(taxAmountResult.subtract(totalAmountPaidResult));
+ BigDecimal arrears = (taxAmountResult != null ? taxAmountResult : BigDecimal.ZERO)
+ .subtract(totalAmountPaidResult != null ? totalAmountPaidResult : BigDecimal.ZERO);
+ ledgerReport.getDemand().setArrears(arrears);📝 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.
| ledgerReport.getDemand().setArrears(taxAmountResult.subtract(totalAmountPaidResult)); | |
| BigDecimal arrears = (taxAmountResult != null ? taxAmountResult : BigDecimal.ZERO) | |
| .subtract(totalAmountPaidResult != null ? totalAmountPaidResult : BigDecimal.ZERO); | |
| ledgerReport.getDemand().setArrears(arrears); |
| Long nextMonthDemGenDateLong =getDemandGenerationDateOfNextMonth(monthlyRecordList, i); | ||
| log.info("nextMonthDemGenDateLong:"+nextMonthDemGenDateLong); | ||
| // Check if the payment date falls on or after the current demand's generation date | ||
| if (transactionDateLong >= currentDemandDate && | ||
| (i + 1 == monthlyRecordList.size() || transactionDateLong < nextMonthDemGenDateLong )) { |
There was a problem hiding this comment.
Handle potential division by zero or null values
In the payment date checks, ensure that nextMonthDemGenDateLong is not zero to prevent logical errors. If nextMonthDemGenDateLong can be zero, additional checks are needed to handle that scenario appropriately.
Apply this diff to add a check for zero values:
if (transactionDateLong >= currentDemandDate &&
- (i + 1 == monthlyRecordList.size() || transactionDateLong < nextMonthDemGenDateLong)) {
+ (i + 1 == monthlyRecordList.size() || (nextMonthDemGenDateLong != 0 && transactionDateLong < nextMonthDemGenDateLong))) {📝 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.
| Long nextMonthDemGenDateLong =getDemandGenerationDateOfNextMonth(monthlyRecordList, i); | |
| log.info("nextMonthDemGenDateLong:"+nextMonthDemGenDateLong); | |
| // Check if the payment date falls on or after the current demand's generation date | |
| if (transactionDateLong >= currentDemandDate && | |
| (i + 1 == monthlyRecordList.size() || transactionDateLong < nextMonthDemGenDateLong )) { | |
| Long nextMonthDemGenDateLong =getDemandGenerationDateOfNextMonth(monthlyRecordList, i); | |
| log.info("nextMonthDemGenDateLong:"+nextMonthDemGenDateLong); | |
| // Check if the payment date falls on or after the current demand's generation date | |
| if (transactionDateLong >= currentDemandDate && | |
| (i + 1 == monthlyRecordList.size() || (nextMonthDemGenDateLong != 0 && transactionDateLong < nextMonthDemGenDateLong))) { |
| if(!ifLastDemandHavePayments){ | ||
| // Add a default PaymentLedgerReport if no payments matched | ||
| log.info("If not matched:"+lastValidDemandReport.getDemand().getMonthAndYear()); | ||
| PaymentLedgerReport defaultPaymentLedgerReport = new PaymentLedgerReport(); | ||
| defaultPaymentLedgerReport.setCollectionDate(null); | ||
| defaultPaymentLedgerReport.setReceiptNo("N/A"); | ||
| defaultPaymentLedgerReport.setPaid(BigDecimal.ZERO); | ||
| defaultPaymentLedgerReport.setBalanceLeft(lastValidDemandReport.getDemand().getTotal_due_amount()); | ||
|
|
||
| if (lastValidDemandReport.getPayment() == null) { | ||
| lastValidDemandReport.setPayment(new ArrayList<>()); | ||
| } | ||
| lastValidDemandReport.getPayment().add(defaultPaymentLedgerReport); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid code duplication when adding default payment ledger reports
The logic for adding a default PaymentLedgerReport when no payments are matched is repeated multiple times in the code. Consider extracting this logic into a separate method to adhere to the DRY (Don't Repeat Yourself) principle.
Apply this diff to create a helper method:
+ private void addDefaultPaymentLedgerReport(LedgerReport ledgerReport, BigDecimal totalDueAmount) {
+ PaymentLedgerReport defaultPaymentLedgerReport = new PaymentLedgerReport();
+ defaultPaymentLedgerReport.setCollectionDate(null);
+ defaultPaymentLedgerReport.setReceiptNo("N/A");
+ defaultPaymentLedgerReport.setPaid(BigDecimal.ZERO);
+ defaultPaymentLedgerReport.setBalanceLeft(totalDueAmount);
+ if (ledgerReport.getPayment() == null) {
+ ledgerReport.setPayment(new ArrayList<>());
+ }
+ ledgerReport.getPayment().add(defaultPaymentLedgerReport);
+ }
...
- // Existing code where default payment ledger report is added
- PaymentLedgerReport defaultPaymentLedgerReport = new PaymentLedgerReport();
- defaultPaymentLedgerReport.setCollectionDate(null);
- defaultPaymentLedgerReport.setReceiptNo("N/A");
- defaultPaymentLedgerReport.setPaid(BigDecimal.ZERO);
- defaultPaymentLedgerReport.setBalanceLeft(ledgerReport.getDemand().getTotal_due_amount());
- if (ledgerReport.getPayment() == null) {
- ledgerReport.setPayment(new ArrayList<>());
- }
- ledgerReport.getPayment().add(defaultPaymentLedgerReport);
+ // Use the helper method
+ addDefaultPaymentLedgerReport(ledgerReport, ledgerReport.getDemand().getTotal_due_amount());Committable suggestion was skipped due to low confidence.
|
|
||
| // Return the demand generation date of the next month | ||
| if (nextMonthLedgerReport != null && nextMonthLedgerReport.getDemand() != null) { | ||
| return nextMonthLedgerReport.getDemand().getDemandGenerationDate(); |
There was a problem hiding this comment.
Handle potential NullPointerException when accessing demand generation date
In the getDemandGenerationDateOfNextMonth method, ensure that getDemandGenerationDate() does not return null to prevent a NullPointerException.
Apply this diff to add a null check:
if (nextMonthLedgerReport != null && nextMonthLedgerReport.getDemand() != null) {
+ if (nextMonthLedgerReport.getDemand().getDemandGenerationDate() != null) {
return nextMonthLedgerReport.getDemand().getDemandGenerationDate();
+ } else {
+ return 0L;
+ }
}Committable suggestion was skipped due to low confidence.
| @@ -208,7 +208,7 @@ public class WsQueryBuilder { | |||
| "ORDER BY startdate"; | |||
|
|
|||
| public static final String TAX_AMOUNT_QUERY="SELECT SUM(taxamount) FROM egbs_demanddetail_v1 WHERE " + | |||
| "demandid IN (SELECT id FROM egbs_demand_v1 WHERE consumercode = ? AND taxperiodto < ? AND status='ACTIVE') and taxheadcode!= 'WS_ADVANCE_CARRYFORWARD';"; | |||
| "demandid IN (SELECT id FROM egbs_demand_v1 WHERE consumercode = ? AND taxperiodto < ? AND status='ACTIVE') "; | |||
There was a problem hiding this comment.
Ensure 'taxperiodto' parameter is added to the prepared statement
In the TAX_AMOUNT_QUERY, the subquery includes a placeholder for taxperiodto:
"demandid IN (SELECT id FROM egbs_demand_v1 WHERE consumercode = ? AND taxperiodto < ? AND status='ACTIVE') ";Make sure that the taxperiodto parameter is properly added to the preparedStatement list when executing this query to prevent potential issues with parameter binding and to ensure the query executes correctly.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores