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.

Sign up for GitHub

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
+842 −750
Open

console recent changes to merged in master #1274

Show file tree
Hide file tree
Changes from 25 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
View file
Open in desktop
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"]

2 changes: 2 additions & 0 deletions ...h-services/project-factory/migration/main/V202422115500__add_locale_generated_resouce.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE eg_cm_generated_resource_details
ADD COLUMN locale VARCHAR(50);
4 changes: 2 additions & 2 deletions health-services/project-factory/package.json
View file
Open in desktop
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
Loading
Loading