Setup Script #2952
Setup Script #2952
Conversation
WalkthroughThe changes introduce a comprehensive, interactive environment setup process. Updates include modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SetupTS as setup.ts
participant SetupModule as scripts/setup/setup.ts
participant FS as File System
participant Inquirer
User->>SetupTS: Execute "npm run setup"
SetupTS->>SetupModule: Call setup() function
SetupModule->>Inquirer: Prompt for configuration inputs
Inquirer-->>SetupModule: Return user responses
SetupModule->>FS: Check for .env file, create backup, update variables
SetupModule-->>SetupTS: Return setup status
SetupTS->>User: Log success or error messages
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
package.json(3 hunks)setup.ts(1 hunks)src/setup/updateEnvVariable.ts(1 hunks)test/setup/administratorEmail.test.ts(1 hunks)test/setup/apiSetup.test.ts(1 hunks)test/setup/cloudbeaverSetup.test.ts(1 hunks)test/setup/minioSetup.test.ts(1 hunks)test/setup/postgresSetup.test.ts(1 hunks)test/setup/setNodeEnviorment.test.ts(1 hunks)vitest.config.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (7)
setup.ts (2)
18-30: Confirm deletion behavior before removing.envif no backup exists.When
originalEnvContentisnullbut.envexists,restoreEnvFiledeletes.env. It might be safer to prompt the user for confirmation, especially if they intend to preserve a manually created.env. This helps prevent accidental data destruction.
64-85: 🧹 Nitpick (assertive)Consider validating email input.
The
administratorEmailfunction simply stores the input intoAPI_ADMINISTRATOR_USER_EMAIL_ADDRESS. If a user enters invalid data, application behavior might break. Add basic validation (e.g., a regex check or additional prompt feedback) to ensure valid email formatting.test/setup/administratorEmail.test.ts (1)
14-22: 🧹 Nitpick (assertive)Test invalid inputs and error flows.
The test suite validates normal use (i.e., a valid email). Consider adding edge cases, such as empty strings or invalid emails, to ensure
administratorEmailgracefully handles unexpected input.test/setup/minioSetup.test.ts (1)
17-23: Verify the security implications of the MINIO_BROWSER setting.The test uses "off" for MINIO_BROWSER. Ensure this aligns with security best practices for your deployment environment.
✅ Verification successful
MinIO browser configuration is properly implemented
The MINIO_BROWSER="off" setting in tests is appropriate and the overall implementation follows security best practices:
- Configuration is documented and follows MinIO's official guidelines
- Production environment prompts for the setting with clear documentation
- Test configuration demonstrates secure usage without affecting production
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check MinIO documentation for recommended MINIO_BROWSER settings curl -s https://min.io/docs/minio/linux/reference/minio-server/minio-server.html | grep -A 5 "MINIO_BROWSER"Length of output: 108
Script:
#!/bin/bash # Search for MINIO_BROWSER configurations and related documentation echo "=== Searching for MINIO_BROWSER usage ===" rg "MINIO_BROWSER" -B 2 -A 2 echo -e "\n=== Checking for MinIO configuration files ===" fd -g "*minio*.{ts,js,json,md}" echo -e "\n=== Looking for setup documentation ===" fd -g "README.md" --exec grep -l -i "minio" {} \;Length of output: 3036
test/setup/setNodeEnviorment.test.ts (2)
49-64: LGTM! Comprehensive error handling test.The error handling test case is well-implemented with proper spy cleanup and error verification.
66-73: LGTM! Good coverage of unchanged environment case.The test case properly verifies that NODE_ENV remains unchanged when not in test environment.
package.json (1)
95-96: LGTM! Setup script added correctly.The setup script is properly configured to use tsx for TypeScript execution.
|
will fix failing tests and implement suggestions |
|
will implement both |
I am planning to go with this approach, is this alright? |
|
@xoldd PTAL at this PR. It has implications for API setup using a script |
|
@gurramkarthiknetha is working on it, we talked on slack, he has started work on this |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
package.json (1)
18-18:⚠️ Potential issueFix invalid package versions.
The specified versions are invalid and could cause installation failures:
[email protected]: Latest version is 16.4.1[email protected]: Latest version is 9.2.15Apply this fix:
- "dotenv": "^16.4.7", + "dotenv": "^16.4.1", - "inquirer": "^12.3.2", + "inquirer": "^9.2.15",Also applies to: 27-27
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
package.json(3 hunks)setup.ts(1 hunks)src/setup/updateEnvVariable.ts(1 hunks)test/setup/administratorEmail.test.ts(1 hunks)test/setup/apiSetup.test.ts(1 hunks)test/setup/cloudbeaverSetup.test.ts(1 hunks)test/setup/minioSetup.test.ts(1 hunks)test/setup/postgresSetup.test.ts(1 hunks)test/setup/setNodeEnviorment.test.ts(1 hunks)vitest.config.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Pull request workflow
vitest.config.ts
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
package.json
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
setup.ts
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
🔇 Additional comments (9)
setup.ts (4)
10-16: Clarify backup method behavior.While
backupEnvFilecorrectly stores the original content of the.envfile, you might consider logging or returning an explicit indicator to confirm whether a backup is performed or skipped (e.g., if the file doesn’t exist). This ensures downstream logic can be more transparent and conditional.🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
32-44: Rename or document overwriting behavior incheckEnvFile.Although it’s called
checkEnvFile, the function overwrites the.envfile with contents fromenvFiles/.env.devcontainer. Consider either renaming it (e.g.,initializeEnvFile) or adding inline comments noting that it destructively replaces the existing.env.🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
381-382: Review.envoverwrite sequence.
checkEnvFile()is invoked immediately beforebackupEnvFile(), meaning the script first overwrites.envfrom the devcontainer file, then backs it up. If your intent is to preserve the user’s existing.env, consider reversing the call order or prompting for confirmation before overwriting.🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
128-163: Ensure secret values are safely stored.You’re storing credentials (e.g.,
API_JWT_SECRET, Minio secret key) directly in the.envfile. While this is common in local development, consider using a secrets manager (e.g., Vault) or encryption at rest for production to reduce credential exposure risk.🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
vitest.config.ts (1)
5-9: Be cautious with ignoring configuration errors.Using
ignoreConfigErrors: truecan hide potential misconfigurations in TypeScript paths. Address any underlying config issues if possible, rather than skipping them. Also, note that the pipeline has flagged unauthorized changes to this file. If this file is deemed sensitive, you may need to add theignore-sensitive-files-prlabel or revert these changes to pass the checks.🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
test/setup/minioSetup.test.ts (1)
15-37: Add validation tests for MinIO configuration.The test suite should include validation for:
- IP address format validation
- Port conflict detection between API and Console ports
- Root credential strength requirements
Example test case:
it("should detect port conflicts", async () => { const mockedAnswers = { ...mockedAnswers, MINIO_API_MAPPED_PORT: "8000", MINIO_CONSOLE_MAPPED_PORT: "8000", // Same port as API }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(mockedAnswers); await expect(minioSetup()).rejects.toThrow("Port conflict detected"); });test/setup/setNodeEnviorment.test.ts (2)
1-74: Fix typo in filename: "setNodeEnviorment" should be "setNodeEnvironment".The test file has a typo in its name that should be corrected to maintain consistency.
Run these commands to fix the filename:
#!/bin/bash # Rename the file git mv test/setup/setNodeEnviorment.test.ts test/setup/setNodeEnvironment.test.ts # Update imports in other files rg -l "setNodeEnviorment" | xargs sed -i 's/setNodeEnviorment/setNodeEnvironment/g'
1-74: Test implementation looks good!The test suite is well-structured with comprehensive coverage of:
- Happy path
- Default values
- Error handling
- Environment-specific behavior
package.json (1)
95-96: Address pipeline failure for sensitive file changes.The pipeline indicates unauthorized changes in sensitive files. The PR requires the 'ignore-sensitive-files-pr' label to proceed.
Run this script to identify sensitive file patterns:
✅ Verification successful
Development configuration scripts are legitimate and follow security best practices
The new scripts are standard development utilities:
setup: An interactive configuration tool with proper environment variable managementupgrade_drizzle_metadata: A standard database schema migration utilityBoth follow security best practices including environment backup/restore functionality and interactive configuration of sensitive data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for sensitive file patterns rg -l "password|secret|credential|key" .Length of output: 11322
Script:
#!/bin/bash # Check setup.ts and related setup files echo "=== setup.ts ===" cat setup.ts 2>/dev/null || echo "File not found" echo -e "\n=== src/setup/updateEnvVariable.ts ===" cat src/setup/updateEnvVariable.ts 2>/dev/null || echo "File not found" echo -e "\n=== Looking for drizzle setup files ===" fd "drizzle.*\.(ts|js|json)" --type f --exec head -n 20 {}Length of output: 13262
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] Unauthorized changes detected in sensitive file. Requires 'ignore-sensitive-files-pr' label to proceed.
|
sir this isn't ready as of moment, I have implemented you requirements, just fixing code_quality issues, then I can look at suggestions all together |
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2952 +/- ##
====================================================
+ Coverage 52.79% 53.55% +0.75%
====================================================
Files 469 472 +3
Lines 35285 35923 +638
Branches 1222 1311 +89
====================================================
+ Hits 18629 19238 +609
- Misses 16654 16680 +26
- Partials 2 5 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (3)
test/setup/cloudbeaverSetup.test.ts (1)
15-45: 🛠️ Refactor suggestionAdd test cases for validation and error scenarios.
The current test only covers the happy path. For security-critical configuration like CloudBeaver, we need additional test cases.
Add these test cases:
it("should validate admin password strength", async () => { const weakPasswordAnswers = { CLOUDBEAVER_ADMIN_PASSWORD: "weak" }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(weakPasswordAnswers); await expect(cloudbeaverSetup()).rejects.toThrow( "Password must be at least 8 characters" ); }); it("should validate server URL format", async () => { const invalidUrlAnswers = { CLOUDBEAVER_SERVER_URL: "invalid-url" }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(invalidUrlAnswers); await expect(cloudbeaverSetup()).rejects.toThrow( "Invalid server URL format" ); }); it("should validate port range", async () => { const invalidPortAnswers = { CLOUDBEAVER_MAPPED_PORT: "999999" }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(invalidPortAnswers); await expect(cloudbeaverSetup()).rejects.toThrow( "Port must be between 1 and 65535" ); });test/setup/apiSetup.test.ts (1)
15-77: 🛠️ Refactor suggestionAdd security validation tests for critical configurations.
While the test covers all environment variables, it lacks validation for security-critical fields like JWT secrets and database credentials.
Add these security validation tests:
it("should validate JWT secret strength", async () => { const weakJwtAnswers = { API_JWT_SECRET: "weak" }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(weakJwtAnswers); await expect(apiSetup()).rejects.toThrow("JWT secret too weak"); }); it("should validate database credentials", async () => { const weakDbAnswers = { API_POSTGRES_PASSWORD: "weak" }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(weakDbAnswers); await expect(apiSetup()).rejects.toThrow("Database password too weak"); }); it("should validate URL formats", async () => { const invalidUrlAnswers = { API_BASE_URL: "invalid-url" }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(invalidUrlAnswers); await expect(apiSetup()).rejects.toThrow("Invalid URL format"); });package.json (1)
18-18:⚠️ Potential issueFix invalid package versions.
The specified versions appear to be invalid:
[email protected]: Latest version is 16.4.1[email protected]: Latest version is 9.2.15Apply this diff to fix the versions:
- "dotenv": "^16.4.7", + "dotenv": "^16.4.1", - "inquirer": "^12.3.2", + "inquirer": "^9.2.15",Also applies to: 27-27
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
package.json(4 hunks)setup.ts(1 hunks)src/setup.ts(1 hunks)src/setup/updateEnvVariable.ts(1 hunks)test/setup/administratorEmail.test.ts(1 hunks)test/setup/apiSetup.test.ts(1 hunks)test/setup/cloudbeaverSetup.test.ts(1 hunks)test/setup/minioSetup.test.ts(1 hunks)test/setup/postgresSetup.test.ts(1 hunks)test/setup/setNodeEnvironment.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (5)
src/setup/updateEnvVariable.ts (1)
23-25: Well done addressing potential ReDoS in regex.Escaping dynamic variables in the pattern helps prevent malicious strings from causing performance issues. This is a secure approach.
src/setup.ts (1)
50-57: Ensure source env files exist.
envFiles/.env.ciandenvFiles/.env.devcontainermust exist prior to reading. Otherwise, this will throw an exception. Consider adding checks or fallback logic to handle missing files or prompting the user to confirm their presence.setup.ts (1)
4-8: Great fallback approach upon errors.Calling
restoreEnvFile()before exiting ensures the user’s original.envfile remains intact if setup fails. This protects against partially-applied configurations. Nice job!test/setup/minioSetup.test.ts (1)
15-39: Add validation tests for MinIO configuration.The test suite should include:
- IP address format validation
- Port conflict detection between API and Console ports
- Root credential strength requirements
Example test case:
it("should detect port conflicts", async () => { const mockedAnswers = { ...validAnswers, MINIO_API_MAPPED_PORT: "8000", MINIO_CONSOLE_MAPPED_PORT: "8000", // Same port as API }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(mockedAnswers); await expect(minioSetup()).rejects.toThrow("Port conflict detected"); });test/setup/postgresSetup.test.ts (1)
15-38: Add test coverage for edge cases and error scenarios.The current test only covers the happy path. Consider adding test cases for:
- Invalid port numbers (e.g., negative, non-numeric, out of range)
- Invalid IP addresses
- Empty or whitespace-only values
- Database name validation
- Error handling when environment variable updates fail
Example test case:
it("should validate port number", async () => { const mockedAnswers = { ...mockedAnswers, POSTGRES_MAPPED_PORT: "-1", // Invalid port }; vi.spyOn(inquirer, "prompt").mockResolvedValueOnce(mockedAnswers); await expect(postgresSetup()).rejects.toThrow("Invalid port number"); });
|
will implement all the new suggestions, the old ones are not relevant as a lot of code was changed. except this, does this fulfill your requirements now @palisadoes ? |
|
I forgot to add documentation for this, will do it tomorrow |
palisadoes
added
the
ignore-sensitive-files-pr
|
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
There was a problem hiding this comment.
Actionable comments posted: 15
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
docs/docs/docs/getting-started/installation.md(1 hunks)scripts/setup/setup.ts(1 hunks)scripts/setup/updateEnvVariable.ts(1 hunks)setup.ts(1 hunks)test/setup/administratorEmail.test.ts(1 hunks)test/setup/apiSetup.test.ts(1 hunks)test/setup/cloudbeaverSetup.test.ts(1 hunks)test/setup/envSetup.spec.ts(1 hunks)test/setup/minioSetup.test.ts(1 hunks)test/setup/postgresSetup.test.ts(1 hunks)test/setup/setup.test.ts(1 hunks)test/setup/updateEnvVariable.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
test/setup/administratorEmail.test.ts (1)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/administratorEmail.test.ts:22-32
Timestamp: 2025-02-19T14:14:32.139Z
Learning: In the Talawa API test suite, environment variable assertions are handled in setup.test.ts, while individual setup function test files (like administratorEmail.test.ts) focus on testing the function's behavior and return values.
test/setup/cloudbeaverSetup.test.ts (3)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Utility functions like validatePort and validateEmail should be tested in dedicated test files (administratorEmail.test.ts for email validation, apiSetup.test.ts for port validation) to maintain a single source of truth and avoid test duplication across the codebase.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Avoid redundant testing of utility functions (like validatePort, validateEmail) that are already thoroughly tested in dedicated test files. Each function should have a single source of truth for its tests.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/cloudbeaverSetup.test.ts:16-30
Timestamp: 2025-02-09T08:29:04.469Z
Learning: CloudBeaver configuration in the setup script requires specific validation:
1. Admin name must be at least 3 characters and contain only alphanumeric characters and underscores
2. Admin password must be at least 8 characters and contain both letters and numbers
3. Server URL must use HTTP/HTTPS protocol with a valid port number
4. Default server URL should match the Docker container's port binding (8978)
test/setup/postgresSetup.test.ts (3)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/postgresSetup.test.ts:16-39
Timestamp: 2025-02-09T08:30:19.949Z
Learning: The Talawa API project uses envConfigSchema.ts for PostgreSQL configuration validation, and additional validation functions in the setup script would be redundant as they're already handled by the schema.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Utility functions like validatePort and validateEmail should be tested in dedicated test files (administratorEmail.test.ts for email validation, apiSetup.test.ts for port validation) to maintain a single source of truth and avoid test duplication across the codebase.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Avoid redundant testing of utility functions (like validatePort, validateEmail) that are already thoroughly tested in dedicated test files. Each function should have a single source of truth for its tests.
test/setup/apiSetup.test.ts (1)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/apiSetup.test.ts:94-131
Timestamp: 2025-02-19T09:07:01.755Z
Learning: The validateURL function in the setup script should handle case-insensitive protocol checks by converting the protocol to lowercase before comparison, while relying on the URL constructor's built-in case-insensitive domain validation.
test/setup/minioSetup.test.ts (2)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Avoid redundant testing of utility functions (like validatePort, validateEmail) that are already thoroughly tested in dedicated test files. Each function should have a single source of truth for its tests.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Utility functions like validatePort and validateEmail should be tested in dedicated test files (administratorEmail.test.ts for email validation, apiSetup.test.ts for port validation) to maintain a single source of truth and avoid test duplication across the codebase.
🪛 markdownlint-cli2 (0.17.2)
docs/docs/docs/getting-started/installation.md
44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
47-47: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
49-49: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
51-51: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
51-51: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Deployment to https://docs-api.talawa.io
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (17)
setup.ts (1)
1-11: Well-structured error handling for the setup process.The error handling is comprehensive, providing detailed diagnostic information including error message, type, code, and stack trace. This approach facilitates better debugging when setup fails.
scripts/setup/updateEnvVariable.ts (3)
3-7: Good JSDoc documentation for the function.The function documentation clearly explains the purpose and parameters, making it easy for other developers to understand and use.
13-16: Implementation handles file backup effectively.Creating a backup before modifying configuration files is a good practice that allows for recovery in case of errors.
42-47: Good error handling with restore mechanism.The function properly restores the original file from backup if an error occurs during file operations, ensuring that the environment file is not left in a corrupt state.
test/setup/cloudbeaverSetup.test.ts (4)
14-21: Good test setup with environment restoration.The test suite properly restores the original environment after each test, ensuring that tests don't affect each other.
22-59: Comprehensive test for CloudBeaver configuration.The test effectively verifies that user-provided CloudBeaver configuration values are correctly processed and stored.
61-83: Thorough error handling test.The test properly verifies error handling behavior, including console logging, backup restoration, and process exit.
86-127: Complete validation tests for CloudBeaver configuration.The validation tests cover all the required validation rules for CloudBeaver admin name, password, and URL as specified in the project requirements.
These tests align with the learnings from past discussions, ensuring that:
- Admin name must be at least 3 characters and contain only alphanumeric characters and underscores
- Admin password must be at least 8 characters and contain both letters and numbers
- Server URL must use HTTP/HTTPS protocol with a valid port number
test/setup/administratorEmail.test.ts (1)
1-100: Well-structured test suite with comprehensive coverage.The test file provides thorough testing of the administratorEmail functionality with proper isolation between tests. It effectively covers validation for various email formats, error handling scenarios, and backup restoration processes. The test structure follows good practices with beforeEach/afterEach hooks for environment cleanup.
test/setup/updateEnvVariable.test.ts (1)
1-80: Comprehensive test coverage for environment variable management.The test suite thoroughly covers the updateEnvVariable functionality including updating existing variables, adding new ones, backup creation/restoration, and error handling. The tests effectively mock filesystem operations and verify both file operations and environment variable updates.
test/setup/setup.test.ts (1)
1-132: Well-structured comprehensive setup test suite.The test file provides thorough coverage of the setup function with different CI configurations, properly validating environment variable settings and error handling paths. The test structure follows good practices with afterEach hooks for environment cleanup.
Also applies to: 158-158
test/setup/minioSetup.test.ts (1)
1-81: Well-structured tests for MinIO configuration.The test suite effectively validates the minioSetup functionality across different scenarios. The separation of concerns in testing follows best practices, avoiding redundant testing of utility functions that are already covered in dedicated test files (in line with the project's established patterns).
Also applies to: 111-134
test/setup/postgresSetup.test.ts (1)
78-100: Verify environment restoration after prompt errors.While this test confirms a backup is restored, consider verifying whether partially updated environment variables are rolled back. This ensures no incomplete or conflicting changes remain if the prompt fails mid-configuration.
test/setup/apiSetup.test.ts (3)
27-40: Verify envReconfigure prompt flow thoroughly.
isEnvConfiguredconditionally includes anenvReconfigureprompt. Ensure you have a test path verifying how subsequent prompts differ when the.envfile already exists vs. when it doesn’t.
99-121: Confirm environment rollbacks on prompt errors.Although the test checks if
.envis restored from.env.backup, it doesn’t verify whether partially updated environment variables are reverted. This helps prevent lingering partial changes.
161-175: Excellent coverage of case-insensitive URL validation.These tests thoroughly validate uppercase, mixed-case protocols, and domains, reflecting real-world scenarios. This is a solid approach to ensure broad URL handling coverage.
scripts/setup/setup.ts (1)
8-10: 🧹 Nitpick (assertive)Use typed fields for
SetupAnswersinstead of a string index signature.Storing boolean or numeric values as strings can be error-prone, requiring frequent conversions and validations. Consider using a more structured and type-safe approach for each field to avoid potential confusion and runtime errors:
-interface SetupAnswers { - [key: string]: string; -} +interface SetupAnswers { + CI: boolean; + API_ADMINISTRATOR_USER_EMAIL_ADDRESS: string; + // ... more strongly typed fields here ... +}⛔ Skipped due to learnings
Learnt from: prayanshchh PR: PalisadoesFoundation/talawa-api#2952 File: src/setup/setup.ts:8-10 Timestamp: 2025-02-11T19:19:42.810Z Learning: The SetupAnswers interface in Talawa API's setup script uses a flexible string map type `[key: string]: string` to allow partial mocking of environment variables in tests without requiring redundant type definitions.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (2)
docs/docs/docs/getting-started/installation.md (1)
44-44: 🧹 Nitpick (assertive)Ensure proper spacing before the heading.
A blank line should precede the "## Running the Setup Script" heading to comply with Markdown best practices (MD022).
Apply this diff:
-## Running the Setup Script + +## Running the Setup Script🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
scripts/setup/updateEnvVariable.ts (1)
9-20: 🧹 Nitpick (assertive)Great implementation of the quoteIfNeeded helper function.
This function properly handles quoting values with spaces and special characters, which is essential for maintaining the integrity of environment variables in the .env file.
However, consider exporting this function to make it testable directly rather than recreating it in the test file.
-function quoteIfNeeded(value: string | number): string { +export function quoteIfNeeded(value: string | number): string {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
docs/docs/docs/getting-started/installation.md(1 hunks)scripts/setup/setup.ts(1 hunks)scripts/setup/updateEnvVariable.ts(1 hunks)test/setup/cloudbeaverSetup.test.ts(1 hunks)test/setup/minioSetup.test.ts(1 hunks)test/setup/updateEnvVariable.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
test/setup/cloudbeaverSetup.test.ts (4)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/cloudbeaverSetup.test.ts:116-126
Timestamp: 2025-02-27T11:26:39.556Z
Learning: Node's URL constructor already validates port numbers and rejects values outside the valid range (0-65535) by throwing an error, making explicit port validation checks unnecessary in code that uses the URL constructor.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Utility functions like validatePort and validateEmail should be tested in dedicated test files (administratorEmail.test.ts for email validation, apiSetup.test.ts for port validation) to maintain a single source of truth and avoid test duplication across the codebase.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Avoid redundant testing of utility functions (like validatePort, validateEmail) that are already thoroughly tested in dedicated test files. Each function should have a single source of truth for its tests.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/cloudbeaverSetup.test.ts:16-30
Timestamp: 2025-02-09T08:29:04.469Z
Learning: CloudBeaver configuration in the setup script requires specific validation:
1. Admin name must be at least 3 characters and contain only alphanumeric characters and underscores
2. Admin password must be at least 8 characters and contain both letters and numbers
3. Server URL must use HTTP/HTTPS protocol with a valid port number
4. Default server URL should match the Docker container's port binding (8978)
test/setup/minioSetup.test.ts (2)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Avoid redundant testing of utility functions (like validatePort, validateEmail) that are already thoroughly tested in dedicated test files. Each function should have a single source of truth for its tests.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Utility functions like validatePort and validateEmail should be tested in dedicated test files (administratorEmail.test.ts for email validation, apiSetup.test.ts for port validation) to maintain a single source of truth and avoid test duplication across the codebase.
🪛 markdownlint-cli2 (0.17.2)
docs/docs/docs/getting-started/installation.md
44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
49-49: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
51-51: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (21)
test/setup/cloudbeaverSetup.test.ts (5)
1-12: Imports and mocks are well-structured.The import statements and mock declarations are clear. This provides a good foundation for the test file.
14-36: Effective setup and teardown strategy.Re-initializing
process.envand resetting mocks inafterEachensures a clean test environment, preventing state leakage between tests.
38-59: Comprehensive prompt response coverage.The test effectively simulates user interactions, verifying that the final answers align with expected environment variables.
61-83: Robust error handling verification.Testing error scenarios ensures reliability. Confirming that
.env.backuprestores a safe state if prompts fail is a good practice.
86-129: Thorough validation tests.The coverage of admin name, password, and URL validation is excellent, ensuring critical parameters are thoroughly vetted.
test/setup/minioSetup.test.ts (5)
1-16: Solid test setup.Spying on
inquirerand resetting mocks inafterEachhelps maintain test isolation and reproducibility.
17-42: Simple scenario coverage is correct.Prompting for minimal Minio configuration (browser, root password, root user) is tested well, ensuring expected defaults.
43-81: Extended configuration tests are well implemented.Verifying environment variable persistence via
dotenv.configis a nice touch. It ensures that the file-based approach is correctly reflected inprocess.env.
82-114: Good user-friendly resolution of port conflicts.Re-prompting for a new console port instead of abruptly failing improves the user experience. The test thoroughly confirms the final port values.
116-137: Robust prompt error handling.Ensuring
.env.backupis restored and the script exits with an error code helps maintain consistent state after failures.scripts/setup/setup.ts (1)
36-45: Properly returns a boolean for confirm prompts.The signature of
promptConfirmaligns with theinquirerconfirm prompt type, avoiding type mismatches.scripts/setup/updateEnvVariable.ts (3)
39-45: Excellent regex pattern escaping for variable names.The regex pattern escaping ensures that special characters in variable names won't break the replacement logic.
46-56: Good implementation of variable replacement and process.env updates.The code properly handles both updating existing variables and adding new ones, with appropriate quoting. Setting process.env directly ensures the changes are available immediately in the runtime environment.
22-26: 🧹 Nitpick (assertive)Function signature looks good, but could benefit from a more descriptive return type.
The function signature is clear, but consider adding a more explicit return type instead of relying on the inferred
void.export function updateEnvVariable(config: { [key: string]: string | number; -}): void { +}): void | never { // Explicitly indicates the function either returns nothing or throwsLikely an incorrect or invalid review comment.
test/setup/updateEnvVariable.test.ts (7)
7-14: Good test setup with clear variable definitions.The test setup is clear and resets mocks properly before each test.
16-28: Great test case for updating existing variables.This test case properly verifies both the file write operation and the process.env update.
30-42: Good test case for adding new variables.This test case properly verifies that a new variable is added to both the file and process.env.
44-51: Good test for backup creation.This test properly verifies that a backup file is created before updating the environment file.
53-65: Excellent test for error handling and backup restoration.This test comprehensively verifies that the backup is restored if an error occurs during the write operation.
67-79: Good test for .env file creation.This test verifies that the function works correctly when the environment file doesn't exist.
81-100: Great test for special character handling.This test verifies that values with spaces and quotes are properly escaped when written to the file.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
scripts/setup/updateEnvVariable.ts (5)
19-22: 🧹 Nitpick (assertive)Improve variable naming for clarity.
The variable name could better indicate this is the original content of the file.
- const existingContent: string = fs.existsSync(envFileName) + const originalContent: string = fs.existsSync(envFileName) ? fs.readFileSync(envFileName, "utf8") : "";
41-47: 🧹 Nitpick (assertive)Consider cleaning up backup files after successful operations.
Your error handling with backup restoration is solid. For better cleanup, consider removing the backup file after a successful write operation.
fs.writeFileSync(envFileName, updatedContent, "utf8"); + // Clean up backup file after successful write + if (fs.existsSync(backupFile)) { + fs.unlinkSync(backupFile); + } } catch (error) { if (fs.existsSync(backupFile)) { fs.copyFileSync(backupFile, envFileName);
25-39:⚠️ Potential issueSecurity issue: Add support for quoting values with spaces and special characters.
The current implementation doesn't handle quoting of values properly, which could lead to parsing issues and security vulnerabilities when the file is read back. This was also flagged by GitHub's security scan.
for (const key in config) { const value = config[key]; + // Quote the value if needed + const formattedValue = quoteIfNeeded(value); const regex = new RegExp( `^${key.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}=.*`, "gm", ); if (regex.test(updatedContent)) { - updatedContent = updatedContent.replace(regex, `${key}=${value}`); + updatedContent = updatedContent.replace(regex, `${key}=${formattedValue}`); } else { - updatedContent += `\n${key}=${value}`; + updatedContent += `\n${key}=${formattedValue}`; } process.env[key] = String(value); } +// Helper function to properly quote values with spaces or special characters +function quoteIfNeeded(value: string | number): string { + const stringValue = String(value); + if (typeof value === 'string' && (stringValue.includes(' ') || stringValue.includes('"') || stringValue.includes("'"))) { + // Escape quotes and wrap in double quotes + return `"${stringValue.replace(/"/g, '\\"')}"`; + } + return stringValue; +}This implements the missing quoting functionality that was previously recommended and addresses the security scan finding about incomplete string escaping.
1-1: 🧹 Nitpick (assertive)Consider using specific imports instead of the whole fs module.
For better maintainability and clarity, import only the file system functions you're using.
-import fs from "node:fs"; +import { existsSync, copyFileSync, readFileSync, writeFileSync } from "node:fs";
13-16: 🧹 Nitpick (assertive)Consider handling existing backup files.
Your backup strategy is good, but doesn't handle cases where a backup file might already exist from a previous failed run, which could lead to overwriting important backups.
const backupFile = `${envFileName}.backup`; if (fs.existsSync(envFileName)) { + // Create a unique backup name with timestamp + const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); + const uniqueBackupFile = `${envFileName}.backup.${timestamp}`; - fs.copyFileSync(envFileName, backupFile); + fs.copyFileSync(envFileName, uniqueBackupFile); + // Use this specific backup for restoration later + const currentBackupFile = uniqueBackupFile;test/setup/updateEnvVariable.test.ts (1)
5-5: 🧹 Nitpick (assertive)Improve mock setup with explicit implementations.
The current mock setup is minimal. For better readability and maintenance, consider using explicit mock implementations:
-vi.mock("fs"); +vi.mock("node:fs", () => ({ + default: { + existsSync: vi.fn(), + readFileSync: vi.fn(), + writeFileSync: vi.fn(), + copyFileSync: vi.fn() + } +}));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
scripts/setup/updateEnvVariable.ts(1 hunks)test/setup/updateEnvVariable.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
|
documentation on how to run the script has been added |
|
|
will make the changes, working on a priority issue, please consider |
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (1)
docs/docs/docs/getting-started/installation.md (1)
44-74: 🧹 Nitpick (assertive)Great addition of the setup script documentation!
This new section provides clear instructions for running the setup script with good explanations of CI vs non-CI modes. The structured format makes it easy for users to understand their options.
However, there are a couple of minor markdown formatting issues to fix:
-## Running the Setup Script + +## Running the Setup Script To configure the .env file, run one of the following commands in your project's root directory: -
- Add a blank line before the heading (line 44)
- Remove one of the consecutive blank lines (line 47-48)
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
47-47: Multiple consecutive blank lines
Expected: 1; Actual: 2(MD012, no-multiple-blanks)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
docs/docs/docs/getting-started/installation.md(1 hunks)scripts/setup/setup.ts(1 hunks)test/scripts/setup/administratorEmail.test.ts(1 hunks)test/scripts/setup/apiSetup.test.ts(1 hunks)test/scripts/setup/caddySetup.test.ts(1 hunks)test/scripts/setup/cloudbeaverSetup.test.ts(1 hunks)test/scripts/setup/envSetup.spec.ts(1 hunks)test/scripts/setup/minioSetup.test.ts(1 hunks)test/scripts/setup/postgresSetup.test.ts(1 hunks)test/scripts/setup/setup.test.ts(1 hunks)test/scripts/setup/updateEnvVariable.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
test/scripts/setup/minioSetup.test.ts (1)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Avoid redundant testing of utility functions (like validatePort, validateEmail) that are already thoroughly tested in dedicated test files. Each function should have a single source of truth for its tests.
test/scripts/setup/administratorEmail.test.ts (2)
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/administratorEmail.test.ts:22-32
Timestamp: 2025-02-19T14:14:32.139Z
Learning: In the Talawa API test suite, environment variable assertions are handled in setup.test.ts, while individual setup function test files (like administratorEmail.test.ts) focus on testing the function's behavior and return values.
Learnt from: prayanshchh
PR: PalisadoesFoundation/talawa-api#2952
File: test/setup/minioSetup.test.ts:16-40
Timestamp: 2025-02-09T08:30:45.416Z
Learning: Utility functions like validatePort and validateEmail should be tested in dedicated test files (administratorEmail.test.ts for email validation, apiSetup.test.ts for port validation) to maintain a single source of truth and avoid test duplication across the codebase.
🪛 markdownlint-cli2 (0.17.2)
docs/docs/docs/getting-started/installation.md
44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
47-47: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
🔇 Additional comments (36)
test/scripts/setup/envSetup.spec.ts (4)
1-42: Well-structured test suite for environment file checks.The tests for
checkEnvFilethoroughly verify the function's behavior for both existing and non-existing.envfiles.
43-60: Good test for environment file initialization based on CI flag.The test verifies that the correct environment file is read based on the CI flag setting.
71-84: Good backup handling test.The test ensures that a backup of the
.envfile is created if it exists.
86-110: Comprehensive error handling tests.The tests properly verify error messages when environment files are missing or cannot be read.
test/scripts/setup/caddySetup.test.ts (1)
1-31: Good test setup for Caddy configuration.The test correctly mocks the user responses for Caddy configuration settings.
test/scripts/setup/cloudbeaverSetup.test.ts (2)
1-61: Well-structured test for CloudBeaver setup.The test thoroughly verifies that user inputs for CloudBeaver configuration are correctly processed and stored in environment variables.
86-131: Comprehensive validation tests.The tests for validation functions thoroughly cover various scenarios including empty inputs, invalid formats, and valid inputs.
test/scripts/setup/updateEnvVariable.test.ts (6)
11-14: Good use of beforeEach for test setup.The test properly resets mocks and sets up the initial conditions before each test case, ensuring test isolation.
16-28: LGTM: Test correctly verifies updating existing variables.The test properly mocks file reading and writing, and verifies both the file content update and the process.env update.
30-42: LGTM: Test correctly verifies adding new variables.This test case properly verifies that new variables are correctly appended to the file and updated in process.env.
44-51: LGTM: Test verifies backup creation.The test properly checks that a backup file is created before making changes to the environment file.
53-65: LGTM: Test verifies restoration from backup on error.The test correctly simulates a write failure and verifies that the backup is restored.
67-79: LGTM: Test verifies .env file creation.The test correctly checks that a new .env file is created with the specified variable if it doesn't exist.
test/scripts/setup/minioSetup.test.ts (4)
10-15: Good use of environment restoration.The test properly saves and restores the original environment after each test, ensuring test isolation.
17-41: LGTM: Test correctly verifies basic Minio configuration.The test properly mocks user responses and verifies that the answers object contains the expected values.
83-115: LGTM: Test correctly handles port conflict.The test properly verifies that port conflicts between API and Console are detected and resolved.
116-138: LGTM: Test properly handles prompt errors.The test correctly simulates a prompt failure and verifies the expected error handling behavior.
test/scripts/setup/setup.test.ts (4)
17-75: LGTM: Test verifies environment setup with CI=false.The test thoroughly checks that all expected environment variables are set correctly in a non-CI environment.
77-133: LGTM: Test verifies environment setup with CI=true.The test correctly verifies that CloudBeaver-related configuration is skipped when CI=true.
134-147: LGTM: Test verifies backup restoration when reconfiguration is declined.The test correctly simulates the user declining to reconfigure and verifies that the process exits cleanly.
149-173: LGTM: Test verifies SIGINT handling.The test properly verifies that a SIGINT signal triggers backup restoration and clean exit.
test/scripts/setup/administratorEmail.test.ts (5)
10-20: LGTM: Test properly manages environment state.The test correctly saves and restores the original environment state to ensure test isolation.
22-32: LGTM: Test verifies email prompting and updates.The test correctly verifies that the user is prompted for an email and that the answers object is updated.
34-49: LGTM: Tests thoroughly verify email validation.The tests properly check multiple valid email formats and various invalid formats with appropriate error messages.
51-75: LGTM: Test verifies error handling with backup.The test correctly verifies that errors during prompting are handled properly and that a backup is restored if available.
77-99: LGTM: Test verifies error handling without backup.The test correctly verifies proper error handling when no backup file exists.
test/scripts/setup/postgresSetup.test.ts (1)
9-40: Great coverage of basic flow.The test successfully verifies standard user input for Postgres configuration. Mock responses are handled cleanly, and the resulting
process.envupdates are confirmed. This ensures your setup logic behaves predictably for typical user scenarios.test/scripts/setup/apiSetup.test.ts (5)
104-126: Error scenario handled properly.Graceful recovery from a prompt failure by restoring
.envfrom backup is tested. This ensures any partial environment changes are discarded. Fine approach for stable reliability.
178-231: URL validation thoroughly tested.You test protocol correctness, IP addresses, edge cases, and domain casing. This robust coverage ensures the setup script won't proceed with invalid URLs. Nicely done.
233-258: Port validation is correct and covers typical edge cases.The test properly rejects out-of-range or non-numeric ports. Everything looks good here.
260-291: Strong secret generation logic.Tests confirm secrets are sufficiently random and capture permission errors gracefully. This should shield you from collisions or insecure secrets.
293-340: No-backup error scenario is well-handled.When
.env.backupis absent, the test ensures correct logging and termination. This is essential for reliable cleanup.scripts/setup/setup.ts (4)
47-80: Secure JWT generation with fallback.
generateJwtSecretcallscrypto.randomByteswith safe length. The catch block logs an error and re-throws if generation fails. This is a good pattern to ensure fail-fast in unusual conditions.
82-128: Validation utilities are carefully structured.Your validators (
validateURL,validatePort,validateEmail,validateCloudBeaverXYZ) handle typical user errors. It's good practice to keep them modular and test them heavily to reduce input vulnerabilities.
138-185: Cautious environment file setup.Creating a backup of
.envand reusing.env.devcontaineror.env.ciis a sensible approach. ThesafeContenttransformation properly escapes problematic characters. Implementation is robust.
504-599: Postgres customization flows are thoroughly prompted.You handle separate mapped vs. default scenarios with validations. This is cohesive. The test coverage in
postgresSetup.test.tsaligns well with these prompts.
|
@tasneemkoushar @meetulr this is ready |
|
@palisadoes PTAL |
|
this has too many commits due to conflicts, can we merge this now? |
palisadoes
merged commit 5674b92
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
created setup script for
develop-postgresIssue Number:
#2688
Snapshots/Videos:
Screencast.from.2025-01-29.08-47-05.webm
Screencast.from.2025-01-29.08-52-29.webm
Screencast.from.2025-01-29.08-54-58.webm
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Documentation