-
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 12 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 |
---|---|---|
|
@@ -4,13 +4,12 @@ import FormData from "form-data"; // Import FormData for handling multipart/form | |
import { defaultheader, httpRequest } from "../utils/request"; // Import httpRequest function for making HTTP requests | ||
import { getFormattedStringForDebug, logger } from "../utils/logger"; // Import logger for logging | ||
import { correctParentValues, findMapValue, generateActivityMessage, getBoundaryRelationshipData, getDataSheetReady, getLocalizedHeaders, sortCampaignDetails, throwError } from "../utils/genericUtils"; // Import utility functions | ||
import { extractCodesFromBoundaryRelationshipResponse, generateFilteredBoundaryData, getConfigurableColumnHeadersBasedOnCampaignType, getFiltersFromCampaignSearchResponse, getLocalizedName } from '../utils/campaignUtils'; // Import utility functions | ||
import { extractCodesFromBoundaryRelationshipResponse, generateFilteredBoundaryData, getConfigurableColumnHeadersBasedOnCampaignType, getFiltersFromCampaignSearchResponse, getLocalizedName, processDataForTargetCalculation } from '../utils/campaignUtils'; // Import utility functions | ||
import { getCampaignSearchResponse, getHierarchy } from './campaignApis'; | ||
const _ = require('lodash'); // Import lodash library | ||
import { getExcelWorkbookFromFileURL } from "../utils/excelUtils"; | ||
import { processMapping } from "../utils/campaignMappingUtils"; | ||
|
||
|
||
//Function to get Workbook with different tabs (for type target) | ||
const getTargetWorkbook = async (fileUrl: string, localizationMap?: any) => { | ||
// Define headers for HTTP request | ||
|
@@ -209,6 +208,7 @@ const getTargetSheetDataAfterCode = async ( | |
for (const sheetName of localizedSheetNames) { | ||
const worksheet = workbook.getWorksheet(sheetName); | ||
const sheetData = getSheetDataFromWorksheet(worksheet); | ||
const jsonData = getJsonData(sheetData,true,true, sheetName); | ||
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 Add consistent error handling. When the + if (!jsonData || jsonData.length === 0) {
+ console.warn("No valid data found in getJsonData.");
+ continue;
+ }
|
||
|
||
// Find the target column index where the first row value matches codeColumnName | ||
const firstRow = sheetData[0]; | ||
|
@@ -224,44 +224,7 @@ const getTargetSheetDataAfterCode = async ( | |
console.warn(`Column "${codeColumnName}" not found in sheet "${sheetName}".`); | ||
continue; | ||
} | ||
|
||
// Process data from sheet | ||
const processedData = sheetData.map((row: any, rowIndex: any) => { | ||
if (rowIndex <= 0) return null; // Skip header row | ||
|
||
let rowData: any = { [codeColumnName]: row[boundaryCodeColumnIndex] }; | ||
|
||
// Add integer values in the target column for the current row | ||
let sumOfCurrentTargets = 0; | ||
let sumOfParentTargets = 0; | ||
const remainingColumns = row.length - boundaryCodeColumnIndex - 1; | ||
const halfPoint = Math.floor(remainingColumns / 2); | ||
let startColIndex = boundaryCodeColumnIndex + 1; | ||
|
||
if (request?.body?.parentCampaign) { | ||
for (let colIndex = startColIndex; colIndex < startColIndex + halfPoint; colIndex++) { | ||
const value = row[colIndex]; | ||
if (typeof value === 'number' && Number.isInteger(value)) { | ||
sumOfParentTargets += value; | ||
} | ||
} | ||
// Add the sum to the row data | ||
rowData['Parent Target at the Selected Boundary level'] = sumOfParentTargets; | ||
|
||
// Calculate middle point of remaining columns | ||
startColIndex = boundaryCodeColumnIndex + 1 + halfPoint; | ||
} | ||
for (let colIndex = startColIndex; colIndex < row.length; colIndex++) { | ||
const value = row[colIndex]; | ||
if (typeof value === 'number' && Number.isInteger(value)) { | ||
sumOfCurrentTargets += value; | ||
} | ||
} | ||
|
||
// Add the sum to the row data | ||
rowData['Target at the Selected Boundary level'] = sumOfCurrentTargets; | ||
return rowData; | ||
}).filter(Boolean); // Remove null entries | ||
const processedData = await processDataForTargetCalculation(request, jsonData, codeColumnName, 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) Computing target data via utility function. Delegating to |
||
|
||
workbookData[sheetName] = processedData; | ||
} | ||
|
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 });
|
||
} | ||
}; | ||
} | ||
|
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: