Changed saving report log to audit db #11502
Conversation
WalkthroughThis update replaces an object reference with a simpler identifier in the report logging mechanism. In the ReportLog class, the previous association with a WebUser is removed and substituted with a non-nullable Long field ( Changes
Poem
✨ Finishing Touches
🪧 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: 0
🧹 Nitpick comments (2)
src/main/java/com/divudi/core/entity/report/ReportLog.java (2)
49-56: Constructor updated to store ID instead of WebUser objectThe constructor has been updated to extract and store the user ID rather than storing the WebUser object reference. This change is consistent with the field type change.
Consider creating an additional constructor that accepts the ID directly, which would be useful for testing or when the WebUser object isn't readily available:
+ public ReportLog(IReportType reportType, Long generatedById, Date startTime, Date endTime) { + this.generatedById = generatedById; + this.startTime = startTime; + this.endTime = endTime; + this.reportType = reportType.getReportType(); + this.reportName = reportType.getReportName(); + this.executionTimeInMillis = endTime.getTime() - startTime.getTime(); + }
4-4: WebUser import still usedThe WebUser import is still needed for the constructor parameter, but this class no longer has a direct relationship with WebUser entities.
If you create an additional constructor that accepts the ID directly (as suggested above), and if the existing constructor is no longer used elsewhere in the codebase, you could remove this import.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/divudi/core/entity/report/ReportLog.java(3 hunks)src/main/java/com/divudi/core/facade/ReportLogFacade.java(1 hunks)
🔇 Additional comments (4)
src/main/java/com/divudi/core/facade/ReportLogFacade.java (1)
11-11:✅ Verification successful
Persistence unit updated to hmisAuditPU
The change from "hmisPU" to "hmisAuditPU" aligns with the PR objective of changing report logging to the audit database. This separates audit data from the main application data, which is a good practice for auditing.
🏁 Script executed:
#!/bin/bash # Check if the hmisAuditPU is properly configured in persistence.xml fd "persistence.xml" | xargs grep -A 10 "hmisAuditPU"Length of output: 409
Persistence Unit Configuration Verified
The use of
@PersistenceContext(unitName = "hmisAuditPU")insrc/main/java/com/divudi/core/facade/ReportLogFacade.javais correct. The output from the search confirms thatpersistence.xmlproperly defines the persistence unit "hmisAuditPU" with the expected configuration. This correctly separates audit logging from the main application data as intended.src/main/java/com/divudi/core/entity/report/ReportLog.java (3)
28-29: Column change from WebUser reference to Long IDReplacing the WebUser reference with just the ID field is appropriate when moving to a separate audit database, as it avoids cross-database relationship issues.
1-116:✅ Verification successful
Overall implementation of ReportLog changes
The changes to this class are well-implemented and align with the PR objective of moving report logs to a separate audit database. The approach of storing just the user ID rather than a direct entity reference is appropriate for this scenario.
🏁 Script executed:
#!/bin/bash # Check for any code that might be affected by these changes # Look for places that might be using the old relationship rg "\.getGeneratedBy\(\)" --type java -A 2 -B 2 # Look for places creating ReportLog objects rg "new ReportLog\(" --type java -A 5 -B 2Length of output: 820
Final Approval: Verified ReportLog Implementation and Integration
The changes in
ReportLog.javahave been confirmed as correctly implemented. The updated class storing only the user ID has been integrated well, and our verification inReportTimerController.java(using thenew ReportLog(constructor) confirms that there are no lingering dependencies on the previous direct entity relationship.
- ReportLog class: Correct modifications for moving report logs to a separate audit database.
- ReportTimerController.java: Usage of the updated constructor shows consistency and no outdated method calls like
.getGeneratedBy().
77-79:❓ Verification inconclusive
Getter updated for new field
The getter has been appropriately updated to return the user ID instead of a WebUser object.
🏁 Script executed:
#!/bin/bash # Check for any code that may be trying to get WebUser from ReportLog rg "getGeneratedBy\(\)" --type java -A 3 -B 3Length of output: 47
Getter update verified – manual confirmation required
The updated getter in
src/main/java/com/divudi/core/entity/report/ReportLog.java(lines 77–79) now returns the user ID (Long) instead of aWebUserobject. A search for calls to the legacy getter (getGeneratedBy()) produced no output, which suggests that no remaining dependencies on the old implementation were detected. However, due to the low-quality inference from the script output, please manually verify that no other parts of the codebase expect aWebUserobject.
- File location:
src/main/java/com/divudi/core/entity/report/ReportLog.java(lines 77–79)- Action: Manually confirm that all references to the previous getter have been updated accordingly.
Summary by CodeRabbit