Skip to content

AMM-1002 : Preprod- Unable to separate a Customization field for #134

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 2 commits into from
Oct 29, 2024
Merged

AMM-1002 : Preprod- Unable to separate a Customization field for #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d9edff
AMM-1002 : Preprod- Unable to separate a Customization field for
ravishanigarapu Oct 25, 2024
a55221a
code rabbit checks
ravishanigarapu Oct 25, 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 src/main/java/com/iemr/common/data/customization/SectionAndFieldsMapping.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import jakarta.persistence.Transient;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotNull;
import lombok.Data;

@Entity
Expand Down Expand Up @@ -96,6 +98,12 @@ public class SectionAndFieldsMapping {
@Column(name = "FieldTitle")
private String fieldTitle;

@Column(name = "ProjectID")
@Expose
@NotNull(message="ProjectId is Required")
@Min(value=1, message = "ProjectId must be possitive")
private Integer projectId;

@Transient
private String[] options;

Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/iemr/common/data/customization/SectionFieldsMappingDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public class SectionFieldsMappingDTO {
private String sectionName;
private String createdBy;
private Integer serviceProviderId;
private Integer projectId;
private List<SectionAndFieldsMapping> fields;

}
9 changes: 5 additions & 4 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,14 +16,15 @@ 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.sectionId = :sectionId AND (sfm.serviceProviderId = :serviceProviderId OR sfm.serviceProviderId= 0)")

@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)")
List<SectionAndFieldsMapping> findSectionIdAndSectionNameAndServiceProviderId(
@Param("sectionId") Integer sectionId, @Param("serviceProviderId") Integer serviceProviderId);
@Param("sectionId") Integer sectionId, @Param("serviceProviderId") Integer serviceProviderId, @Param("projectId") Integer projectId);
Comment on lines +20 to +22
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 name doesn't match its implementation.

The method name suggests it finds by sectionId and sectionName, but the query filters by fieldName. This mismatch could lead to confusion. Consider renaming the method to better reflect its functionality.

-List<SectionAndFieldsMapping> findSectionIdAndSectionNameAndServiceProviderId(
+List<SectionAndFieldsMapping> findByFieldNameWithFallback(

Committable suggestion was skipped due to low confidence.


@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.id = :id")
SectionAndFieldsMapping getById(@Param("id") Integer id);

@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.fieldName = :fieldName and sfm.serviceProviderId = :serviceProviderId")
List<SectionAndFieldsMapping> getByFieldName(@Param("fieldName") String fieldName,@Param("serviceProviderId") Integer serviceProviderId);
@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.fieldName = :fieldName and sfm.serviceProviderId = :serviceProviderId and sfm.projectId = :projectId")
List<SectionAndFieldsMapping> getByFieldName(@Param("fieldName") String fieldName,@Param("serviceProviderId") Integer serviceProviderId,@Param("projectId") Integer projectId);
Comment on lines +27 to +28
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

Inconsistent fallback behavior between queries.

While the first query allows fallback to global settings (projectId=0), this query enforces strict matching. This inconsistency could lead to unexpected behavior where some queries find global settings and others don't.

Consider applying the same fallback pattern:

-@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.fieldName = :fieldName and sfm.serviceProviderId = :serviceProviderId and sfm.projectId = :projectId")
+@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)")
📝 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
@Query("SELECT sfm FROM SectionAndFieldsMapping sfm WHERE sfm.fieldName = :fieldName and sfm.serviceProviderId = :serviceProviderId and sfm.projectId = :projectId")
List<SectionAndFieldsMapping> getByFieldName(@Param("fieldName") String fieldName,@Param("serviceProviderId") Integer serviceProviderId,@Param("projectId") Integer projectId);
@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)")
List<SectionAndFieldsMapping> getByFieldName(@Param("fieldName") String fieldName,@Param("serviceProviderId") Integer serviceProviderId,@Param("projectId") Integer projectId);


}
17 changes: 13 additions & 4 deletions src/main/java/com/iemr/common/service/customization/CustomizationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,15 @@ public String saveSectionAndFields(SectionFieldsMappingDTO sectionFieldsMappingD
try {

if (sectionFieldsMappingDTO != null && sectionFieldsMappingDTO.getFields() != null
&& sectionFieldsMappingDTO.getFields().size() > 0) {
&& sectionFieldsMappingDTO.getFields().size() > 0 && sectionFieldsMappingDTO.getProjectId() != null) {
SectionAndFieldsMapping sectionAndFieldsMapping;
List<SectionAndFieldsMapping> sectionAndFieldsMappingList = new ArrayList<>();
for (SectionAndFieldsMapping sectionFieldsMapping : sectionFieldsMappingDTO.getFields()) {
List<SectionAndFieldsMapping> byFieldName = sectionAndFieldsMappingRepo
.getByFieldName(sectionFieldsMapping.getFieldName(),sectionFieldsMapping.getServiceProviderId());
.getByFieldName(sectionFieldsMapping.getFieldName(),sectionFieldsMapping.getServiceProviderId(),sectionFieldsMappingDTO.getProjectId());
sectionAndFieldsMapping = new SectionAndFieldsMapping();
sectionAndFieldsMapping.setSectionId(sectionFieldsMappingDTO.getSectionId());
sectionAndFieldsMapping.setProjectId(sectionFieldsMappingDTO.getProjectId());
sectionAndFieldsMapping.setCreatedBy(sectionFieldsMappingDTO.getCreatedBy());
sectionAndFieldsMapping.setAllowMax(sectionFieldsMapping.getAllowMax());
sectionAndFieldsMapping.setFieldName(sectionFieldsMapping.getFieldName());
Expand Down Expand Up @@ -284,16 +285,19 @@ public String fetchMappedFields(String request, String Authorization) throws Exc
SectionAndFieldsMapping sectionAndFieldsMapping = InputMapper.gson().fromJson(request,
SectionAndFieldsMapping.class);
try {
if(sectionAndFieldsMapping.getProjectId() == null) {
throw new IllegalArgumentException("ProjectId is required");
}
List<SectionAndFieldsMapping> resultSet = sectionAndFieldsMappingRepo
.findSectionIdAndSectionNameAndServiceProviderId(sectionAndFieldsMapping.getSectionId(),
sectionAndFieldsMapping.getServiceProviderId());
sectionAndFieldsMapping.getServiceProviderId(), sectionAndFieldsMapping.getProjectId());

String sectionName = sectionMasterCustomizationRepo.findSectionName(sectionAndFieldsMapping.getSectionId());
Map<String, Object> responseMap = new HashMap<>();
responseMap.put("sectionId", sectionAndFieldsMapping.getSectionId());
responseMap.put("sectionName", sectionName);
responseMap.put("serviceProviderId", sectionAndFieldsMapping.getServiceProviderId());

responseMap.put("projectId", sectionAndFieldsMapping.getProjectId());
Comment on lines +293 to +300
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

Add null check for projectId

While the changes correctly include projectId in the query and response, consider adding a null check for projectId to prevent potential NullPointerException.

- .findSectionIdAndSectionNameAndServiceProviderId(sectionAndFieldsMapping.getSectionId(),
-   sectionAndFieldsMapping.getServiceProviderId(), sectionAndFieldsMapping.getProjectId());
+ if (sectionAndFieldsMapping.getProjectId() == null) {
+   throw new IllegalArgumentException("ProjectId is required");
+ }
+ .findSectionIdAndSectionNameAndServiceProviderId(sectionAndFieldsMapping.getSectionId(),
+   sectionAndFieldsMapping.getServiceProviderId(), sectionAndFieldsMapping.getProjectId());

Committable suggestion was skipped due to low confidence.

List<Map<String, Object>> fieldsList = new ArrayList<>();
for (SectionAndFieldsMapping field : resultSet) {
Map<String, Object> fieldMap = new HashMap<>();
Expand Down Expand Up @@ -372,6 +376,11 @@ public String updateSectionAndFields(String request, String Authorization) throw
if (sectionAndFieldsMapping.getId() != null) {
SectionAndFieldsMapping response = sectionAndFieldsMappingRepo.getById(sectionAndFieldsMapping.getId());
if (response != null) {
if(sectionAndFieldsMapping.getProjectId() != null) {
ProjectCustomization project = projectCustomizationRepo.findById(sectionAndFieldsMapping.getProjectId())
.orElseThrow(() -> new IllegalArgumentException("Invalid projectId"));
response.setProjectId(sectionAndFieldsMapping.getProjectId());
}
if (sectionAndFieldsMapping.getFieldName() != null)
response.setFieldName(sectionAndFieldsMapping.getFieldName());
if (sectionAndFieldsMapping.getIsRequired() != null)
Expand Down
Loading