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

checklist-bugs-fixes #1513

Merged
merged 2 commits into from
Oct 15, 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 @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import PreviewComponent from "../../components/PreviewComponent";
import { isError, set, template } from "lodash";
import { value } from "jsonpath";
import data_hook from "../../hooks/data_hook";
import def from "ajv/dist/vocabularies/discriminator";
import { QuestionContext } from "../../components/CreateQuestionContext";
// import { LabelFieldPair } from "@egovernments/digit-ui-react-components";
import _ from 'lodash';
Expand All @@ -25,6 +24,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 +41,60 @@ 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 urlMd = window.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH");
const reqCriteriaResource = {
// url: `/mdms-v2/v2/_search`,
url: `/${urlMd}/v2/_search`,
body: {
MdmsCriteria: {
tenantId: tenantId,
// schemaCode: "HCMadminconsole.checklisttemplates"
schemaCode: "HCM-ADMIN-CONSOLE.ChecklistTemplates"
}
},
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(()=>{
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);
} else {
}
} else {
}
Comment on lines +85 to +88
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 empty else blocks to improve code readability

The else blocks at lines 85-86 and 87-88 are empty and do not perform any actions. Removing these empty blocks enhances code readability.

Apply this diff to remove the empty else blocks:

        } else {
-       }
-     } else {
-       }
        }

Refactored code:

useEffect(() => {
  if (localization?.messages?.length > 0) {
    let matchedItem = localization.messages.find(item => item.message === checklistType);
    if (matchedItem) {
      let code = matchedItem.code;
      let res = code.replace("HCM_CHECKLIST_TYPE_", "");
      setChecklistTypeCode(res);
    }
  }
}, [localization]);


}, [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 +103,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 +193,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 +217,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 +328,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,6 +358,13 @@ module = "HCM";
const data = await mutateAsync(payload); // Use mutateAsync for await support
// Handle successful checklist creation
// Proceed with localization if needed
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;
const localisationResult = await localisationMutateAsync(localisations);
Expand Down Expand Up @@ -452,12 +498,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
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