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

checklist-bugs-fixes #1513

Merged
merged 2 commits into from
Oct 15, 2024
Merged

checklist-bugs-fixes #1513

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87bf9ba
checklist-bugs-fixes
Oct 14, 2024
2346626
cahgnes
Oct 15, 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 ...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 @@ -166,7 +166,7 @@ const CreateQuestionContext = ({ onSelect, ...props }) => {
}
};
// const [initialState, setInitialState] = useState([{ id: crypto.randomUUID(), parentId: null, level: 1, key: 1, title: null, type: { "code": "SingleValueList" }, value: null, isRequired: false }])

const [initialState, setInitialState] = useState(()=>{
const savedQuestions = localStorage.getItem("questions");
return savedQuestions ? JSON.parse(savedQuestions) : [{ id: crypto.randomUUID(), parentId: null, level: 1, key: 1, title: null, type: { "code": "SingleValueList" }, value: null, isRequired: false }]
Expand Down
100 changes: 76 additions & 24 deletions ...icro-ui-internals/packages/modules/campaign-manager/src/pages/employee/CreateChecklist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ const CreateChecklist = () => {
const checklistType = searchParams.get("checklistType");
const projectName = searchParams.get("campaignName");
const campagnType = searchParams.get("campaignType");
const [checklistTypeCode, setChecklistTypeCode] = useState(null);
const [roleCode, setRoleCode] = useState(null);
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 state variable 'roleCode'

The state variable roleCode and its setter setRoleCode are declared but not used anywhere in the code. Consider removing them to clean up the code.

Apply this diff to remove the unused state variable:

- const [roleCode, setRoleCode] = useState(null);
📝 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 [roleCode, setRoleCode] = useState(null);

const flow = searchParams.get("flow");
const role = searchParams.get("role");
const campaignName = searchParams.get("campaignName");
Expand All @@ -40,27 +42,62 @@ const CreateChecklist = () => {
const history = useHistory();
let data_mdms=[]
let template_data=[]
Comment on lines 41 to 43
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 variables data_mdms and template_data

The variables data_mdms and template_data are declared but not used anywhere in the code. Removing unused variables helps maintain code cleanliness and readability.

Apply this diff to remove the unused variables:

- let data_mdms=[]
- let template_data=[]
...
- useEffect(()=>{
-   if(data_mdms && data_mdms.length!=0) template_data=data_mdms;
- }, [mdms])

Also applies to: 93-95

🧰 Tools
🪛 Biome

[error] 42-42: This let declares a variable that is only assigned once.

'data_mdms' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

const reqCriteriaResource = {
url: `/mdms-v2/v2/_search`,
body: {
MdmsCriteria: {
const reqCriteriaResource = {
url: `/mdms-v2/v2/_search`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

take the mdms url from global config

body: {
MdmsCriteria: {
tenantId: tenantId,
schemaCode: "HCMadminconsole.checklisttemplates"
Copy link
Collaborator

Choose a reason for hiding this comment

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

can u use schema code as

HCM-ADMIN-CONSOLE.ChecklistTemplates

@suryansh-egov ?

}
},
config: {
enabled: true,
select: (data) => {
return data?.mdms?.[0]?.data?.data;
},
},
// changeQueryName:"checklsit template "
};
const { isLoading, data: mdms, isFetching } = Digit.Hooks.useCustomAPIHook(reqCriteriaResource);
const reqCriteria = {

url: `/localization/messages/v1/_search`,
body:{
tenantId: tenantId
},
params: {
locale: "en_MZ",
tenantId: tenantId,
schemaCode: "HCMadminconsole.checklisttemplates"
}
},
config: {
enabled: true,
select: (data) => {
return data?.mdms?.[0]?.data?.data;
module: "hcm-campaignmanager"
},
},
};
const { isLoading, data: mdms, isFetching } = Digit.Hooks.useCustomAPIHook(reqCriteriaResource);
}
const { isLoading1, data: localization, isFetching1 } = Digit.Hooks.useCustomAPIHook(reqCriteria);
useEffect(()=>{
console.log("the achieved localization is", localization);
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove logs

if (localization?.messages?.length > 0) {
let matchedItem = localization.messages.find(item => item.message === checklistType);
// If a match is found, assign the 'code' to 'checklistcode'
if (matchedItem) {
console.log("matched", matchedItem);
let code = matchedItem.code;
let res = code.replace("HCM_CHECKLIST_TYPE_", "");
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

Use 'const' instead of 'let' for variables that are not reassigned

The variables matchedItem, code, and res are declared with let but are never reassigned. Consider using const instead to indicate that these variables are not meant to change after initialization.

Apply this diff:

- let matchedItem = localization.messages.find(item => item.message === checklistType);
+ const matchedItem = localization.messages.find(item => item.message === checklistType);

- let code = matchedItem.code;
+ const code = matchedItem.code;

- let res = code.replace("HCM_CHECKLIST_TYPE_", "");
+ const res = code.replace("HCM_CHECKLIST_TYPE_", "");
📝 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
let matchedItem = localization.messages.find(item => item.message === checklistType);
// If a match is found, assign the 'code' to 'checklistcode'
if (matchedItem) {
console.log("matched", matchedItem);
let code = matchedItem.code;
let res = code.replace("HCM_CHECKLIST_TYPE_", "");
const matchedItem = localization.messages.find(item => item.message === checklistType);
// If a match is found, assign the 'code' to 'checklistcode'
if (matchedItem) {
console.log("matched", matchedItem);
const code = matchedItem.code;
const res = code.replace("HCM_CHECKLIST_TYPE_", "");
🧰 Tools
🪛 Biome

[error] 78-78: This let declares a variable that is only assigned once.

'matchedItem' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[error] 82-82: This let declares a variable that is only assigned once.

'code' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[error] 83-83: This let declares a variable that is only assigned once.

'res' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

console.log("the res", res);
setChecklistTypeCode(res);
} else {
console.log('No matching checklist type found.');
}
} else {
console.log('Localization or messages array is not available.');
}
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 unnecessary console.log statements

The console.log statements within the useEffect hook may have been used for debugging purposes. Consider removing them or replacing them with a proper logging mechanism appropriate for the production environment.

Apply this diff to remove the console.log statements:

- console.log("the achieved localization is", localization);
- console.log("matched", matchedItem);
- console.log("the res", res);
- console.log('No matching checklist type found.');
- console.log('Localization or messages array is not available.');
📝 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
useEffect(()=>{
console.log("the achieved localization is", localization);
if (localization?.messages?.length > 0) {
let matchedItem = localization.messages.find(item => item.message === checklistType);
// If a match is found, assign the 'code' to 'checklistcode'
if (matchedItem) {
console.log("matched", matchedItem);
let code = matchedItem.code;
let res = code.replace("HCM_CHECKLIST_TYPE_", "");
console.log("the res", res);
setChecklistTypeCode(res);
} else {
console.log('No matching checklist type found.');
}
} else {
console.log('Localization or messages array is not available.');
}
useEffect(()=>{
if (localization?.messages?.length > 0) {
let matchedItem = localization.messages.find(item => item.message === checklistType);
// If a match is found, assign the 'code' to 'checklistcode'
if (matchedItem) {
let code = matchedItem.code;
let res = code.replace("HCM_CHECKLIST_TYPE_", "");
setChecklistTypeCode(res);
}
}
}, [localization, checklistType, setChecklistTypeCode]);
🧰 Tools
🪛 Biome

[error] 78-78: This let declares a variable that is only assigned once.

'matchedItem' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[error] 82-82: This let declares a variable that is only assigned once.

'code' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[error] 83-83: This let declares a variable that is only assigned once.

'res' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)


}, [localization])


useEffect(()=>{
if(data_mdms && data_mdms.length!=0) template_data=data_mdms;
}, [mdms])

module = "HCM";
module = "hcm-checklist";
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

Avoid reassigning the module variable

At line 97, the variable module is reassigned with a new value "hcm-checklist". This may lead to confusion since module was initially assigned from URL parameters. Consider using a new variable name to preserve the original value from searchParams.

Apply this diff:

- module = "hcm-checklist";
+ const moduleChecklist = "hcm-checklist";

And update references to use moduleChecklist instead of module where appropriate.

Committable suggestion was skipped due to low confidence.

const { mutateAsync: localisationMutateAsync } = Digit.Hooks.campaign.useUpsertLocalisation(tenantId, module, locale);

let processedData = [];
Expand All @@ -69,10 +106,10 @@ module = "HCM";
},[])


const [checklistName, setChecklistName] = useState("");
const addChecklistName = (data) => {
setChecklistName(data);
}
const [checklistName, setChecklistName] = useState(`${checklistType} ${role}`);
// const addChecklistName = (data) => {
// setChecklistName(data);
// }
Comment on lines +107 to +109
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 code

The code in lines 110-112 appears to be commented out and may not be needed. Removing unused code helps maintain codebase cleanliness and readability.

Apply this diff:

- // const addChecklistName = (data) => {
- //   setChecklistName(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
// const addChecklistName = (data) => {
// setChecklistName(data);
// }


const closeToast = () => {
setShowToast(null);
Expand Down Expand Up @@ -159,15 +196,18 @@ module = "HCM";
}
codes[question.id] = code;

let moduleChecklist = "hcm-checklist";
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

Use 'const' instead of 'let' for 'moduleChecklist'

The variable moduleChecklist is assigned once and never reassigned. Consider declaring it with const to indicate that it won't change.

Apply this diff:

- let moduleChecklist = "hcm-checklist";
+ const moduleChecklist = "hcm-checklist";
📝 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
let moduleChecklist = "hcm-checklist";
const moduleChecklist = "hcm-checklist";
🧰 Tools
🪛 Biome

[error] 199-199: This let declares a variable that is only assigned once.

'moduleChecklist' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)


let checklistTypeTemp = checklistType.toUpperCase().replace(/ /g, "_");
let roleTemp = role.toUpperCase().replace(/ /g, "_");
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

Use 'const' instead of 'let' for 'roleTemp'

The variable roleTemp is assigned once and never reassigned. Declaring it with const enhances code readability and ensures it cannot be accidentally modified.

Apply this diff:

- let roleTemp = role.toUpperCase().replace(/ /g, "_");
+ const roleTemp = role.toUpperCase().replace(/ /g, "_");
📝 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
let roleTemp = role.toUpperCase().replace(/ /g, "_");
const roleTemp = role.toUpperCase().replace(/ /g, "_");
🧰 Tools
🪛 Biome

[error] 202-202: This let declares a variable that is only assigned once.

'roleTemp' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

if(checklistTypeCode) checklistTypeTemp=checklistTypeCode;
let formattedString = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${code}`;
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

Use 'const' instead of 'let' for 'formattedString'

The variable formattedString is never reassigned after its initial assignment. Consider using const to indicate its immutability.

Apply this diff:

- let formattedString = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${code}`;
+ const formattedString = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${code}`;
📝 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
let formattedString = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${code}`;
const formattedString = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${code}`;
🧰 Tools
🪛 Biome

[error] 204-204: This let declares a variable that is only assigned once.

'formattedString' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)



const obj = {
"code": formattedString,
"message": String(question.title),
"module": module,
"module": moduleChecklist,
"locale": locale
}
local.push(obj);
Expand All @@ -180,12 +220,13 @@ module = "HCM";
const optionval = option.label;
const upperCaseString = optionval.toUpperCase();
const transformedString = upperCaseString.replace(/ /g, '_');
if(checklistTypeCode) checklistTypeTemp=checklistTypeCode;
option.label = transformedString;
let formattedStringTemp = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${option.label}`;
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

Use 'const' instead of 'let' for 'formattedStringTemp'

The variable formattedStringTemp is assigned once and not reassigned. Using const clarifies that its value remains constant.

Apply this diff:

- let formattedStringTemp = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${option.label}`;
+ const formattedStringTemp = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${option.label}`;
📝 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
let formattedStringTemp = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${option.label}`;
const formattedStringTemp = `${campaignName}.${checklistTypeTemp}.${roleTemp}.${option.label}`;
🧰 Tools
🪛 Biome

[error] 225-225: This let declares a variable that is only assigned once.

'formattedStringTemp' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

const obj = {
"code": formattedStringTemp,
"message": String(optionval),
"module": module, // to be dynamic
"module": moduleChecklist, // to be dynamic
"locale": locale //to be dynamic
}
local.push(obj);
Expand Down Expand Up @@ -290,6 +331,7 @@ module = "HCM";
);
let checklistTypeTemp = checklistType.toUpperCase().replace(/ /g, "_");
let roleTemp = role.toUpperCase().replace(/ /g, "_");
if(checklistTypeCode) checklistTypeTemp=checklistTypeCode;
let code_of_checklist = `${campaignName}.${checklistTypeTemp}.${roleTemp}`;
return {
tenantId: tenantId,
Expand Down Expand Up @@ -319,8 +361,18 @@ module = "HCM";
const data = await mutateAsync(payload); // Use mutateAsync for await support
// Handle successful checklist creation
// Proceed with localization if needed
console.log("data success", data);
console.log("ul", uniqueLocal);
let checklistTypeTemp = checklistType.toUpperCase().replace(/ /g, "_");
if(checklistTypeCode) checklistTypeTemp=checklistTypeCode;
let roleTemp = role.toUpperCase().replace(/ /g, "_");
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

Use 'const' instead of 'let' for 'roleTemp'

The variable roleTemp is assigned once and never reassigned. Declaring it with const improves code clarity.

Apply this diff:

- let roleTemp = role.toUpperCase().replace(/ /g, "_");
+ const roleTemp = role.toUpperCase().replace(/ /g, "_");
📝 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
let roleTemp = role.toUpperCase().replace(/ /g, "_");
const roleTemp = role.toUpperCase().replace(/ /g, "_");
🧰 Tools
🪛 Biome

[error] 368-368: This let declares a variable that is only assigned once.

'roleTemp' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

uniqueLocal.push({code: `${campaignName}_${checklistTypeTemp}_${roleTemp}`,
locale: locale,
message: `${checklistType} ${role}`,
module: "hcm-checklist" });
if (data.success) { // Replace with your actual condition
const localisations = uniqueLocal;
console.log("unique local", uniqueLocal);
const localisationResult = await localisationMutateAsync(localisations);
// Check if localization succeeded
if (!localisationResult.success) {
Expand Down Expand Up @@ -452,12 +504,12 @@ module = "HCM";
<div style={{ display: "flex" }}>
<div style={{ width: "26%", fontWeight: "500", marginTop: "0.7rem" }}>{t("NAME_OF_CHECKLIST")}</div>
<TextInput
isRequired={true}
disabled={true}
className="tetxinput-example"
type={"text"}
name={t("NAME_OF_CHECKLIST")}
value={checklistName || ""}
onChange={(event) => addChecklistName(event.target.value)}
value={`${checklistType} ${role}`}
// onChange={(event) => addChecklistName(event.target.value)}
Comment on lines +501 to +506
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)

Consider displaying checklist name as static text instead of a disabled input

Using a disabled TextInput to display the checklist name might not provide the best user experience. Consider displaying the checklist name as static text or a read-only field to improve clarity.

Example change:

- <TextInput
-   disabled={true}
-   className="tetxinput-example"
-   type={"text"}
-   name={t("NAME_OF_CHECKLIST")}
-   value={`${checklistType} ${role}`}
-   placeholder={"Checklist Name"}
- />
+ <div style={{ marginTop: "0.7rem", fontWeight: "500" }}>
+   {`${checklistType} ${role}`}
+ </div>
📝 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
disabled={true}
className="tetxinput-example"
type={"text"}
name={t("NAME_OF_CHECKLIST")}
value={checklistName || ""}
onChange={(event) => addChecklistName(event.target.value)}
value={`${checklistType} ${role}`}
// onChange={(event) => addChecklistName(event.target.value)}
<div style={{ marginTop: "0.7rem", fontWeight: "500" }}>
{`${checklistType} ${role}`}
</div>
// onChange={(event) => addChecklistName(event.target.value)}

placeholder={"Checklist Name"}
/>
</div>
Expand Down
1 change: 1 addition & 0 deletions ...icro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SearchChecklist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const SearchChecklist = () => {
};

const createNewChecklist = () => {
localStorage.removeItem("questions");
history.push(
`/${window.contextPath}/employee/campaign/checklist/create?checklistType=${list?.list}&campaignName=${stateData?.name}&role=${code?.code}&campaignType=${stateData?.campaignType}`
);
Expand Down