Fix: Missing & Incorrect API URLs in .env.example #3512
Conversation
|
Warning Rate limit exceeded@JaiPannu-IITI has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request updates the local environment configuration for Talawa-admin by modifying endpoint URLs in the Changes
Suggested labels
Suggested reviewers
Poem
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
The issue has not been addressed. The setup script has not been altered according to the issue statement. The setup script was obviously not run as a part of this PR.
Please fix the following.
- The setup script does no use
.env.example. And must never do so. It relies on coded values which are always certain. - If a pre-exisiting .env file has trailing slashes on graphql endpoints, the script doesn't strip them.
- Here is the unchanged .env URLS
REACT_APP_TALAWA_URL=http://localhost:4000/graphql/ REACT_APP_BACKEND_WEBSOCKET_URL=ws://localhost:4000/graphql/ - If the
.envfile doesn't exist, the script fails. A test needs to be written for this.
- The setup script defaults to URLs with trailing slashes on graphql endpoints
- Here is the resulting .env file based on the defaults
REACT_APP_TALAWA_URL=http://localhost:4000/graphql/ REACT_APP_BACKEND_WEBSOCKET_URL=ws://localhost:4000/graphql/ REACT_APP_DOCKER_TALAWA_URL=http://host.docker.internal:4000/graphql/
|
Implementing the required fixes. |
|
@palisadoes, I believe this commit aligns with our requirements. Attached are the demonstration and code coverage results for your review. script.modified.movLet me know if any adjustments are needed. |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/setup/checkEnvFile/checkEnvFile.ts (1)
Line range hint
18-27: Add error handling for file operations.The function performs file operations without try-catch blocks, which could lead to uncaught exceptions if file operations fail.
Apply this diff to add error handling:
export function modifyEnvFile(): void { + try { const env = dotenv.parse(fs.readFileSync('.env')); const envSample = dotenv.parse(fs.readFileSync('.env.example')); const misplaced = Object.keys(envSample).filter((key) => !(key in env)); if (misplaced.length > 0) { const config = dotenv.parse(fs.readFileSync('.env.example')); misplaced.map((key) => fs.appendFileSync('.env', `${key}=${config[key]}\n`), ); } + } catch (error) { + console.error('Error modifying .env file:', error); + throw error; + } }
🧹 Nitpick comments (4)
src/setup/checkConnection/checkConnection.spec.ts (1)
44-44: LGTM! Consider adding more edge cases.The failed connection test URL has been updated correctly. Consider adding test cases for other URL format edge cases (e.g., malformed URLs, URLs with query parameters).
docs/docs/docs/getting-started/installation.md (1)
Line range hint
211-241: Add language specifiers to code blocks.The code blocks lack language specifiers, which affects syntax highlighting and documentation readability.
Update the code blocks by adding the
envlanguage specifier:-``` +```env REACT_APP_TALAWA_URL="http://API-IP-ADRESS:4000/graphql"Apply this change to all similar code blocks in this section. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 227-227: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 234-234: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 241-241: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </blockquote></details> <details> <summary>src/setup/checkEnvFile/checkEnvFile.spec.ts (1)</summary><blockquote> Line range hint `17-56`: **Add test case for error handling in modifyEnvFile.** The tests cover the happy path scenarios but should also verify error handling when file operations fail. Add this test case: ```typescript it('should handle file operation errors', () => { vi.spyOn(fs, 'readFileSync').mockImplementation(() => { throw new Error('File read error'); }); expect(() => modifyEnvFile()).toThrow('File read error'); });docs/docs/auto-docs/setup/checkEnvFile/checkEnvFile/functions/checkEnvFile.md (1)
7-13: Add function description to improve documentation.The documentation should include a description of what the function does and when it returns true/false.
Add this description:
> **checkEnvFile**(): `boolean` +Checks for the existence of environment files and creates .env if needed. +Returns true if .env exists or is successfully created, false if setup cannot proceed. + ## Returns `boolean`
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS.md(1 hunks)docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS_EMPTY.md(1 hunks)docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS_ERROR.md(1 hunks)docs/docs/auto-docs/screens/OrganizationFundCampaign/OrganizationFundCampagins/functions/default.md(1 hunks)docs/docs/auto-docs/screens/OrganizationFunds/OrganizationFunds/functions/default.md(1 hunks)docs/docs/auto-docs/setup/checkEnvFile/checkEnvFile/functions/checkEnvFile.md(1 hunks)docs/docs/auto-docs/setup/checkEnvFile/checkEnvFile/functions/modifyEnvFile.md(1 hunks)docs/docs/docs/getting-started/installation.md(2 hunks)setup.ts(2 hunks)src/setup/askForTalawaApiUrl/askForTalawaApiUrl.spec.ts(3 hunks)src/setup/askForTalawaApiUrl/askForTalawaApiUrl.ts(1 hunks)src/setup/askForTalawaApiUrl/setupTalawaWebSocketUrl.spec.ts(1 hunks)src/setup/checkConnection/checkConnection.spec.ts(3 hunks)src/setup/checkEnvFile/checkEnvFile.spec.ts(4 hunks)src/setup/checkEnvFile/checkEnvFile.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- docs/docs/auto-docs/setup/checkEnvFile/checkEnvFile/functions/modifyEnvFile.md
- docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS_EMPTY.md
- src/setup/askForTalawaApiUrl/askForTalawaApiUrl.ts
- docs/docs/auto-docs/screens/OrganizationFundCampaign/OrganizationFundCampagins/functions/default.md
- docs/docs/auto-docs/screens/OrganizationFunds/OrganizationFunds/functions/default.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/docs/docs/getting-started/installation.md
211-211: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
218-218: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
234-234: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
241-241: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (10)
src/setup/checkConnection/checkConnection.spec.ts (2)
6-6: LGTM! URL format updated correctly.The removal of the trailing slash from the mock URL aligns with the PR objectives and maintains consistency with the new URL format.
30-30: LGTM! Test URL updated consistently.The test URL format matches the mock implementation and follows the new URL format standard.
src/setup/askForTalawaApiUrl/setupTalawaWebSocketUrl.spec.ts (1)
36-36: LGTM! URL format updated correctly.The endpoint URL has been updated to remove the trailing slash, which aligns with the PR objectives and maintains consistency across the codebase.
src/setup/askForTalawaApiUrl/askForTalawaApiUrl.spec.ts (1)
Line range hint
20-53: LGTM! URL formats standardized correctly.All endpoint URLs have been consistently updated to remove trailing slashes, maintaining the standardized format across test cases.
docs/docs/docs/getting-started/installation.md (1)
Line range hint
205-242: LGTM! Environment configuration examples updated correctly.The URL formats in the environment configuration examples have been standardized by removing trailing slashes, which aligns with the PR objectives.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
204-204: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
211-211: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
218-218: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS_ERROR.md (1)
7-9: Clarify and Validate Updated Union Type for MOCKS_ERRORThe updated documentation now reflects a revised and more complex union type for the
MOCKS_ERRORconstant. In particular, note the following improvements:
- The updated reference to the definition location (now linked to line 416 in the source) confirms that the documentation is in sync with the code.
- The union includes multiple variants for GraphQL query responses and error objects, with some fields explicitly typed as
undefinedto denote their absence in certain scenarios.It would be beneficial to verify that every union member accurately represents the underlying data structures. If possible, consider breaking down the longer type definition into multiple lines or leveraging TypeScript’s optional property shorthand (e.g., using
prop?: type) for improved readability in auto-generated docs.docs/docs/auto-docs/screens/OrganizationActionItems/OrganizationActionItem.mocks/variables/MOCKS.md (1)
7-9: Streamline and Ensure Consistency in the MOCKS Type DefinitionThe changes to the
MOCKSvariable’s type definition now offer a more streamlined representation of the various GraphQL request/response scenarios. Key points include:
- The removal or consolidation of some redundant
undefineddeclarations appears to simplify the union type, improving clarity.- It is important to ensure that fields that are optional across multiple union members consistently use either an explicit
undefinedor adopt the optional property notation. This consistency improves both maintainability and readability.- The documentation also correctly provides a reference link that indicates where the source definition now resides (line 21). Verify that this link remains up to date after further changes.
Overall, this update aids in aligning the documentation with the current codebase expectations.
src/setup/checkEnvFile/checkEnvFile.ts (1)
6-16: LGTM! Good improvement on the return type.The function now properly indicates success/failure through its boolean return type, which is a better design for error handling.
setup.ts (1)
65-71: LGTM! Good error handling flow.The changes properly handle the return value from
checkEnvFileand sequence the operations correctly.src/setup/checkEnvFile/checkEnvFile.spec.ts (1)
58-100: LGTM! Good test coverage for checkEnvFile.The tests thoroughly cover all scenarios including file existence checks and error cases.
There was a problem hiding this comment.
- The script doesn't trim trailing slashes in URLs ending in
graphql/
REACT_APP_TALAWA_URL=http://localhost:4000/graphql/ REACT_APP_BACKEND_WEBSOCKET_URL=ws://localhost:4000/graphql/ REACT_APP_DOCKER_TALAWA_URL=http://host.docker.internal:4000/graphql/ - This is required for anyone already using Talawa-Admin
|
@palisadoes, Please confirm that you want to remove trailing slashes from manually entered URL's ?? |
Yes, but only if the URL ends in |
|
|
@palisadoes ,I couldn't find a test file for setup.ts, where do we find spec files for file outside src? That's all left here which is decreasing code coverage by 0.09%. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3512 +/- ##
====================================================
- Coverage 88.68% 88.65% -0.04%
====================================================
Files 341 341
Lines 8638 8626 -12
Branches 1925 1925
====================================================
- Hits 7661 7647 -14
+ Misses 644 635 -9
- Partials 333 344 +11 ☔ View full report in Codecov by Sentry. |
palisadoes
merged commit 735869e
into
PalisadoesFoundation:develop-postgres
Issue Reference
This PR addresses missing and incorrect API URL values in .env.example, which caused Talawa-API connection with Talawa-Admin to fail.
Related issue: #3493
Bug Description
During the migration to the develop-postgres branch, an issue prevents users from accessing the portal correctly.
Fix Implemented
Changes in .env.example
Before (Incorrect)
REACT_APP_TALAWA_URL= REACT_APP_BACKEND_WEBSOCKET_URL=ws://localhost:4000/graphql/After(Correct)
REACT_APP_TALAWA_URL=http://localhost:4000/graphql REACT_APP_BACKEND_WEBSOCKET_URL=ws://localhost:4000/graphqlExpected Behavior
Screenshots
Testing
connection.test.mov
Additional Details
This PR ensures that:
The setup script generates a correct .env file with REACT_APP_TALAWA_URL.
WebSocket URLs are properly formatted to prevent connection failures.
This fix improves the Talawa-Admin setup process, making it fully functional.
Summary by CodeRabbit
.envfile and its creation if absent.