changemaker.lite/mkdocs/docs/v2/contributing/pull-requests.md

Pull Request Guidelines

This guide covers the complete pull request (PR) process for contributing code to Changemaker Lite V2, from creation to merge.

Before Submitting a PR

1. Create or Find an Issue

For features:

• Search existing issues first
• If none exists, create a feature request
• Wait for maintainer approval before implementing
• Discuss implementation approach in the issue

For bugs:

• Search existing bug reports first
• If none exists, create a bug report
• Include reproduction steps and expected vs actual behavior
• Verify bug still exists on latest v2 branch

!!! tip "Avoid Wasted Effort" Always create an issue and get approval before spending time on a large feature. Maintainers may have alternative approaches or priorities.

2. Test Your Changes

Run all checks locally before submitting:

# Type checking
cd api && npx tsc --noEmit
cd admin && npx tsc --noEmit

# Linting
cd api && npm run lint
cd admin && npm run lint

# Unit tests
cd api && npm test
cd admin && npm test

# Integration tests (if applicable)
cd api && npm run test:integration

# Build
cd api && npm run build
cd admin && npm run build

All checks must pass before submitting PR.

3. Update Documentation

If your changes affect:

• API endpoints: Update API reference
• User features: Update user guides
• Environment variables: Update .env.example
• Database schema: Update database docs
• Configuration: Update deployment guides

!!! warning "Documentation is Required" PRs with new features will not be merged without corresponding documentation updates.

PR Title Format

Use Conventional Commits format:

type(scope): short description

Types

Type	When to Use
feat	New feature for users
fix	Bug fix
docs	Documentation changes
style	Code formatting (no behavior change)
refactor	Code restructuring (no behavior change)
perf	Performance improvements
test	Test additions or fixes
chore	Build process, tooling, dependencies
ci	CI/CD configuration
revert	Reverting a previous commit

Scopes

Common scopes by area:

Backend:

• api - General API changes
• auth - Authentication/authorization
• campaigns - Campaign module
• locations - Location module
• shifts - Shift module
• canvass - Canvassing module
• email - Email sending
• database - Database schema/migrations

Frontend:

• admin - Admin pages
• public - Public pages
• volunteer - Volunteer portal
• components - React components
• store - Zustand stores
• ui - UI/UX changes

Infrastructure:

• docker - Docker/Docker Compose
• nginx - Nginx configuration
• monitoring - Prometheus/Grafana
• deployment - Deployment scripts/config

Examples

Good titles:

• ✅ feat(campaigns): add campaign export to CSV
• ✅ fix(geocoding): handle null responses from Nominatim
• ✅ docs(api): document campaign endpoints
• ✅ refactor(auth): extract JWT middleware to separate file
• ✅ perf(locations): add database index on postalCode

Bad titles:

• ❌ Update campaigns.tsx (too vague)
• ❌ Bug fix (no scope or description)
• ❌ WIP: New feature (don't submit WIP PRs)
• ❌ Fixed everything (not descriptive)

PR Description Template

Use this template for your PR description:

## What

[Clear description of what this PR does]

## Why

[Why this change is needed, link to issue]

## How

[Brief explanation of implementation approach]

## Testing

[How to test these changes]

## Screenshots

[For UI changes, include before/after screenshots]

## Checklist

- [ ] Tests added/updated
- [ ] Documentation updated
- [ ] No console errors
- [ ] All CI checks pass
- [ ] Follows code style guidelines

Fixes #[issue-number]

Example PR Description

## What

Adds a CSV export button to the campaigns page that allows admins to download all campaigns with their metadata.

## Why

Users need to export campaign data for reporting and analysis in external tools like Excel.

Fixes #456

## How

- Added export button to CampaignsPage header
- Created `/api/influence/campaigns/export` endpoint
- Implemented CSV generation using `csv-stringify` library
- Added SUPER_ADMIN/INFLUENCE_ADMIN role check

## Testing

1. Login as admin user
2. Navigate to Campaigns page (/app/influence/campaigns)
3. Click "Export CSV" button in page header
4. Verify CSV file downloads with correct data
5. Open CSV in Excel/Google Sheets to verify formatting

## Screenshots

![Export button in campaigns page](https://user-images.githubusercontent.com/export-button.png)

## Checklist

- [x] Tests added (export endpoint integration test)
- [x] Documentation updated (API reference, admin guide)
- [x] No console errors
- [x] All CI checks pass
- [x] Follows code style guidelines

Fixes #456

Creating the PR

1. Push Your Branch

# Ensure your branch is up to date
git fetch upstream
git rebase upstream/v2  # or: git merge upstream/v2

# Push to your fork
git push origin feature/your-feature-name

# If you rebased, force push (with care!)
git push --force-with-lease origin feature/your-feature-name

2. Open PR on GitHub

1. Go to your fork on GitHub
2. Click "Pull requests" tab
3. Click "New pull request"
4. Base repository: changemaker-lite/v2 base: v2
5. Head repository: YOUR-USERNAME/changemaker-lite compare: feature/your-feature-name
6. Click "Create pull request"
7. Fill out the PR template (see above)
8. Click "Create pull request"

3. Request Reviewers

• PRs are automatically assigned to maintainers
• You can request specific reviewers if you know who to ask
• For urgent PRs, mention @changemaker-lite/maintainers

Code Review Process

Automated Checks

After submitting, CI/CD runs these checks:

1. Lint: ESLint rules
2. Type Check: TypeScript compilation
3. Tests: Unit + integration tests
4. Build: Production build
5. Security: Dependency vulnerability scan

Status badges appear on your PR:

• ✅ Green checkmark: All checks passed
• ❌ Red X: Some checks failed
• 🟡 Yellow dot: Checks in progress

Fix failing checks before requesting review.

Maintainer Review

A maintainer will review your code and provide feedback:

Review categories:

1. Code quality:

   • Follows code style guidelines
   • No unnecessary complexity
   • Proper error handling
   • No security vulnerabilities

2. Functionality:

   • Solves the problem correctly
   • Edge cases handled
   • No regressions

3. Tests:

   • Adequate test coverage (>80%)
   • Tests are meaningful
   • Tests pass consistently

4. Documentation:

   • Code comments for complex logic
   • API documentation updated
   • User guide updated (if needed)

Review Outcomes

Approved ✅:

• Maintainer approves PR
• Ready to merge (after squash)

Request Changes 🔄:

• Maintainer requests modifications
• Address feedback and push new commits
• Re-request review after changes

Comment 💬:

• Feedback without blocking merge
• Optional to address

Addressing Feedback

1. Read Feedback Carefully

• Understand the requested change
• Ask clarifying questions if unclear
• Don't take criticism personally (it's about code, not you)

2. Make Changes

# Make requested changes
# Edit files...

# Commit changes
git add .
git commit -m "refactor: address review feedback

- Extracted duplicate logic into helper function
- Added error handling for edge case
- Updated tests to cover new scenario"

# Push to same branch
git push origin feature/your-feature-name

Commits are added to existing PR automatically.

3. Respond to Comments

• Acknowledge feedback: "Good catch, fixed in abc1234"
• Explain changes: "Refactored this to use a switch statement instead"
• Ask questions: "I'm not sure how to handle X, suggestions?"
• Mark resolved: Click "Resolve conversation" after addressing

4. Re-Request Review

After addressing all feedback:

1. Click "Reviewers" section
2. Click circular arrow next to reviewer's name
3. Or comment @reviewer Ready for re-review

Common Review Feedback

Code Quality Issues

Issue: Large function with too many responsibilities

Feedback:

This function is doing too much. Can you extract the geocoding logic into a separate function?

Fix:

// Before
async function createLocation(data) {
  // 50 lines of validation, geocoding, database insert...
}

// After
async function createLocation(data) {
  const validated = validateLocationData(data);
  const geocoded = await geocodeAddress(validated.address);
  return insertLocation({ ...validated, ...geocoded });
}

Issue: Magic numbers or strings

Feedback:

What does 30 represent here? Use a named constant.

Fix:

// Before
if (visits.length >= 30) { }

// After
const VISIT_RATE_LIMIT = 30;
if (visits.length >= VISIT_RATE_LIMIT) { }

Issue: Missing error handling

Feedback:

What happens if the API call fails? Add error handling.

Fix:

// Before
const reps = await fetch(url).then(r => r.json());

// After
try {
  const response = await fetch(url);
  if (!response.ok) {
    throw new Error(`API returned ${response.status}`);
  }
  const reps = await response.json();
} catch (error) {
  logger.error('Failed to fetch representatives', error);
  throw new Error('Unable to lookup representatives');
}

Test Coverage Issues

Issue: Missing test for edge case

Feedback:

Add a test for when postal code is invalid.

Fix:

it('should return 400 for invalid postal code', async () => {
  const response = await request(app)
    .post('/api/influence/representatives/lookup')
    .send({ postalCode: 'INVALID' });

  expect(response.status).toBe(400);
  expect(response.body.success).toBe(false);
});

Documentation Issues

Issue: Missing API documentation

Feedback:

Add this endpoint to the API reference docs.

Fix: Update docs/v2/api-reference/campaigns.md with new endpoint.

Performance Issues

Issue: N+1 query problem

Feedback:

This causes N+1 queries. Use Prisma include to join.

Fix:

// Before (N+1)
const campaigns = await prisma.campaign.findMany();
for (const campaign of campaigns) {
  campaign.createdBy = await prisma.user.findUnique({ where: { id: campaign.createdByUserId } });
}

// After (single query)
const campaigns = await prisma.campaign.findMany({
  include: { createdBy: true }
});

Merge Process

Squash and Merge

Changemaker Lite uses squash and merge for all PRs:

1. Maintainer clicks "Squash and merge"
2. All commits in PR are squashed into one commit
3. Commit message = PR title + description summary
4. Merged to v2 branch

Why squash?

• Clean linear history
• Easier to revert if needed
• No messy "WIP" or "fix typo" commits

After Merge

Once your PR is merged:

1. Celebrate! 🎉 You've contributed to Changemaker Lite
2. Update your fork:

   git checkout v2
   git pull upstream v2
   git push origin v2
   

3. Delete feature branch (optional):

   git branch -d feature/your-feature-name
   git push origin --delete feature/your-feature-name
   

4. Update issue: GitHub auto-closes issue with Fixes #N
5. Check release notes: Your contribution will be mentioned in next release

PR Checklist

Use this before submitting:

Pre-Submission

• Issue created and approved by maintainer
• Branch created from latest v2
• Changes implemented following code style
• Self-review - read your own code critically
• Manual testing - verify changes work as expected

Code Quality

• TypeScript: No type errors (npx tsc --noEmit)
• Linting: No lint errors (npm run lint)
• Formatting: Code formatted (npm run format)
• No console logs: Remove debug statements
• No commented code: Remove old code
• Error handling: All errors caught and logged

Tests

• Unit tests: Added/updated tests
• Tests pass: npm test succeeds
• Coverage: Maintained or improved (>80%)
• Integration tests: Added if needed
• Edge cases: Tested invalid inputs

Documentation

• Code comments: Complex logic documented
• API docs: New endpoints documented
• User docs: User guide updated (if user-facing)
• README: Updated if needed
• .env.example: New env vars added

UI (if applicable)

• Responsive: Works on mobile/tablet/desktop
• Accessibility: Keyboard navigation works
• Browser testing: Works in Chrome, Firefox, Safari
• Loading states: Spinners for async operations
• Error states: Error messages shown to user
• Screenshots: Included in PR description

Final Checks

• CI passing: All automated checks green
• PR template: Description complete
• Commit messages: Follow conventional commits
• No merge conflicts: Branch rebased/merged with v2
• Reviewers requested: Maintainers notified

Troubleshooting PRs

CI Checks Failing

Lint failures:

cd api && npm run lint:fix
cd admin && npm run lint:fix
git add . && git commit -m "chore: fix lint errors" && git push

Type errors:

cd api && npx tsc --noEmit  # Shows errors
# Fix type errors in code
git add . && git commit -m "fix: resolve type errors" && git push

Test failures:

cd api && npm test  # Run locally to see errors
# Fix failing tests
git add . && git commit -m "test: fix failing tests" && git push

Merge Conflicts

Resolving conflicts:

# Fetch latest upstream
git fetch upstream

# Rebase onto v2
git rebase upstream/v2

# If conflicts, resolve them
# Edit conflicted files, then:
git add .
git rebase --continue

# Force push (since history changed)
git push --force-with-lease origin feature/your-feature-name

PR Not Getting Reviewed

If no review after 5 business days:

1. Check CI: Ensure all checks pass
2. Ping maintainer: Comment "@changemaker-lite/maintainers Friendly ping for review"
3. Join Discord: Ask in #contributors channel
4. Email: dev@cmlite.org for urgent PRs

Reasons for delays:

• Maintainers busy with other priorities
• PR too large (break into smaller PRs)
• Missing context (add more details to description)
• Waiting on related PRs to merge first

Related Documentation

• Contributing Guide - Overview
• Development Setup - Environment setup
• Code of Conduct - Community standards
• Roadmap - Future plans

Questions?

• Discord: #contributors channel
• Discussions: Ask in Q&A
• Email: dev@cmlite.org

Thank you for contributing! Every PR helps make Changemaker Lite better. 🚀