feat: AMM-1267 ECD Quality Supervisor- Question Configuration - #104
Conversation
WalkthroughAdds role/roles handling across entities, DTOs, and services: introduces persistent String role and transient List roles, normalization via flattenRoles(), parsing/splitting/composing roles in service methods, and updates version in pom.xml from 3.1.0 to 3.4.0. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as QualityAuditQuestionConfigurationImpl
participant DAO as QualityAuditQuestionConfig (DAO)
participant DB
Client->>Service: create/update Questionnaire (roles: List<String>)
Service->>DAO: question.flattenRoles() (join list -> CSV)
DAO->>DB: Persist role (String, CSV)
DB-->>DAO: Save OK
DAO-->>Service: Result
Service-->>Client: Acknowledgement
sequenceDiagram
participant Client
participant Service as QualityAuditImpl / Q.A. Config Impl
participant View as V_get_Qualityaudit_SectionQuestionaireValues
participant DB
Client->>Service: Fetch questions/sections
Service->>DB: Query view/entity
DB-->>Service: Rows with Role (CSV)
Service->>Service: Split CSV -> roles List<String>
Service-->>Client: DTOs with roles
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/main/java/com/iemr/ecd/service/quality/QualityAuditQuestionConfigurationImpl.java (1)
58-88: Critical: optionsSet leaks across iterations and can save previous question’s options on subsequent questionsoptionsSet is declared outside the loop and only reinitialized when options are present. For a question without options, the previous iteration’s optionsSet will be reused, causing duplication and misassociation with the new questionId.
Fix by reinitializing optionsSet per iteration and validating options/scores length:
- Set<QualityAuditQuestionnaireValues> optionsSet = null; + Set<QualityAuditQuestionnaireValues> optionsSet = null; for (QualityAuditQuestionConfig question : qualityAuditQuestionConfig) { + // ensure a fresh set per question + optionsSet = new java.util.HashSet<>(); if (question.getRoles() != null && !question.getRoles().isEmpty()) { question.flattenRoles(); } - if (question.getOptions() != null && question.getOptions().length > 0) { - int j = 0; - Integer[] scoreArr = question.getScores(); - optionsSet = new HashSet<>(); - for (String option : question.getOptions()) { - questionnaireOptions = new QualityAuditQuestionnaireValues(); - questionnaireOptions.setQuestionValues(option); - questionnaireOptions.setScore(scoreArr[j]); - questionnaireOptions.setPsmId(question.getPsmId()); - questionnaireOptions.setCreatedBy(question.getCreatedBy()); - optionsSet.add(questionnaireOptions); - j++; - } - } + if (question.getOptions() != null && question.getOptions().length > 0) { + Integer[] scoreArr = question.getScores(); + if (scoreArr == null || scoreArr.length != question.getOptions().length) { + throw new ECDException("Options and scores length mismatch for question: " + question.getQuestionnaire()); + } + for (int j = 0; j < question.getOptions().length; j++) { + questionnaireOptions = new QualityAuditQuestionnaireValues(); + questionnaireOptions.setQuestionValues(question.getOptions()[j]); + questionnaireOptions.setScore(scoreArr[j]); + questionnaireOptions.setPsmId(question.getPsmId()); + questionnaireOptions.setCreatedBy(question.getCreatedBy()); + optionsSet.add(questionnaireOptions); + } + } QualityAuditQuestionConfig resultSet = qualityAuditQuestionConfigRepo.save(question); - if (resultSet != null && resultSet.getId() != null && optionsSet != null && optionsSet.size() > 0) { + if (resultSet != null && resultSet.getId() != null && optionsSet != null && !optionsSet.isEmpty()) { for (QualityAuditQuestionnaireValues optionsValues : optionsSet) { optionsValues.setQuestionId(resultSet.getId()); } qualityAuditQuestionnaireValuesRepo.saveAll(optionsSet); } }
🧹 Nitpick comments (3)
src/main/java/com/iemr/ecd/dto/ResponseCallAuditSectionQuestionMapDTO.java (1)
51-52: Roles field addition: consider suppressing nulls in JSON outputIf roles is often absent, consider omitting it from serialized JSON when null to reduce payload noise.
Outside selected lines, you can annotate the DTO:
import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; @JsonInclude(Include.NON_NULL) public class ResponseCallAuditSectionQuestionMapDTO { // ... }src/main/java/com/iemr/ecd/service/quality/QualityAuditImpl.java (1)
369-371: Trim and de-duplicate roles when splittingSplitting on commas can leave leading/trailing spaces and empty tokens. Normalize to avoid UI/logic issues.
Apply this change:
- if(obj.getRole() != null && !obj.getRole().isEmpty()) { - responseDTO.setRoles(Arrays.asList(obj.getRole().split(","))); - } + if (obj.getRole() != null && !obj.getRole().isEmpty()) { + responseDTO.setRoles(Arrays.stream(obj.getRole().split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .distinct() + .toList()); + }Outside selected lines, add the import if needed:
import java.util.stream.Collectors; // if using collect(Collectors.toList())src/main/java/com/iemr/ecd/service/quality/QualityAuditQuestionConfigurationImpl.java (1)
144-146: Trim and filter roles when splitting from SP outputAvoid whitespace/null entries and duplicates for cleaner downstream behavior.
Apply this diff:
- if (strArr[14] != null && !strArr[14].isEmpty()) { - obj.setRoles(Arrays.asList(strArr[14].split(","))); - } + if (strArr[14] != null && !strArr[14].isEmpty()) { + obj.setRoles(Arrays.stream(strArr[14].split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .distinct() + .toList()); + }Outside selected lines, add import if needed:
import java.util.stream.Collectors; // if using collect(Collectors.toList())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
pom.xml(1 hunks)src/main/java/com/iemr/ecd/dao/QualityAuditQuestionConfig.java(2 hunks)src/main/java/com/iemr/ecd/dao/V_get_Qualityaudit_SectionQuestionaireValues.java(1 hunks)src/main/java/com/iemr/ecd/dto/QualityAuditorSectionQuestionaireResponseDTO.java(1 hunks)src/main/java/com/iemr/ecd/dto/ResponseCallAuditSectionQuestionMapDTO.java(1 hunks)src/main/java/com/iemr/ecd/service/quality/QualityAuditImpl.java(1 hunks)src/main/java/com/iemr/ecd/service/quality/QualityAuditQuestionConfigurationImpl.java(4 hunks)
🔇 Additional comments (4)
pom.xml (1)
14-14: No hard-coded WAR names detected; artifact name is dynamic
- pom.xml (line 253) defines
<finalName>${artifactId}-${version}</finalName>- No occurrences of
ecd-api-3.1.0.waror other versioned WAR filenames were found in the repositorySince the WAR name is derived from the updated
<version>3.4.0</version>, no further changes are needed in CI/CD scripts within this repo.src/main/java/com/iemr/ecd/dao/V_get_Qualityaudit_SectionQuestionaireValues.java (1)
79-81: Confirm DB view column and reserved word handling for "Role"Ensure the view V_get_Qualityaudit_Sectionquestionairevalues exposes a column named Role exactly as mapped here. Also verify your RDBMS doesn’t require quoting for ROLE (reserved in some versions) to avoid runtime mapping errors.
src/main/java/com/iemr/ecd/dto/QualityAuditorSectionQuestionaireResponseDTO.java (1)
57-58: LGTM: roles added to response DTOThe addition aligns with service-side parsing and should serialize as expected with Lombok @DaTa.
src/main/java/com/iemr/ecd/service/quality/QualityAuditQuestionConfigurationImpl.java (1)
62-65: Good: flatten roles before persistenceFlattening early ensures consistent storage for Role while accepting a List in input.
2 participants
📋 Description
JIRA ID: AMM-1267
Makes the necessary changes for Quality related role mapping in Question Configuration
✅ Type of Change
Summary by CodeRabbit