thrillwiki_django_no_react/CODE_QUALITY_REVIEW.md

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

# 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:

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

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

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

Fix: Explicitly list allowed fields:

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

# 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:

# 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):

# 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:

# 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:

# 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:

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:

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

Fix: Create reusable permission class:

# 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:

// 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

# 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:

# 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

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

Recommendation: Extend rate limiting:

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:

# 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:

# 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:

# 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

# 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

# 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

# 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

# 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.