Skip to content
This repository has been archived by the owner on Nov 26, 2024. It is now read-only.

[ISNE-304] - set createdtime and createdby in audit details for demand and water reindexing #977

Merged
merged 8 commits into from
Oct 14, 2024
Merged

[ISNE-304] - set createdtime and createdby in audit details for demand and water reindexing #977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
402c30e
Merge pull request #972 from egovernments/ISTE-487
pradeepkumarcm-egov Oct 10, 2024
30ea854
set createdtime and createdby in audit details
Taniya-eGov Oct 11, 2024
5c4f023
set createdtime and createdtime in auditdetails
Taniya-eGov Oct 11, 2024
e82b5e0
added null pointer check
Taniya-eGov Oct 11, 2024
d87fd5c
Merge pull request #976 from egovernments/ISNE-304
pradeepkumarcm-egov Oct 14, 2024
b34f5d3
added auditdetails check condition
Taniya-eGov Oct 14, 2024
b359c05
check currAuditDetails condition
Taniya-eGov Oct 14, 2024
564d24a
Merge pull request #979 from egovernments/ISNE-304
pradeepkumarcm-egov Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,14 @@ public DemandResponse updateAsync(DemandRequest demandRequest, PaymentBackUpdate
RequestInfo requestInfo = demandRequest.getRequestInfo();
List<Demand> demands = demandRequest.getDemands();
AuditDetails auditDetail = util.getAuditDetail(requestInfo);
for (Demand demand : demands) {
AuditDetails currAuditDetails = demand.getAuditDetails();
if (currAuditDetails != null) {
auditDetail.setCreatedTime(currAuditDetails.getCreatedTime());
auditDetail.setCreatedBy(currAuditDetails.getCreatedBy());
}
demand.setAuditDetails(auditDetail);
}
Comment on lines +232 to +239
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid reusing the same AuditDetails instance across demands

The current implementation modifies a single auditDetail instance for all demands. This can lead to unintended side effects, as all demands will share the same AuditDetails object. To ensure each demand has its own unique audit details, create a new AuditDetails instance inside the loop for each demand.

Apply this diff to create a new AuditDetails for each demand:

 AuditDetails auditDetail = util.getAuditDetail(requestInfo);
 for (Demand demand : demands) {
+    AuditDetails demandAuditDetail = new AuditDetails();
     AuditDetails currAuditDetails = demand.getAuditDetails();
     if (currAuditDetails != null) {
-        auditDetail.setCreatedTime(currAuditDetails.getCreatedTime());
-        auditDetail.setCreatedBy(currAuditDetails.getCreatedBy());
+        demandAuditDetail.setCreatedTime(currAuditDetails.getCreatedTime());
+        demandAuditDetail.setCreatedBy(currAuditDetails.getCreatedBy());
     }
-    demand.setAuditDetails(auditDetail);
+    demand.setAuditDetails(demandAuditDetail);
 }
📝 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.

Suggested change
for (Demand demand : demands) {
AuditDetails currAuditDetails = demand.getAuditDetails();
if (currAuditDetails != null) {
auditDetail.setCreatedTime(currAuditDetails.getCreatedTime());
auditDetail.setCreatedBy(currAuditDetails.getCreatedBy());
}
demand.setAuditDetails(auditDetail);
}
AuditDetails auditDetail = util.getAuditDetail(requestInfo);
for (Demand demand : demands) {
AuditDetails demandAuditDetail = new AuditDetails();
AuditDetails currAuditDetails = demand.getAuditDetails();
if (currAuditDetails != null) {
demandAuditDetail.setCreatedTime(currAuditDetails.getCreatedTime());
demandAuditDetail.setCreatedBy(currAuditDetails.getCreatedBy());
}
demand.setAuditDetails(demandAuditDetail);
}

🛠️ Refactor suggestion

Consider copying the entire currAuditDetails

Instead of selectively copying createdTime and createdBy, you might want to copy all relevant audit details to preserve the full audit trail. This ensures consistency and completeness of audit information.

Apply this diff to copy the audit details:

 for (Demand demand : demands) {
-    AuditDetails currAuditDetails = demand.getAuditDetails();
-    if (currAuditDetails != null) {
-        auditDetail.setCreatedTime(currAuditDetails.getCreatedTime());
-        auditDetail.setCreatedBy(currAuditDetails.getCreatedBy());
-    }
-    demand.setAuditDetails(auditDetail);
+    AuditDetails currAuditDetails = demand.getAuditDetails();
+    if (currAuditDetails != null) {
+        demand.setAuditDetails(currAuditDetails);
+    } else {
+        demand.setAuditDetails(util.getAuditDetail(requestInfo));
+    }
 }

Committable suggestion was skipped due to low confidence.


List<Demand> newDemands = new ArrayList<>();

Expand Down
6 changes: 5 additions & 1 deletion ...ervices/ws-services/src/main/java/org/egov/waterconnection/service/EnrichmentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,13 @@ private List<String> getIdList(RequestInfo requestInfo, String tenantId, String
*
* @param waterConnectionRequest WaterConnectionRequest Object
*/
public void enrichUpdateWaterConnection(WaterConnectionRequest waterConnectionRequest) {
public void enrichUpdateWaterConnection(AuditDetails currentAuditDetails, WaterConnectionRequest waterConnectionRequest) {
AuditDetails auditDetails = waterServicesUtil
.getAuditDetails(waterConnectionRequest.getRequestInfo().getUserInfo().getUuid(), false);
if (currentAuditDetails != null) {
auditDetails.setCreatedBy(currentAuditDetails.getCreatedBy());
auditDetails.setCreatedTime(currentAuditDetails.getCreatedTime());
}
waterConnectionRequest.getWaterConnection().setAuditDetails(auditDetails);
WaterConnection connection = waterConnectionRequest.getWaterConnection();
if (!CollectionUtils.isEmpty(connection.getDocuments())) {
Expand Down
9 changes: 6 additions & 3 deletions ...services/ws-services/src/main/java/org/egov/waterconnection/service/WaterServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public List<WaterConnection> updateWaterConnection(WaterConnectionRequest waterC
throw new CustomException("DUPLICATE_OLD_CONNECTION_NUMBER",
"Duplicate Old connection number");
}

AuditDetails auditDetails=waterConnection.get(0).getAuditDetails();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential NullPointerException when accessing waterConnection.get(0)

In the methods updateWaterConnection (line 243) and updateWaterConnectionForModifyFlow (line 334), you are accessing waterConnection.get(0) without checking if waterConnection is not null and not empty. This could lead to a NullPointerException or IndexOutOfBoundsException if waterConnection is null or empty. Ensure that you perform a null check and size check before accessing elements of the list.

Apply this diff to fix the issue:

+        if (waterConnection != null && !waterConnection.isEmpty()) {
             AuditDetails auditDetails = waterConnection.get(0).getAuditDetails();
+        } else {
+            // Handle the null or empty waterConnection case appropriately
+            // For example, throw a CustomException or initialize auditDetails differently
+        }

Also applies to: 334-334

mDMSValidator.validateMasterData(waterConnectionRequest, WCConstants.UPDATE_APPLICATION);
Property property = validateProperty.getOrValidateProperty(waterConnectionRequest);
validateProperty.validatePropertyFields(property, waterConnectionRequest.getRequestInfo());
Expand All @@ -252,7 +252,7 @@ public List<WaterConnection> updateWaterConnection(WaterConnectionRequest waterC
String previousApplicationStatus = workflowService.getApplicationStatus(waterConnectionRequest.getRequestInfo(),
waterConnectionRequest.getWaterConnection().getApplicationNo(),
waterConnectionRequest.getWaterConnection().getTenantId(), config.getBusinessServiceValue());
enrichmentService.enrichUpdateWaterConnection(waterConnectionRequest);
enrichmentService.enrichUpdateWaterConnection(auditDetails, waterConnectionRequest);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure auditDetails is not null before passing to enrichUpdateWaterConnection

In the methods updateWaterConnection (line 255) and updateWaterConnectionForModifyFlow (line 346), you are passing auditDetails to enrichUpdateWaterConnection. If auditDetails is null, this may cause a NullPointerException within enrichUpdateWaterConnection. Ensure that auditDetails is properly initialized and not null before using it.

Apply this diff to validate auditDetails:

         // Previous code ensuring waterConnection is not null or empty
+        if (auditDetails != null) {
             enrichmentService.enrichUpdateWaterConnection(auditDetails, waterConnectionRequest);
+        } else {
+            // Handle the null auditDetails case appropriately
+            // For example, initialize auditDetails or handle the error
+        }

Also applies to: 346-346

actionValidator.validateUpdateRequest(waterConnectionRequest, businessService, previousApplicationStatus);
waterConnectionValidator.validateUpdate(waterConnectionRequest, searchResult, WCConstants.UPDATE_APPLICATION);
userService.updateUser(waterConnectionRequest, searchResult);
Expand Down Expand Up @@ -331,6 +331,7 @@ private List<WaterConnection> updateWaterConnectionForModifyFlow(WaterConnection
throw new CustomException("DUPLICATE_OLD_CONNECTION_NUMBER",
"Duplicate Old connection number");
}
AuditDetails auditDetails=waterConnection.get(0).getAuditDetails();
mDMSValidator.validateMasterData(waterConnectionRequest, WCConstants.MODIFY_CONNECTION);
BusinessService businessService = workflowService.getBusinessService(
waterConnectionRequest.getWaterConnection().getTenantId(), waterConnectionRequest.getRequestInfo(),
Expand All @@ -342,7 +343,9 @@ private List<WaterConnection> updateWaterConnectionForModifyFlow(WaterConnection
String previousApplicationStatus = workflowService.getApplicationStatus(waterConnectionRequest.getRequestInfo(),
waterConnectionRequest.getWaterConnection().getApplicationNo(),
waterConnectionRequest.getWaterConnection().getTenantId(), config.getModifyWSBusinessServiceName());
enrichmentService.enrichUpdateWaterConnection(waterConnectionRequest);
if (auditDetails != null) {
enrichmentService.enrichUpdateWaterConnection(auditDetails, waterConnectionRequest);
}
actionValidator.validateUpdateRequest(waterConnectionRequest, businessService, previousApplicationStatus);
userService.updateUser(waterConnectionRequest, searchResult);
waterConnectionValidator.validateUpdate(waterConnectionRequest, searchResult, WCConstants.MODIFY_CONNECTION);
Expand Down
Loading