Skip to content
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.

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

not selected added in single value list #1824

Merged
merged 3 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ const CreateQuestion = ({ onSelect, className, level = 1, initialQuestionData, p
MdmsCriteria: {
tenantId: tenantId,
schemaCode: "HCM-ADMIN-CONSOLE.appFieldTypes",
isActive: true
}
}
}), [tenantId]); // Only recreate if tenantId changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
MdmsCriteria: {
tenantId: tenantId,
schemaCode: `HCM-ADMIN-CONSOLE.ChecklistTemplates`,
isActive: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Document the active-only filtering behavior.

Consider adding a code comment to document that this configuration only returns active checklist templates. This helps other developers understand the filtering behavior.

 schemaCode: `HCM-ADMIN-CONSOLE.ChecklistTemplates`,
+// Only fetch active checklist templates
 isActive: true,
 filters : {},
📝 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
isActive: true,
schemaCode: `HCM-ADMIN-CONSOLE.ChecklistTemplates`,
// Only fetch active checklist templates
isActive: true,
filters : {},

filters : {},
limit: 10,
offset: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ const useBoundaryHome = ({ screenType = "campaign", defaultHierarchyType = "", h
MdmsCriteria: {
tenantId: tenantId,
schemaCode: `${CONSOLE_MDMS_MODULENAME}.HierarchySchema`,
isActive: true
},
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ const CreateChecklist = () => {
filters: {
role: role,
checklistType: checklistType
}
},
isActive: true
}
},
});
Expand Down Expand Up @@ -329,6 +330,9 @@ const CreateChecklist = () => {
return upperCaseString.replace(/ /g, '_');
});
}
if(String(item?.type?.code) === "SingleValueList"){
labelsArray.push("NOT_SELECTED");
}

const questionObject = {
id: item.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const SearchChecklist = () => {
MdmsCriteria: {
tenantId: tenantId,
schemaCode: `${CONSOLE_MDMS_MODULENAME}.rolesForChecklist`,
isActive: true
}
}
};
Expand All @@ -71,7 +72,8 @@ const SearchChecklist = () => {
MdmsCriteria: {
tenantId: tenantId,
schemaCode: "HCM.CHECKLIST_TYPES",
filters: {"type": "DEFAULT"}
filters: {"type": "DEFAULT"},
isActive: true
}
},
changeQueryName: "HCM"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@ const UpdateChecklist = () => {
return upperCaseString.replace(/ /g, '_');
});
}
if(String(item?.type?.code) === "SingleValueList"){
labelsArray.push("NOT_SELECTED");
}
Comment on lines +279 to +281
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

Improve error handling specificity

The error handling could be more specific about what went wrong during the update process.

Consider enhancing the error handling:

 } catch (error) {
-    setShowToast({ label: "CHECKLIST_UPDATE_FAILED", isError: "true" });
+    const errorMessage = error.response?.data?.message || "CHECKLIST_UPDATE_FAILED";
+    setShowToast({ 
+      label: errorMessage,
+      isError: "true",
+      details: error.message 
+    });
     // console.error("Error creating checklist:", error);
 }

Also applies to: 279-281


🧹 Nitpick (assertive)

Improve string comparison robustness

The string comparison for question type could be more resilient to edge cases.

Consider this improvement:

-if(String(item?.type?.code) === "SingleValueList"){
+if(String(item?.type?.code)?.toUpperCase() === "SINGLEVALUELIST"){

This change makes the comparison case-insensitive and more maintainable.

📝 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
if(String(item?.type?.code) === "SingleValueList"){
labelsArray.push("NOT_SELECTED");
}
if(String(item?.type?.code)?.toUpperCase() === "SINGLEVALUELIST"){
labelsArray.push("NOT_SELECTED");
}


const questionObject = {
id: item.id,
Expand Down