Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

console recent changes to merged in master #1274

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
355a8ab
Enabled commands for debug remotely for project factory pod (#1249)
ashish-egov Dec 9, 2024
2bdd736
changed package json for debug (#1258)
ashish-egov Dec 9, 2024
016dc01
boundary code in update flow to be persisted in db (#1259)
nitish-egov Dec 10, 2024
d96bb2b
chnaged header creation logic in generate flow (#1261)
nitish-egov Dec 10, 2024
19283e0
made few changes for fetching boundaries from boundariesCombined (#1263)
nitish-egov Dec 10, 2024
74700da
some boundary bulk and microplan user changes (#1268)
ashish-egov Dec 11, 2024
9dee69b
localisation-cache-fix (#1270)
ashish-egov Dec 11, 2024
3ed33e6
Microplan fix (#1269)
ashish-egov Dec 12, 2024
49b5801
Boundary fix message (#1279)
ashish-egov Dec 12, 2024
b373ac4
Facility Fix (#1282)
ashish-egov Dec 17, 2024
b69ba14
added gzip handler in middleware (#1290)
nitish-egov Dec 19, 2024
ad9a5b8
refactor target generate and validate for generic type (#1296)
nitish-egov Dec 30, 2024
f13f4cf
Facility Singular Creation (#1302)
ashish-egov Dec 31, 2024
9df4256
BUGFIX campaignUtils.ts for update target fix (#1304)
ashish-egov Jan 2, 2025
d832441
Allow facility capacity to be 0 in sheet uploads for microplan (#1303)
ashish-egov Jan 2, 2025
fc6122e
Implemented functionality to include all boundaries present in the fa…
ashish-egov Jan 2, 2025
bfccb0a
Optimized latitude and longitude validation logic for improved effici…
ashish-egov Jan 3, 2025
5420d8e
Implemented locale and campaign check (#1300)
ashish-egov Jan 3, 2025
bd673e7
refactored target tabs validate logic (#1330)
nitish-egov Jan 7, 2025
97b4b32
Update campaignUtils.ts (#1332)
ashish-egov Jan 8, 2025
b5be48e
added config for retry for resource creation completion (#1333)
nitish-egov Jan 9, 2025
396673c
refactored code for header localization (#1338)
nitish-egov Jan 13, 2025
3fc3a41
feat: Implement facility list with inactive by default and toggle act…
ashish-egov Jan 22, 2025
343393e
Console dropdown logics (#1359)
nitish-egov Jan 28, 2025
d94b380
added locale column in generated resource table for generating templa…
nitish-egov Jan 29, 2025
b5aee12
Created User issue fixed (#1370)
ashish-egov Jan 30, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions health-services/project-factory/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,4 @@ COPY . .
EXPOSE 3000

CMD ["yarn", "prod"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding DEBUG environment variable

According to the summary, the prod script's behavior depends on the DEBUG environment variable, but it's not defined in the Dockerfile.

Add the DEBUG environment variable definition:

 EXPOSE 3000
+
+# Set DEBUG environment variable (default to false for production)
+ENV DEBUG=false
 
 CMD ["yarn", "prod"]

Committable suggestion skipped: line range outside the PR's diff.

# Replaced by CMD ["yarn", "prod"]


# Replaced by CMD ["yarn", "prod"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Remove 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"]

4 changes: 2 additions & 2 deletions health-services/project-factory/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"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",

"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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

⚠️ Potential issue

Reconsider debug configuration in production

The current implementation has several concerns:

  1. Copying debug configuration in production is unusual and could lead to unexpected behavior
  2. No validation if tsconfig.debug.json exists
  3. No cleanup of temporary configuration changes
  4. Could affect build artifacts

Consider:

  1. Using separate npm scripts for debug and production builds
  2. Implementing proper error handling for missing configuration files
  3. Cleaning up temporary configurations after use

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"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'",

"watch-ts": "tsc --watch"
},
"repository": {
Expand Down
13 changes: 0 additions & 13 deletions health-services/project-factory/src/server/api/campaignApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1344,7 +1344,6 @@ async function performAndSaveResourceActivity(
createAndSearchConfig?.createBulkDetails?.createPath,
chunkData
);
creationTime = Date.now();
if (type == "facility" || type == "facilityMicroplan") {
await handeFacilityProcess(
request,
Expand Down Expand Up @@ -1849,21 +1848,9 @@ async function projectCreate(projectCreateBody: any, request: any) {
logger.info(
`for boundary type ${projectCreateResponse?.Project[0]?.address?.boundaryType} and code ${projectCreateResponse?.Project[0]?.address?.boundary}`
);
// if (
// !request.body.newlyCreatedBoundaryProjectMap[
// projectCreateBody?.Projects?.[0]?.address?.boundary
// ]
// ) {
// request.body.newlyCreatedBoundaryProjectMap[
// projectCreateBody?.Projects?.[0]?.address?.boundary
// ] = {};
// }
request.body.boundaryProjectMapping[
projectCreateBody?.Projects?.[0]?.address?.boundary
].projectId = projectCreateResponse?.Project[0]?.id;
// request.body.newlyCreatedBoundaryProjectMap[
// projectCreateBody?.Projects?.[0]?.address?.boundary
// ].projectId = projectCreateResponse?.Project[0]?.id;
} else {
throwError(
"PROJECT",
Expand Down
43 changes: 3 additions & 40 deletions health-services/project-factory/src/server/api/genericApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add consistent error handling.

When the getJsonData call fails or returns invalid data, ensure the code handles it gracefully instead of proceeding.

+ if (!jsonData || jsonData.length === 0) {
+   console.warn("No valid data found in getJsonData.");
+   continue;
+ }

Committable suggestion skipped: line range outside the PR's diff.


// Find the target column index where the first row value matches codeColumnName
const firstRow = sheetData[0];
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Computing target data via utility function.

Delegating to processDataForTargetCalculation centralizes logic for target calculations. Ensure the function handles edge cases (e.g., empty data).


workbookData[sheetName] = processedData;
}
Expand Down
2 changes: 1 addition & 1 deletion health-services/project-factory/src/server/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class App {
extended: true,
})
);
this.app.use(bodyParser.json());
// this.app.use(bodyParser.json());
this.app.use(tracingMiddleware);
this.app.use(requestMiddleware);
this.app.use(errorLogger);
Expand Down
5 changes: 5 additions & 0 deletions health-services/project-factory/src/server/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Make the facility code & type configurable.

Both facilityCodeColumn and facilityType are hardcoded. If there’s a need to adapt or extend these values in the future, consider sourcing them from environment variables instead of hardcoding. This ensures flexibility and easier maintenance.

},
user: {
userTab: process.env.USER_TAB_NAME || "HCM_ADMIN_CONSOLE_USER_LIST",
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use Number.parseInt instead of global parseInt

For better consistency and to adhere to modern JavaScript standards, use Number.parseInt instead of the global parseInt function.

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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"),
🧰 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.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)


[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

},
// targetColumnsForSpecificCampaigns: {
// bedNetCampaignColumns: ["HCM_ADMIN_CONSOLE_TARGET"],
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Rethrow the original error to preserve stack trace

Using throw new Error(e); replaces the original error's stack trace, making debugging difficult. Instead, rethrow the original error to maintain the stack trace and error message.

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 });

Committable suggestion skipped: line range outside the PR's diff.

}
};
}
Expand Down
Loading
Loading