Staging to Master

[priyanka-TL]

Summary by CodeRabbit

• New Features

  • Added a health-check API endpoint and runtime routes to expose service health.
  • Added startup environment-variable validation that halts startup with clear error messages on failure.

• Documentation

  • Updated API docs to group scheduler endpoints under "jobs" and added health-check specs with response examples.
  • Added health-check usage and configuration guidance in project docs.

• Chores

  • Added health-check tooling and supporting runtime configuration.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds a health-check feature: API docs and Postman collection include /scheduler/health; application startup now validates environment variables and initializes a health-check router; new health-check modules and config (Redis + microservice checks) plus related dependencies were added.

Changes

Cohort / File(s)	Summary
API Documentation src/api-doc/MentorED-Scheduler.postman_collection.json, src/api-doc/api-doc.yaml	Added healthCheck tag and /scheduler/health GET endpoint; reorganized Default → jobs tags; adjusted internal reference identifiers and operation metadata.
Route Configuration src/constants/interface-routes/configs.json	Appended /scheduler/health (GET) route entry with MUST_HAVE priority, no rate limiting.
Health Check Implementation src/healthCheck/index.js, src/healthCheck/health-check.js, src/healthCheck/health.config.js	New health-check initializer wiring /health and /healthCheckStatus; health-check service functions producing standardized payloads and error handling; Redis URL-based config exported.
Application Startup src/app.js, src/envVariables.js	Added environment variable validator module and invoke it early in startup; initialize health-check routes before main config load.
Documentation src/healthCheck/README.md	New README describing config shape, built-in checks, microservice check examples, env usage, and minimal valid config.
Dependencies src/package.json	Added cli-table ^0.3.11 and elevate-services-health-check ^0.0.4.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application (src/app.js)
    participant Env as Env Validator (src/envVariables.js)
    participant HealthInit as Health Init (src/healthCheck/index.js)
    participant Router as Express Router
    participant HCService as Health Service (src/healthCheck/health-check.js)
    participant Redis as Redis

    App->>Env: run validation()
    alt success
        Env-->>App: { success: true }
        App->>HealthInit: initialize(app)
        HealthInit->>Router: register /health, /healthCheckStatus
    else failure
        Env-->>App: { success: false }
        App->>App: log error and exit
    end

    Note over Router,HCService: runtime request flow
    Router->>HCService: GET /health
    HCService->>Redis: perform Redis check (via configured URL)
    alt Redis reachable
        Redis-->>HCService: OK
        HCService-->>Router: 200 { healthy: true, details... }
    else Redis failure
        Redis-->>HCService: error
        HCService-->>Router: 400/500 { healthy: false, error... }
    end
    Router-->>Client: health response

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review env validation logic and conditional requiredIf handling (src/envVariables.js)
• Validate health-check error paths and standardized response construction (src/healthCheck/health-check.js)
• Confirm route wiring and startup order in src/app.js and src/healthCheck/index.js
• Verify API doc tag changes and consistency across YAML and Postman files

"I hopped in with a jittery cheer,
Brought pings and checks to make things clear.
Redis hums, endpoints sing,
A tiny carrot for uptime's spring.
— your rabbit dev 🐇"

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Staging to Master' is vague and generic—it describes a branch merge operation rather than the specific changes being introduced (health check endpoints, environment validation, API reorganization).	Consider a more descriptive title that captures the main technical changes, such as 'Add health check endpoints and environment validation' or 'Introduce scheduler health check with API reorganization'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch staging

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (6)

src/healthCheck/health.config.js (1)

8-18: Avoid hardcoding service version in health config.

Config shape looks correct, but version: '1.0.0' will easily drift from the real service version. Prefer deriving this from package.json or an env variable so health responses always report the actual deployed version. Optionally, consider driving redis.enabled from configuration as well.

src/healthCheck/README.md (1)

1-135: Fix package name in docs and address markdown tab formatting.

• The README refers to elevate-project-services-health-check on line 3, but src/package.json declares the dependency as elevate-services-health-check. Update the README to match the actual package name.
• Hard tabs appear in code block examples (lines 23–27, 72–86, 120–127). Replace these with spaces to comply with markdownlint (MD010).

src/envVariables.js (2)

3-4: Make the validator idempotent by scoping success and tableData inside the function

Because success and tableData are module‑level variables, multiple calls to the exported function will:

• Keep success stuck in false once any run has failed, even if later runs pass.
• Keep accumulating rows in tableData, duplicating table entries on each call.

For a startup‑time check this might be acceptable today, but making the validator idempotent will avoid surprises if it’s ever reused (e.g., in a health/self‑check).

Consider:

• Initializing let success = true and let tableData = new table() inside module.exports = function () { ... }.
• Optionally treating enviromentVariables as immutable (e.g., don’t permanently flip .optional based on requiredIf, or clone before mutating) so repeated invocations see a clean schema.

Also applies to: 56-57, 98-138

---

26-30: Align DISABLE_LOG default type and messaging with downstream expectations

DISABLE_LOG has:

• Message: “Required true or false for logs”
• Default: true (boolean)

Elsewhere in the app this env var may be read as a string ('true' / 'false') from process.env, but the default here assigns a boolean. That type mismatch can cause subtle issues in strict comparisons or config loaders.

Consider:

• Making the default a string ('true' / 'false') for consistency with typical env usage, or
• Updating downstream code to treat it as a boolean consistently (and adjusting the message to reflect that).

Also applies to: 121-127

src/api-doc/api-doc.yaml (2)

450-460: operationId for /scheduler/jobs/updateDelay appears to be copy‑pasted

The post operation under /scheduler/jobs/updateDelay still uses:

operationId: /scheduler/jobs/create

which doesn’t match the path and can confuse client generation and API documentation.

Update it to reflect the correct operation:

-      operationId: /scheduler/jobs/create
+      operationId: /scheduler/jobs/updateDelay

---

618-752: Clarify /scheduler/health description and assign a meaningful operationId

For the /scheduler/health GET:

• The description mentions mandatory request parameters and “This is a mandatory parameter”, but the operation defines no parameters.
• operationId is an empty string, which is awkward for tools that rely on it.

You could simplify and correct this block as follows:

-      description: >-
-        This API is used to verify the health status of the Scheduler Service
-        and its dependencies. 
-
-        * The API Endpoint for health check is /scheduler/health
-
-        * It is mandatory to provide values for parameters that are marked as
-        `required`
-
-        * This is a mandatory parameter and cannot be empty or null
-      operationId: ''
+      description: >-
+        This API is used to verify the health status of the Scheduler Service
+        and its dependencies. It performs a basic self-check of the scheduler
+        and downstream services such as Redis.
+
+        * The API endpoint for health check is `/scheduler/health`.
+        * This endpoint does not accept any request parameters.
+      operationId: /scheduler/health

This keeps the intent while removing references to non‑existent required parameters and making the operationId usable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 918abf0 and 1162a2d.

📒 Files selected for processing (10)

• src/api-doc/MentorED-Scheduler.postman_collection.json (1 hunks)
• src/api-doc/api-doc.yaml (9 hunks)
• src/app.js (1 hunks)
• src/constants/interface-routes/configs.json (1 hunks)
• src/envVariables.js (1 hunks)
• src/healthCheck/README.md (1 hunks)
• src/healthCheck/health-check.js (1 hunks)
• src/healthCheck/health.config.js (1 hunks)
• src/healthCheck/index.js (1 hunks)
• src/package.json (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (5)

src/healthCheck/index.js (2)

src/app.js (2)

• require (7-7)
• app (24-24)

src/healthCheck/health-check.js (1)

• require (9-9)

src/app.js (1)

src/healthCheck/health-check.js (1)

• require (9-9)

src/envVariables.js (1)

src/configs/bull.js (1)

• process (4-4)

src/healthCheck/health-check.js (1)

src/app.js (1)

• require (7-7)

src/healthCheck/health.config.js (1)

src/configs/bull.js (1)

• process (4-4)

🪛 markdownlint-cli2 (0.18.1)

src/healthCheck/README.md

23-23: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

24-24: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

25-25: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

26-26: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

27-27: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

72-72: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

73-73: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

74-74: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

75-75: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

76-76: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

77-77: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

78-78: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

79-79: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

80-80: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

81-81: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

82-82: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

83-83: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

84-84: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

85-85: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

86-86: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

120-120: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

121-121: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

122-122: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

123-123: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

124-124: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

125-125: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

126-126: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

127-127: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🔇 Additional comments (1)

src/package.json (1)

14-32: Verify cli-table and elevate-services-health-check package legitimacy and security status.

Both packages exist on npm at the specified versions (0.3.11 and 0.0.4 respectively). However, confirm that elevate-services-health-check is the correct scoped/internal package for this project and check both packages for known vulnerabilities before merging.

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

src/envVariables.js (2)

5-54: Typo in variable name and minor formatting issue.

• Line 5: enviromentVariables should be environmentVariables (missing 'n').
• Line 39: Missing space after colon in default:'notificationtopic'.

Apply this diff to fix both issues:

-let enviromentVariables = {
+let environmentVariables = {
 	API_DOC_URL: {
 		message: 'Required api doc url',
 		optional: true,
 		default: '/scheduler/api-doc',
 	},
 	APPLICATION_BASE_URL: {
 		message: 'Required application base url',
 		optional: true,
 		default: '/scheduler/',
 	},
 	APPLICATION_PORT: {
 		message: 'Required application port number',
 		optional: true,
 		default: '4000'
 	},
 	DEFAULT_QUEUE: {
 		message: 'Required queue name',
 		optional: true,
 		default: 'emailqueue',
 	},
 	DISABLE_LOG: {
 		message: 'Required true or false for logs',
 		optional: true,
 		default: true,
 	},
 	ERROR_LOG_LEVEL: {
 		message: 'Required error log level',
 		optional: true,
 		default: 'silly'
 	},
 	NOTIFICATION_KAFKA_TOPIC: {
 		message: 'Required notification topic',
 		optional: true,
-		default:'notificationtopic'
+		default: 'notificationtopic'
 	},
 	KAFKA_URL: {
 		message: 'Required kafka url',
 		optional: false,
 	},
 	REDIS_HOST: {
 		message: 'Required redis host',
 		optional: false,
 	},
 	REDIS_PORT: {
 		message: 'Required redis port number',
 		optional: false,
 	},
 
 }

Note: You'll also need to update all references to enviromentVariables throughout the file.

---

129-135: Add undefined/null check for message.

Line 130 checks if message !== '' but doesn't verify that message is defined. If the message property is undefined or null, the check would pass but line 131 would fail.

Apply this diff:

 		if (!keyCheckPass) {
-			if (enviromentVariables[eachEnvironmentVariable].message !== '') {
+			if (enviromentVariables[eachEnvironmentVariable].message && enviromentVariables[eachEnvironmentVariable].message !== '') {
 				tableObj[eachEnvironmentVariable] = enviromentVariables[eachEnvironmentVariable].message
 			} else {
 				tableObj[eachEnvironmentVariable] = `FAILED - ${eachEnvironmentVariable} is required`
 			}
 		}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1162a2d and 2853742.

📒 Files selected for processing (2)

• src/app.js (1 hunks)
• src/envVariables.js (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/app.js

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: priyanka-TL
Repo: ELEVATE-Project/scheduler PR: 61
File: src/healthCheck/index.js:8-27
Timestamp: 2025-12-03T11:02:00.466Z
Learning: In the ELEVATE-Project/scheduler repository, health check routes in src/healthCheck/index.js are defined without the /scheduler prefix (e.g., /health) because the prefix is added at a different layer such as app mounting, reverse proxy, or API gateway configuration.

🔇 Additional comments (1)

src/envVariables.js (1)

1-3: Previous critical issue resolved.

The validRequiredIfOperators declaration fixes the ReferenceError that was flagged in the previous review.