Add contributing guidelines, feature flag utilities, and route integrity tests
This commit is contained in:
126
CONTRIBUTING.md
Normal file
126
CONTRIBUTING.md
Normal file
@@ -0,0 +1,126 @@
|
|||||||
|
# Contributing to SaaS-PDF
|
||||||
|
|
||||||
|
## Safety Rules
|
||||||
|
|
||||||
|
These rules are **non-negotiable**. Every PR must comply.
|
||||||
|
|
||||||
|
### 1. Never Delete Existing Routes
|
||||||
|
|
||||||
|
All routes are registered in `frontend/src/config/routes.ts`. This file is the canonical source of truth. The route safety test (`routes.test.ts`) verifies that every registered route exists in `App.tsx`.
|
||||||
|
|
||||||
|
- **Adding a route:** Append to `routes.ts` → add `<Route>` in `App.tsx`
|
||||||
|
- **Removing a route:** ❌ NEVER. Deprecate by redirecting instead.
|
||||||
|
|
||||||
|
### 2. Never Modify Existing Working Tools
|
||||||
|
|
||||||
|
Each tool lives in its own file under `frontend/src/components/tools/`. Do not change a tool's:
|
||||||
|
|
||||||
|
- Public API (props, accepted file types, output format)
|
||||||
|
- Route path
|
||||||
|
- Backend endpoint it calls
|
||||||
|
|
||||||
|
If you need to change behavior, add a new option behind a feature flag.
|
||||||
|
|
||||||
|
### 3. Never Break Existing Tests
|
||||||
|
|
||||||
|
Run the full test suites before pushing:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Frontend
|
||||||
|
cd frontend && npx vitest run
|
||||||
|
|
||||||
|
# Backend
|
||||||
|
cd backend && python -m pytest tests/ -q
|
||||||
|
```
|
||||||
|
|
||||||
|
If a test fails after your change, **fix your code**, not the test — unless the test itself is wrong (and you explain why in the PR).
|
||||||
|
|
||||||
|
### 4. Add New Functionality in Isolated Modules
|
||||||
|
|
||||||
|
- New tools → new file under `components/tools/`
|
||||||
|
- New pages → new file under `pages/`
|
||||||
|
- New backend routes → new file or append to existing route file
|
||||||
|
- New services → new file under `services/` or `utils/`
|
||||||
|
|
||||||
|
Never add new logic inline to existing tool components. Keep changes isolated so they can be reverted independently.
|
||||||
|
|
||||||
|
### 5. Use Feature Flags When Needed
|
||||||
|
|
||||||
|
#### Backend
|
||||||
|
|
||||||
|
Feature flags are defined in `backend/config/__init__.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
FEATURE_EDITOR = os.getenv("FEATURE_EDITOR", "true").lower() == "true"
|
||||||
|
```
|
||||||
|
|
||||||
|
Check them in routes:
|
||||||
|
|
||||||
|
```python
|
||||||
|
from flask import current_app
|
||||||
|
if not current_app.config.get("FEATURE_EDITOR"):
|
||||||
|
return jsonify(error="Feature disabled"), 403
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Frontend
|
||||||
|
|
||||||
|
Feature flags are defined in `frontend/src/config/featureFlags.ts`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import { isFeatureEnabled } from '@/config/featureFlags';
|
||||||
|
|
||||||
|
if (isFeatureEnabled('EDITOR')) {
|
||||||
|
// render the tool
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Set via environment variables:
|
||||||
|
|
||||||
|
```env
|
||||||
|
VITE_FEATURE_EDITOR=true # enabled (default)
|
||||||
|
VITE_FEATURE_OCR=false # disabled
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Development Workflow
|
||||||
|
|
||||||
|
### Branch Naming
|
||||||
|
|
||||||
|
| Type | Pattern | Example |
|
||||||
|
|------|---------|---------|
|
||||||
|
| Feature | `feature/tool-name` | `feature/pdf-merger` |
|
||||||
|
| Fix | `fix/issue-description` | `fix/ocr-language-flag` |
|
||||||
|
| SEO | `feature/seo-*` | `feature/seo-content` |
|
||||||
|
|
||||||
|
### PR Checklist
|
||||||
|
|
||||||
|
- [ ] No existing routes removed (checked by `routes.test.ts`)
|
||||||
|
- [ ] No existing tool components modified (unless bug fix)
|
||||||
|
- [ ] All tests pass (`vitest run` + `pytest`)
|
||||||
|
- [ ] Build succeeds (`npx vite build`)
|
||||||
|
- [ ] New routes added to `routes.ts` registry
|
||||||
|
- [ ] New i18n keys added to all 3 language files (en, ar, fr)
|
||||||
|
- [ ] Feature flag added if the feature can be disabled
|
||||||
|
|
||||||
|
### File Structure Convention
|
||||||
|
|
||||||
|
```
|
||||||
|
frontend/src/
|
||||||
|
├── components/
|
||||||
|
│ ├── layout/ # Header, Footer, AdSlot
|
||||||
|
│ ├── shared/ # Reusable components (ToolCard, ErrorBoundary)
|
||||||
|
│ ├── seo/ # SEOHead, ToolLandingPage, FAQSection
|
||||||
|
│ └── tools/ # One file per tool (PdfToWord.tsx, etc.)
|
||||||
|
├── config/
|
||||||
|
│ ├── routes.ts # Canonical route registry (NEVER delete entries)
|
||||||
|
│ ├── featureFlags.ts # Frontend feature flag reader
|
||||||
|
│ ├── seoData.ts # SEO metadata for all tools
|
||||||
|
│ └── toolLimits.ts # File size limits
|
||||||
|
├── hooks/ # Custom React hooks
|
||||||
|
├── i18n/ # Translation files (en.json, ar.json, fr.json)
|
||||||
|
├── pages/ # Page components (HomePage, AboutPage, etc.)
|
||||||
|
├── services/ # API clients, analytics
|
||||||
|
├── stores/ # Zustand stores
|
||||||
|
└── utils/ # Pure utility functions (seo.ts, etc.)
|
||||||
|
```
|
||||||
23
frontend/src/config/featureFlags.ts
Normal file
23
frontend/src/config/featureFlags.ts
Normal file
@@ -0,0 +1,23 @@
|
|||||||
|
/**
|
||||||
|
* Feature flag utilities for the frontend.
|
||||||
|
*
|
||||||
|
* Feature flags are read from VITE_FEATURE_* environment variables.
|
||||||
|
* When a flag is absent or set to "true", the feature is ENABLED (opt-out model).
|
||||||
|
* Set a flag to "false" to disable a feature.
|
||||||
|
*
|
||||||
|
* Usage:
|
||||||
|
* import { isFeatureEnabled } from '@/config/featureFlags';
|
||||||
|
* if (isFeatureEnabled('EDITOR')) { ... }
|
||||||
|
*/
|
||||||
|
|
||||||
|
type FeatureName = 'EDITOR' | 'OCR' | 'REMOVEBG';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check whether a feature is enabled.
|
||||||
|
* Defaults to `true` if the env var is not set.
|
||||||
|
*/
|
||||||
|
export function isFeatureEnabled(feature: FeatureName): boolean {
|
||||||
|
const value = import.meta.env[`VITE_FEATURE_${feature}`];
|
||||||
|
if (value === undefined || value === '') return true; // enabled by default
|
||||||
|
return value.toLowerCase() !== 'false';
|
||||||
|
}
|
||||||
52
frontend/src/config/routes.test.ts
Normal file
52
frontend/src/config/routes.test.ts
Normal file
@@ -0,0 +1,52 @@
|
|||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { readFileSync } from 'fs';
|
||||||
|
import { resolve, dirname } from 'path';
|
||||||
|
import { fileURLToPath } from 'url';
|
||||||
|
import { ALL_ROUTES } from '@/config/routes';
|
||||||
|
|
||||||
|
const __filename = fileURLToPath(import.meta.url);
|
||||||
|
const __dirname = dirname(__filename);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* SAFETY TEST — Route Integrity
|
||||||
|
*
|
||||||
|
* Ensures that every route in the canonical registry (routes.ts)
|
||||||
|
* has a matching <Route path="..."> in App.tsx.
|
||||||
|
*
|
||||||
|
* If this test fails it means either:
|
||||||
|
* 1. A route was removed from App.tsx (NEVER do this)
|
||||||
|
* 2. A route was added to routes.ts but not yet wired in App.tsx
|
||||||
|
*/
|
||||||
|
describe('Route safety', () => {
|
||||||
|
const appSource = readFileSync(
|
||||||
|
resolve(__dirname, '../App.tsx'),
|
||||||
|
'utf-8'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Extract all path="..." values from <Route> elements
|
||||||
|
const routePathRegex = /path="([^"]+)"/g;
|
||||||
|
const appPaths = new Set<string>();
|
||||||
|
let match: RegExpExecArray | null;
|
||||||
|
while ((match = routePathRegex.exec(appSource)) !== null) {
|
||||||
|
if (match[1] !== '*') appPaths.add(match[1]);
|
||||||
|
}
|
||||||
|
|
||||||
|
it('App.tsx contains routes for every entry in the route registry', () => {
|
||||||
|
const missing = ALL_ROUTES.filter((r) => !appPaths.has(r));
|
||||||
|
expect(missing, `Missing routes in App.tsx: ${missing.join(', ')}`).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('route registry is not empty', () => {
|
||||||
|
expect(ALL_ROUTES.length).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('no duplicate routes in the registry', () => {
|
||||||
|
const seen = new Set<string>();
|
||||||
|
const duplicates: string[] = [];
|
||||||
|
for (const route of ALL_ROUTES) {
|
||||||
|
if (seen.has(route)) duplicates.push(route);
|
||||||
|
seen.add(route);
|
||||||
|
}
|
||||||
|
expect(duplicates, `Duplicate routes: ${duplicates.join(', ')}`).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
71
frontend/src/config/routes.ts
Normal file
71
frontend/src/config/routes.ts
Normal file
@@ -0,0 +1,71 @@
|
|||||||
|
/**
|
||||||
|
* Canonical route registry — single source of truth for all application routes.
|
||||||
|
*
|
||||||
|
* SAFETY RULE: Never remove a route from this list.
|
||||||
|
* New routes may only be appended. The route safety test
|
||||||
|
* (routes.test.ts) will fail if any existing route is deleted.
|
||||||
|
*/
|
||||||
|
|
||||||
|
// ─── Page routes ─────────────────────────────────────────────────
|
||||||
|
export const PAGE_ROUTES = [
|
||||||
|
'/',
|
||||||
|
'/about',
|
||||||
|
'/account',
|
||||||
|
'/forgot-password',
|
||||||
|
'/reset-password',
|
||||||
|
'/privacy',
|
||||||
|
'/terms',
|
||||||
|
'/contact',
|
||||||
|
] as const;
|
||||||
|
|
||||||
|
// ─── Tool routes ─────────────────────────────────────────────────
|
||||||
|
export const TOOL_ROUTES = [
|
||||||
|
// PDF Tools
|
||||||
|
'/tools/pdf-to-word',
|
||||||
|
'/tools/word-to-pdf',
|
||||||
|
'/tools/compress-pdf',
|
||||||
|
'/tools/merge-pdf',
|
||||||
|
'/tools/split-pdf',
|
||||||
|
'/tools/rotate-pdf',
|
||||||
|
'/tools/pdf-to-images',
|
||||||
|
'/tools/images-to-pdf',
|
||||||
|
'/tools/watermark-pdf',
|
||||||
|
'/tools/protect-pdf',
|
||||||
|
'/tools/unlock-pdf',
|
||||||
|
'/tools/page-numbers',
|
||||||
|
'/tools/pdf-editor',
|
||||||
|
'/tools/pdf-flowchart',
|
||||||
|
'/tools/pdf-to-excel',
|
||||||
|
'/tools/remove-watermark-pdf',
|
||||||
|
'/tools/reorder-pdf',
|
||||||
|
'/tools/extract-pages',
|
||||||
|
|
||||||
|
// Image Tools
|
||||||
|
'/tools/image-converter',
|
||||||
|
'/tools/image-resize',
|
||||||
|
'/tools/compress-image',
|
||||||
|
'/tools/ocr',
|
||||||
|
'/tools/remove-background',
|
||||||
|
|
||||||
|
// Convert Tools
|
||||||
|
'/tools/html-to-pdf',
|
||||||
|
|
||||||
|
// AI Tools
|
||||||
|
'/tools/chat-pdf',
|
||||||
|
'/tools/summarize-pdf',
|
||||||
|
'/tools/translate-pdf',
|
||||||
|
'/tools/extract-tables',
|
||||||
|
|
||||||
|
// Other Tools
|
||||||
|
'/tools/qr-code',
|
||||||
|
'/tools/video-to-gif',
|
||||||
|
'/tools/word-counter',
|
||||||
|
'/tools/text-cleaner',
|
||||||
|
] as const;
|
||||||
|
|
||||||
|
// ─── All routes combined ─────────────────────────────────────────
|
||||||
|
export const ALL_ROUTES = [...PAGE_ROUTES, ...TOOL_ROUTES] as const;
|
||||||
|
|
||||||
|
export type PageRoute = (typeof PAGE_ROUTES)[number];
|
||||||
|
export type ToolRoute = (typeof TOOL_ROUTES)[number];
|
||||||
|
export type AppRoute = (typeof ALL_ROUTES)[number];
|
||||||
Reference in New Issue
Block a user