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.

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

cleanup and roles from mdms v2 #1634

Merged
merged 8 commits into from
Oct 23, 2024
Merged

cleanup and roles from mdms v2 #1634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7c4a6a5
cleanup and roles from mdms v2
suryansh-egov Oct 23, 2024
6ddcfb0
Merge branch 'console' into feature/cl-final
suryansh-egov Oct 23, 2024
b96ed50
Update CreateQuestionContext.js
suryansh-egov Oct 23, 2024
53b35f0
using global config for mdms-v2 and update checklist code modified
suryansh-egov Oct 23, 2024
910d68a
Merge branch 'feature/cl-final' of https://github.com/egovernments/DI…
suryansh-egov Oct 23, 2024
d075746
Update health/micro-ui/web/micro-ui-internals/packages/modules/campai…
jagankumar-egov Oct 23, 2024
e3c7ec8
Update health/micro-ui/web/micro-ui-internals/packages/modules/campai…
jagankumar-egov Oct 23, 2024
cc8bf7f
Update health/micro-ui/web/micro-ui-internals/packages/modules/campai…
jagankumar-egov Oct 23, 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
301 changes: 153 additions & 148 deletions ...web/micro-ui-internals/packages/modules/campaign-manager/src/components/CreateQuestion.js
View file
Open in desktop

Large diffs are not rendered by default.

90 changes: 29 additions & 61 deletions ...ro-ui-internals/packages/modules/campaign-manager/src/components/CreateQuestionContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@ const CreateQuestionContext = ({ onSelect, ...props }) => {
return [...action.payload];
}
else return state;
case "CLEAR_DATA":
const newState = state.map(item => {
// Set isActive to false for all previous items
return { ...item, isActive: false };
});
const updatedState = [
...newState,
{
...action.payload[0],
}
];
return updatedState;

Comment on lines +41 to +53
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

⚠️ Potential issue

Add block scoping and payload validation to the CLEAR_DATA case.

The current implementation has potential issues:

  1. Switch case declarations need block scoping
  2. Missing payload validation

Apply this fix:

 case "CLEAR_DATA": {
+  if (!action.payload?.[0]) {
+    return state;
+  }
   const newState = state.map(item => ({
     ...item,
     isActive: false
   }));
   return [
     ...newState,
     { ...action.payload[0] }
   ];
 }

Consider filtering out inactive items to improve memory efficiency:

 case "CLEAR_DATA": {
   if (!action.payload?.[0]) {
     return state;
   }
-  const newState = state.map(item => ({
-    ...item,
-    isActive: false
-  }));
+  const newState = state
+    .filter(item => item.isActive)
+    .map(item => ({
+      ...item,
+      isActive: false
+    }));
   return [
     ...newState,
     { ...action.payload[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
case "CLEAR_DATA":
const newState = state.map(item => {
// Set isActive to false for all previous items
return { ...item, isActive: false };
});
const updatedState = [
...newState,
{
...action.payload[0],
}
];
return updatedState;
case "CLEAR_DATA": {
if (!action.payload?.[0]) {
return state;
}
const newState = state
.filter(item => item.isActive)
.map(item => ({
...item,
isActive: false
}));
return [
...newState,
{ ...action.payload[0] }
];
}
🧰 Tools
🪛 Biome

[error] 42-45: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

[error] 46-51: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

case "ADD_QUESTION":
if(action?.payload?.level>3) return state;
return [
Expand Down Expand Up @@ -81,63 +94,6 @@ const CreateQuestionContext = ({ onSelect, ...props }) => {
// Return the original object if the id doesn't match
return i;
});




// let id = action?.payload?.id;
// const deleteQuestionAndSubquestions = (state, questionId) => {
// // Recursive function to find and delete subquestions based on option ids
// const findRelatedSubquestions = (questions, optionIds) => {
// return questions.reduce((acc, question) => {
// // If the current question's parentId matches any of the optionIds
// if (optionIds.includes(question.parentId)) {
// // Add the current question's id to the list of ids to delete
// let relatedIds = [question.id];

// // Check if the current question has options
// if (question.options && question.options.length > 0) {
// const newOptionIds = question.options.map(option => option.id);
// // Recursively find and delete subquestions related to the current question's options
// relatedIds = relatedIds.concat(findRelatedSubquestions(questions, newOptionIds));
// }

// return [...acc, ...relatedIds];
// }
// return acc;
// }, []);
// };

// // Get the question to be deleted
// const questionToDelete = state.find(q => q.id === questionId);

// if (!questionToDelete) {
// return state;
// }

// // Start by collecting IDs of the question's options (if any)
// let idsToDelete = [questionId]; // Start with the main question's ID

// if (questionToDelete.options && questionToDelete.options.length > 0) {
// const optionIds = questionToDelete.options.map(option => option.id);
// // Find and delete subquestions related to the options' ids
// idsToDelete = idsToDelete.concat(findRelatedSubquestions(state, optionIds));
// }

// // Filter out the questions whose ids are in idsToDelete
// const newState = state.filter(question => !idsToDelete.includes(question.id));

// // Print the updated state for verification

// return newState;
// }
// const newState = deleteQuestionAndSubquestions(state, id);
// state = newState;
// return state


// return state.filter((i) => i.key !== action?.payload?.index).map((i, n) => ({ ...i, key: n + 1 }));
break;
case "UPDATE_REQUIRED":
return state.map((i) => {
if (i.id === action?.payload?.id) {
Expand Down Expand Up @@ -209,10 +165,22 @@ const CreateQuestionContext = ({ onSelect, ...props }) => {
if (props?.props?.data !== 0) {
// Dispatch only if the data is different
setTypeOfCall(props?.props?.typeOfCall);
dispatchQuestionData({
type: "UPDATE_QUESTION_DATA",
payload: props?.props?.data,
});
if(props?.props?.typeOfCall === "clear")
{
dispatchQuestionData({
type: "CLEAR_DATA",
payload: props?.props?.data
})

}
else{
dispatchQuestionData({
type: "UPDATE_QUESTION_DATA",
payload: props?.props?.data,
});

}

Comment on lines +168 to +183
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

Improve useEffect robustness and clarity.

The current implementation has several potential issues:

  1. The condition props?.props?.data !== 0 is unclear and could be improved
  2. Deep prop access without proper null checks
  3. Missing cleanup function

Apply these improvements:

 useEffect(() => {
-  if (props?.props?.data !== 0) {
+  // Ensure we have valid data to process
+  if (!props?.props) {
+    return;
+  }
+
+  const { data, typeOfCall } = props.props;
+  if (!data) {
+    return;
+  }
+
   setTypeOfCall(props?.props?.typeOfCall);
-  if(props?.props?.typeOfCall === "clear") {
+  if(typeOfCall === "clear") {
     dispatchQuestionData({
       type: "CLEAR_DATA",
-      payload: props?.props?.data
+      payload: data
     });
   } else {
     dispatchQuestionData({
       type: "UPDATE_QUESTION_DATA",
-      payload: props?.props?.data,
+      payload: data,
     });
   }
-  }
+
+  // Cleanup function
+  return () => {
+    setTypeOfCall(null);
+  };
 }, [props?.props?.data]);
📝 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(props?.props?.typeOfCall === "clear")
{
dispatchQuestionData({
type: "CLEAR_DATA",
payload: props?.props?.data
})
}
else{
dispatchQuestionData({
type: "UPDATE_QUESTION_DATA",
payload: props?.props?.data,
});
}
// Ensure we have valid data to process
if (!props?.props) {
return;
}
const { data, typeOfCall } = props.props;
if (!data) {
return;
}
setTypeOfCall(props?.props?.typeOfCall);
if(typeOfCall === "clear") {
dispatchQuestionData({
type: "CLEAR_DATA",
payload: data
});
} else {
dispatchQuestionData({
type: "UPDATE_QUESTION_DATA",
payload: data,
});
}
// Cleanup function
return () => {
setTypeOfCall(null);
};


}
}, [props?.props?.data]); // Ensure that the initialState is included in the dependency array
Expand Down
74 changes: 38 additions & 36 deletions .../web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,19 @@ const businessServiceMap = {};
const inboxModuleNameMap = {};


const updateServiceDefinition = async (newStatus) => {
const updateServiceDefinition = async (tenantId, newStatus, sdcode) => {
try {
const res = await Digit.CustomService.getResponse({
url: "/service-request/service/definition/v1/_update",
body: {
ServiceDefinition: {
"tenantId": tenantId,
"code": serviceCode,
"code": sdcode,
"isActive": newStatus
},
},
});
if (res) {
rowDataCache[serviceCode].isActive = newStatus;
if (apiCache[serviceCode]) {
apiCache[serviceCode].ServiceDefinitions[0].isActive = newStatus;
}
}
return res;
} catch (error) {
Expand Down Expand Up @@ -67,61 +63,67 @@ export const UICustomizations = {

additionalCustomizations: (row, key, column, value, searchResult) => {
const { t } = useTranslation();
const tenantId = Digit.ULBService.getCurrentTenantId();
const history = useHistory();
const location = useLocation();
const searchParams = new URLSearchParams(location.search);
const campaignName = searchParams.get("name");
switch (key) {

case "STATUS":
// const [localIsActive, setLocalIsActive] = useState(rowDataCache[serviceCode].isActive);

// const toggle = async () => {
// const newStatus = !localIsActive;
// const res = await updateServiceDefinition(newStatus);
// if (res) {
// rowDataCache[serviceCode].isActive = newStatus;
// setLocalIsActive(newStatus);
// }
// };


// const [localIsActive, setLocalIsActive] = useState(rowDataCache[serviceCode].isActive);
const toggle = async () => {
const prev = row?.ServiceRequest?.[0]?.isActive;
const sdcode = row?.ServiceRequest?.[0]?.code;
const res = await updateServiceDefinition(tenantId, !prev, sdcode);
if(res)
{

}
};
Comment on lines +75 to +84
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 user feedback for toggle action.

The toggle function should provide feedback to users about the success or failure of the status update.

Consider adding toast notifications:

 const toggle = async () => {
   const prev = row?.ServiceRequest?.[0]?.isActive;
   const sdcode = row?.ServiceRequest?.[0]?.code;
-  const res = await updateServiceDefinition(tenantId, !prev, sdcode);
-  if(res)
-  {
-
-  }
+  try {
+    const res = await updateServiceDefinition(tenantId, !prev, sdcode);
+    if(res) {
+      Digit.Utils.toast.success("Status updated successfully");
+    }
+  } catch (error) {
+    Digit.Utils.toast.error("Failed to update status");
+  }
 };
📝 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
// const [localIsActive, setLocalIsActive] = useState(rowDataCache[serviceCode].isActive);
const toggle = async () => {
const prev = row?.ServiceRequest?.[0]?.isActive;
const sdcode = row?.ServiceRequest?.[0]?.code;
const res = await updateServiceDefinition(tenantId, !prev, sdcode);
if(res)
{
}
};
// const [localIsActive, setLocalIsActive] = useState(rowDataCache[serviceCode].isActive);
const toggle = async () => {
const prev = row?.ServiceRequest?.[0]?.isActive;
const sdcode = row?.ServiceRequest?.[0]?.code;
try {
const res = await updateServiceDefinition(tenantId, !prev, sdcode);
if(res) {
Digit.Utils.toast.success("Status updated successfully");
}
} catch (error) {
Digit.Utils.toast.error("Failed to update status");
}
};
🧰 Tools
🪛 Biome

[error] 76-84: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


// const switchText = localIsActive ? "Active" : "Inactive";
return (
<Switch
isCheckedInitially={row?.ServiceRequest?.[0]?.isActive}
label={""}
onToggle={()=>{}}
onToggle={toggle}
/>
);
case "ACTION":
// if (rowDataCache[serviceCode].attributes) {
// return (
// <Button
// type="button"
// size="medium"
// icon="View"
// variation="secondary"
// label={t("VIEW")}
// onClick={() => {
// history.push(`/${window.contextPath}/employee/campaign/checklist/view?campaignName=${campaignName}&role=${role_code}&checklistType=${cl_code}`)
// }}
// />
// );
// } else {
return (
const role_code = row?.data?.role;
const cl_code = row?.data?.checklistType;
const sd = row?.ServiceRequest?.[0];
if(sd)
{
return (
<Button
type="button"
size="medium"
icon="View"
// icon="View"
variation="secondary"
label={t("VIEW")}
onClick={() => {
history.push(`/${window.contextPath}/employee/campaign/checklist/view?campaignName=${campaignName}&role=${role_code}&checklistType=${cl_code}`)
}}
/>
)
}
else{
return (
<Button
type="button"
size="medium"
// icon="View"
variation="secondary"
label={t("CREATE")}
onClick={() => {
history.push(`/${window.contextPath}/employee/campaign/checklist/create?campaignName=${campaignName}&role=${role_code}&checklistType=${cl_code}`)
}}
/>
);
// }
)
}
Comment on lines +95 to +126
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

Fix switch case variable declarations to prevent scope leakage.

Variables declared in switch cases should be wrapped in blocks to prevent them from being accessible in other cases.

Apply this fix:

 case "ACTION":
+  {
   const role_code = row?.data?.role;
   const cl_code = row?.data?.checklistType;
   const sd = row?.ServiceRequest?.[0];
   if(sd) {
     return (
       <Button
         type="button"
         size="medium"
         variation="secondary"
         label={t("VIEW")}
         onClick={() => {
           history.push(`/${window.contextPath}/employee/campaign/checklist/view?campaignName=${campaignName}&role=${role_code}&checklistType=${cl_code}`)
         }}
       />
     )
   } else {
     return (
       <Button
         type="button"
         size="medium"
         variation="secondary"
         label={t("CREATE")}
         onClick={() => {
           history.push(`/${window.contextPath}/employee/campaign/checklist/create?campaignName=${campaignName}&role=${role_code}&checklistType=${cl_code}`)
         }}
       />
     )
   }
+  }

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 Biome

[error] 95-95: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

[error] 96-96: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

[error] 97-97: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

default:
return value;
}
Expand Down
5 changes: 3 additions & 2 deletions ...micro-ui-internals/packages/modules/campaign-manager/src/configs/checklistSearchConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@

const tenantId = Digit.ULBService.getCurrentTenantId();
const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
export const checklistSearchConfig = [
{
label: "Checklist Search",
type: "search",
apiDetails: {
serviceName: "/mdms-v2/v2/_search",
serviceName: `/${mdms_context_path}/v2/_search`,
requestParam: {
"tenantId":tenantId,
},
requestBody: {
MdmsCriteria: {
tenantId: tenantId,
// schemaCode: "HCMadminconsole.checklisttemplates"
schemaCode: "HCM-ADMIN-CONSOLE.ChecklistTemplates_DEMO",
schemaCode: "HCM-ADMIN-CONSOLE.Checklist_Templates",
filters : {}
}
Comment on lines 16 to 19
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)

Remove commented out schema code.

Clean up the old commented schema code to improve code readability.

-            // schemaCode: "HCMadminconsole.checklisttemplates"
             schemaCode: "HCM-ADMIN-CONSOLE.Checklist_Templates",
📝 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
// schemaCode: "HCMadminconsole.checklisttemplates"
schemaCode: "HCM-ADMIN-CONSOLE.ChecklistTemplates_DEMO",
schemaCode: "HCM-ADMIN-CONSOLE.Checklist_Templates",
filters : {}
}
schemaCode: "HCM-ADMIN-CONSOLE.Checklist_Templates",
filters : {}
}

},
Expand Down
4 changes: 3 additions & 1 deletion ...i-internals/packages/modules/campaign-manager/src/hooks/services/createTypeOfChecklist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const createTypeOfChecklist = async (req, tenantId) => {
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

Remove unused tenantId parameter.

The function accepts a tenantId parameter but never uses it. Either remove the unused parameter or document why it's needed for future use.

-const createTypeOfChecklist = async (req, tenantId) => {
+const createTypeOfChecklist = async (req) => {

Committable suggestion was skipped due to low confidence.

const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
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

Consider moving configuration retrieval outside the function.

The MDMS context path configuration is retrieved on every function call. Consider moving this to a module-level constant or a configuration hook for better performance and reusability.

+const MDMS_CONTEXT_PATH = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
+
 const createTypeOfChecklist = async (req, tenantId) => {
-  const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
-
   try {
     const response = await Digit.CustomService.getResponse({
-      url: `/${mdms_context_path}/v1/_search`,
+      url: `/${MDMS_CONTEXT_PATH}/v1/_search`,
📝 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
const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
const MDMS_CONTEXT_PATH = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";


try {
const response = await Digit.CustomService.getResponse({
url: "/mdms-v2/v1/_search",
url: `/${mdms_context_path}/v1/_search`,
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

Validate MDMS context path to prevent path injection.

The URL is constructed using a configurable value without validation. This could potentially lead to path injection if the configuration is compromised.

Add validation for the context path:

+const validateContextPath = (path) => {
+  return /^[a-zA-Z0-9-]+$/.test(path) ? path : "mdms-v2";
+};
+
 const createTypeOfChecklist = async (req, tenantId) => {
-  const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
+  const mdms_context_path = validateContextPath(window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH"));

Committable suggestion was skipped due to low confidence.

body: {
MdmsCriteria: req,
},
Expand Down
Loading