-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
fix: fix mongo db urls mismatch inside .env
and docker-compose.dev.yaml
file, along with some other setup fixes
#2879
Conversation
…, also correct the default mongodb url when not setting up with docker
…aml file, also correct the default mongodb url when not setting up with docker" This reverts commit 8af7c63.
…, also correct the default mongodb url when not setting up with docker
…tabase not able to connect automatically/throws error after docker containers have started and fix the test for connecting to mongodb with correct url
WalkthroughThe pull request introduces modifications to the MongoDB connection string across multiple files. The changes primarily involve appending query parameters Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
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.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
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.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
src/utilities/loadDefaultOrg.ts (1)
Line range hint
15-15
: Fix typo in function name.The function name has a typo: "Organiation" should be "Organization".
-export async function loadDefaultOrganiation(dbName?: string): Promise<void> { +export async function loadDefaultOrganization(dbName?: string): Promise<void> {setup.ts (1)
Line range hint
1-1379
:⚠️ Pipeline failure: Unauthorized file modification.The pipeline indicates that this file is protected and cannot be modified. Please:
- Check if you have the necessary permissions to modify this file
- Consult with the repository maintainers about the correct process for updating protected files
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt. This file is protected and cannot be changed.
🧹 Nitpick comments (3)
src/setup/MongoDB.ts (1)
79-80
: Document the MongoDB URL parameters.The default URL now includes specific parameters (
replicaSet=rs0&directConnection=true
). Please add a comment explaining why these parameters are required and their impact.{ type: "input", name: "url", message: "Enter your MongoDB URL:", + // Default URL includes replica set and direct connection parameters required for proper MongoDB connection + // replicaSet=rs0: Enables replica set mode for transaction support + // directConnection=true: Ensures direct connection to the specified MongoDB instance default: "mongodb://localhost:27017/talawa-api?replicaSet=rs0&directConnection=true", },tests/setup/mongoDB.spec.ts (1)
73-75
: Add test cases for invalid MongoDB URLs.While the current tests cover the happy path with the new URL format, consider adding test cases for:
- URLs without required parameters
- Malformed URLs
- URLs with incorrect replica set names
Example test case:
it("should fail for URLs missing required parameters", async () => { const invalidUrl = "mongodb://localhost:27017/talawa-api"; vi.spyOn(module, "checkConnection").mockImplementationOnce(() => Promise.resolve(false) ); const result = await module.checkConnection(invalidUrl); expect(result).toBe(false); });Also applies to: 103-105, 113-114
setup.ts (1)
448-448
: Consider extracting MongoDB URL parameters to a constant.The MongoDB URL parameters are duplicated. Consider extracting them to a constant for better maintainability.
+const MONGODB_URL_PARAMS = "?replicaSet=rs0&directConnection=true"; + // In the mongoDB function -DB_URL = `${url?.endsWith("/talawa-api?replicaSet=rs0&directConnection=true") ? url : `${url}/talawa-api?replicaSet=rs0&directConnection=true`}`; +DB_URL = `${url?.endsWith(`/talawa-api${MONGODB_URL_PARAMS}`) ? url : `${url}/talawa-api${MONGODB_URL_PARAMS}`}`;Also applies to: 454-454
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt. This file is protected and cannot be changed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
setup.ts
(2 hunks)src/db.ts
(1 hunks)src/setup/MongoDB.ts
(1 hunks)src/utilities/loadDefaultOrg.ts
(1 hunks)tests/setup/mongoDB.spec.ts
(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
setup.ts
[error] Unauthorized file modification or deletion attempt. This file is protected and cannot be changed.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (2)
src/utilities/loadDefaultOrg.ts (1)
3-3
: LGTM! Good improvement in path handling.The change to use
fileURLToPath
is a better approach for ES modules as it handles URL encoding/decoding properly.Also applies to: 7-7
setup.ts (1)
1167-1168
: LGTM! Docker MongoDB URL is now consistent.The Docker MongoDB URL now includes the correct parameters, ensuring consistency with non-Docker environments.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Unauthorized file modification or deletion attempt. This file is protected and cannot be changed.
@@ -14,7 +13,7 @@ export const connect = async (dbName?: string): Promise<void> => { | |||
|
|||
// If no connection exists, attempt to establish a new connection. | |||
try { | |||
await mongoose.connect(MONGO_DB_URL as string, { | |||
await mongoose.connect(process.env.MONGO_DB_URL as string, { |
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.
🛠️ Refactor suggestion
Add validation for MONGO_DB_URL environment variable.
While using process.env
directly is fine, we should validate that the URL is set and properly formatted before using it.
+ const mongoUrl = process.env.MONGO_DB_URL;
+ if (!mongoUrl) {
+ throw new Error('MONGO_DB_URL environment variable is not set');
+ }
- await mongoose.connect(process.env.MONGO_DB_URL as string, {
+ await mongoose.connect(mongoUrl, {
📝 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.
await mongoose.connect(process.env.MONGO_DB_URL as string, { | |
const mongoUrl = process.env.MONGO_DB_URL; | |
if (!mongoUrl) { | |
throw new Error('MONGO_DB_URL environment variable is not set'); | |
} | |
await mongoose.connect(mongoUrl, { |
@aakarsh-2004 Please fix the failed tests. |
Please add |
@aakarsh-2004 Open a new PR against upstream develop-postgres branch. Closing for now |
What kind of change does this PR introduce?
bug fix, when setting up the project locally using docker
Issue Number:
Fixes #2864
Snapshots/Videos:
The error looked some like
![mongodb-not-connecting-error](https://private-user-images.githubusercontent.com/98646540/405163552-592c752f-4ea9-4326-97fa-47974179867d.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1NzYzNDEsIm5iZiI6MTczOTU3NjA0MSwicGF0aCI6Ii85ODY0NjU0MC80MDUxNjM1NTItNTkyYzc1MmYtNGVhOS00MzI2LTk3ZmEtNDc5NzQxNzk4NjdkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE0VDIzMzQwMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWU2YjM2N2E5ZDczMjYxNDUyNTMxNTI5NTM4YTMxZjMyYzEyMDAwYjQxZDcxOWI2YjAyMzJhODc4YTkyY2IxZmImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.DbGdUjIAUDItjDqJGMJ28TYjEQT4n5Vl28ncKTqe9EI)
Not everything works correctly and mongo starts successfully after docker setup
![image](https://private-user-images.githubusercontent.com/98646540/405163883-9b841816-afe4-4d1f-ab19-10646363f815.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1NzYzNDEsIm5iZiI6MTczOTU3NjA0MSwicGF0aCI6Ii85ODY0NjU0MC80MDUxNjM4ODMtOWI4NDE4MTYtYWZlNC00ZDFmLWFiMTktMTA2NDYzNjNmODE1LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE0VDIzMzQwMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWQ1ZDI1NzYxZDMxNzViMTk2ZDRiNWNjY2M4ODE2MjQwZGM3NGJiNjI4ZjY5ZmMxZDk0NWRhNjZlYjY2OGJmN2YmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.GvXPh2hA4B5PItDb-2C29Z475NthN9XyEfCE-fVoWUY)
Previously default mongodb urls mismatched in
.env
file anddocker-compose.dev.yaml
fileMONGO_DB_URL
inside .env previously.MONGO_DB_URL
inside docker-compose.dev.yaml previously.Now the default urls match!
MONGO_DB_URL
inside .env now.MONGO_DB_URL
inside docker-compose.dev.yaml now.I also fixed the Import Default Organization functionality was not working when setting up with docker and this error was being thrown.
![Screenshot 2025-01-19 130059](https://private-user-images.githubusercontent.com/98646540/405166694-12b0bbb5-88e5-4291-811a-02e3a3c8e7d6.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1NzYzNDEsIm5iZiI6MTczOTU3NjA0MSwicGF0aCI6Ii85ODY0NjU0MC80MDUxNjY2OTQtMTJiMGJiYjUtODhlNS00MjkxLTgxMWEtMDJlM2EzYzhlN2Q2LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE0VDIzMzQwMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWFiYWNmZDAzYzRhNTE2YzdhMzg5OWI2ODliNjg0OTI3ZjNkNDk2YjNjNGI1NWEzMWIzYWJkMTRkMTRkYTc1NWQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.ReESSusijXv2M7UM1mSD5BCv96-s-E1HfVBWFUMul2Y)
This was happening because when setting up for the first time or from scratch
constants.ts
had storedMONGO_DB_URL
was empty asMONGO_DB_URL
later gets populated after user runsnpm run setup
. So instead we usedprocess.env.MONGO_DB_URL
for connecting to the MongoDB Container.Also I followed the same file importing convention inside
loadDefaultOrg.ts
as used inloadSampleData.ts
for better readability.Summary
This PR ensures that users have easy time setting up this project locally without having to think much about it or so that users do not have to solve bugs on how to setup this project locally.
Checklist
CodeRabbit AI Review
Test Coverage
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Release Notes
Configuration Updates
Path Resolution
fileURLToPath
Testing
These changes improve database connection reliability and configuration consistency across different environments.