Project factory bff #259
Closed
Project factory bff #259
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Fix : Fixed duplicate caching issue in add screen * Fix : Fixed caching issue in MDMSAddV4 screen * feat: implemented cache deletion for specific screens
* Feat : Created Enpoints for transform data * Feat : Created Endpoints to transform xlsx data * Refactor : Refactored parsing logics * Refactor: Reafactored Kafka Listener * Refactor: Reafactored Kafka Listener * Refactor: Refactored Config for helm chart * Feat : Implemented Dynamic url * Feat : Integrated multiple ParsingTemplates and ingestion message in kafka * Feat : Integrated with ingestion api * Feat : Integrated transform service for unified-dev
…/DIGIT-Frontend into project-factory-bff
…oad of type boundarywith target
…/DIGIT-Frontend into project-factory-bff
…/DIGIT-Frontend into project-factory-bff
…/DIGIT-Frontend into project-factory-bff
…/DIGIT-Frontend into project-factory-bff
WalkthroughWalkthroughThe recent updates across various utility services focus on enhancing data handling and processing capabilities, particularly around Excel file manipulation, boundary and hierarchy management, and data validation. New functionalities include the generation of unique IDs, project creation, resource management, and advanced filtering for data processing. These changes aim to streamline operations, improve data accuracy, and facilitate more efficient project and campaign management. Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 12
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
utilities/project-factory/package.jsonis excluded by!**/*.json
Files selected for processing (8)
- utilities/egov-bff/src/server/api/index.ts (1 hunks)
- utilities/egov-bff/src/server/utils/index.ts (1 hunks)
- utilities/project-factory/src/server/api/campaignApis.ts (3 hunks)
- utilities/project-factory/src/server/api/genericApis.ts (3 hunks)
- utilities/project-factory/src/server/controllers/dataManage/dataManage.controller.ts (1 hunks)
- utilities/project-factory/src/server/utils/campaignUtils.ts (3 hunks)
- utilities/project-factory/src/server/utils/genericUtils.ts (4 hunks)
- utilities/project-factory/src/server/utils/validators/campaignValidators.ts (2 hunks)
Files not summarized due to errors (1)
- utilities/egov-bff/src/server/utils/index.ts: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
- utilities/project-factory/src/server/controllers/dataManage/dataManage.controller.ts
Additional comments not posted (9)
utilities/project-factory/src/server/api/campaignApis.ts (2)
379-396: LGTM! ThegenerateHierarchyListfunction correctly generates hierarchy chains from boundary data.
9-9: Acknowledged the mention ofgenerateHierarchyfunction added incampaignUtils. Ensure its implementation aligns with the intended functionality.utilities/project-factory/src/server/api/genericApis.ts (1)
6-8: Acknowledged the addition ofgetBoundaryRelationshipDataandgenerateFilteredBoundaryData. Ensure their implementations align with the intended functionality and error handling is adequately addressed.utilities/project-factory/src/server/utils/campaignUtils.ts (1)
666-677: EnsurevalidateFiltersfunction properly handles all possible edge cases and inputs.utilities/egov-bff/src/server/api/index.ts (4)
61-82: Be cautious with logging the search result in thesearchMDMSfunction to avoid potential data leakage. Verify the consistency of the error structure to ensure safe access to error details.
87-106: Consider removing or modifying the log statement that logs the request data in thegetCampaignNumberfunction to avoid potential data leakage. Ensure that thehttpRequestfunction used for making the request has adequate error handling.
176-192: Ensure that thehttpRequestfunction used increateAndUploadFilefor uploading the file has adequate error handling. Consider adding explicit error handling in this function if necessary.
194-211: LGTM! ThegenerateHierarchyListfunction correctly implements recursion to generate hierarchy chains and follows best practices.utilities/egov-bff/src/server/utils/index.ts (1)
94-99: Ensure proper validation of the cache key in thecacheResponsefunction to prevent potential security risks.Consider adding a validation step to ensure that the key does not contain any sensitive information or patterns that could be exploited.
Comment on lines
+681
to
+685
| async function getBoundaryRelationshipData(request: any, params: any) { | ||
| const url = `${config.host.boundaryHost}${config.paths.boundaryRelationship}`; | ||
| const boundaryRelationshipResponse = await httpRequest(url, request.body, params); | ||
| return boundaryRelationshipResponse?.TenantBoundary?.[0]?.boundary; | ||
| } |
There was a problem hiding this comment.
Consider adding error handling for the external API call in getBoundaryRelationshipData.
const boundaryRelationshipResponse = await httpRequest(url, request.body, params).catch(e => {
logger.error(`Failed to fetch boundary relationship data: ${e.message}`);
throw new Error(`Failed to fetch boundary relationship data: ${e.message}`);
});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.
Suggested change
| async function getBoundaryRelationshipData(request: any, params: any) { | |
| const url = `${config.host.boundaryHost}${config.paths.boundaryRelationship}`; | |
| const boundaryRelationshipResponse = await httpRequest(url, request.body, params); | |
| return boundaryRelationshipResponse?.TenantBoundary?.[0]?.boundary; | |
| } | |
| async function getBoundaryRelationshipData(request: any, params: any) { | |
| const url = `${config.host.boundaryHost}${config.paths.boundaryRelationship}`; | |
| const boundaryRelationshipResponse = await httpRequest(url, request.body, params).catch(e => { | |
| logger.error(`Failed to fetch boundary relationship data: ${e.message}`); | |
| throw new Error(`Failed to fetch boundary relationship data: ${e.message}`); | |
| }); | |
| return boundaryRelationshipResponse?.TenantBoundary?.[0]?.boundary; | |
| } |
Comment on lines
+687
to
+721
| async function getDataSheetReady(boundaryData: any, request: any) { | ||
| if (!boundaryData) { | ||
| throw new Error("No boundary data provided."); | ||
| } | ||
| const boundaryType = boundaryData?.[0].boundaryType; | ||
| const boundaryList = generateHierarchyList(boundaryData) | ||
| if (!Array.isArray(boundaryList) || boundaryList.length === 0) { | ||
| throw new Error("Boundary list is empty or not an array."); | ||
| } | ||
| const boundaryCodes = boundaryList.map(boundary => boundary.split(',').pop()); | ||
| const string = boundaryCodes.join(', '); | ||
| const boundaryEntityResponse = await httpRequest(config.host.boundaryHost + config.paths.boundaryServiceSearch, request.body, { tenantId: "pg", codes: string }); | ||
|
|
||
| const boundaryCodeNameMapping: { [key: string]: string } = {}; | ||
| boundaryEntityResponse?.Boundary?.forEach((data: any) => { | ||
| boundaryCodeNameMapping[data?.code] = data?.additionalDetails?.name; | ||
| }); | ||
|
|
||
| const hierarchy = await getHierarchy(request, request?.query?.tenantId, request?.query?.hierarchyType); | ||
| const startIndex = boundaryType ? hierarchy.indexOf(boundaryType) : -1; | ||
| const reducedHierarchy = startIndex !== -1 ? hierarchy.slice(startIndex) : hierarchy; | ||
| const headers = [...reducedHierarchy, "Boundary Code", "Target at the Selected Boundary level", "Start Date of Campaign (Optional Field)", "End Date of Campaign (Optional Field)"]; | ||
| const data = boundaryList.map(boundary => { | ||
| const boundaryParts = boundary.split(','); | ||
| const boundaryCode = boundaryParts[boundaryParts.length - 1]; | ||
| const rowData = boundaryParts.concat(Array(Math.max(0, reducedHierarchy.length - boundaryParts.length)).fill('')); | ||
| const mappedRowData = rowData.map((cell: any, index: number) => | ||
| index === reducedHierarchy.length ? '' : cell !== '' ? boundaryCodeNameMapping[cell] || cell : '' | ||
| ); | ||
| const boundaryCodeIndex = reducedHierarchy.length; | ||
| mappedRowData[boundaryCodeIndex] = boundaryCode; | ||
| return mappedRowData; | ||
| }); | ||
| return await createExcelSheet(data, headers); | ||
| } |
There was a problem hiding this comment.
Refactor getDataSheetReady to improve error handling and readability.
- if (!boundaryData) {
- throw new Error("No boundary data provided.");
- }
+ if (!Array.isArray(boundaryData) || boundaryData.length === 0) {
+ throw new Error("Boundary data is empty or not provided.");
+ }Also, consider validating boundaryType and hierarchy to ensure they are not undefined before using them.
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.
Suggested change
| async function getDataSheetReady(boundaryData: any, request: any) { | |
| if (!boundaryData) { | |
| throw new Error("No boundary data provided."); | |
| } | |
| const boundaryType = boundaryData?.[0].boundaryType; | |
| const boundaryList = generateHierarchyList(boundaryData) | |
| if (!Array.isArray(boundaryList) || boundaryList.length === 0) { | |
| throw new Error("Boundary list is empty or not an array."); | |
| } | |
| const boundaryCodes = boundaryList.map(boundary => boundary.split(',').pop()); | |
| const string = boundaryCodes.join(', '); | |
| const boundaryEntityResponse = await httpRequest(config.host.boundaryHost + config.paths.boundaryServiceSearch, request.body, { tenantId: "pg", codes: string }); | |
| const boundaryCodeNameMapping: { [key: string]: string } = {}; | |
| boundaryEntityResponse?.Boundary?.forEach((data: any) => { | |
| boundaryCodeNameMapping[data?.code] = data?.additionalDetails?.name; | |
| }); | |
| const hierarchy = await getHierarchy(request, request?.query?.tenantId, request?.query?.hierarchyType); | |
| const startIndex = boundaryType ? hierarchy.indexOf(boundaryType) : -1; | |
| const reducedHierarchy = startIndex !== -1 ? hierarchy.slice(startIndex) : hierarchy; | |
| const headers = [...reducedHierarchy, "Boundary Code", "Target at the Selected Boundary level", "Start Date of Campaign (Optional Field)", "End Date of Campaign (Optional Field)"]; | |
| const data = boundaryList.map(boundary => { | |
| const boundaryParts = boundary.split(','); | |
| const boundaryCode = boundaryParts[boundaryParts.length - 1]; | |
| const rowData = boundaryParts.concat(Array(Math.max(0, reducedHierarchy.length - boundaryParts.length)).fill('')); | |
| const mappedRowData = rowData.map((cell: any, index: number) => | |
| index === reducedHierarchy.length ? '' : cell !== '' ? boundaryCodeNameMapping[cell] || cell : '' | |
| ); | |
| const boundaryCodeIndex = reducedHierarchy.length; | |
| mappedRowData[boundaryCodeIndex] = boundaryCode; | |
| return mappedRowData; | |
| }); | |
| return await createExcelSheet(data, headers); | |
| } | |
| async function getDataSheetReady(boundaryData: any, request: any) { | |
| if (!Array.isArray(boundaryData) || boundaryData.length === 0) { | |
| throw new Error("Boundary data is empty or not provided."); | |
| } | |
| const boundaryType = boundaryData?.[0].boundaryType; | |
| const boundaryList = generateHierarchyList(boundaryData) | |
| if (!Array.isArray(boundaryList) || boundaryList.length === 0) { | |
| throw new Error("Boundary list is empty or not an array."); | |
| } | |
| const boundaryCodes = boundaryList.map(boundary => boundary.split(',').pop()); | |
| const string = boundaryCodes.join(', '); | |
| const boundaryEntityResponse = await httpRequest(config.host.boundaryHost + config.paths.boundaryServiceSearch, request.body, { tenantId: "pg", codes: string }); | |
| const boundaryCodeNameMapping: { [key: string]: string } = {}; | |
| boundaryEntityResponse?.Boundary?.forEach((data: any) => { | |
| boundaryCodeNameMapping[data?.code] = data?.additionalDetails?.name; | |
| }); | |
| const hierarchy = await getHierarchy(request, request?.query?.tenantId, request?.query?.hierarchyType); | |
| const startIndex = boundaryType ? hierarchy.indexOf(boundaryType) : -1; | |
| const reducedHierarchy = startIndex !== -1 ? hierarchy.slice(startIndex) : hierarchy; | |
| const headers = [...reducedHierarchy, "Boundary Code", "Target at the Selected Boundary level", "Start Date of Campaign (Optional Field)", "End Date of Campaign (Optional Field)"]; | |
| const data = boundaryList.map(boundary => { | |
| const boundaryParts = boundary.split(','); | |
| const boundaryCode = boundaryParts[boundaryParts.length - 1]; | |
| const rowData = boundaryParts.concat(Array(Math.max(0, reducedHierarchy.length - boundaryParts.length)).fill('')); | |
| const mappedRowData = rowData.map((cell: any, index: number) => | |
| index === reducedHierarchy.length ? '' : cell !== '' ? boundaryCodeNameMapping[cell] || cell : '' | |
| ); | |
| const boundaryCodeIndex = reducedHierarchy.length; | |
| mappedRowData[boundaryCodeIndex] = boundaryCode; | |
| return mappedRowData; | |
| }); | |
| return await createExcelSheet(data, headers); | |
| } |
Comment on lines
+39
to
+50
| const throwError = ( | ||
| message = "Internal Server Error", | ||
| code = "INTERNAL_SERVER_ERROR", | ||
| status = 500 | ||
| ) => { | ||
| let error = new Error(message); | ||
| // error.status = status; | ||
| // error.code = code; | ||
| logger.error("Error : " + error); | ||
|
|
||
| throw error; | ||
| }; |
There was a problem hiding this comment.
Consider adding error code and status to the thrown error in the throwError function.
let error = new Error(message);
+ error.status = status;
+ error.code = code;This change ensures that the error object contains all relevant information, making it easier to handle downstream.
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.
Suggested change
| const throwError = ( | |
| message = "Internal Server Error", | |
| code = "INTERNAL_SERVER_ERROR", | |
| status = 500 | |
| ) => { | |
| let error = new Error(message); | |
| // error.status = status; | |
| // error.code = code; | |
| logger.error("Error : " + error); | |
| throw error; | |
| }; | |
| const throwError = ( | |
| message = "Internal Server Error", | |
| code = "INTERNAL_SERVER_ERROR", | |
| status = 500 | |
| ) => { | |
| let error = new Error(message); | |
| error.status = status; | |
| error.code = code; | |
| logger.error("Error : " + error); | |
| throw error; | |
| }; |
Comment on lines
+188
to
+215
| async function generateXlsxFromJson(request: any, response: any, simplifiedData: any) { | ||
| try { | ||
| const ws = XLSX.utils.json_to_sheet(simplifiedData); | ||
|
|
||
| // Create a new workbook | ||
| const wb = XLSX.utils.book_new(); | ||
| XLSX.utils.book_append_sheet(wb, ws, 'Sheet 1'); | ||
| const buffer = XLSX.write(wb, { bookType: 'xlsx', type: 'buffer' }); | ||
| const formData = new FormData(); | ||
| formData.append('file', buffer, 'filename.xlsx'); | ||
| formData.append('tenantId', request?.body?.RequestInfo?.userInfo?.tenantId); | ||
| formData.append('module', 'pgr'); | ||
|
|
||
| logger.info("File uploading url : " + config.host.filestore + config.paths.filestore); | ||
| var fileCreationResult = await httpRequest(config.host.filestore + config.paths.filestore, formData, undefined, undefined, undefined, | ||
| { | ||
| 'Content-Type': 'multipart/form-data', | ||
| 'auth-token': request?.body?.RequestInfo?.authToken | ||
| } | ||
| ); | ||
| const responseData = fileCreationResult?.files; | ||
| logger.info("Response data after File Creation : " + JSON.stringify(responseData)); | ||
| return responseData; | ||
| } catch (e: any) { | ||
| const errorMessage = "Error occurred while fetching the file store ID: " + e.message; | ||
| logger.error(errorMessage) | ||
| return errorResponder({ message: errorMessage + " Check Logs" }, request, response); | ||
| } |
There was a problem hiding this comment.
Add error handling for potential failures in external calls within the generateXlsxFromJson function.
} catch (e: any) {
+ logger.error("Failed to generate XLSX from JSON: ", e);
const errorMessage = "Error occurred while fetching the file store ID: " + e.message;
logger.error(errorMessage)
return errorResponder({ message: errorMessage + " Check Logs" }, request, response);
}This ensures that any errors encountered during the process are logged, providing more context for debugging.
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.
Suggested change
| async function generateXlsxFromJson(request: any, response: any, simplifiedData: any) { | |
| try { | |
| const ws = XLSX.utils.json_to_sheet(simplifiedData); | |
| // Create a new workbook | |
| const wb = XLSX.utils.book_new(); | |
| XLSX.utils.book_append_sheet(wb, ws, 'Sheet 1'); | |
| const buffer = XLSX.write(wb, { bookType: 'xlsx', type: 'buffer' }); | |
| const formData = new FormData(); | |
| formData.append('file', buffer, 'filename.xlsx'); | |
| formData.append('tenantId', request?.body?.RequestInfo?.userInfo?.tenantId); | |
| formData.append('module', 'pgr'); | |
| logger.info("File uploading url : " + config.host.filestore + config.paths.filestore); | |
| var fileCreationResult = await httpRequest(config.host.filestore + config.paths.filestore, formData, undefined, undefined, undefined, | |
| { | |
| 'Content-Type': 'multipart/form-data', | |
| 'auth-token': request?.body?.RequestInfo?.authToken | |
| } | |
| ); | |
| const responseData = fileCreationResult?.files; | |
| logger.info("Response data after File Creation : " + JSON.stringify(responseData)); | |
| return responseData; | |
| } catch (e: any) { | |
| const errorMessage = "Error occurred while fetching the file store ID: " + e.message; | |
| logger.error(errorMessage) | |
| return errorResponder({ message: errorMessage + " Check Logs" }, request, response); | |
| } | |
| async function generateXlsxFromJson(request: any, response: any, simplifiedData: any) { | |
| try { | |
| const ws = XLSX.utils.json_to_sheet(simplifiedData); | |
| // Create a new workbook | |
| const wb = XLSX.utils.book_new(); | |
| XLSX.utils.book_append_sheet(wb, ws, 'Sheet 1'); | |
| const buffer = XLSX.write(wb, { bookType: 'xlsx', type: 'buffer' }); | |
| const formData = new FormData(); | |
| formData.append('file', buffer, 'filename.xlsx'); | |
| formData.append('tenantId', request?.body?.RequestInfo?.userInfo?.tenantId); | |
| formData.append('module', 'pgr'); | |
| logger.info("File uploading url : " + config.host.filestore + config.paths.filestore); | |
| var fileCreationResult = await httpRequest(config.host.filestore + config.paths.filestore, formData, undefined, undefined, undefined, | |
| { | |
| 'Content-Type': 'multipart/form-data', | |
| 'auth-token': request?.body?.RequestInfo?.authToken | |
| } | |
| ); | |
| const responseData = fileCreationResult?.files; | |
| logger.info("Response data after File Creation : " + JSON.stringify(responseData)); | |
| return responseData; | |
| } catch (e: any) { | |
| logger.error("Failed to generate XLSX from JSON: ", e); | |
| const errorMessage = "Error occurred while fetching the file store ID: " + e.message; | |
| logger.error(errorMessage) | |
| return errorResponder({ message: errorMessage + " Check Logs" }, request, response); | |
| } | |
| } |
Comment on lines
+361
to
+381
| function validateFilters(request: any) { | ||
| const boundaries = request?.body?.Filters?.boundaries; | ||
|
|
||
| if (!Array.isArray(boundaries)) { | ||
| throw new Error("Boundaries should be an array"); | ||
| } | ||
|
|
||
| const rootBoundaries = boundaries.filter((boundary: any) => boundary.isRoot); | ||
|
|
||
| if (rootBoundaries.length !== 1) { | ||
| throw new Error("Invalid Filter Criteria: Either more than one root or no root present"); | ||
| } | ||
|
|
||
| const boundaryTypeOfRoot = rootBoundaries[0]?.boundaryType; | ||
|
|
||
| const boundariesOfTypeOfSameAsRoot = boundaries.filter((boundary: any) => boundary.boundaryType === boundaryTypeOfRoot); | ||
|
|
||
| if (boundariesOfTypeOfSameAsRoot.length > 1) { | ||
| throw new Error("Invalid Filter Criteria: Only one root can be there for a given Boundary level"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider enhancing the error messages in validateFilters to provide more context and refactoring for improved readability.
function validateFilters(request: any) {
const boundaries = request?.body?.Filters?.boundaries;
if (!Array.isArray(boundaries)) {
- throw new Error("Boundaries should be an array");
+ throw new Error("Invalid Filter Criteria: Boundaries should be provided as an array.");
}
const rootBoundaries = boundaries.filter((boundary: any) => boundary.isRoot);
if (rootBoundaries.length !== 1) {
- throw new Error("Invalid Filter Criteria: Either more than one root or no root present");
+ throw new Error("Invalid Filter Criteria: Exactly one root boundary is required.");
}
const boundaryTypeOfRoot = rootBoundaries[0]?.boundaryType;
const boundariesOfTypeOfSameAsRoot = boundaries.filter((boundary: any) => boundary.boundaryType === boundaryTypeOfRoot);
if (boundariesOfTypeOfSameAsRoot.length > 1) {
- throw new Error("Invalid Filter Criteria: Only one root can be there for a given Boundary level");
+ throw new Error("Invalid Filter Criteria: Multiple root boundaries found for the same boundary level.");
}
}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.
Suggested change
| function validateFilters(request: any) { | |
| const boundaries = request?.body?.Filters?.boundaries; | |
| if (!Array.isArray(boundaries)) { | |
| throw new Error("Boundaries should be an array"); | |
| } | |
| const rootBoundaries = boundaries.filter((boundary: any) => boundary.isRoot); | |
| if (rootBoundaries.length !== 1) { | |
| throw new Error("Invalid Filter Criteria: Either more than one root or no root present"); | |
| } | |
| const boundaryTypeOfRoot = rootBoundaries[0]?.boundaryType; | |
| const boundariesOfTypeOfSameAsRoot = boundaries.filter((boundary: any) => boundary.boundaryType === boundaryTypeOfRoot); | |
| if (boundariesOfTypeOfSameAsRoot.length > 1) { | |
| throw new Error("Invalid Filter Criteria: Only one root can be there for a given Boundary level"); | |
| } | |
| } | |
| function validateFilters(request: any) { | |
| const boundaries = request?.body?.Filters?.boundaries; | |
| if (!Array.isArray(boundaries)) { | |
| throw new Error("Invalid Filter Criteria: Boundaries should be provided as an array."); | |
| } | |
| const rootBoundaries = boundaries.filter((boundary: any) => boundary.isRoot); | |
| if (rootBoundaries.length !== 1) { | |
| throw new Error("Invalid Filter Criteria: Exactly one root boundary is required."); | |
| } | |
| const boundaryTypeOfRoot = rootBoundaries[0]?.boundaryType; | |
| const boundariesOfTypeOfSameAsRoot = boundaries.filter((boundary: any) => boundary.boundaryType === boundaryTypeOfRoot); | |
| if (boundariesOfTypeOfSameAsRoot.length > 1) { | |
| throw new Error("Invalid Filter Criteria: Multiple root boundaries found for the same boundary level."); | |
| } | |
| } |
Comment on lines
+29
to
+55
| } | ||
| const getSheetData = async (fileUrl: string, sheetName: string, getRow = false) => { | ||
| const headers = { | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'application/pdf', | ||
| }; | ||
|
|
||
| const responseFile = await httpRequest(fileUrl, null, {}, 'get', 'arraybuffer', headers); | ||
| const workbook = XLSX.read(responseFile, { type: 'buffer' }); | ||
|
|
||
| // Check if the specified sheet exists in the workbook | ||
| if (!workbook.Sheets.hasOwnProperty(sheetName)) { | ||
| throw new Error(`Sheet with name "${sheetName}" is not present in the file.`); | ||
| } | ||
|
|
||
| const sheetData = XLSX.utils.sheet_to_json(workbook.Sheets[sheetName]); | ||
| const jsonData = sheetData.map((row: any, index: number) => { | ||
| const rowData: any = {}; | ||
| Object.keys(row).forEach(key => { | ||
| rowData[key] = row[key] === undefined || row[key] === '' ? null : row[key]; | ||
| }); | ||
| if (getRow) rowData['!row#number!'] = index + 1; // Adding row number | ||
| return rowData; | ||
| }); | ||
| logger.info("Sheet Data : " + JSON.stringify(jsonData)) | ||
| return jsonData; | ||
| }; |
There was a problem hiding this comment.
Consider removing or modifying the log statement that logs the entire sheet data to avoid potential data leakage and performance issues.
- logger.info("Sheet Data : " + JSON.stringify(jsonData))Also, improve error handling to include the original error message for better debugging.
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.
Suggested change
| } | |
| const getSheetData = async (fileUrl: string, sheetName: string, getRow = false) => { | |
| const headers = { | |
| 'Content-Type': 'application/json', | |
| Accept: 'application/pdf', | |
| }; | |
| const responseFile = await httpRequest(fileUrl, null, {}, 'get', 'arraybuffer', headers); | |
| const workbook = XLSX.read(responseFile, { type: 'buffer' }); | |
| // Check if the specified sheet exists in the workbook | |
| if (!workbook.Sheets.hasOwnProperty(sheetName)) { | |
| throw new Error(`Sheet with name "${sheetName}" is not present in the file.`); | |
| } | |
| const sheetData = XLSX.utils.sheet_to_json(workbook.Sheets[sheetName]); | |
| const jsonData = sheetData.map((row: any, index: number) => { | |
| const rowData: any = {}; | |
| Object.keys(row).forEach(key => { | |
| rowData[key] = row[key] === undefined || row[key] === '' ? null : row[key]; | |
| }); | |
| if (getRow) rowData['!row#number!'] = index + 1; // Adding row number | |
| return rowData; | |
| }); | |
| logger.info("Sheet Data : " + JSON.stringify(jsonData)) | |
| return jsonData; | |
| }; | |
| } | |
| const getSheetData = async (fileUrl: string, sheetName: string, getRow = false) => { | |
| const headers = { | |
| 'Content-Type': 'application/json', | |
| Accept: 'application/pdf', | |
| }; | |
| const responseFile = await httpRequest(fileUrl, null, {}, 'get', 'arraybuffer', headers); | |
| const workbook = XLSX.read(responseFile, { type: 'buffer' }); | |
| // Check if the specified sheet exists in the workbook | |
| if (!workbook.Sheets.hasOwnProperty(sheetName)) { | |
| throw new Error(`Sheet with name "${sheetName}" is not present in the file.`); | |
| } | |
| const sheetData = XLSX.utils.sheet_to_json(workbook.Sheets[sheetName]); | |
| const jsonData = sheetData.map((row: any, index: number) => { | |
| const rowData: any = {}; | |
| Object.keys(row).forEach(key => { | |
| rowData[key] = row[key] === undefined || row[key] === '' ? null : row[key]; | |
| }); | |
| if (getRow) rowData['!row#number!'] = index + 1; // Adding row number | |
| return rowData; | |
| }); | |
| return jsonData; | |
| }; |
Comment on lines
+1437
to
+1458
| for (const item of boundaryData) { | ||
| if (item.District && !uniqueDistricts.includes(item.District)) { | ||
| uniqueDistricts.push(item.District); | ||
| } | ||
| } | ||
| for (const district of uniqueDistricts) { | ||
| const districtDataFiltered = boundaryData.filter(item => item.District === district); | ||
| const districtIndex = Object.keys(districtDataFiltered[0]).indexOf('District'); | ||
| const headers = Object.keys(districtDataFiltered[0]).slice(districtIndex); | ||
| const newSheetData = [headers]; | ||
| for (const data of districtDataFiltered) { | ||
| const rowData = Object.values(data).slice(districtIndex).map(value => value === null ? '' : String(value)); // Replace null with empty string | ||
| newSheetData.push(rowData); | ||
| } | ||
| const ws = XLSX.utils.aoa_to_sheet(newSheetData); | ||
| XLSX.utils.book_append_sheet(workbook, ws, district); | ||
| } | ||
| return workbook; | ||
| } catch (error) { | ||
| throw Error("An error occurred while appending sheets:"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Refactor the appendSheetsToWorkbook function to improve readability and error handling.
try {
const uniqueDistricts: string[] = [];
const uniqueDistrictsForMainSheet: string[] = [];
const workbook = XLSX.utils.book_new();
const mainSheetData: any[] = [];
const headersForMainSheet = Object.keys(boundaryData[0]);
mainSheetData.push(headersForMainSheet);
// Existing logic...
} catch (error) {
- throw Error("An error occurred while appending sheets:");
+ logger.error("An error occurred while appending sheets: ", error);
+ throw error;
}This change ensures that errors are logged for easier debugging and that the original error is thrown to preserve the stack trace.
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.
Suggested change
| for (const item of boundaryData) { | |
| if (item.District && !uniqueDistricts.includes(item.District)) { | |
| uniqueDistricts.push(item.District); | |
| } | |
| } | |
| for (const district of uniqueDistricts) { | |
| const districtDataFiltered = boundaryData.filter(item => item.District === district); | |
| const districtIndex = Object.keys(districtDataFiltered[0]).indexOf('District'); | |
| const headers = Object.keys(districtDataFiltered[0]).slice(districtIndex); | |
| const newSheetData = [headers]; | |
| for (const data of districtDataFiltered) { | |
| const rowData = Object.values(data).slice(districtIndex).map(value => value === null ? '' : String(value)); // Replace null with empty string | |
| newSheetData.push(rowData); | |
| } | |
| const ws = XLSX.utils.aoa_to_sheet(newSheetData); | |
| XLSX.utils.book_append_sheet(workbook, ws, district); | |
| } | |
| return workbook; | |
| } catch (error) { | |
| throw Error("An error occurred while appending sheets:"); | |
| } | |
| } | |
| try { | |
| const uniqueDistricts: string[] = []; | |
| const uniqueDistrictsForMainSheet: string[] = []; | |
| const workbook = XLSX.utils.book_new(); | |
| const mainSheetData: any[] = []; | |
| const headersForMainSheet = Object.keys(boundaryData[0]); | |
| mainSheetData.push(headersForMainSheet); | |
| for (const item of boundaryData) { | |
| if (item.District && !uniqueDistricts.includes(item.District)) { | |
| uniqueDistricts.push(item.District); | |
| } | |
| } | |
| for (const district of uniqueDistricts) { | |
| const districtDataFiltered = boundaryData.filter(item => item.District === district); | |
| const districtIndex = Object.keys(districtDataFiltered[0]).indexOf('District'); | |
| const headers = Object.keys(districtDataFiltered[0]).slice(districtIndex); | |
| const newSheetData = [headers]; | |
| for (const data of districtDataFiltered) { | |
| const rowData = Object.values(data).slice(districtIndex).map(value => value === null ? '' : String(value)); // Replace null with empty string | |
| newSheetData.push(rowData); | |
| } | |
| const ws = XLSX.utils.aoa_to_sheet(newSheetData); | |
| XLSX.utils.book_append_sheet(workbook, ws, district); | |
| } | |
| return workbook; | |
| } catch (error) { | |
| logger.error("An error occurred while appending sheets: ", error); | |
| throw error; | |
| } | |
| } |
Comment on lines
+347
to
+358
| const params = { | ||
| ...request?.query, | ||
| includeChildren: true | ||
| }; | ||
| const boundaryData = await getBoundaryRelationshipData(request, params); | ||
| logger.info("boundaryData for sheet " + JSON.stringify(boundaryData)) | ||
| if (boundaryData) { | ||
| const boundaryList = generateHierarchyList(boundaryData) | ||
| if (Array.isArray(boundaryList) && boundaryList.length > 0) { | ||
| const boundaryCodes = boundaryList.map(boundary => boundary.split(',').pop()); | ||
| const string = boundaryCodes.join(', '); | ||
| const boundaryEntityResponse = await httpRequest(config.host.boundaryHost + config.paths.boundaryServiceSearch, request.body, { tenantId: "pg", codes: string }); | ||
|
|
||
| const boundaryCodeNameMapping: { [key: string]: string } = {}; | ||
| boundaryEntityResponse?.Boundary?.forEach((data: any) => { | ||
| boundaryCodeNameMapping[data?.code] = data?.additionalDetails?.name; | ||
| }); | ||
|
|
||
| const hierarchy = await getHierarchy(request, request?.query?.tenantId, request?.query?.hierarchyType); | ||
| const startIndex = boundaryType ? hierarchy.indexOf(boundaryType) : -1; | ||
| const reducedHierarchy = startIndex !== -1 ? hierarchy.slice(startIndex) : hierarchy; | ||
| const headers = [...reducedHierarchy, "Boundary Code", "Target at the Selected Boundary level", "Start Date of Campaign (Optional Field)", "End Date of Campaign (Optional Field)"]; | ||
| const data = boundaryList.map(boundary => { | ||
| const boundaryParts = boundary.split(','); | ||
| const boundaryCode = boundaryParts[boundaryParts.length - 1]; | ||
| const rowData = boundaryParts.concat(Array(Math.max(0, reducedHierarchy.length - boundaryParts.length)).fill('')); | ||
| const mappedRowData = rowData.map((cell: any, index: number) => | ||
| index === reducedHierarchy.length ? '' : cell !== '' ? boundaryCodeNameMapping[cell] || cell : '' | ||
| ); | ||
| const boundaryCodeIndex = reducedHierarchy.length; | ||
| mappedRowData[boundaryCodeIndex] = boundaryCode; | ||
| return mappedRowData; | ||
| }); | ||
| return await createExcelSheet(data, headers); | ||
| } | ||
| if (request?.body?.Filters) { | ||
| const filteredBoundaryData = await generateFilteredBoundaryData(request); | ||
| return await getDataSheetReady(filteredBoundaryData, request); | ||
| } | ||
| else { | ||
| return await getDataSheetReady(boundaryData, request); |
There was a problem hiding this comment.
Refactor getBoundarySheetData for improved readability by reducing nested conditions.
async function getBoundarySheetData(request: any) {
const params = { ...request?.query, includeChildren: true };
const boundaryData = await getBoundaryRelationshipData(request, params);
logger.info("boundaryData for sheet " + JSON.stringify(boundaryData));
+ const dataToProcess = request?.body?.Filters ? await generateFilteredBoundaryData(request) : boundaryData;
- if (request?.body?.Filters) {
- const filteredBoundaryData = await generateFilteredBoundaryData(request);
- return await getDataSheetReady(filteredBoundaryData, request);
- } else {
- return await getDataSheetReady(boundaryData, request);
- }
+ return await getDataSheetReady(dataToProcess, request);
}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.
Suggested change
| const params = { | |
| ...request?.query, | |
| includeChildren: true | |
| }; | |
| const boundaryData = await getBoundaryRelationshipData(request, params); | |
| logger.info("boundaryData for sheet " + JSON.stringify(boundaryData)) | |
| if (boundaryData) { | |
| const boundaryList = generateHierarchyList(boundaryData) | |
| if (Array.isArray(boundaryList) && boundaryList.length > 0) { | |
| const boundaryCodes = boundaryList.map(boundary => boundary.split(',').pop()); | |
| const string = boundaryCodes.join(', '); | |
| const boundaryEntityResponse = await httpRequest(config.host.boundaryHost + config.paths.boundaryServiceSearch, request.body, { tenantId: "pg", codes: string }); | |
| const boundaryCodeNameMapping: { [key: string]: string } = {}; | |
| boundaryEntityResponse?.Boundary?.forEach((data: any) => { | |
| boundaryCodeNameMapping[data?.code] = data?.additionalDetails?.name; | |
| }); | |
| const hierarchy = await getHierarchy(request, request?.query?.tenantId, request?.query?.hierarchyType); | |
| const startIndex = boundaryType ? hierarchy.indexOf(boundaryType) : -1; | |
| const reducedHierarchy = startIndex !== -1 ? hierarchy.slice(startIndex) : hierarchy; | |
| const headers = [...reducedHierarchy, "Boundary Code", "Target at the Selected Boundary level", "Start Date of Campaign (Optional Field)", "End Date of Campaign (Optional Field)"]; | |
| const data = boundaryList.map(boundary => { | |
| const boundaryParts = boundary.split(','); | |
| const boundaryCode = boundaryParts[boundaryParts.length - 1]; | |
| const rowData = boundaryParts.concat(Array(Math.max(0, reducedHierarchy.length - boundaryParts.length)).fill('')); | |
| const mappedRowData = rowData.map((cell: any, index: number) => | |
| index === reducedHierarchy.length ? '' : cell !== '' ? boundaryCodeNameMapping[cell] || cell : '' | |
| ); | |
| const boundaryCodeIndex = reducedHierarchy.length; | |
| mappedRowData[boundaryCodeIndex] = boundaryCode; | |
| return mappedRowData; | |
| }); | |
| return await createExcelSheet(data, headers); | |
| } | |
| if (request?.body?.Filters) { | |
| const filteredBoundaryData = await generateFilteredBoundaryData(request); | |
| return await getDataSheetReady(filteredBoundaryData, request); | |
| } | |
| else { | |
| return await getDataSheetReady(boundaryData, request); | |
| const params = { | |
| ...request?.query, | |
| includeChildren: true | |
| }; | |
| const boundaryData = await getBoundaryRelationshipData(request, params); | |
| logger.info("boundaryData for sheet " + JSON.stringify(boundaryData)) | |
| const dataToProcess = request?.body?.Filters ? await generateFilteredBoundaryData(request) : boundaryData; | |
| return await getDataSheetReady(dataToProcess, request); |
Comment on lines
+679
to
+731
| function filterBoundaries(boundaryData: any[], filters: any): any { | ||
| function filterRecursive(boundary: any): any { | ||
| const boundaryFilters = filters && filters.boundaries; // Accessing boundaries array from filters object | ||
| const filter = boundaryFilters?.find((f: any) => f.code === boundary.code && f.boundaryType === boundary.boundaryType); | ||
|
|
||
| if (!filter) { | ||
| return { | ||
| ...boundary, | ||
| children: boundary.children.map(filterRecursive) | ||
| }; | ||
| } | ||
|
|
||
| if (!boundary.children.length) { | ||
| if (!filter.includeAllChildren) { | ||
| throw new Error("Boundary cannot have includeAllChildren filter false if it does not have any children"); | ||
| } | ||
| // If boundary has no children and includeAllChildren is true, return as is | ||
| return { | ||
| ...boundary, | ||
| children: [] | ||
| }; | ||
| } | ||
|
|
||
| if (filter.includeAllChildren) { | ||
| // If includeAllChildren is true, return boundary with all children | ||
| return { | ||
| ...boundary, | ||
| children: boundary.children.map(filterRecursive) | ||
| }; | ||
| } | ||
|
|
||
| const filteredChildren: any[] = []; | ||
| boundary.children.forEach((child: any) => { | ||
| const matchingFilter = boundaryFilters.find((f: any) => f.code === child.code && f.boundaryType === child.boundaryType); | ||
| if (matchingFilter) { | ||
| filteredChildren.push(filterRecursive(child)); | ||
| } | ||
| }); | ||
| return { | ||
| ...boundary, | ||
| children: filteredChildren | ||
| }; | ||
| } | ||
| try { | ||
| const filteredData = boundaryData.map(filterRecursive); | ||
| return filteredData; | ||
| } | ||
| catch (e: any) { | ||
| const errorMessage = "Error occurred while fetching boundaries: " + e.message; | ||
| logger.error(errorMessage) | ||
| throw new Error(errorMessage); | ||
| } | ||
| } |
There was a problem hiding this comment.
Refactor filterBoundaries to simplify logic and improve readability.
Consider using a more declarative approach for filtering and mapping the boundaries.
Comment on lines
+733
to
+770
| function generateHierarchy(boundaries: any[]) { | ||
| // Create an object to store boundary types and their parents | ||
| const parentMap: any = {}; | ||
|
|
||
| // Populate the object with boundary types and their parents | ||
| for (const boundary of boundaries) { | ||
| parentMap[boundary.boundaryType] = boundary.parentBoundaryType; | ||
| } | ||
|
|
||
| // Traverse the hierarchy to generate the hierarchy list | ||
| const hierarchyList = []; | ||
| for (const boundaryType in parentMap) { | ||
| if (Object.prototype.hasOwnProperty.call(parentMap, boundaryType)) { | ||
| const parentBoundaryType = parentMap[boundaryType]; | ||
| if (parentBoundaryType === null) { | ||
| // This boundary type has no parent, add it to the hierarchy list | ||
| hierarchyList.push(boundaryType); | ||
| // Traverse its children recursively | ||
| traverseChildren(boundaryType, parentMap, hierarchyList); | ||
| } | ||
| } | ||
| } | ||
| return hierarchyList; | ||
| } | ||
|
|
||
| function traverseChildren(parent: any, parentMap: any, hierarchyList: any[]) { | ||
| for (const boundaryType in parentMap) { | ||
| if (Object.prototype.hasOwnProperty.call(parentMap, boundaryType)) { | ||
| const parentBoundaryType = parentMap[boundaryType]; | ||
| if (parentBoundaryType === parent) { | ||
| // This boundary type has the current parent, add it to the hierarchy list | ||
| hierarchyList.push(boundaryType); | ||
| // Traverse its children recursively | ||
| traverseChildren(boundaryType, parentMap, hierarchyList); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The provided implementation of generateHierarchy and traverseChildren functions correctly establishes parent-child relationships by building a hierarchy list from a set of boundaries. However, there is no explicit mechanism observed in the code to detect or prevent cycles in the hierarchy. This means that while the functions are designed to traverse and organize boundaries into a hierarchy based on their parent-child relationships, they do not explicitly handle the case where cycles might exist in the input data.
Analysis chain
In generateHierarchy, verify that the parent-child relationships are correctly established and that there are no cycles in the hierarchy.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that generateHierarchy correctly handles parent-child relationships without creating cycles
grep -C3 "function generateHierarchy" utilities/project-factory/src/server/utils/campaignUtils.ts
Length of output: 254
Script:
#!/bin/bash
# Extract the full implementation of generateHierarchy and traverseChildren functions
grep -A30 "function generateHierarchy" utilities/project-factory/src/server/utils/campaignUtils.ts
grep -A20 "function traverseChildren" utilities/project-factory/src/server/utils/campaignUtils.ts
Length of output: 2334
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes