Skip to content

Custom FieldName Patching #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 18, 2024
Merged

Custom FieldName Patching #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a70d9a2
missed file
ravishanigarapu Nov 4, 2024
76f1d39
FieldName Patching
ravishanigarapu Nov 18, 2024
01e9871
Null Conditions added
ravishanigarapu Nov 18, 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
2 changes: 1 addition & 1 deletion src/main/java/com/iemr/common/dto/identity/BeneficiariesDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public class BeneficiariesDTO {
private String otherFields;
private String genderName;
private String maritalStatusName;

private String community;
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent naming of community field across DTOs needs standardization

The codebase shows mixed usage of both community and communityName fields:

  • community is used in 8 DTOs/models including BeneficiariesDTO
  • communityName is used in 3 models including BeneficiaryModel

This inconsistency should be addressed by standardizing on one naming convention across all DTOs and models. Notably, IdentityEditDTO has both fields which indicates potential duplication.

🔗 Analysis chain

Consider standardizing community field implementation.

While the addition of the community field is appropriate, consider these improvements:

  1. Ensure consistent naming with related DTOs (other files use communityName)
  2. Add Javadoc to document the field's purpose
  3. Consider using an enumeration or reference data table for standardized community values

Example improvement:

+ /** Community/social group of the beneficiary */
  private String community;

Let's verify the naming consistency across DTOs:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for community-related fields in other DTOs
rg -t java "private\s+String\s+community[^a-zA-Z]" src/
rg -t java "private\s+String\s+communityName[^a-zA-Z]" src/

Length of output: 1191

// End Outreach

// Start 1097
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/iemr/common/dto/identity/IdentityEditDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,5 @@ public class IdentityEditDTO {
private boolean emergencyRegistration;
private String otherFields;
// private Object otherFields;
private String communityName;
}
3 changes: 2 additions & 1 deletion src/main/java/com/iemr/common/model/beneficiary/BeneficiaryModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ public class BeneficiaryModel implements Comparable<BeneficiaryModel> {
private String monthlyFamilyIncome;
@Expose
private boolean emergencyRegistration;

@Expose
private String communityName;
Comment on lines +283 to +284
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation constraints and documentation for the new communityName field

While the field addition follows the class's pattern, consider the following improvements:

  1. Add validation constraints to prevent invalid data:
 @Expose
+@Size(max = 50)
+@Pattern(regexp = "^[a-zA-Z0-9\\s-]*$", message = "Community name contains invalid characters")
 private String communityName;
  1. Add Javadoc to document the field's purpose and any business rules:
+/**
+ * Represents the beneficiary's community/ethnicity information.
+ * This field should be handled with appropriate privacy considerations.
+ */
 @Expose
 private String communityName;

Committable suggestion skipped: line range outside the PR's diff.

@Expose
private Boolean passToNurse = false;
private String otherFields;
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/iemr/common/repo/customization/SectionAndFieldsMappingRepo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ public interface SectionAndFieldsMappingRepo extends CrudRepository<SectionAndFi
// List<SectionAndFieldsMapping> findBySectionIdAndSectionNameAndServiceProviderId(
// @Param("sectionId") Integer sectionId, @Param("serviceProviderId") Integer serviceProviderId);


@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.fieldName = :fieldName AND (sfm.serviceProviderId = :serviceProviderId OR sfm.serviceProviderId = 0) AND (sfm.projectId = :projectId OR sfm.projectId = 0)")
@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.sectionId = :sectionId AND (sfm.serviceProviderId = :serviceProviderId OR sfm.serviceProviderId= 0) AND (sfm.projectId = :projectId OR sfm.projectId= 0)")
List<SectionAndFieldsMapping> findSectionIdAndSectionNameAndServiceProviderId(
@Param("sectionId") Integer sectionId, @Param("serviceProviderId") Integer serviceProviderId, @Param("projectId") Integer projectId);
Comment on lines +19 to 21
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Method naming and implementation concerns need attention

Several issues need to be addressed in the new query method:

  1. The method name doesn't follow Spring Data JPA naming conventions and is inconsistent with its functionality. Consider renaming to better reflect the query's purpose.
  2. The removal of the deleted=false condition might expose soft-deleted records.
  3. The method name suggests it returns sectionName, but the query doesn't specifically select this field.

Consider applying these changes:

-@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.sectionId = :sectionId AND (sfm.serviceProviderId = :serviceProviderId OR sfm.serviceProviderId= 0) AND (sfm.projectId = :projectId OR sfm.projectId= 0)")
-List<SectionAndFieldsMapping> findSectionIdAndSectionNameAndServiceProviderId(
+@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.sectionId = :sectionId " +
+       "AND (sfm.serviceProviderId = :serviceProviderId OR sfm.serviceProviderId = 0) " +
+       "AND (sfm.projectId = :projectId OR sfm.projectId = 0) " +
+       "AND sfm.deleted = false")
+List<SectionAndFieldsMapping> findBySectionIdAndServiceProviderIdAndProjectId(
     @Param("sectionId") Integer sectionId,
     @Param("serviceProviderId") Integer serviceProviderId,
     @Param("projectId") Integer projectId);

Consider adding documentation to explain:

  1. The significance of serviceProviderId = 0 and projectId = 0
  2. The expected behavior when these special values are matched
/**
 * Finds section and fields mappings based on section ID, service provider ID, and project ID.
 * 
 * @param sectionId The ID of the section to filter by
 * @param serviceProviderId The service provider ID (0 represents a special case that matches all providers)
 * @param projectId The project ID (0 represents a special case that matches all projects)
 * @return List of matching section and fields mappings, excluding soft-deleted records
 */


Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/iemr/common/service/beneficiary/IEMRSearchUserServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,8 @@ public List<BeneficiaryModel> getBeneficiaryListFromMapper(List<BeneficiariesDTO
benDetailForOutboundDTOList.forEach(beneficiaryModel -> {

BeneficiaryModel beneficiary = benCompleteMapper.benDetailForOutboundDTOToIBeneficiary(beneficiaryModel);
if(null != beneficiaryModel.getBeneficiaryDetails() && null != beneficiaryModel.getBeneficiaryDetails().getCommunity())
beneficiary.setCommunityName(beneficiaryModel.getBeneficiaryDetails().getCommunity());
if (beneficiary.getAge() == null) {
beneficiary.setAge(beneficiary.getActualAge());
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/iemr/common/service/beneficiary/RegisterBenificiaryServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ public Integer updateBenificiary(BeneficiaryModel benificiaryDetails, String aut
Integer updatedRows = 0;

IdentityEditDTO identityEditDTO = identityBenEditMapper.BenToIdentityEditMapper(benificiaryDetails);
if(null != benificiaryDetails.getI_bendemographics())
identityEditDTO.setCommunity(benificiaryDetails.getI_bendemographics().getCommunityName());
if (benificiaryDetails.getBeneficiaryIdentities() != null
&& benificiaryDetails.getBeneficiaryIdentities().size() > 0) {
identityEditDTO.setIdentities(Identity.createIdentity(benificiaryDetails.getBeneficiaryIdentities(),
Expand Down
Loading