AMM-1137 | Modify feedback register API- Add param "BeneficiaryConsent" #141
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FeedbackDetails
User->>FeedbackDetails: Create FeedbackDetails(feedbackID, institutionID, ..., beneficiaryConsent)
FeedbackDetails->>FeedbackDetails: Set beneficiaryConsent
FeedbackDetails-->>User: FeedbackDetails object created
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
🧹 Outside diff range and nitpick comments (4)
src/main/java/com/iemr/common/data/feedback/FeedbackDetails.java (4)
103-106: Remove unnecessary blank lines around field declaration.The field declaration is correctly annotated but has excessive spacing.
private String instituteName = ""; - - @Column(name = "BeneficiaryConsent") @Expose private Boolean beneficiaryConsent;
Line range hint
311-333: Add validation for beneficiaryConsent parameter.Since this field represents user consent, it's critical to ensure it's not null to prevent ambiguous consent states.
public FeedbackDetails(Long feedbackID, Long institutionID,String instiName, Integer designationID, Integer severityID, Integer feedbackTypeID, Integer feedbackStatusID, String feedback, Long beneficiaryRegID, Integer serviceID, Integer userID, String sMSPhoneNo, Timestamp serviceAvailDate, Boolean deleted, String createdBy, Timestamp createdDate, String modifiedBy, Timestamp lastModDate, String feedbackAgainst, Boolean beneficiaryConsent) { + if (beneficiaryConsent == null) { + throw new IllegalArgumentException("Beneficiary consent cannot be null"); + } super(); this.feedbackID = feedbackID; // ... other assignments ... this.beneficiaryConsent = beneficiaryConsent; }
Line range hint
573-642: Consider using Builder pattern to improve code maintainability.The static factory method has grown to accept 34 parameters, which makes it hard to maintain and use correctly. Consider implementing a Builder pattern to make the code more maintainable and less error-prone.
Example implementation:
public static class FeedbackDetailsBuilder { private Long feedbackID; private User mUser; private Long institutionID; // ... other fields ... private Boolean beneficiaryConsent; public FeedbackDetailsBuilder withFeedbackID(Long feedbackID) { this.feedbackID = feedbackID; return this; } // ... other builder methods ... public FeedbackDetailsBuilder withBeneficiaryConsent(Boolean beneficiaryConsent) { this.beneficiaryConsent = beneficiaryConsent; return this; } public FeedbackDetails build() { FeedbackDetails details = new FeedbackDetails(); details.feedbackID = this.feedbackID; // ... other assignments ... details.beneficiaryConsent = this.beneficiaryConsent; return details; } }
beneficiaryConsentfield not initialized in all constructorsThe class has 4 constructors:
- Empty constructor with no initialization
- Full constructor that properly initializes
beneficiaryConsent- Two partial constructors that don't initialize
beneficiaryConsentSince
beneficiaryConsentis a Boolean field, not initializing it in constructors could lead to null pointer exceptions when the getter is called. The field should be initialized in all constructors, even if just to a default value.🔗 Analysis chain
Line range hint
1-694: Verify initialization in all constructors.The
beneficiaryConsentfield is only initialized in one constructor. This could lead to NPEs when using other constructors.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other constructors in the class that might need updating ast-grep --pattern 'class FeedbackDetails { $$$ public FeedbackDetails($$$) { $$$ } $$$ }'Length of output: 104
Script:
#!/bin/bash # Let's try a simpler pattern to find constructors rg "public FeedbackDetails\(" -A 5 src/main/java/com/iemr/common/data/feedback/FeedbackDetails.javaLength of output: 1313
src/main/java/com/iemr/common/data/feedback/FeedbackDetails.java
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
📋 Description
JIRA ID: AMM-1137 | Modify feedback register API- Add param "BeneficiaryConsent
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
beneficiaryConsent, to capture consent status related to feedback.beneficiaryConsentproperty.beneficiaryConsentparameter.beneficiaryConsentproperty.