hierarchy #1647
hierarchy #1647
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several enhancements across multiple components within the campaign manager module. Key changes include the addition of a delay function in Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js (1)
Line range hint
232-238: Improve the file download implementation.The current implementation has potential reliability issues:
- The hardcoded 2-second delay might not be sufficient for all file sizes
- No loading state management for user feedback
Consider implementing a polling mechanism instead of a fixed delay:
const downloadExcelTemplate = async () => { + try { const res = await generateFile(); - await delay(2000); + let attempts = 0; + const maxAttempts = 10; + while (attempts < maxAttempts) { + const resFile = await generateTemplate(); + if (resFile?.GeneratedResource?.[0]?.fileStoreid) { + const fileNameWithoutExtension = row?.hierarchyType; + Digit.Utils.campaign.downloadExcelWithCustomName({ + fileStoreId: resFile.GeneratedResource[0].fileStoreid, + customName: fileNameWithoutExtension + }); + return; + } + attempts++; + await delay(1000); + } + throw new Error('File generation timed out'); + } catch (error) { + console.error('Error downloading template:', error); + // Add error notification here + } - const resFile = await generateTemplate(); - if (resFile && resFile?.GeneratedResource?.[0]?.fileStoreid) { - const fileNameWithoutExtension = row?.hierarchyType; - Digit.Utils.campaign.downloadExcelWithCustomName({ fileStoreId: resFile?.GeneratedResource?.[0]?.fileStoreid, customName: fileNameWithoutExtension }); - } }🧰 Tools
🪛 Biome
[error] 234-234: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (1)
Line range hint
108-112: Ensure consistent casing for 'fileStoreId'In the
downloadExcelTemplatefunction, you're accessingresFile?.GeneratedResource?.[0]?.fileStoreidwith a lowercase 'i' in 'fileStoreid'. However, elsewhere in the code and typically in JavaScript conventions, it's used asfileStoreIdwith an uppercase 'I'. This inconsistency might lead toundefinedvalues and potential errors if the API actually returnsfileStoreId.Apply this diff to correct the property name:
- if (resFile && resFile?.GeneratedResource?.[0]?.fileStoreid) { + if (resFile && resFile?.GeneratedResource?.[0]?.fileStoreId) {🧰 Tools
🪛 Biome
[error] 110-110: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Boundary.js (5 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Boundary.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (1)
Pattern
**/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js
[error] 215-215: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Boundary.js
[error] 204-204: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js
[error] 196-196: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 198-198: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
🔇 Additional comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Boundary.js (1)
201-201: 🧹 Nitpick (assertive)Modify
Toastcomponent to be self-closingJSX elements without children should be self-closing to adhere to style guidelines.
Apply this diff:
-{toast && - <Toast label={t("USER_NOT_AUTHORISED")} type={"error"} onClose={() => setToast(false)} ></Toast>} +{toast && + <Toast label={t("USER_NOT_AUTHORISED")} type={"error"} onClose={() => setToast(false)} />}Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (1)
154-191: Verify API response handling in 'callCreateDataApi'After calling the
_createAPI, you extractidandtypeOfDatafromcreateResponse?.ResourceDetails. Ensure thatcreateResponse?.ResourceDetailsexists and contains these properties to prevent potentialundefinedaccess.Please verify that
createResponse?.ResourceDetailsalways containsidandtypewhen the API call is successful.
| @@ -212,6 +212,7 @@ export const UICustomizations = { | |||
| }); | |||
| return res; | |||
| } | |||
| const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms)); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing the delay utility function.
The delay function could be improved for better maintainability and reusability.
-const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms));
+/**
+ * Creates a Promise that resolves after the specified delay
+ * @param {number} ms - The delay duration in milliseconds
+ * @returns {Promise<void>}
+ */
+const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms));Consider moving this utility function to a shared utilities file for reuse across the application.
📝 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.
| const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms)); | |
| /** | |
| * Creates a Promise that resolves after the specified delay | |
| * @param {number} ms - The delay duration in milliseconds | |
| * @returns {Promise<void>} | |
| */ | |
| const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms)); |
🧰 Tools
🪛 Biome
[error] 215-215: 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)
Fix switch case declaration scope.
The delay function is declared within a switch case, which could lead to scoping issues.
Move the declaration outside the switch statement:
+const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms));
+
switch (key) {
case "ACTION":
const tenantId = Digit.ULBService.getCurrentTenantId();
const generateFile = async () => {
// ...
}
- const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms));
const generateTemplate = async () => {
// ...
}
// ...
}Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 215-215: 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 [config, setConfig] = useState({defaultHierarchyType: "DEFAULTBOUNDARY", | ||
| // hierarchyType: "DEMOONCONSOLE"}); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove commented-out code to improve code cleanliness
The commented-out code appears to be unnecessary. Removing it can help keep the codebase clean and maintainable.
| isDisabled={!directView && !authorized} | ||
| variation={"secondary"} | ||
| label={t("EDIT_BOUNDARY_DATA")} | ||
| onClick={() => { | ||
| // setShowPopUp(false); | ||
| callDirectView(); | ||
| if(authorized) | ||
| { | ||
| callDirectView(); | ||
| } | ||
| else{ | ||
| setToast(true); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align isDisabled property with authorized state for the "Edit Boundary Data" button
For consistency, adjust the isDisabled property and simplify the onClick handler by relying on the button's disabled state.
Apply this diff:
-isDisabled={!directView && !authorized}
+isDisabled={!authorized}
...
onClick={() => {
- if(authorized)
- {
callDirectView();
- }
- else{
- setToast(true);
- }
}}📝 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.
| isDisabled={!directView && !authorized} | |
| variation={"secondary"} | |
| label={t("EDIT_BOUNDARY_DATA")} | |
| onClick={() => { | |
| // setShowPopUp(false); | |
| callDirectView(); | |
| if(authorized) | |
| { | |
| callDirectView(); | |
| } | |
| else{ | |
| setToast(true); | |
| } | |
| isDisabled={!authorized} | |
| variation={"secondary"} | |
| label={t("EDIT_BOUNDARY_DATA")} | |
| onClick={() => { | |
| // setShowPopUp(false); | |
| callDirectView(); |
| const [defaultHierarchyType, setDefaultHierarchyType] = useState(config?.defaultHierarchyType || searchParams.get("defaultHierarchyType")); | ||
| const [hierarchyType, setHierarchyType] = useState(config?.hierarchyType || searchParams.get("hierarchyType")); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify optional chaining for the config object
Since config is always defined in this scope, you can remove the optional chaining config?. to simplify the code.
Apply this diff:
-const [defaultHierarchyType, setDefaultHierarchyType] = useState(config?.defaultHierarchyType || searchParams.get("defaultHierarchyType"));
+const [defaultHierarchyType, setDefaultHierarchyType] = useState(config.defaultHierarchyType || searchParams.get("defaultHierarchyType"));
-const [hierarchyType, setHierarchyType] = useState(config?.hierarchyType || searchParams.get("hierarchyType"));
+const [hierarchyType, setHierarchyType] = useState(config.hierarchyType || searchParams.get("hierarchyType"));📝 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.
| const [defaultHierarchyType, setDefaultHierarchyType] = useState(config?.defaultHierarchyType || searchParams.get("defaultHierarchyType")); | |
| const [hierarchyType, setHierarchyType] = useState(config?.hierarchyType || searchParams.get("hierarchyType")); | |
| const [defaultHierarchyType, setDefaultHierarchyType] = useState(config.defaultHierarchyType || searchParams.get("defaultHierarchyType")); | |
| const [hierarchyType, setHierarchyType] = useState(config.hierarchyType || searchParams.get("hierarchyType")); |
| if (JSON.stringify(hierarchyType) !== JSON.stringify(newHierarchyTypes)) { | ||
| setHierarchyType(newHierarchyTypes); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use direct comparison for hierarchy types instead of JSON.stringify
When comparing hierarchyType and newHierarchyTypes, using JSON.stringify may be unnecessary if they are strings or simple types. Consider using direct comparison for better performance.
Apply this diff:
-if (JSON.stringify(hierarchyType) !== JSON.stringify(newHierarchyTypes)) {
+if (hierarchyType !== newHierarchyTypes) {Similarly, for defaultHierarchyType:
-if (JSON.stringify(defaultHierarchyType) !== JSON.stringify(newDefHierarchyTypes)) {
+if (defaultHierarchyType !== newDefHierarchyTypes) {Committable suggestion was skipped due to low confidence.
| // isDisabled={!authorized} | ||
| variation={"secondary"} | ||
| label={t("CREATE_NEW_BOUNDARY_DATA")} | ||
| onClick={() => { | ||
| if (direct && geoPodeData) { | ||
| if(authorized) | ||
| { | ||
| if (direct) { | ||
| callDirectView(); | ||
| } else { | ||
| setShowPopUp(true); | ||
| } | ||
| } | ||
| else{ | ||
| setToast(true); | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent handling of authorized checks and isDisabled properties
To improve user experience and code consistency, consider re-enabling the isDisabled={!authorized} property for the "Create New Boundary Data" button and removing the authorized check from the onClick handler. This prevents unauthorized users from interacting with the button.
Apply this diff:
-// isDisabled={!authorized}
+isDisabled={!authorized}
...
onClick={() => {
- if(authorized)
- {
if (direct) {
callDirectView();
} else {
setShowPopUp(true);
}
- }
- else{
- setToast(true);
- }
}}📝 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.
| // isDisabled={!authorized} | |
| variation={"secondary"} | |
| label={t("CREATE_NEW_BOUNDARY_DATA")} | |
| onClick={() => { | |
| if (direct && geoPodeData) { | |
| if(authorized) | |
| { | |
| if (direct) { | |
| callDirectView(); | |
| } else { | |
| setShowPopUp(true); | |
| } | |
| } | |
| else{ | |
| setToast(true); | |
| } | |
| isDisabled={!authorized} | |
| variation={"secondary"} | |
| label={t("CREATE_NEW_BOUNDARY_DATA")} | |
| onClick={() => { | |
| if (direct) { | |
| callDirectView(); | |
| } else { | |
| setShowPopUp(true); | |
| } | |
| }} |
| setDataCreationGoing(false); | ||
| return true; // Stop polling once status is "completed" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Double-check success toast message placement
In the polling function, upon successful completion (status === "completed"), you're setting a success toast message inside pollForStatusCompletion. Verify if displaying the toast here aligns with the user experience, or if it should be managed after polling completes in callCreateDataApi.
| let label = `${t("WBH_BOUNDARY_CREATION_FAIL")}: `; | ||
| resp?.response?.data?.Errors?.map((err, idx) => { | ||
| if (idx === resp?.response?.data?.Errors?.length - 1) { | ||
| label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + "."; | ||
| label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + "."; | ||
| } else { | ||
| label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + ", "; | ||
| label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + ", "; | ||
| } | ||
| }); | ||
| setShowToast({ label, isError: "error" }); | ||
| setDataCreationGoing(false); | ||
| return {}; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use template literals instead of string concatenation
The static analysis tool has flagged the use of string concatenation in building the label string. Using template literals improves readability and consistency.
Apply this diff to refactor the code:
let label = `${t("WBH_BOUNDARY_CREATION_FAIL")}: `;
resp?.response?.data?.Errors?.map((err, idx) => {
if (idx === resp?.response?.data?.Errors?.length - 1) {
- label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + ".";
+ label = `${label}${t(Digit.Utils.locale.getTransformedLocale(err?.code))}.`;
} else {
- label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + ", ";
+ label = `${label}${t(Digit.Utils.locale.getTransformedLocale(err?.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.
| let label = `${t("WBH_BOUNDARY_CREATION_FAIL")}: `; | |
| resp?.response?.data?.Errors?.map((err, idx) => { | |
| if (idx === resp?.response?.data?.Errors?.length - 1) { | |
| label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + "."; | |
| label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + "."; | |
| } else { | |
| label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + ", "; | |
| label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + ", "; | |
| } | |
| }); | |
| setShowToast({ label, isError: "error" }); | |
| setDataCreationGoing(false); | |
| return {}; | |
| }); | |
| let label = `${t("WBH_BOUNDARY_CREATION_FAIL")}: `; | |
| resp?.response?.data?.Errors?.map((err, idx) => { | |
| if (idx === resp?.response?.data?.Errors?.length - 1) { | |
| label = `${label}${t(Digit.Utils.locale.getTransformedLocale(err?.code))}.`; | |
| } else { | |
| label = `${label}${t(Digit.Utils.locale.getTransformedLocale(err?.code))}, `; | |
| } | |
| }); |
🧰 Tools
🪛 Biome
[error] 196-196: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 198-198: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| const pollForStatusCompletion = async (id, typeOfData) => { | ||
| const pollInterval = 1000; // Poll every 1 seconds | ||
| const maxRetries = 10; // Limit the number of retries to avoid infinite loop | ||
| let retries = 0; | ||
|
|
||
| const poll = async () => { | ||
| if (retries >= maxRetries) { | ||
| throw new Error("Max retries reached"); | ||
| } | ||
|
|
||
| const searchResponse = await Digit.CustomService.getResponse({ | ||
| url: "/project-factory/v1/data/_search", | ||
| params: {}, | ||
| body: { | ||
| SearchCriteria:{ | ||
| id: [id], | ||
| tenantId: tenantId, | ||
| type: typeOfData | ||
| } | ||
|
|
||
| }, | ||
| }); | ||
|
|
||
| const status = searchResponse?.ResourceDetails?.status; | ||
|
|
||
| if (status === "completed") { | ||
| setShowToast({ label: `${t("WBH_HIERARCHY_STATUS_COMPLETED")}`, isError: "success" }); | ||
| setDataCreationGoing(false); | ||
| return true; // Stop polling once status is "completed" | ||
| } else { | ||
| retries++; | ||
| setTimeout(poll, pollInterval); // Retry after the interval | ||
| } | ||
| }; | ||
|
|
||
| await poll(); | ||
| }; |
There was a problem hiding this comment.
Handle 'maxRetries' scenario in polling mechanism
In the pollForStatusCompletion function, if the maxRetries limit is reached without the status being "completed", the function throws an error but does not reset the dataCreationGoing state. This could leave the UI in a loading state indefinitely.
Apply this diff to ensure the state is properly reset:
if (retries >= maxRetries) {
+ setDataCreationGoing(false);
throw new Error("Max retries reached");
}📝 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.
| const pollForStatusCompletion = async (id, typeOfData) => { | |
| const pollInterval = 1000; // Poll every 1 seconds | |
| const maxRetries = 10; // Limit the number of retries to avoid infinite loop | |
| let retries = 0; | |
| const poll = async () => { | |
| if (retries >= maxRetries) { | |
| throw new Error("Max retries reached"); | |
| } | |
| const searchResponse = await Digit.CustomService.getResponse({ | |
| url: "/project-factory/v1/data/_search", | |
| params: {}, | |
| body: { | |
| SearchCriteria:{ | |
| id: [id], | |
| tenantId: tenantId, | |
| type: typeOfData | |
| } | |
| }, | |
| }); | |
| const status = searchResponse?.ResourceDetails?.status; | |
| if (status === "completed") { | |
| setShowToast({ label: `${t("WBH_HIERARCHY_STATUS_COMPLETED")}`, isError: "success" }); | |
| setDataCreationGoing(false); | |
| return true; // Stop polling once status is "completed" | |
| } else { | |
| retries++; | |
| setTimeout(poll, pollInterval); // Retry after the interval | |
| } | |
| }; | |
| await poll(); | |
| }; | |
| const pollForStatusCompletion = async (id, typeOfData) => { | |
| const pollInterval = 1000; // Poll every 1 seconds | |
| const maxRetries = 10; // Limit the number of retries to avoid infinite loop | |
| let retries = 0; | |
| const poll = async () => { | |
| if (retries >= maxRetries) { | |
| setDataCreationGoing(false); | |
| throw new Error("Max retries reached"); | |
| } | |
| const searchResponse = await Digit.CustomService.getResponse({ | |
| url: "/project-factory/v1/data/_search", | |
| params: {}, | |
| body: { | |
| SearchCriteria:{ | |
| id: [id], | |
| tenantId: tenantId, | |
| type: typeOfData | |
| } | |
| }, | |
| }); | |
| const status = searchResponse?.ResourceDetails?.status; | |
| if (status === "completed") { | |
| setShowToast({ label: `${t("WBH_HIERARCHY_STATUS_COMPLETED")}`, isError: "success" }); | |
| setDataCreationGoing(false); | |
| return true; // Stop polling once status is "completed" | |
| } else { | |
| retries++; | |
| setTimeout(poll, pollInterval); // Retry after the interval | |
| } | |
| }; | |
| await poll(); | |
| }; |
|
|
||
| if (status === "completed") { | ||
| setShowToast({ label: `${t("WBH_HIERARCHY_STATUS_COMPLETED")}`, isError: "success" }); | ||
| setDataCreationGoing(false); |
There was a problem hiding this comment.
if dataCreationGoing show a loader linke in upload screen
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (1)
Pattern
**/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js
[error] 278-278: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.(lint/complexity/noUselessCatch)
[error] 293-293: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 295-295: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
🔇 Additional comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (1)
293-295: Use template literals instead of string concatenationThis issue has already been noted in previous reviews.
🧰 Tools
🪛 Biome
[error] 293-293: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 295-295: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| @@ -105,7 +104,7 @@ const ViewHierarchy = () => { | |||
|
|
|||
| } | |||
| const downloadExcelTemplate = async() => { | |||
| const res = await generateFile() | |||
| // const res = await generateFile() | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider removing commented-out code
The line // const res = await generateFile() is commented out. If this code is no longer needed, consider removing it to keep the codebase clean.
| // const callCreateDataApi = async () => { | ||
| // setDisable(true); | ||
| // setDataCreationGoing(true); | ||
| // try { | ||
| // setDataCreateToast(true); | ||
|
|
||
| // // Call the create API | ||
| // const createResponse = await Digit.CustomService.getResponse({ | ||
| // url: "/project-factory/v1/data/_create", | ||
| // params: {}, | ||
| // body: { | ||
| // ResourceDetails: { | ||
| // tenantId: tenantId, | ||
| // type: "boundaryManagement", | ||
| // fileStoreId: fileStoreId, | ||
| // action: "create", | ||
| // hierarchyType: hierarchyType, | ||
| // additionalDetails: { | ||
| // source: "boundary", | ||
| // }, | ||
| // campaignId: "default" | ||
| // }, | ||
| // }, | ||
| // }); | ||
|
|
||
| // // Extract the id from the response | ||
| // const id = createResponse?.ResourceDetails?.id; | ||
| // const typeOfData = createResponse?.ResourceDetails?.type; | ||
|
|
||
| // if (id) { | ||
| // // Start polling the search API | ||
| // await pollForStatusCompletion(id, typeOfData); | ||
| // } | ||
|
|
||
| // setDataCreateToast(false); | ||
| // setShowToast({ label: `${t("WBH_HIERARCHY_CREATED")}`, isError: "success" }); | ||
| // return createResponse; | ||
| // } catch (resp) { | ||
| // setDisable(false); | ||
| // let label = `${t("WBH_BOUNDARY_CREATION_FAIL")}: `; | ||
| // resp?.response?.data?.Errors?.map((err, idx) => { | ||
| // if (idx === resp?.response?.data?.Errors?.length - 1) { | ||
| // label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + "."; | ||
| // } else { | ||
| // label = label + t(Digit.Utils.locale.getTransformedLocale(err?.code)) + ", "; | ||
| // } | ||
| // }); | ||
| // setShowToast({ label, isError: "error" }); | ||
| // setDataCreationGoing(false); | ||
| // return {}; | ||
| // } | ||
| // }; | ||
|
|
||
| // // Function to poll the search API until status is "completed" | ||
| // const pollForStatusCompletion = async (id, typeOfData) => { | ||
| // const pollInterval = 1000; // Poll every 1 seconds | ||
| // const maxRetries = 10; // Limit the number of retries to avoid infinite loop | ||
| // let retries = 0; | ||
|
|
||
| // const poll = async () => { | ||
| // if (retries >= maxRetries) { | ||
| // throw new Error("Max retries reached"); | ||
| // } | ||
|
|
||
| // const searchResponse = await Digit.CustomService.getResponse({ | ||
| // url: "/project-factory/v1/data/_search", | ||
| // params: {}, | ||
| // body: { | ||
| // SearchCriteria:{ | ||
| // id: [id], | ||
| // tenantId: tenantId, | ||
| // type: typeOfData | ||
| // } | ||
|
|
||
| // }, | ||
| // }); | ||
|
|
||
| // const status = searchResponse?.ResourceDetails?.status; | ||
|
|
||
| // if (status === "completed") { | ||
| // setShowToast({ label: `${t("WBH_HIERARCHY_STATUS_COMPLETED")}`, isError: "success" }); | ||
| // setDataCreationGoing(false); | ||
| // return true; // Stop polling once status is "completed" | ||
| // } else { | ||
| // retries++; | ||
| // setTimeout(poll, pollInterval); // Retry after the interval | ||
| // } | ||
| // }; | ||
|
|
||
| // await poll(); | ||
| // }; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unnecessary commented-out code
The block of code from lines 153 to 244 is commented out. If this code is obsolete or no longer needed, consider removing it to improve code readability and maintainability.
| {previewPage && ( | ||
| <Card type={"primary"} variant={"viewcard"} className={"example-view-card"}> | ||
| <div style={{fontSize:"2.5rem", fontWeight:700}}>{t("CONFIRM_BOUNDARY_DATA")}</div> | ||
| <div style={{height:"1.5rem"}}></div> | ||
| <XlsPreviewNew file={fileData} onDownload={() => {}} onBack={() => {setShowPreview(false); setUploadPage(true)}} /> | ||
| {!dataCreationGoing && <XlsPreviewNew file={fileData} onDownload={() => {}} onBack={() => {setShowPreview(false); setUploadPage(true)}} />} |
There was a problem hiding this comment.
Fix undefined state variables in onBack handler
In the onBack handler of XlsPreviewNew, you're calling setShowPreview and setUploadPage, but these state setters are not defined in the component. It seems you intended to use setPreviewPage and setFirstPage, which are the defined state variables.
Apply this diff to fix the state setters:
{!dataCreationGoing && (
<XlsPreviewNew
file={fileData}
onDownload={() => {}}
onBack={() => {
- setShowPreview(false);
- setUploadPage(true);
+ setPreviewPage(false);
+ setFirstPage(true);
}}
/>
)}📝 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.
| {!dataCreationGoing && <XlsPreviewNew file={fileData} onDownload={() => {}} onBack={() => {setShowPreview(false); setUploadPage(true)}} />} | |
| {!dataCreationGoing && ( | |
| <XlsPreviewNew | |
| file={fileData} | |
| onDownload={() => {}} | |
| onBack={() => { | |
| setPreviewPage(false); | |
| setFirstPage(true); | |
| }} | |
| /> | |
| )} |
| throw pollError; // Propagate polling errors to the outer catch block | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove unnecessary catch clause that rethrows the error
The catch block catches pollError only to rethrow it without any additional handling. This is redundant and can be removed to simplify the code.
Apply this diff to remove the unnecessary try-catch block:
if (id) {
- try {
await pollForStatusCompletion(id, typeOfData);
setDataCreateToast(false);
setShowToast({ label: `${t("WBH_HIERARCHY_CREATED")}`, isError: "success" });
- } catch (pollError) {
- throw pollError; // Propagate polling errors to the outer catch block
- }
}📝 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.
| throw pollError; // Propagate polling errors to the outer catch block | |
| } | |
| } | |
| if (id) { | |
| await pollForStatusCompletion(id, typeOfData); | |
| setDataCreateToast(false); | |
| setShowToast({ label: `${t("WBH_HIERARCHY_CREATED")}`, isError: "success" }); | |
| } |
🧰 Tools
🪛 Biome
[error] 278-278: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.(lint/complexity/noUselessCatch)
Choose the appropriate template for your PR:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation