-
Notifications
You must be signed in to change notification settings - Fork 24
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
console recent changes to merged in master #1274
base: master
Are you sure you want to change the base?
Changes from 11 commits
355a8ab
2bdd736
016dc01
d96bb2b
19283e0
74700da
9dee69b
3ed33e6
49b5801
b373ac4
b69ba14
ad9a5b8
f13f4cf
9df4256
d832441
fc6122e
bfccb0a
5420d8e
bd673e7
97b4b32
b5be48e
396673c
3fc3a41
343393e
d94b380
b5aee12
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,4 @@ COPY . . | |
EXPOSE 3000 | ||
|
||
CMD ["yarn", "prod"] | ||
# Replaced by CMD ["yarn", "prod"] | ||
|
||
|
||
# Replaced by CMD ["yarn", "prod"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Remove redundant comment The comment about CMD replacement is redundant as Git history already tracks these changes. Consider removing this line for cleaner code. -# Replaced by CMD ["yarn", "prod"] |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -10,11 +10,11 @@ | |||||||
"build": "yarn run build-ts", | ||||||||
"build-ts": "tsc", | ||||||||
"clean": "rm -rf ./dist", | ||||||||
"serve": "node dist/index.js", | ||||||||
"serve": "if [ \"$DEBUG\" = \"true\" ]; then node --inspect=0.0.0.0:9229 dist/index.js; else node dist/index.js; fi", | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security: Restrict debug port binding The debug port is currently bound to all network interfaces (0.0.0.0), which could expose the debugger in production environments. Consider restricting it to localhost or specific interfaces. Apply this change to improve security: - "serve": "if [ \"$DEBUG\" = \"true\" ]; then node --inspect=0.0.0.0:9229 dist/index.js; else node dist/index.js; fi",
+ "serve": "if [ \"$DEBUG\" = \"true\" ]; then node --inspect=127.0.0.1:9229 dist/index.js; else node dist/index.js; fi", 📝 Committable suggestion
Suggested change
|
||||||||
"start": "yarn run dev", | ||||||||
"test": "jest", | ||||||||
"dev": "ts-node-dev --respawn src/server/index.ts", | ||||||||
"prod": "yarn build && yarn serve", | ||||||||
"prod": "if [ \"$DEBUG\" = \"true\" ]; then cp tsconfig.debug.json tsconfig.json; fi && yarn build && yarn serve", | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Reconsider debug configuration in production The current implementation has several concerns:
Consider:
Suggested refactor: - "prod": "if [ \"$DEBUG\" = \"true\" ]; then cp tsconfig.debug.json tsconfig.json; fi && yarn build && yarn serve",
+ "prod": "yarn build && yarn serve",
+ "debug": "[ -f tsconfig.debug.json ] && cp tsconfig.debug.json tsconfig.json && yarn build && yarn serve || echo 'Error: tsconfig.debug.json not found'", 📝 Committable suggestion
Suggested change
|
||||||||
"watch-ts": "tsc --watch" | ||||||||
}, | ||||||||
"repository": { | ||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -39,6 +39,8 @@ const config = { | |||||||||
}, | ||||||||||
facility: { | ||||||||||
facilityTab: process.env.FACILITY_TAB_NAME || "HCM_ADMIN_CONSOLE_FACILITIES", | ||||||||||
facilityCodeColumn : "HCM_ADMIN_CONSOLE_FACILITY_CODE", | ||||||||||
facilityType : "facility" | ||||||||||
Comment on lines
+44
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Make the facility code & type configurable. Both |
||||||||||
}, | ||||||||||
user: { | ||||||||||
userTab: process.env.USER_TAB_NAME || "HCM_ADMIN_CONSOLE_USER_LIST", | ||||||||||
|
@@ -95,6 +97,8 @@ const config = { | |||||||||
defaultLocale: process.env.LOCALE || "en_MZ", | ||||||||||
boundaryPrefix: "hcm-boundary", | ||||||||||
localizationModule: process.env.LOCALIZATION_MODULE || "hcm-admin-schemas", | ||||||||||
localizationWaitTimeInBoundaryCreation: parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"), | ||||||||||
localizationChunkSizeForBoundaryCreation: parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"), | ||||||||||
Comment on lines
+102
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use For better consistency and to adhere to modern JavaScript standards, use Apply this diff to update the code: - localizationWaitTimeInBoundaryCreation: parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"),
- localizationChunkSizeForBoundaryCreation: parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"),
+ localizationWaitTimeInBoundaryCreation: Number.parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"),
+ localizationChunkSizeForBoundaryCreation: Number.parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"), 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 100-100: Use Number.parseInt instead of the equivalent global. ES2015 moved some globals into the Number namespace for consistency. (lint/style/useNumberNamespace) [error] 101-101: Use Number.parseInt instead of the equivalent global. ES2015 moved some globals into the Number namespace for consistency. (lint/style/useNumberNamespace) |
||||||||||
}, | ||||||||||
// targetColumnsForSpecificCampaigns: { | ||||||||||
// bedNetCampaignColumns: ["HCM_ADMIN_CONSOLE_TARGET"], | ||||||||||
|
@@ -148,6 +152,7 @@ const config = { | |||||||||
localizationSearch: process.env.EGOV_LOCALIZATION_SEARCH || "localization/messages/v1/_search", | ||||||||||
localizationCreate: "localization/messages/v1/_upsert", | ||||||||||
projectTypeSearch: "project-factory/v1/project-type/search", | ||||||||||
cacheBurst: process.env.CACHE_BURST || "localization/messages/cache-bust", | ||||||||||
boundaryRelationshipCreate: "boundary-service/boundary-relationships/_create", | ||||||||||
healthIndividualSearch: process.env.EGOV_HEALTH_INDIVIDUAL_SEARCH || "health-individual/v1/_search", | ||||||||||
projectFacilitySearch: process.env.EGOV_HEALTH_PROJECT_FACILITY_SEARCH || "health-project/facility/v1/_search", | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import { logger } from "../../utils/logger"; | |
import { httpRequest } from "../../utils/request"; | ||
import config from "../../config/index"; | ||
import { convertLocalisationResponseToMap } from "../../utils/localisationUtils"; | ||
import { defaultRequestInfo } from "../../api/coreApis"; | ||
|
||
let cachedResponse = {}; | ||
|
||
|
@@ -82,6 +83,20 @@ class Localisation { | |
); | ||
cachedResponse = { ...this.cachedResponse }; | ||
}; | ||
|
||
// Calls the cache burst API of localization | ||
public cacheBurst = async ( | ||
) => { | ||
logger.info(`Calling localization cache burst api`); | ||
const RequestInfo = defaultRequestInfo; | ||
const requestBody = { | ||
RequestInfo | ||
} | ||
await httpRequest( | ||
this.localizationHost + config.paths.cacheBurst, | ||
requestBody) | ||
}; | ||
|
||
|
||
private checkCacheAndDeleteIfExists = (module: string, locale: "string") => { | ||
logger.info( | ||
|
@@ -132,6 +147,7 @@ class Localisation { | |
// Log and handle any errors that occur during the process | ||
console.log(e); | ||
logger.error(String(e)); | ||
throw new Error(e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rethrow the original error to preserve stack trace Using Apply this diff to rethrow the original error: - throw new Error(e);
+ throw e; If additional context is needed, wrap the error while preserving the original error: - throw new Error(e);
+ throw new Error(`Failed to create localisation: ${e.message}`);
+ // Optionally, include the original error object
+ // throw new Error(`Failed to create localisation: ${e.message}`, { cause: e });
|
||
} | ||
}; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -958,7 +958,7 @@ async function enrichAndPersistCampaignWithError(requestBody: any, error: any) { | |
requestBody.CampaignDetails.status = campaignStatuses?.failed; | ||
// requestBody.CampaignDetails.isActive = false; | ||
requestBody.CampaignDetails.boundaryCode = | ||
getRootBoundaryCode(requestBody?.CampaignDetails?.boundaries) || null; | ||
getRootBoundaryCode(requestBody?.boundariesCombined) || null; | ||
requestBody.CampaignDetails.projectType = | ||
requestBody?.CampaignDetails?.projectType || null; | ||
requestBody.CampaignDetails.hierarchyType = | ||
|
@@ -1035,7 +1035,7 @@ async function enrichAndPersistCampaignForCreate( | |
request.body.CampaignDetails.status = | ||
action == "create" ? campaignStatuses.started : campaignStatuses.drafted; | ||
request.body.CampaignDetails.boundaryCode = getRootBoundaryCode( | ||
request.body.CampaignDetails.boundaries | ||
request.body?.boundariesCombined | ||
); | ||
request.body.CampaignDetails.projectType = | ||
request?.body?.CampaignDetails?.projectType || null; | ||
|
@@ -1111,7 +1111,7 @@ async function enrichAndPersistCampaignForUpdate( | |
? campaignStatuses.started | ||
: campaignStatuses.drafted; | ||
const boundaryCode = !request?.body?.CampaignDetails?.projectId | ||
? getRootBoundaryCode(request.body.CampaignDetails.boundaries) | ||
? getRootBoundaryCode(request.body?.boundariesCombined) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider improving readability of the boundary code logic. The nested ternary operator could be simplified for better readability. Consider this alternative: - const boundaryCode = !request?.body?.CampaignDetails?.projectId
- ? getRootBoundaryCode(request.body?.boundariesCombined)
- : request?.body?.CampaignDetails?.boundaryCode ||
- ExistingCampaignDetails?.boundaryCode;
+ const boundaryCode = request?.body?.CampaignDetails?.projectId
+ ? (request?.body?.CampaignDetails?.boundaryCode || ExistingCampaignDetails?.boundaryCode)
+ : getRootBoundaryCode(request.body?.boundariesCombined);
|
||
: request?.body?.CampaignDetails?.boundaryCode || | ||
ExistingCampaignDetails?.boundaryCode; | ||
request.body.CampaignDetails.boundaryCode = boundaryCode; | ||
|
@@ -2414,6 +2414,7 @@ async function processAfterPersist(request: any, actionInUrl: any) { | |
localizationMap | ||
); | ||
} | ||
delete request.body?.boundariesCombined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid using the Deleting properties from objects can negatively impact performance. Consider setting the property to Apply this diff: - delete request.body?.boundariesCombined;
+ request.body.boundariesCombined = undefined;
🧰 Tools🪛 Biome (1.9.4)[error] 2417-2417: Avoid the delete operator which can impact performance. (lint/performance/noDelete) |
||
} catch (error: any) { | ||
console.log(error); | ||
logger.error(error); | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -374,7 +374,7 @@ async function fullProcessFlowForNewEntry(newEntryResponse: any, generatedResour | |||||||||||||||||||||||||||||||
const localizationMapModule = await getLocalizedMessagesHandler(request, request?.query?.tenantId); | ||||||||||||||||||||||||||||||||
const localizationMap = { ...localizationMapHierarchy, ...localizationMapModule }; | ||||||||||||||||||||||||||||||||
let fileUrlResponse: any; | ||||||||||||||||||||||||||||||||
if(type != 'boundaryManagement' && request?.query?.campaignId != 'default' && type != 'boundaryGeometryManagement'){ | ||||||||||||||||||||||||||||||||
if (type != 'boundaryManagement' && request?.query?.campaignId != 'default' && type != 'boundaryGeometryManagement') { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Standardize equality operators. Replace loose equality operators with strict equality for consistent type comparison: - if (type != 'boundaryManagement' && request?.query?.campaignId != 'default' && type != 'boundaryGeometryManagement') {
+ if (type !== 'boundaryManagement' && request?.query?.campaignId !== 'default' && type !== 'boundaryGeometryManagement') {
- else if (type == 'boundaryManagement' || type === 'boundaryGeometryManagement') {
+ else if (type === 'boundaryManagement' || type === 'boundaryGeometryManagement') { Also applies to: 387-387 |
||||||||||||||||||||||||||||||||
const responseFromCampaignSearch = await getCampaignSearchResponse(request); | ||||||||||||||||||||||||||||||||
const campaignObject = responseFromCampaignSearch?.CampaignDetails?.[0]; | ||||||||||||||||||||||||||||||||
logger.info(`checks for parent campaign for type: ${type}`) | ||||||||||||||||||||||||||||||||
|
@@ -407,7 +407,7 @@ async function fullProcessFlowForNewEntry(newEntryResponse: any, generatedResour | |||||||||||||||||||||||||||||||
await produceModifiedMessages(generatedResourceNew, updateGeneratedResourceTopic); | ||||||||||||||||||||||||||||||||
request.body.generatedResource = finalResponse; | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
else if (type == 'boundaryManagement' || type === 'boundaryGeometryManagement'){ | ||||||||||||||||||||||||||||||||
else if (type == 'boundaryManagement' || type === 'boundaryGeometryManagement') { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Unify equality checks for consistency. |
||||||||||||||||||||||||||||||||
// get boundary data from boundary relationship search api | ||||||||||||||||||||||||||||||||
logger.info("Generating Boundary Data") | ||||||||||||||||||||||||||||||||
const boundaryDataSheetGeneratedBeforeDifferentTabSeparation = await getBoundaryDataService(request, false); | ||||||||||||||||||||||||||||||||
|
@@ -601,10 +601,10 @@ function setAndFormatHeaders(worksheet: any, mainHeader: any, headerSet: any) { | |||||||||||||||||||||||||||||||
async function createReadMeSheet(request: any, workbook: any, mainHeader: any, localizationMap = {}) { | ||||||||||||||||||||||||||||||||
const isSourceMicroplan = await isMicroplanRequest(request); | ||||||||||||||||||||||||||||||||
let readMeConfig: any; | ||||||||||||||||||||||||||||||||
if(isSourceMicroplan) { | ||||||||||||||||||||||||||||||||
if (isSourceMicroplan) { | ||||||||||||||||||||||||||||||||
readMeConfig = await getReadMeConfigForMicroplan(request); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
else{ | ||||||||||||||||||||||||||||||||
else { | ||||||||||||||||||||||||||||||||
readMeConfig = await getReadMeConfig(request); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
const headerSet = new Set(); | ||||||||||||||||||||||||||||||||
|
@@ -749,13 +749,13 @@ async function createFacilityAndBoundaryFile(facilitySheetData: any, boundaryShe | |||||||||||||||||||||||||||||||
hideUniqueIdentifierColumn(facilitySheet, createAndSearch?.["facility"]?.uniqueIdentifierColumn); | ||||||||||||||||||||||||||||||||
changeFirstRowColumnColour(facilitySheet, 'E06666'); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
let receivedDropdowns=request.body?.dropdowns; | ||||||||||||||||||||||||||||||||
logger.info("started adding dropdowns in facility",JSON.stringify(receivedDropdowns)) | ||||||||||||||||||||||||||||||||
let receivedDropdowns = request.body?.dropdowns; | ||||||||||||||||||||||||||||||||
logger.info("started adding dropdowns in facility", JSON.stringify(receivedDropdowns)) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if(!receivedDropdowns||Object.keys(receivedDropdowns)?.length==0){ | ||||||||||||||||||||||||||||||||
if (!receivedDropdowns || Object.keys(receivedDropdowns)?.length == 0) { | ||||||||||||||||||||||||||||||||
logger.info("No dropdowns found"); | ||||||||||||||||||||||||||||||||
receivedDropdowns= setDropdownFromSchema(request,schema,localizationMap); | ||||||||||||||||||||||||||||||||
logger.info("refetched drodowns",JSON.stringify(receivedDropdowns)) | ||||||||||||||||||||||||||||||||
receivedDropdowns = setDropdownFromSchema(request, schema, localizationMap); | ||||||||||||||||||||||||||||||||
logger.info("refetched drodowns", JSON.stringify(receivedDropdowns)) | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Dropdown initialization refetch logic. Gracefully re-fetching dropdowns if none are found prevents unexpected null references. Consider adding error handling around this fallback. |
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
await handledropdownthings(facilitySheet, receivedDropdowns); | ||||||||||||||||||||||||||||||||
await handleHiddenColumns(facilitySheet, request.body?.hiddenColumns); | ||||||||||||||||||||||||||||||||
|
@@ -777,7 +777,7 @@ async function handledropdownthings(sheet: any, dropdowns: any) { | |||||||||||||||||||||||||||||||
logger.info("Dropdowns provided:", dropdowns); | ||||||||||||||||||||||||||||||||
for (const key of Object.keys(dropdowns)) { | ||||||||||||||||||||||||||||||||
if (dropdowns[key]) { | ||||||||||||||||||||||||||||||||
logger.info(`Processing dropdown key: ${key} with values: ${dropdowns[key]}`); | ||||||||||||||||||||||||||||||||
logger.info(`Processing dropdown key: ${key} with values: ${dropdowns[key]}`); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Be mindful of potential clutter in logs. |
||||||||||||||||||||||||||||||||
const firstRow = sheet.getRow(1); | ||||||||||||||||||||||||||||||||
firstRow.eachCell({ includeEmpty: true }, (cell: any, colNumber: any) => { | ||||||||||||||||||||||||||||||||
if (cell.value === key) { | ||||||||||||||||||||||||||||||||
|
@@ -815,7 +815,7 @@ async function handledropdownthings(sheet: any, dropdowns: any) { | |||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
async function handleHiddenColumns(sheet: any, hiddenColumns: any) { | ||||||||||||||||||||||||||||||||
// logger.info(sheet) | ||||||||||||||||||||||||||||||||
logger.info("hiddenColumns",hiddenColumns); | ||||||||||||||||||||||||||||||||
logger.info("hiddenColumns", hiddenColumns); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Hidden columns logging. While printing hidden columns can help debugging, consider logging structured data (like JSON) for better clarity. |
||||||||||||||||||||||||||||||||
if (hiddenColumns) { | ||||||||||||||||||||||||||||||||
for (const columnName of hiddenColumns) { | ||||||||||||||||||||||||||||||||
const firstRow = sheet.getRow(1); | ||||||||||||||||||||||||||||||||
|
@@ -849,13 +849,13 @@ async function createUserAndBoundaryFile(userSheetData: any, boundarySheetData: | |||||||||||||||||||||||||||||||
addDataToSheet(request, userSheet, userSheetData, undefined, undefined, true, false, localizationMap, fileUrl, schema); | ||||||||||||||||||||||||||||||||
hideUniqueIdentifierColumn(userSheet, createAndSearch?.["user"]?.uniqueIdentifierColumn); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
let receivedDropdowns=request.body?.dropdowns; | ||||||||||||||||||||||||||||||||
logger.info("started adding dropdowns in user",JSON.stringify(receivedDropdowns)) | ||||||||||||||||||||||||||||||||
let receivedDropdowns = request.body?.dropdowns; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Use const for immutable variables. The variable is never reassigned, use - let receivedDropdowns = request.body?.dropdowns;
+ const receivedDropdowns = request.body?.dropdowns; 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 854-854: This let declares a variable that is only assigned once. 'receivedDropdowns' is never reassigned. Safe fix: Use const instead. (lint/style/useConst) |
||||||||||||||||||||||||||||||||
logger.info("started adding dropdowns in user", JSON.stringify(receivedDropdowns)) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if(!receivedDropdowns||Object.keys(receivedDropdowns)?.length==0){ | ||||||||||||||||||||||||||||||||
if (!receivedDropdowns || Object.keys(receivedDropdowns)?.length == 0) { | ||||||||||||||||||||||||||||||||
logger.info("No dropdowns found"); | ||||||||||||||||||||||||||||||||
receivedDropdowns= setDropdownFromSchema(request,schema,localizationMap); | ||||||||||||||||||||||||||||||||
logger.info("refetched drodowns",JSON.stringify(receivedDropdowns)) | ||||||||||||||||||||||||||||||||
receivedDropdowns = setDropdownFromSchema(request, schema, localizationMap); | ||||||||||||||||||||||||||||||||
logger.info("refetched drodowns", JSON.stringify(receivedDropdowns)) | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
await handledropdownthings(userSheet, receivedDropdowns); | ||||||||||||||||||||||||||||||||
await handleHiddenColumns(userSheet, request.body?.hiddenColumns); | ||||||||||||||||||||||||||||||||
|
@@ -870,6 +870,8 @@ async function createUserAndBoundaryFile(userSheetData: any, boundarySheetData: | |||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
async function generateFacilityAndBoundarySheet(tenantId: string, request: any, localizationMap?: { [key: string]: string }, filteredBoundary?: any, fileUrl?: any) { | ||||||||||||||||||||||||||||||||
const type = request?.query?.type || request?.body?.ResourceDetails?.type; | ||||||||||||||||||||||||||||||||
const typeWithoutWith = type.includes('With') ? type.split('With')[0] : type; | ||||||||||||||||||||||||||||||||
Comment on lines
+883
to
+884
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type safety in request handling. Add validation and type checking for the extracted type: - const type = request?.query?.type || request?.body?.ResourceDetails?.type;
+ const type = request?.query?.type || request?.body?.ResourceDetails?.type;
+ if (!type) {
+ throwError("COMMON", 400, "VALIDATION_ERROR", "Type is required");
+ }
+ if (typeof type !== 'string') {
+ throwError("COMMON", 400, "VALIDATION_ERROR", "Type must be a string");
+ }
|
||||||||||||||||||||||||||||||||
// Get facility and boundary data | ||||||||||||||||||||||||||||||||
logger.info("Generating facilities started"); | ||||||||||||||||||||||||||||||||
const allFacilities = await getAllFacilities(tenantId, request.body); | ||||||||||||||||||||||||||||||||
|
@@ -881,10 +883,10 @@ async function generateFacilityAndBoundarySheet(tenantId: string, request: any, | |||||||||||||||||||||||||||||||
if (fileUrl) { | ||||||||||||||||||||||||||||||||
/* fetch facility from processed file | ||||||||||||||||||||||||||||||||
and generate facility sheet data */ | ||||||||||||||||||||||||||||||||
schema = await callMdmsTypeSchema(request, tenantId, true, typeWithoutWith, "all"); | ||||||||||||||||||||||||||||||||
const processedFacilitySheetData = await getSheetData(fileUrl, localizedFacilityTab, false, undefined, localizationMap); | ||||||||||||||||||||||||||||||||
const modifiedProcessedFacilitySheetData = modifyProcessedSheetData(request, processedFacilitySheetData, localizationMap); | ||||||||||||||||||||||||||||||||
const modifiedProcessedFacilitySheetData = modifyProcessedSheetData(typeWithoutWith, processedFacilitySheetData, schema, localizationMap); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Modifying processed data. Calling |
||||||||||||||||||||||||||||||||
facilitySheetData = modifiedProcessedFacilitySheetData; | ||||||||||||||||||||||||||||||||
schema = await callMdmsTypeSchema(request, tenantId, true, "facility", "all"); | ||||||||||||||||||||||||||||||||
setDropdownFromSchema(request, schema, localizationMap); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
else { | ||||||||||||||||||||||||||||||||
|
@@ -903,9 +905,11 @@ async function generateFacilityAndBoundarySheet(tenantId: string, request: any, | |||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
async function generateUserSheet(request: any, localizationMap?: { [key: string]: string }, filteredBoundary?: any, userData?: any, fileUrl?: any) { | ||||||||||||||||||||||||||||||||
const tenantId = request?.query?.tenantId; | ||||||||||||||||||||||||||||||||
const type = request?.query?.type || request?.body?.ResourceDetails?.type; | ||||||||||||||||||||||||||||||||
const typeWithoutWith = type.includes('With') ? type.split('With')[0] : type; | ||||||||||||||||||||||||||||||||
let schema: any; | ||||||||||||||||||||||||||||||||
const isUpdate = fileUrl ? true : false; | ||||||||||||||||||||||||||||||||
schema = await callMdmsTypeSchema(request, tenantId, isUpdate, "user"); | ||||||||||||||||||||||||||||||||
schema = await callMdmsTypeSchema(request, tenantId, isUpdate, typeWithoutWith); | ||||||||||||||||||||||||||||||||
setDropdownFromSchema(request, schema, localizationMap); | ||||||||||||||||||||||||||||||||
const headers = schema?.columns; | ||||||||||||||||||||||||||||||||
const localizedHeaders = getLocalizedHeaders(headers, localizationMap); | ||||||||||||||||||||||||||||||||
|
@@ -917,7 +921,7 @@ async function generateUserSheet(request: any, localizationMap?: { [key: string] | |||||||||||||||||||||||||||||||
/* fetch facility from processed file | ||||||||||||||||||||||||||||||||
and generate facility sheet data */ | ||||||||||||||||||||||||||||||||
const processedUserSheetData = await getSheetData(fileUrl, localizedUserTab, false, undefined, localizationMap); | ||||||||||||||||||||||||||||||||
const modifiedProcessedUserSheetData = modifyProcessedSheetData(request, processedUserSheetData, localizationMap); | ||||||||||||||||||||||||||||||||
const modifiedProcessedUserSheetData = modifyProcessedSheetData(typeWithoutWith, processedUserSheetData, schema, localizationMap); | ||||||||||||||||||||||||||||||||
userSheetData = modifiedProcessedUserSheetData; | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
else { | ||||||||||||||||||||||||||||||||
|
@@ -1075,7 +1079,7 @@ async function handleGenerateError(newEntryResponse: any, generatedResource: any | |||||||||||||||||||||||||||||||
code: error.code, | ||||||||||||||||||||||||||||||||
description: error.description, | ||||||||||||||||||||||||||||||||
message: error.message | ||||||||||||||||||||||||||||||||
} || String(error) | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error object handling. Add type checking and validation for error properties to ensure consistent error reporting: item.status = generatedResourceStatuses.failed,
item.additionalDetails = {
...item.additionalDetails,
error: {
- status: error.status,
- code: error.code,
- description: error.description,
- message: error.message
+ status: error.status || 500,
+ code: error.code || 'UNKNOWN_ERROR',
+ description: error.description || null,
+ message: typeof error.message === 'string' ? error.message : 'Unknown error occurred'
}
}
|
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||
generatedResource = { generatedResource: newEntryResponse }; | ||||||||||||||||||||||||||||||||
|
@@ -1303,12 +1307,12 @@ async function getDataSheetReady(boundaryData: any, request: any, localizationMa | |||||||||||||||||||||||||||||||
mappedRowData[boundaryCodeIndex] = boundaryCode; | ||||||||||||||||||||||||||||||||
return mappedRowData; | ||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||
if(type == "boundaryManagement"){ | ||||||||||||||||||||||||||||||||
if (type == "boundaryManagement") { | ||||||||||||||||||||||||||||||||
logger.info("Processing data for boundaryManagement type") | ||||||||||||||||||||||||||||||||
const latLongBoundaryMap = await getLatLongMapForBoundaryCodes(request, boundaryCodeList); | ||||||||||||||||||||||||||||||||
for (let d of data) { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Use const instead of let. The variable declared here is never reassigned. Switching to - let d of data
+ const d of data 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 1290-1290: This let declares a variable that is only assigned once. 'd' is never reassigned. Safe fix: Use const instead. (lint/style/useConst) |
||||||||||||||||||||||||||||||||
const boundaryCode = d[d.length - 1]; // Assume last element is the boundary code | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
Comment on lines
+1323
to
+1328
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve type safety in boundary management processing. The boundary code access assumes array structure without validation. Add validation: if (type == "boundaryManagement") {
logger.info("Processing data for boundaryManagement type")
const latLongBoundaryMap = await getLatLongMapForBoundaryCodes(request, boundaryCodeList);
for (let d of data) {
+ if (!Array.isArray(d) || d.length === 0) {
+ logger.error("Invalid data structure for boundary management");
+ continue;
+ }
const boundaryCode = d[d.length - 1];
if (latLongBoundaryMap[boundaryCode]) {
const [latitude = null, longitude = null] = latLongBoundaryMap[boundaryCode];
d.push(latitude);
d.push(longitude);
}
}
} 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 1313-1313: This let declares a variable that is only assigned once. 'd' is never reassigned. Safe fix: Use const instead. (lint/style/useConst) |
||||||||||||||||||||||||||||||||
if (latLongBoundaryMap[boundaryCode]) { | ||||||||||||||||||||||||||||||||
const [latitude = null, longitude = null] = latLongBoundaryMap[boundaryCode]; // Destructure lat/long | ||||||||||||||||||||||||||||||||
d.push(latitude); // Append latitude | ||||||||||||||||||||||||||||||||
|
@@ -1359,7 +1363,7 @@ function modifyDataBasedOnDifferentTab(boundaryData: any, differentTabsBasedOnLe | |||||||||||||||||||||||||||||||
async function getLocalizedMessagesHandler(request: any, tenantId: any, module = config.localisation.localizationModule, overrideCache = false) { | ||||||||||||||||||||||||||||||||
const localisationcontroller = Localisation.getInstance(); | ||||||||||||||||||||||||||||||||
const locale = getLocaleFromRequest(request); | ||||||||||||||||||||||||||||||||
const localizationResponse = await localisationcontroller.getLocalisedData(module, locale, tenantId,overrideCache); | ||||||||||||||||||||||||||||||||
const localizationResponse = await localisationcontroller.getLocalisedData(module, locale, tenantId, overrideCache); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider caching optimization for localization. The localization data fetching could benefit from caching to improve performance. Consider implementing a caching mechanism for frequently accessed localization data to reduce database load and improve response times. This could be achieved using the existing |
||||||||||||||||||||||||||||||||
return localizationResponse; | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,36 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { Request } from "express"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import * as zlib from "zlib"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Update Node.js import to use the node: protocol For better clarity and explicit Node.js module identification, update the zlib import. -import * as zlib from "zlib";
+import * as zlib from "node:zlib"; 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 2-2: A Node.js builtin module should be imported with the node: protocol. Using the node: protocol is more explicit and signals that the imported module belongs to Node.js. (lint/style/useNodejsImportProtocol) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const handleGzipRequest = async (req: Request): Promise<void> => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const buffers: Buffer[] = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Collect data chunks from the request | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await new Promise<void>((resolve, reject) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
req.on("data", (chunk: any) => buffers.push(chunk)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
req.on("end", resolve); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
req.on("error", reject); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Concatenate and decompress the data | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const gzipBuffer = Buffer.concat(buffers); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const decompressedData = await decompressGzip(gzipBuffer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
req.body = decompressedData; // Assign the parsed data to req.body | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (err: any) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw new Error(`Failed to process Gzip data: ${err.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+4
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add memory safeguards for buffer collection The current implementation collects all chunks without size limits, which could lead to memory issues with large requests. Add a size limit check: export const handleGzipRequest = async (req: Request): Promise<void> => {
const buffers: Buffer[] = [];
+ let totalSize = 0;
+ const MAX_SIZE = 10 * 1024 * 1024; // 10MB limit
// Collect data chunks from the request
await new Promise<void>((resolve, reject) => {
- req.on("data", (chunk: any) => buffers.push(chunk));
+ req.on("data", (chunk: Buffer) => {
+ totalSize += chunk.length;
+ if (totalSize > MAX_SIZE) {
+ reject(new Error("Request payload too large"));
+ return;
+ }
+ buffers.push(chunk);
+ });
req.on("end", resolve);
req.on("error", reject);
}); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Helper function to decompress Gzip data | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const decompressGzip = (gzipBuffer: Buffer): Promise<any> => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return new Promise((resolve, reject) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
zlib.gunzip(gzipBuffer, (err, result) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (err) return reject(err); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
resolve(JSON.parse(result.toString())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (parseErr) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
reject(new Error("Invalid JSON format in decompressed data")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+24
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error handling and type safety in decompressGzip The current implementation could be improved with more specific error handling and type safety. -const decompressGzip = (gzipBuffer: Buffer): Promise<any> => {
+const decompressGzip = (gzipBuffer: Buffer): Promise<unknown> => {
return new Promise((resolve, reject) => {
zlib.gunzip(gzipBuffer, (err, result) => {
- if (err) return reject(err);
+ if (err) {
+ return reject(new Error(`Gzip decompression failed: ${err.message}`));
+ }
try {
- resolve(JSON.parse(result.toString()));
+ const parsed = JSON.parse(result.toString('utf-8'));
+ if (typeof parsed !== 'object' || parsed === null) {
+ throw new Error('Decompressed content must be a JSON object');
+ }
+ resolve(parsed);
} catch (parseErr) {
- reject(new Error("Invalid JSON format in decompressed data"));
+ reject(new Error(`Invalid JSON format: ${parseErr.message}`));
}
});
});
}; 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -62,9 +62,8 @@ export async function getUserDataFromMicroplanSheet(request: any, fileStoreId: a | |||||
export function getAllUserData(request: any, userMapping: any, localizationMap: any) { | ||||||
const emailKey = getLocalizedName("HCM_ADMIN_CONSOLE_USER_EMAIL_MICROPLAN", localizationMap); | ||||||
const nameKey = getLocalizedName("HCM_ADMIN_CONSOLE_USER_NAME_MICROPLAN", localizationMap); | ||||||
const phoneNumberKey = getLocalizedName("HCM_ADMIN_CONSOLE_USER_PHONE_NUMBER_MICROPLAN", localizationMap); | ||||||
validateInConsistency(request, userMapping, emailKey, nameKey); | ||||||
validateNationalDuplicacy(request, userMapping, phoneNumberKey); | ||||||
validateNationalDuplicacy(request, userMapping, localizationMap); | ||||||
const dataToCreate: any = []; | ||||||
for (const phoneNumber of Object.keys(userMapping)) { | ||||||
const roles = userMapping[phoneNumber].map((user: any) => user.role).join(','); | ||||||
|
@@ -94,30 +93,30 @@ function validateInConsistency(request: any, userMapping: any, emailKey: any, na | |||||
request.body.sheetErrorDetails = Array.isArray(request.body.sheetErrorDetails) ? [...request.body.sheetErrorDetails, ...overallInconsistencies] : overallInconsistencies; | ||||||
} | ||||||
|
||||||
function validateNationalDuplicacy(request: any, userMapping: any, phoneNumberKey: any) { | ||||||
function validateNationalDuplicacy(request: any, userMapping: any, localizationMap: any) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Add TypeScript type annotations for function parameters. The function parameters lack type annotations, which could lead to runtime errors. -function validateNationalDuplicacy(request: any, userMapping: any, localizationMap: any) {
+interface UserMappingType {
+ [phoneNumber: string]: Array<{
+ role?: string;
+ "!sheet#name!"?: string;
+ "!row#number!"?: number;
+ }>;
+}
+
+function validateNationalDuplicacy(
+ request: { body: { ResourceDetails: { status: string }, sheetErrorDetails?: any[] } },
+ userMapping: UserMappingType,
+ localizationMap: { [key: string]: string }
+) { |
||||||
const duplicates: any[] = []; | ||||||
|
||||||
for (const phoneNumber in userMapping) { | ||||||
const roleMap: any = {}; | ||||||
const users = userMapping[phoneNumber]; | ||||||
|
||||||
for (const user of users) { | ||||||
if (user.role?.startsWith("Root ")) { | ||||||
const userRole = user?.role; | ||||||
if (userRole?.startsWith("ROOT_")) { | ||||||
// Trim the role | ||||||
const trimmedRole = user.role.replace("Root ", "").trim().toLowerCase(); | ||||||
const trimmedRoleWithCapital = trimmedRole.charAt(0).toUpperCase() + trimmedRole.slice(1); | ||||||
const trimmedRole = userRole.replace("ROOT_", ""); | ||||||
Comment on lines
+104
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix inconsistent role handling logic. The role handling logic is inconsistent between the if and else blocks:
Apply this diff to fix the logic: if (userRole?.startsWith("ROOT_")) {
const trimmedRole = userRole.replace("ROOT_", "");
// ... existing code ...
} else {
- const trimmedRole = userRole;
- const errorMessage: any = `An user with ${getLocalizedName("ROOT_" + trimmedRole, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
+ const trimmedRole = userRole;
+ const errorMessage: any = `A user with ${getLocalizedName(trimmedRole, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
// ... existing code ...
} Also applies to: 118-119 |
||||||
|
||||||
// Check for duplicates in the roleMap | ||||||
if (roleMap[trimmedRole] && roleMap[trimmedRole]["!sheet#name!"] != user["!sheet#name!"]) { | ||||||
const errorMessage: any = `An user with ${trimmedRoleWithCapital} role can’t be assigned to ${user.role} role`; | ||||||
const errorMessage: any = `An user with ${getLocalizedName(trimmedRole, localizationMap)} role can’t be assigned to ${getLocalizedName(userRole, localizationMap)} role`; | ||||||
duplicates.push({ rowNumber: user["!row#number!"], sheetName: user["!sheet#name!"], status: "INVALID", errorDetails: errorMessage }); | ||||||
} else { | ||||||
roleMap[trimmedRole] = user; | ||||||
} | ||||||
} | ||||||
else { | ||||||
const trimmedRole = user.role.toLowerCase(); | ||||||
const errorMessage: any = `An user with ${"Root " + trimmedRole} role can’t be assigned to ${user.role} role`; | ||||||
const trimmedRole = userRole; | ||||||
const errorMessage: any = `An user with ${getLocalizedName("ROOT_" + trimmedRole, localizationMap)} role can’t be assigned to ${getLocalizedName(userRole, localizationMap)} role`; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Use template literals and correct the grammatical error in the error message To enhance readability and maintain consistency, prefer using template literals over string concatenation. Also, replace 'An user' with 'A user' for grammatical correctness in the error message. Apply this diff to implement the changes: - const errorMessage: any = `An user with ${getLocalizedName("ROOT_" + trimmedRole, localizationMap)} role can’t be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
+ const errorMessage: any = `A user with ${getLocalizedName(`ROOT_${trimmedRole}`, localizationMap)} role can’t be assigned to ${getLocalizedName(userRole, localizationMap)} role`; 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 119-119: Template literals are preferred over string concatenation. Unsafe fix: Use a template literal. (lint/style/useTemplate) |
||||||
if (roleMap[trimmedRole] && roleMap[trimmedRole]["!sheet#name!"] != user["!sheet#name!"]) { | ||||||
duplicates.push({ rowNumber: user["!row#number!"], sheetName: user["!sheet#name!"], status: "INVALID", errorDetails: errorMessage }); | ||||||
} else { | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding DEBUG environment variable
According to the summary, the
prod
script's behavior depends on theDEBUG
environment variable, but it's not defined in the Dockerfile.Add the DEBUG environment variable definition: