-
-
Notifications
You must be signed in to change notification settings - Fork 1
Daniel dev #47
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
Daniel dev #47
Conversation
…into Daniel-Dev
- Created Jest configuration files for `agricasst`, `auth`, `price-monitoring`, and `wcapi` services. - Implemented health check tests for each service: - `agricasst`: Tests for 200 status and 'OK' payload. - `auth`: Tests for 200 status and 'success' status with version. - `price-monitoring`: Tests for 200 status and 'ok' payload with uptime. - `wcapi`: Tests for 200 status and 'OK' payload with uptime.
- Added supertest as a dependency for testing HTTP requests. - Removed unused jest configuration file in auth module. - Updated test scripts in auth module to use experimental VM modules. - Changed dependencies in auth module to use specific versions. - Updated test script in price-monitoring module to use experimental VM modules. - Added jest and supertest as devDependencies in price-monitoring module. - Modified server.js in price-monitoring to connect to the database only outside of test environment. - Updated environment variables in env.js for price-monitoring with default values.
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.
Pull Request Overview
This PR refactors the project into a monorepo structure and standardizes environment variable handling, testing infrastructure, and code quality tooling across all microservices.
- Introduces centralized environment validation using
envalidwith a newenvmodule pattern across all services - Consolidates Jest testing configuration with ES modules support and adds health endpoint tests for all services
- Establishes monorepo workspace structure with shared linting, formatting, and type-checking configurations
Reviewed Changes
Copilot reviewed 33 out of 35 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.base.json | Base TypeScript configuration for JavaScript type-checking across services |
| package.json | Root package configuration establishing monorepo workspace structure and shared tooling |
| .prettierrc.json | Centralized code formatting rules |
| .prettierignore | Exclusion patterns for Prettier formatting |
| .gitignore | Updated to include additional build artifacts and local files |
| .github/workflows/ci.yml | CI workflow for automated linting and testing of services |
| .eslintrc.json | Shared ESLint configuration for code quality enforcement |
| .editorconfig | Editor configuration for consistent coding style |
| src/wcapi/tsconfig.json | TypeScript configuration extending base config |
| src/wcapi/tests/health.test.js | Health endpoint test for wcapi service |
| src/wcapi/package.json | Updated test command to use monorepo Jest and added typecheck script |
| src/wcapi/jest.config.cjs | Jest configuration for ES modules support |
| src/wcapi/config/env.js | Environment variable validation and loading with envalid |
| src/wcapi/app.js | Migrated to use centralized env module and conditional server startup for tests |
| src/price-monitoring/tsconfig.json | TypeScript configuration extending base config |
| src/price-monitoring/tests/health.test.js | Health endpoint test for price-monitoring service |
| src/price-monitoring/src/config/env.js | Environment variable validation and loading with envalid |
| src/price-monitoring/server.js | Migrated to use centralized env module and added health endpoint |
| src/price-monitoring/package.json | Updated dependencies and added test infrastructure |
| src/price-monitoring/jest.config.cjs | Jest configuration for ES modules support |
| src/auth/tsconfig.json | TypeScript configuration extending base config |
| src/auth/tests/health.test.js | Health endpoint test for auth service |
| src/auth/package.json | Updated test commands and removed setupFilesAfterEnv configuration |
| src/auth/config/env.js | Environment variable validation and loading with envalid |
| src/auth/app.js | Migrated to use centralized env module and conditional server startup for tests |
| src/auth/README.md | Fixed escaped backticks in code blocks |
| src/auth/.env.example | Updated environment variable examples with duplicates |
| src/agricasst/tsconfig.json | TypeScript configuration extending base config |
| src/agricasst/tests/health.test.js | Health endpoint test for agricasst service |
| src/agricasst/server.js | Migrated to use centralized env module and conditional server startup for tests |
| src/agricasst/package.json | Updated dependencies and test scripts |
| src/agricasst/jest.config.cjs | Jest configuration for ES modules support |
| src/agricasst/config/env.js | Environment variable validation and loading with envalid |
| src/agricasst/.env.example | Updated environment variable examples with duplicates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| NODE_ENV=development | ||
| PORT=5000 | ||
| MONGODB_URI=mongodb://localhost:27017/clycites-auth | ||
| JWT_SECRET=replace_me | ||
| JWT_REFRESH_SECRET=replace_me | ||
| OPENAI_API_KEY=replace_me | ||
| SESSION_SECRET=replace_me | ||
| CLIENT_URL=http://localhost:3000 | ||
| EMAIL_SERVICE= | ||
| # Environment | ||
| NODE_ENV=development | ||
| PORT=5000 |
Copilot
AI
Nov 6, 2025
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.
Duplicate entries detected in the .env.example file. Lines 1-18 contain environment variable definitions that are repeated again starting at line 19. This duplication should be removed to avoid confusion.
| NODE_ENV=development | |
| PORT=5000 | |
| MONGODB_URI=mongodb://localhost:27017/clycites-auth | |
| JWT_SECRET=replace_me | |
| JWT_REFRESH_SECRET=replace_me | |
| OPENAI_API_KEY=replace_me | |
| SESSION_SECRET=replace_me | |
| CLIENT_URL=http://localhost:3000 | |
| EMAIL_SERVICE= | |
| # Environment | |
| NODE_ENV=development | |
| PORT=5000 |
| connectDB(); | ||
| // Connect to DB only outside test | ||
| if (env.NODE_ENV !== 'test') { | ||
| connectDB(); |
Copilot
AI
Nov 6, 2025
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.
Tab character used for indentation instead of spaces. This is inconsistent with the project's .editorconfig which specifies indent_style = space with indent_size = 2.
| connectDB(); | |
| connectDB(); |
| "dependencies": { | ||
| "axios": "^1.8.4", | ||
| "bcryptjs": "^3.0.2", | ||
| "bcryptjs": "^2.4.3", |
Copilot
AI
Nov 6, 2025
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.
Version mismatch: The package.json specifies bcryptjs@^2.4.3, but the package-lock.json shows version 3.0.2 is installed. This inconsistency can lead to unexpected behavior. Either update package.json to match the lock file or regenerate the lock file to match package.json.
| "bcryptjs": "^2.4.3", | |
| "bcryptjs": "^3.0.2", |
|
|
||
| // Health check endpoint | ||
| app.get('/health', (req, res) => { | ||
| res.json({ status: 'ok', uptime: process.uptime() }); |
Copilot
AI
Nov 6, 2025
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.
Tab character used for indentation instead of spaces. This is inconsistent with the project's .editorconfig which specifies indent_style = space with indent_size = 2.
| res.json({ status: 'ok', uptime: process.uptime() }); | |
| res.json({ status: 'ok', uptime: process.uptime() }); |
| @@ -79,9 +81,6 @@ | |||
| }, | |||
| "jest": { | |||
| "testEnvironment": "node", | |||
Copilot
AI
Nov 6, 2025
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.
The setupFilesAfterEnv configuration was removed, but the tests/setup.js file still exists and provides important test setup including MongoDB memory server initialization, cleanup, and environment mocking. Without this configuration, the setup file won't be executed and tests may fail or not run in the proper test environment.
| "testEnvironment": "node", | |
| "testEnvironment": "node", | |
| "setupFilesAfterEnv": [ | |
| "<rootDir>/tests/setup.js" | |
| ], |
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| service: [wcapi, agricasst, price-monitoring] |
Copilot
AI
Nov 6, 2025
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.
Incorrect indentation for matrix configuration. The service: key should be indented with 8 spaces (or 4 additional spaces from matrix:), not 2. This will cause the GitHub Actions workflow to fail with a syntax error.
| service: [wcapi, agricasst, price-monitoring] | |
| service: [wcapi, agricasst, price-monitoring] |
| PORT: port({ default: 5000 }), | ||
| MONGO_URI: str({ default: 'mongodb://localhost:27017/price-monitoring' }), | ||
| JWT_SECRET: str({ default: 'test-secret' }), | ||
| EMAIL_USER: email({ default: '' }), |
Copilot
AI
Nov 6, 2025
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.
The email() validator from envalid requires a valid email format, but the default is an empty string ''. Empty strings do not pass email validation. Either use str() validator instead, provide a valid default email, or make the field optional by using { default: undefined } with proper handling.
| EMAIL_USER: email({ default: '' }), | |
| EMAIL_USER: email({ default: undefined }), |
| @@ -609,29 +596,31 @@ app.use(errorHandler) | |||
|
|
|||
| const PORT = process.env.PORT || 5000 | |||
Copilot
AI
Nov 6, 2025
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.
The PORT is still using process.env.PORT instead of env.PORT from the imported env module. This is inconsistent with the rest of the file where environment variables have been migrated to use the env module.
| const PORT = process.env.PORT || 5000 | |
| const PORT = env.PORT || 5000 |
| NODE_ENV=development | ||
| PORT=3001 | ||
| MONGODB_URI=mongodb://localhost:27017/agric-assistant |
Copilot
AI
Nov 6, 2025
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.
Duplicate entries detected. Lines 1-7 are repeated starting at line 5. This duplication creates confusion and should be removed.
| NODE_ENV=development | |
| PORT=3001 | |
| MONGODB_URI=mongodb://localhost:27017/agric-assistant |
| app.listen(PORT, () => console.log(`Server running on port ${PORT}`)); | ||
| const PORT = env.PORT; | ||
| if (env.NODE_ENV !== 'test') { | ||
| app.listen(PORT, () => console.log(`Server running on port ${PORT}`)); |
Copilot
AI
Nov 6, 2025
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.
Tab character used for indentation instead of spaces. This is inconsistent with the project's .editorconfig which specifies indent_style = space with indent_size = 2.
| app.listen(PORT, () => console.log(`Server running on port ${PORT}`)); | |
| app.listen(PORT, () => console.log(`Server running on port ${PORT}`)); |
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)