Condition to not update createdtime in update demand call for reindexing #969
Conversation
WalkthroughThe changes in this pull request primarily involve modifications to various service classes and a utility class within the billing service. The Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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: 2
🧹 Outside diff range and nitpick comments (6)
business-services/billing-service/src/main/java/org/egov/demand/util/Util.java (1)
Line range hint
1-427: Summary of changes and potential impactThe main change in this file is the modification of the
getAuditDetailmethod in theUtilclass. This change allows for more flexible creation of audit details, distinguishing between create and update operations. While this change improves the functionality, it has the following potential impacts:
- It introduces a backwards-incompatible change to the method's signature.
- It may require updates to all parts of the codebase that call this method.
- It might affect the behavior of audit trails in the system, depending on how it's used.
Please ensure that all callers of this method are updated and that the new behavior aligns with the expected audit trail requirements across the system.
Consider the following architectural advice:
- If this change is part of a larger refactoring effort to improve audit trails, ensure that it's consistently applied across all relevant services.
- Consider creating a separate AuditService if audit-related functionality grows more complex in the future.
- Evaluate if this change aligns with any existing audit policies or regulations that the system needs to comply with.
business-services/billing-service/src/main/java/org/egov/demand/service/BillService.java (1)
365-365: Summary: Updated audit detail generation for billsThe change to
util.getAuditDetail(requestInfo, true)introduces a new boolean parameter in the audit detail generation process for bills. While this change is minimal, it may have implications on how audit information is recorded and processed throughout the system.Recommendations:
- Ensure that the
Util.getAuditDetailmethod correctly handles this new boolean parameter.- Verify that all other calls to
getAuditDetailacross the codebase have been updated consistently.- Update any relevant documentation or comments related to audit detail generation.
- Consider adding a comment explaining the purpose of the
trueparameter for future maintainability.business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java (2)
Line range hint
638-1022: Consider refactoring thegetAllDemandsmethodThe
getAllDemandsmethod is quite long and complex, making it difficult to understand and maintain. Consider breaking it down into smaller, more focused methods to improve readability and maintainability.Some suggestions for refactoring:
- Extract the demand filtering logic into a separate method.
- Create a method for calculating various totals (e.g.,
currentmonthBill,totalAreas, etc.).- Consider using the Builder pattern for constructing the
AggregatedDemandDetailResponse.Example of extracting demand filtering logic:
private List<Demand> filterActiveDemands(List<Demand> demands) { return demands.stream() .filter(demand -> demand.getStatus().equals(Demand.StatusEnum.ACTIVE)) .collect(Collectors.toList()); }
Line range hint
638-1022: Add comments to explain complex stream operationsThere are multiple instances of complex stream operations and business logic throughout the
getAllDemandsmethod. Adding comments to explain the purpose and logic of these operations would greatly improve code readability and maintainability.For example, consider adding comments before complex stream operations like:
// Calculate the total areas without penalty for specific tax head codes totalAreas = remainingMonthDemandDetailList.stream() .filter(dd -> taxHeadCodesToFilterWithoutPenalty.contains(dd.getTaxHeadMasterCode())) .map(dd -> dd.getTaxAmount().subtract(dd.getCollectionAmount())) .reduce(BigDecimal.ZERO, BigDecimal::add);business-services/billing-service/src/main/java/org/egov/demand/web/validator/AmendmentValidator.java (2)
Line range hint
237-237: Typo in method name 'getIsAmendmentworkflowEnabed'The method
getIsAmendmentworkflowEnabed()appears to have a typo in the word "Enabed". It should likely begetIsAmendmentWorkflowEnabled().This occurs in the following lines:
- Line 237:
if (props.getIsAmendmentworkflowEnabed()) {- Line 246:
if (!props.getIsAmendmentworkflowEnabed()) {Apply this diff to correct the method name:
- if (props.getIsAmendmentworkflowEnabed()) { + if (props.getIsAmendmentWorkflowEnabled()) {Ensure that the method name is updated in the
ApplicationPropertiesclass and any other references to maintain consistency.Also applies to: 246-246
Line range hint
263-263: Typographical error in error messageThere's a typo in the error message string:
- Current:
"Mandatory workflow fileds missing in the update request, Please add all the following fields module, businessservice, businessid and action"- Corrected:
"Mandatory workflow fields missing in the update request. Please add all the following fields: module, businessservice, businessid, and action"Apply this diff to correct the typo and improve clarity:
- "Mandatory workflow fileds missing in the update request, Please add all the following fields module, businessservice, businessid and action" + "Mandatory workflow fields missing in the update request. Please add all the following fields: module, businessservice, businessid, and action"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- business-services/billing-service/src/main/java/org/egov/demand/service/AmendmentService.java (1 hunks)
- business-services/billing-service/src/main/java/org/egov/demand/service/BillService.java (1 hunks)
- business-services/billing-service/src/main/java/org/egov/demand/service/BillServicev2.java (1 hunks)
- business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java (2 hunks)
- business-services/billing-service/src/main/java/org/egov/demand/util/Util.java (1 hunks)
- business-services/billing-service/src/main/java/org/egov/demand/web/validator/AmendmentValidator.java (2 hunks)
🧰 Additional context used
🔇 Additional comments (7)
business-services/billing-service/src/main/java/org/egov/demand/util/Util.java (1)
115-127: Approve changes with suggestions for improvementThe changes to the
getAuditDetailmethod improve its flexibility by allowing different behavior for create and update operations. This is a good improvement. However, there are a few points to consider:
- The method signature change is backwards incompatible. Ensure all callers of this method are updated accordingly.
- Consider adding Javadoc to explain the purpose of the
isCreateparameter.- The
createdByis set even whenisCreateis false. This might be confusing. Consider setting it to null in the update case.To verify the impact of this change, please run the following script:
This script will help identify all places where the
getAuditDetailmethod is called and whereAuditDetailsare created, allowing you to ensure all usages are updated correctly.✅ Verification successful
Verification Successful
All usages of the
getAuditDetailmethod have been updated to include the newisCreateparameter, ensuring backward compatibility is maintained.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all occurrences of getAuditDetail method calls in the codebase # Search for getAuditDetail method calls echo "Searching for getAuditDetail method calls:" rg --type java "getAuditDetail\s*\(" -A 2 # Search for AuditDetails creation echo "\nSearching for AuditDetails creation:" rg --type java "AuditDetails\s*\." -A 3Length of output: 34219
business-services/billing-service/src/main/java/org/egov/demand/service/BillService.java (2)
365-365: Verify consistency ofgetAuditDetailusageWhile the change to
getAuditDetailappears isolated, it's important to ensure that this new boolean parameter is used consistently across the codebase. This change might require updates in other services or classes that use theUtil.getAuditDetailmethod.Let's check for other occurrences of
getAuditDetailin the codebase:#!/bin/bash # Description: Find other occurrences of getAuditDetail method calls # Expected result: All calls should now include a boolean parameter rg 'getAuditDetail\s*\(' --type java
365-365: Audit details now include a boolean parameterThe
getAuditDetailmethod call has been updated to include a boolean parameter set totrue. This change might affect how audit details are generated or logged for bills.To ensure this change doesn't introduce any unintended side effects, let's verify the implementation of the
getAuditDetailmethod in theUtilclass:✅ Verification successful
[/run_scripts]
Verified:
getAuditDetailmethod now accepts a boolean parameterThe
getAuditDetailmethod in theUtilclass has been successfully updated to include a boolean parameter (isCreate). This aligns with the change in theBillServiceclass and ensures that audit details are generated or logged appropriately based on the provided parameter.
- Location:
business-services/billing-service/src/main/java/org/egov/demand/util/Util.java🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the implementation of getAuditDetail method in Util class # Expected result: The method should accept a boolean parameter ast-grep --lang java --pattern 'class Util { $$$ getAuditDetail($_, boolean $_) { $$$ } $$$ }'Length of output: 2319
Script:
#!/bin/bash # Description: Verify that the getAuditDetail method in Util class accepts a boolean parameter # Expected result: The method signature includes a boolean parameter rg 'public\s+AuditDetails\s+getAuditDetail\s*\(\s*RequestInfo\s+\w+,\s*Boolean\s+\w+\s*\)' --type java business-services/billing-service/src/main/java/org/egov/demand/util/Util.javaLength of output: 263
business-services/billing-service/src/main/java/org/egov/demand/service/BillServicev2.java (1)
459-459: LGTM! Verify consistency with Util class.The change to
util.getAuditDetail(requestInfo, true)looks good. It appears to be part of an update to how audit details are generated for new bills.To ensure consistency, let's check the
Utilclass for the updatedgetAuditDetailmethod:business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java (2)
147-147: Verify consistency of audit detail retrievalThe
getAuditDetailmethod now includes a boolean parameter set totrue. This change likely affects how audit details are generated or processed during the creation of demands.Please ensure that this change is consistent with the intended behavior and is applied correctly in other relevant parts of the code. Run the following script to check for other occurrences of
getAuditDetailand verify if they need similar updates:✅ Verification successful
Audit detail retrieval consistency verified
The addition of the
trueparameter in thegetAuditDetailmethod within thecreatemethod is consistent with its usage across the codebase. Different usages ofgetAuditDetailwithtrueandfalseare appropriate based on the context of the operation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of getAuditDetail rg "getAuditDetail\s*\(" -t javaLength of output: 2086
231-231: Verify intentional difference in audit detail retrievalThe
getAuditDetailmethod call inupdateAsyncnow includes a boolean parameter set tofalse, while in thecreatemethod it's set totrue. This difference suggests distinct behavior for creation and updating of demands.Please confirm that this difference is intentional and correctly implemented. Consider adding a comment explaining the reason for the different boolean values if it's not immediately obvious. Run the following script to check the implementation of
getAuditDetail:business-services/billing-service/src/main/java/org/egov/demand/web/validator/AmendmentValidator.java (1)
218-218: Correct usage of 'createdTime' during creationIn the
enrichAmendmentForCreatemethod, passingtruetoutil.getAuditDetail(requestInfo, true)correctly ensures that thecreatedTimeis set for new amendments.
| @@ -165,7 +165,7 @@ public void updateDemandWithAmendmentTax(RequestInfo requestInfo, Amendment amen | |||
| List<Demand> demands = demandService.getDemands(demandCriteria, requestInfo); | |||
| if(!CollectionUtils.isEmpty(demands)) { | |||
|
|
|||
| AuditDetails auditDetails = util.getAuditDetail(requestInfo); | |||
| AuditDetails auditDetails = util.getAuditDetail(requestInfo,true); | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistency Found in util.getAuditDetail Method Calls
There is an inconsistent usage of the boolean parameter in the util.getAuditDetail method:
- Most calls use
trueas the second parameter. - One call in
DemandService.javausesfalse.
Please verify whether the false parameter in DemandService.java is intentional and aligns with the intended behavior. If it is not intended, consider updating it to true to maintain consistency.
🔗 Analysis chain
Verify the impact of the new parameter in util.getAuditDetail
The util.getAuditDetail method now includes an additional boolean parameter set to true. This change might affect how audit details are generated or processed. Please ensure that:
- The
Utilclass has been updated to handle this new parameter correctly. - The behavior change aligns with the intended functionality for this specific use case.
- Other calls to
getAuditDetailacross the codebase have been updated consistently, if necessary.
To ensure consistency across the codebase, please run the following script:
This script will help identify any inconsistencies in the usage of util.getAuditDetail across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for inconsistencies in util.getAuditDetail method calls
# Search for all occurrences of util.getAuditDetail
echo "Occurrences of util.getAuditDetail:"
rg --type java "util\.getAuditDetail\(" -A 1
# Search for the updated method call with the boolean parameter
echo "\nOccurrences of util.getAuditDetail with boolean parameter:"
rg --type java "util\.getAuditDetail\([^)]+,\s*true\)" -A 1
# Compare the results to identify any inconsistencies
Length of output: 3802
| @@ -286,7 +286,7 @@ public Amendment validateAndEnrichAmendmentForUpdate(@Valid AmendmentUpdateReque | |||
| * enriching the update object | |||
| */ | |||
| amendmentUpdate.setAdditionalDetails(util.jsonMerge(amendments.get(0).getAdditionalDetails(), amendmentUpdate.getAdditionalDetails())); | |||
| amendmentUpdate.setAuditDetails(util.getAuditDetail(amendmentUpdateRequest.getRequestInfo())); | |||
| amendmentUpdate.setAuditDetails(util.getAuditDetail(amendmentUpdateRequest.getRequestInfo(),true)); | |||
There was a problem hiding this comment.
Avoid updating 'createdTime' during update operations
In the validateAndEnrichAmendmentForUpdate method, passing true to util.getAuditDetail(amendmentUpdateRequest.getRequestInfo(), true) will update the createdTime. For update operations, you should preserve the original createdTime and only update the lastModifiedTime.
Apply this diff to prevent updating the createdTime:
- amendmentUpdate.setAuditDetails(util.getAuditDetail(amendmentUpdateRequest.getRequestInfo(), true));
+ amendmentUpdate.setAuditDetails(util.getAuditDetail(amendmentUpdateRequest.getRequestInfo(), false));This ensures that only the lastModifiedTime is updated during amendments.
📝 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.
| amendmentUpdate.setAuditDetails(util.getAuditDetail(amendmentUpdateRequest.getRequestInfo(),true)); | |
| amendmentUpdate.setAuditDetails(util.getAuditDetail(amendmentUpdateRequest.getRequestInfo(), false)); |
| @@ -286,7 +286,7 @@ public Amendment validateAndEnrichAmendmentForUpdate(@Valid AmendmentUpdateReque | |||
| * enriching the update object | |||
| */ | |||
| amendmentUpdate.setAdditionalDetails(util.jsonMerge(amendments.get(0).getAdditionalDetails(), amendmentUpdate.getAdditionalDetails())); | |||
| amendmentUpdate.setAuditDetails(util.getAuditDetail(amendmentUpdateRequest.getRequestInfo())); | |||
| amendmentUpdate.setAuditDetails(util.getAuditDetail(amendmentUpdateRequest.getRequestInfo(),true)); | |||
There was a problem hiding this comment.
check here it might required false here as it is update call
| @@ -165,7 +165,7 @@ public void updateDemandWithAmendmentTax(RequestInfo requestInfo, Amendment amen | |||
| List<Demand> demands = demandService.getDemands(demandCriteria, requestInfo); | |||
| if(!CollectionUtils.isEmpty(demands)) { | |||
|
|
|||
| AuditDetails auditDetails = util.getAuditDetail(requestInfo); | |||
| AuditDetails auditDetails = util.getAuditDetail(requestInfo,true); | |||
There was a problem hiding this comment.
its a update call make it false here as well
Summary by CodeRabbit
New Features
Bug Fixes
Documentation