Add comprehensive contribution documentation #533
Conversation
- Add CONTRIBUTION_GUIDE.md with detailed contribution steps - Add FIRST_CONTRIBUTION_CHECKLIST.md for first-time contributors - Add .env.example with all environment variable templates - Add setup.sh script for automated development setup - Improve onboarding experience for new contributors These files will help new contributors get started quickly and understand the contribution process better.
|
@PankajSingh34 is attempting to deploy a commit to the Renu's projects Team on Vercel. A member of the Team first needs to authorize it. |
🎉 Thanks for the PR, @PankajSingh34!We really appreciate you taking the time to contribute to ExpenseFlow! 💙 ⭐ Love this project?Please give us a star! It helps the project grow and reach more developers! 🌟 🔗 https://github.com/Renu-code123/ExpenseFlow ✅ PR ChecklistBefore we review, please ensure:
🙌 Thank You for Contributing!We truly appreciate your interest in contributing to this project.
We'll review your PR as soon as possible. Keep up the great work! ✨ |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive contribution documentation to help new contributors get started with the ExpenseFlow project. The documentation provides detailed guidance on environment setup, contribution workflows, and best practices.
Changes:
- Added detailed contribution guide (CONTRIBUTION_GUIDE.md) with step-by-step instructions, code examples, and resource links
- Added first-time contributor checklist (FIRST_CONTRIBUTION_CHECKLIST.md) with comprehensive task lists for the entire contribution process
- Added environment configuration template (.env.example) with extensive variable documentation for all integrations
- Added automated setup script (setup.sh) to streamline development environment initialization
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
| setup.sh | Automated bash script to check prerequisites, install dependencies, create .env file, verify MongoDB, and create necessary directories |
| FIRST_CONTRIBUTION_CHECKLIST.md | Comprehensive checklist covering pre-contribution setup, issue selection, development workflow, submission process, and post-PR activities |
| CONTRIBUTION_GUIDE.md | Detailed guide with contribution ideas categorized by difficulty, step-by-step workflow, project structure overview, and practical examples |
| .env.example | Environment variable template documenting all configuration options including server, database, authentication, email, integrations, and feature flags |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### **2. Make changes** | ||
| Edit `public/script.js` to add a new category icon: |
There was a problem hiding this comment.
Example references non-existent file. The example workflow mentions editing public/script.js (line 270), but this file doesn't exist in the repository. The public directory contains many specific JavaScript files like public/dashboard.js, public/trackerscript.js, public/index.js, etc., but no generic script.js file. Update this example to reference an actual file from the codebase, or clarify that this is a hypothetical example. Consider using a real file like public/trackerscript.js or public/dashboard.js to make the example more concrete and accurate.
| - [ ] Wrote descriptive commit message: | ||
| ``` | ||
| Add feature: descriptive title | ||
|
|
||
| - Detailed change 1 | ||
| - Detailed change 2 | ||
| - Detailed change 3 | ||
|
|
||
| Fixes #issue-number | ||
| ``` |
There was a problem hiding this comment.
The example commit message format doesn't follow the convention described in CONTRIBUTING.md. CONTRIBUTING.md (lines 106-120) specifies using imperative mood and present tense (e.g., "Add feature" not "Added feature"), with 72 character limit for first line. However, this example uses "Add feature: descriptive title" which includes a colon. While not wrong, for consistency with the existing CONTRIBUTING.md file, consider aligning the format. The example here is actually fine, but the checklist should reference the CONTRIBUTING.md convention for consistency.
| CORS_ORIGIN=http://localhost:3000,http://localhost:5000 | ||
| # Add your frontend URL in production |
There was a problem hiding this comment.
CORS origin configuration may be redundant or incorrect. The default value includes both http://localhost:3000 and http://localhost:5000, but the PORT is set to 5000 in this file (line 5), and server.js defaults to 3000 (line 46 of server.js). This creates confusion about which port is actually being used. If the server runs on port 5000 (as configured in this file), then http://localhost:3000 in CORS_ORIGIN would be incorrect unless there's a separate frontend server. Clarify which port(s) should be in CORS_ORIGIN based on the actual deployment setup, or add a comment explaining when multiple origins are needed (e.g., separate frontend dev server).
| CORS_ORIGIN=http://localhost:3000,http://localhost:5000 | |
| # Add your frontend URL in production | |
| # Comma-separated list of allowed frontend origins. | |
| # Example: http://localhost:3000 for a React/Vue dev server, http://localhost:5000 for the API server itself. | |
| # In most setups, the backend runs on PORT=5000 (see above) and the frontend dev server on 3000. | |
| # Update/remove entries as needed for your actual deployment, and add your production frontend URL(s) here. | |
| CORS_ORIGIN=http://localhost:3000,http://localhost:5000 |
| - [ ] MongoDB running (local or Atlas) | ||
| - [ ] Project runs successfully (`npm run dev`) | ||
|
|
||
| ### Repository Setup |
There was a problem hiding this comment.
Extra trailing whitespace on line 14 after "Repository Setup". While this doesn't affect functionality, removing trailing whitespace is a common best practice in documentation for consistency and clean diffs.
| ### Repository Setup | |
| ### Repository Setup |
| ENABLE_OCR=false | ||
| ENABLE_AI_CATEGORIZATION=true | ||
| ENABLE_NOTIFICATIONS=true | ||
| ENABLE_MULTI_CURRENCY=true |
There was a problem hiding this comment.
Missing critical environment variables that are used in the codebase. The following environment variables should be added to this file:
SMTP_HOST,SMTP_PORT,SMTP_USER,SMTP_PASS- used by services/alertService.js, services/inviteService.js, and services/reminderService.js for email functionalityFRONTEND_URLorAPP_URL- used by server.js line 40, 98, and throughout services for generating URLsREDIS_HOST,REDIS_PORT,REDIS_DB- used by middleware/rateLimiter.js for distributed rate limitingADMIN_EMAILorADMIN_EMAILS- used by services/alertService.js for admin notificationsSUPPORT_EMAIL- used by services/emailService.jsALPHA_VANTAGE_API_KEY- used by services/investmentService.js and services/priceUpdateService.jsCOINGECKO_API_KEY,FINNHUB_API_KEY,POLYGON_API_KEY- used by services/priceUpdateService.jsDEFAULT_CURRENCY,DEFAULT_LOCALE- used by services/analyticsService.jsEMAIL_SERVICE- used by services/notificationService.jsPLAID_WEBHOOK_SECRET- used by services/openBankingService.jsALERT_LOG_FILE- used by services/alertService.js
These variables are necessary for various features to work properly.
| ENABLE_MULTI_CURRENCY=true | |
| ENABLE_MULTI_CURRENCY=true | |
| # SMTP Configuration (for email functionality) | |
| SMTP_HOST=smtp.gmail.com | |
| SMTP_PORT=587 | |
| SMTP_USER=your-email@gmail.com | |
| SMTP_PASS=your-app-specific-password | |
| # Frontend URL (used for generating links in emails, redirects, etc.) | |
| FRONTEND_URL=http://localhost:3000 | |
| # Redis Configuration (for distributed rate limiting, caching, etc.) | |
| REDIS_HOST=localhost | |
| REDIS_PORT=6379 | |
| REDIS_DB=0 | |
| # Admin & Support Emails | |
| ADMIN_EMAILS=admin1@example.com,admin2@example.com | |
| SUPPORT_EMAIL=support@example.com | |
| # Market Data & Investment APIs | |
| ALPHA_VANTAGE_API_KEY=your_alpha_vantage_api_key | |
| COINGECKO_API_KEY=your_coingecko_api_key | |
| FINNHUB_API_KEY=your_finnhub_api_key | |
| POLYGON_API_KEY=your_polygon_api_key | |
| # Analytics Defaults | |
| DEFAULT_CURRENCY=USD | |
| DEFAULT_LOCALE=en-US | |
| # Notification Service (e.g., smtp, sendgrid, mailgun) | |
| EMAIL_SERVICE=smtp | |
| # Plaid Webhook (used to verify Plaid webhook signatures) | |
| PLAID_WEBHOOK_SECRET=your_plaid_webhook_secret | |
| # Alert Logging | |
| ALERT_LOG_FILE=logs/alerts.log |
| # JWT Configuration | ||
| JWT_SECRET=your_super_secret_jwt_key_change_this_in_production | ||
| JWT_EXPIRE=7d | ||
| JWT_REFRESH_SECRET=your_refresh_token_secret_change_this | ||
| JWT_REFRESH_EXPIRE=30d |
There was a problem hiding this comment.
Weak example JWT secrets could be accidentally used in production. Lines 14 and 16 contain example secrets like "your_super_secret_jwt_key_change_this_in_production" and "your_refresh_token_secret_change_this". While these include warnings to change them, developers might forget. Consider adding a more prominent warning comment above the JWT section, such as: "# CRITICAL: Generate strong random secrets for production. Never use these example values!" Also consider adding a note about using a tool like openssl rand -base64 32 to generate secure secrets.
| EMAIL_HOST=smtp.gmail.com | ||
| EMAIL_PORT=587 | ||
| EMAIL_USER=your-email@gmail.com | ||
| EMAIL_PASSWORD=your-app-specific-password |
There was a problem hiding this comment.
The environment variable name is inconsistent with the codebase. The code uses EMAIL_PASS (see services/emailService.js line 11, services/notificationService.js line 153), but this file specifies EMAIL_PASSWORD. This will cause email functionality to fail because the password won't be read correctly. Change EMAIL_PASSWORD to EMAIL_PASS to match the actual codebase usage.
| EMAIL_PASSWORD=your-app-specific-password | |
| EMAIL_PASS=your-app-specific-password |
| - [ ] Read CONTRIBUTING.md | ||
| - [ ] Read CONTRIBUTION_GUIDE.md |
There was a problem hiding this comment.
Inconsistent reference to contribution documentation. This checklist references "CONTRIBUTING.md" on line 22, but the PR adds a new file called "CONTRIBUTION_GUIDE.md" (which is being reviewed). The existing CONTRIBUTING.md file already exists in the repository. This creates confusion about which file contributors should read. Either update this reference to point to CONTRIBUTION_GUIDE.md, or clarify that both files should be read, specifying the purpose of each.
| - [ ] Read CONTRIBUTING.md | |
| - [ ] Read CONTRIBUTION_GUIDE.md | |
| - [ ] Read CONTRIBUTING.md (general contribution guidelines and policies) | |
| - [ ] Read CONTRIBUTION_GUIDE.md (step-by-step guide for making your first contribution) |
| @@ -0,0 +1,79 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Missing execute permissions notice. While the script has a proper shebang line, new contributors might not know they need to make it executable. Consider adding a comment in the script or in the documentation about running chmod +x setup.sh before executing it, or mention that users should run it with bash setup.sh if they encounter permission issues.
| ## 💻 Making Your Contribution | ||
|
|
||
| ### Branch Creation | ||
| - [ ] Updated main branch: `git checkout main && git pull upstream main` |
There was a problem hiding this comment.
Inconsistent Git workflow between files. FIRST_CONTRIBUTION_CHECKLIST.md line 48 correctly instructs to git pull upstream main (pulling from the original repository after forking), but CONTRIBUTION_GUIDE.md line 109 shows git pull origin main (pulling from your own fork). For contributors who have forked the repository, they should consistently pull from 'upstream' to get the latest changes from the main repository. Update line 109 in CONTRIBUTION_GUIDE.md to use git pull upstream main for consistency, or add a note explaining the difference between 'origin' (your fork) and 'upstream' (original repo).
|
hy @Renu-code123 , |
- Add consistent spacing between sections - Improve markdown formatting for better readability - Fix quote inconsistencies in code examples - Enhance visual hierarchy of content This makes the contribution guide more accessible and easier to follow for new contributors.
- Add complete openInviteModal() function with form handling - Implement closeInviteModal() to properly close modal - Add updateRolePermissions() to display role-specific permissions - Create handleInviteSubmit() for async API integration - Include loading states and error handling - Display role permissions dynamically based on selection - Integrate with existing workspace invitation API endpoint Resolves TODO at line 333 in workspace-feature.js Closes Renu-code123#1 - Invite Modal Implementation
- Install Jest, Supertest, and testing dependencies - Create jest.config.js with coverage thresholds (50%) - Add test scripts to package.json (test, test:watch, test:unit, test:integration) - Create test directory structure (tests/unit, tests/integration) - Implement User model unit tests (18 test cases) - Schema validation tests - Password hashing tests - Data sanitization tests - Currency and locale tests - Budget limit validation - Implement utility function tests (40+ test cases) - Email validation - Input sanitization - Currency formatting - Percentage calculations - Date utilities - String and array utilities - Create integration test template for Authentication API - Add comprehensive test documentation (tests/README.md) - Update .gitignore to exclude coverage reports - Configure global test setup with mockdate Test Results: 30 passing tests Coverage: Models 20.61%, User.js 22.98% Resolves Renu-code123#2 - Add Unit Tests
- Document both completed contributions - Include technical details and impact - Add test statistics and coverage metrics - Provide pull request template - List future contribution ideas This summary documents the completion of: 1. Invite Modal Functionality 2. Unit Testing Infrastructure
|
@PankajSingh34 mention issue no.# |
2 participants
These files will help new contributors get started quickly and understand the contribution process better.
📝 Description
Provide a brief summary of your changes. Does this PR affect the Vanilla JS frontend, the Node.js API, or the PWA service workers?
🔗 Related Issue
Fixes # (issue number)
🛠️ Type of Change
🧪 Testing & Validation
npm startand confirmed there are no console errors.🚩 Checklist:
public/,models/,routes/)..env.examplefile if I added new environment variables..envsecrets or private keys.mainbranch (or the designated development branch).📸 Visuals (if applicable)