# ThrillWiki Codebase Quality Review

**Date:** January 2026
**Scope:** Full-stack analysis (Django backend, frontend, infrastructure, tests)

---

## Executive Summary

This codebase is a **well-architected Django 5.2 application** with HTMX/Alpine.js frontend, PostgreSQL/PostGIS database, Redis caching, and Celery task queue. The project demonstrates strong engineering fundamentals but has accumulated technical debt in several areas that, if addressed, would significantly improve maintainability, performance, and security.

### Overall Assessment

| Area | Score | Notes |
|------|-------|-------|
| Architecture | ⭐⭐⭐⭐ | Well-organized modular Django apps |
| Code Quality | ⭐⭐⭐ | Good patterns but inconsistencies exist |
| Security | ⭐⭐⭐ | Solid foundation with some XSS risks |
| Performance | ⭐⭐⭐ | Good caching but N+1 queries present |
| Testing | ⭐⭐⭐ | 70% coverage with gaps |
| Frontend | ⭐⭐⭐ | Clean JS but no tooling/types |
| Infrastructure | ⭐⭐⭐⭐ | Comprehensive CI/CD and deployment |

---

## 🔴 Critical Issues (Fix Immediately)

### 1. XSS Vulnerabilities in Admin Panel

**Location:** `backend/apps/moderation/admin.py`

```python
# Line 228 - changes_preview() method
return mark_safe("".join(html))  # User data not escaped!

# Line 740 - context_preview() method
return mark_safe("".join(html))  # Context data not escaped!
```

**Risk:** Attackers could inject malicious JavaScript through edit submissions.

**Fix:**
```python
from django.utils.html import escape

# In changes_preview():
html.append(f"<td>{escape(str(old))}</td><td>{escape(str(new))}</td>")
```

### 2. Debug Print Statements in Production Code

**Location:** `backend/apps/parks/models/parks.py:375-426`

```python
print(f"\nLooking up slug: {slug}")  # DEBUG CODE IN PRODUCTION
print(f"Found current park with slug: {slug}")
print(f"Checking historical slugs...")
```

**Fix:** Remove or convert to `logging.debug()`.

### 3. Mass Assignment Vulnerability in Serializers

**Location:** `backend/apps/api/v1/accounts/serializers.py`

```python
class UserProfileUpdateInputSerializer(serializers.ModelSerializer):
    class Meta:
        model = UserProfile
        fields = "__all__"  # DANGEROUS - exposes all fields for update
```

**Fix:** Explicitly list allowed fields:
```python
fields = ["display_name", "bio", "location", "website", "social_links"]
```

### 4. N+1 Query in Trip Planning Views

**Location:** `backend/apps/parks/views.py:577-583, 635-639, 686-690`

```python
# Current (N+1 problem - one query per park):
for tid in _get_session_trip(request):
    try:
        parks.append(Park.objects.get(id=tid))
    except Park.DoesNotExist:
        continue

# Fix (single query):
park_ids = _get_session_trip(request)
parks = list(Park.objects.filter(id__in=park_ids))
```

---

## 🟠 High Priority Issues

### 5. Fat Models with Business Logic

The following models have 200+ lines of business logic that should be extracted to services:

| Model | Location | Lines | Issue |
|-------|----------|-------|-------|
| `Park` | `parks/models/parks.py` | 220-428 | FSM transitions, slug resolution, computed fields |
| `EditSubmission` | `moderation/models.py` | 76-371 | Full approval workflow |
| `PhotoSubmission` | `moderation/models.py` | 668-903 | Photo approval workflow |

**Recommendation:** Create service classes:
```
apps/parks/services/
├── park_service.py          # FSM transitions, computed fields
├── slug_service.py          # Historical slug resolution
└── ...

apps/moderation/services/
├── submission_service.py    # EditSubmission workflow
├── photo_service.py         # PhotoSubmission workflow
└── ...
```

### 6. Missing Database Indexes

**Critical indexes to add:**

```python
# ParkPhoto - No index on frequently-filtered FK
class ParkPhoto(models.Model):
    park = models.ForeignKey(Park, on_delete=models.CASCADE, db_index=True)  # ADD db_index

# UserNotification - Missing composite index
class Meta:
    indexes = [
        models.Index(fields=["user", "created_at"]),  # ADD for sorting
    ]

# RideCredit - Missing index for ordering
class Meta:
    indexes = [
        models.Index(fields=["user", "display_order"]),  # ADD
    ]

# Company - Missing status filter index
class Meta:
    indexes = [
        models.Index(fields=["status", "founded_year"]),  # ADD
    ]
```

### 7. Inconsistent API Response Formats

**Current state (3+ different formats):**

```python
# Format 1: rides endpoint
{"rides": [...], "total_count": X, "strategy": "...", "has_more": bool}

# Format 2: parks endpoint
{"parks": [...], "total_count": X, "strategy": "..."}

# Format 3: DRF paginator
{"results": [...], "count": X, "next": "...", "previous": "..."}

# Format 4: Success responses
{"success": True, "data": {...}}  # vs
{"detail": "Success message"}     # vs
{"message": "Success"}
```

**Recommendation:** Create a standard response wrapper:

```python
# apps/core/api/responses.py
class StandardResponse:
    @staticmethod
    def success(data=None, message=None, meta=None):
        return {
            "success": True,
            "data": data,
            "message": message,
            "meta": meta  # pagination, counts, etc.
        }

    @staticmethod
    def error(message, code=None, details=None):
        return {
            "success": False,
            "error": {
                "message": message,
                "code": code,
                "details": details
            }
        }
```

### 8. Overly Broad Exception Handling

**Pattern found 15+ times:**

```python
# BAD - masks actual errors
try:
    queryset = self.apply_filters(queryset)
except Exception as e:
    log_exception(e)
    return Park.objects.none()  # Silent failure!
```

**Fix:** Catch specific exceptions:

```python
from django.core.exceptions import ValidationError
from django.db import DatabaseError

try:
    queryset = self.apply_filters(queryset)
except ValidationError as e:
    messages.warning(request, f"Invalid filter: {e}")
    return base_queryset
except DatabaseError as e:
    logger.error("Database error in filter", exc_info=True)
    raise  # Let it bubble up or return error response
```

### 9. Duplicated Permission Checks

**Found in 6+ locations:**

```python
# Repeated pattern in views:
if not (request.user == instance.uploaded_by or request.user.is_staff):
    raise PermissionDenied()
```

**Fix:** Create reusable permission class:

```python
# apps/core/permissions.py
class IsOwnerOrStaff(permissions.BasePermission):
    def has_object_permission(self, request, view, obj):
        if request.method in permissions.SAFE_METHODS:
            return True
        owner_field = getattr(view, 'owner_field', 'user')
        owner = getattr(obj, owner_field, None)
        return owner == request.user or request.user.is_staff
```

---

## 🟡 Medium Priority Issues

### 10. Frontend Has No Build Tooling

**Current state:**
- No `package.json` or npm dependencies
- No TypeScript (vanilla JS only)
- No ESLint/Prettier configuration
- No minification/bundling pipeline
- No source maps for debugging

**Impact:**
- No type safety in 8,000+ lines of JavaScript
- Manual debugging without source maps
- No automated code quality checks

**Recommendation:** Add minimal tooling:

```json
// package.json
{
  "scripts": {
    "lint": "eslint backend/static/js/",
    "format": "prettier --write 'backend/static/js/**/*.js'",
    "typecheck": "tsc --noEmit"
  },
  "devDependencies": {
    "eslint": "^8.0.0",
    "prettier": "^3.0.0",
    "typescript": "^5.0.0"
  }
}
```

### 11. Test Coverage Gaps

**Disabled tests (technical debt):**
- `tests_disabled/test_models.py` - Park model tests
- `tests_disabled/test_filters.py` - Filter tests
- `tests_disabled/test_search.py` - Search/autocomplete tests

**Missing test coverage:**
- Celery async tasks (not tested)
- Cache hit/miss behavior
- Concurrent operations/race conditions
- Performance benchmarks
- Component-level accessibility

**Recommendation:**
1. Re-enable disabled tests with updated model references
2. Add Celery task tests with `CELERY_TASK_ALWAYS_EAGER = True`
3. Implement Page Object Model for E2E tests

### 12. Celery Configuration Issues

**Location:** `backend/config/celery.py`

```python
# Issue 1: No retry policy visible
# Tasks that fail don't have automatic retry with backoff

# Issue 2: Beat schedule lacks jitter
# All daily tasks run at midnight - thundering herd problem
CELERYBEAT_SCHEDULE = {
    "daily-ban-expiry": {"schedule": crontab(hour=0, minute=0)},
    "daily-deletion-processing": {"schedule": crontab(hour=0, minute=0)},
    "daily-closing-checks": {"schedule": crontab(hour=0, minute=0)},
    # All at midnight!
}
```

**Fix:**

```python
# Add retry policy to tasks
@app.task(bind=True, max_retries=3, default_retry_delay=60)
def process_submission(self, submission_id):
    try:
        # ... task logic
    except TransientError as e:
        raise self.retry(exc=e, countdown=60 * (2 ** self.request.retries))

# Stagger beat schedule
CELERYBEAT_SCHEDULE = {
    "daily-ban-expiry": {"schedule": crontab(hour=0, minute=0)},
    "daily-deletion-processing": {"schedule": crontab(hour=0, minute=15)},
    "daily-closing-checks": {"schedule": crontab(hour=0, minute=30)},
}
```

### 13. Rate Limiting Only on Auth Endpoints

**Location:** `backend/apps/core/middleware/rate_limiting.py`

```python
RATE_LIMITED_PATHS = {
    "/api/v1/auth/login/": {...},
    "/api/v1/auth/signup/": {...},
    # Missing: file uploads, form submissions, search endpoints
}
```

**Recommendation:** Extend rate limiting:

```python
RATE_LIMITED_PATHS = {
    # Auth
    "/api/v1/auth/login/": {"per_minute": 5, "per_hour": 30},
    "/api/v1/auth/signup/": {"per_minute": 3, "per_hour": 10},
    "/api/v1/auth/password-reset/": {"per_minute": 3, "per_hour": 10},

    # File uploads
    "/api/v1/photos/upload/": {"per_minute": 10, "per_hour": 100},

    # Search (prevent abuse)
    "/api/v1/search/": {"per_minute": 30, "per_hour": 500},

    # Submissions
    "/api/v1/submissions/": {"per_minute": 5, "per_hour": 50},
}
```

### 14. Inconsistent Status Field Implementations

**Three different patterns used:**

```python
# Pattern 1: RichFSMField (Park)
status = RichFSMField(default=ParkStatus.OPERATING, ...)

# Pattern 2: CharField with choices (Company)
status = models.CharField(max_length=20, choices=STATUS_CHOICES, ...)

# Pattern 3: RichChoiceField (User role)
role = RichChoiceField(choices=UserRole.choices, ...)
```

**Recommendation:** Standardize on one approach (RichFSMField for state machines, RichChoiceField for enums).

### 15. Magic Numbers Throughout Code

**Examples found:**

```python
# auth/views.py
get_random_string(64)  # Why 64?
timeout=300  # Why 300 seconds?
MAX_AVATAR_SIZE = 10 * 1024 * 1024  # Inline constant

# Various files
page_size = 20  # vs 24 in other places
```

**Fix:** Create constants module:

```python
# apps/core/constants.py
class Security:
    MFA_TOKEN_LENGTH = 64
    MFA_TOKEN_TIMEOUT_SECONDS = 300
    MAX_AVATAR_SIZE_BYTES = 10 * 1024 * 1024

class Pagination:
    DEFAULT_PAGE_SIZE = 20
    MAX_PAGE_SIZE = 100
```

---

## 🟢 Low Priority / Nice-to-Have

### 16. Deprecated Code Not Removed

**Location:** `backend/static/js/moderation/history.js`
- File marked as DEPRECATED but still present
- Should be removed or migrated

### 17. Unused Imports

Multiple files have duplicate or unused imports:
- `backend/apps/api/v1/rides/views.py` - `Q` imported twice

### 18. Missing Docstrings on Complex Methods

Many service methods and complex views lack docstrings explaining:
- Expected inputs/outputs
- Business rules applied
- Side effects

### 19. Template `|safe` Filter Usage

**Files using `|safe` that should use `|sanitize`:**
- `templates/components/ui/icon.html:61`
- `templates/components/navigation/breadcrumbs.html:116`

---

## Architecture Recommendations

### 1. Adopt Service Layer Pattern Consistently

```
apps/
├── parks/
│   ├── models/           # Data models only
│   ├── services/         # Business logic
│   │   ├── park_service.py
│   │   ├── slug_service.py
│   │   └── media_service.py
│   ├── selectors/        # Read queries (already exists)
│   └── api/              # Serializers, viewsets
```

### 2. Create Shared Response/Error Handling

```python
# apps/core/api/
├── responses.py          # StandardResponse class
├── exceptions.py         # Custom exceptions with codes
├── error_handlers.py     # DRF exception handler
└── mixins.py            # Reusable view mixins
```

### 3. Implement Repository Pattern for Complex Queries

```python
# apps/parks/repositories/park_repository.py
class ParkRepository:
    @staticmethod
    def get_by_slug_with_history(slug: str) -> Park | None:
        """Resolve slug including historical slugs."""
        # Move 60+ lines from Park.get_by_slug() here
```

### 4. Add Event-Driven Architecture for Cross-App Communication

```python
# Instead of direct imports between apps:
from apps.parks.models import Park  # Tight coupling

# Use signals/events:
# apps/core/events.py
park_status_changed = Signal()

# apps/parks/services/park_service.py
park_status_changed.send(sender=Park, park=park, old_status=old, new_status=new)

# apps/notifications/handlers.py
@receiver(park_status_changed)
def notify_followers(sender, park, **kwargs):
    ...
```

---

## Performance Optimization Opportunities

### 1. Database Query Optimization

| Issue | Location | Impact |
|-------|----------|--------|
| N+1 in trip views | `parks/views.py:577` | High - loops with `.get()` |
| Missing indexes | Multiple models | Medium - slow filters |
| No query count monitoring | Production | Unknown query count |

### 2. Caching Strategy Improvements

```python
# Add cache key versioning
CACHE_VERSION = "v1"

def get_park_cache_key(park_id):
    return f"park:{CACHE_VERSION}:{park_id}"

# Add cache tags for invalidation
from django.core.cache import cache

def invalidate_park_caches(park_id):
    cache.delete_pattern(f"park:*:{park_id}:*")
```

### 3. Frontend Performance

- Add `loading="lazy"` to images below the fold
- Implement virtual scrolling for long lists
- Add service worker for offline capability

---

## Security Hardening Checklist

- [ ] Fix XSS in admin `mark_safe()` calls
- [ ] Replace `fields = "__all__"` in serializers
- [ ] Add rate limiting to file upload endpoints
- [ ] Review `|safe` template filter usage
- [ ] Add Content Security Policy headers
- [ ] Implement API request signing for sensitive operations
- [ ] Add audit logging for admin actions
- [ ] Review OAuth state management consistency

---

## Recommended Action Plan

### Phase 1: Critical Fixes (This Sprint)
1. Fix XSS vulnerabilities in admin
2. Remove debug print statements
3. Fix mass assignment in serializers
4. Fix N+1 queries in trip views
5. Add missing database indexes

### Phase 2: High Priority (Next 2 Sprints)
1. Extract business logic to services
2. Standardize API response format
3. Fix overly broad exception handling
4. Re-enable disabled tests
5. Add rate limiting to more endpoints

### Phase 3: Medium Priority (Next Quarter)
1. Add frontend build tooling
2. Implement TypeScript for type safety
3. Improve Celery configuration
4. Standardize status field patterns
5. Add comprehensive E2E tests

### Phase 4: Ongoing
1. Remove deprecated code
2. Add missing docstrings
3. Monitor and optimize queries
4. Security audits

---

## Conclusion

This codebase has a solid foundation with good architectural decisions (modular apps, service layer beginnings, comprehensive configuration). The main areas needing attention are:

1. **Security:** XSS vulnerabilities and mass assignment risks
2. **Performance:** N+1 queries and missing indexes
3. **Maintainability:** Fat models and inconsistent patterns
4. **Testing:** Re-enable disabled tests and expand coverage

Addressing the critical and high-priority issues would significantly improve code quality and reduce technical debt. The codebase is well-positioned for scaling with relatively minor refactoring efforts.