mirror of
https://github.com/pacnpal/thrillwiki_django_no_react.git
synced 2026-02-05 09:25:18 -05:00
Compare commits
2 Commits
claude/cod
...
b37aedf82e
| Author | SHA1 | Date | |
|---|---|---|---|
|
b37aedf82e | ||
|
|
fa570334fc |
@@ -1,592 +0,0 @@
|
||||
# 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.
|
||||
@@ -690,7 +690,8 @@ class SmartRideLoader:
|
||||
if category in category_labels:
|
||||
return category_labels[category]
|
||||
else:
|
||||
raise ValueError(f"Unknown ride category: {category}")
|
||||
# Return original value as fallback for unknown categories
|
||||
return category.replace("_", " ").title()
|
||||
|
||||
def _get_status_label(self, status: str) -> str:
|
||||
"""Convert status code to human-readable label."""
|
||||
@@ -707,7 +708,8 @@ class SmartRideLoader:
|
||||
if status in status_labels:
|
||||
return status_labels[status]
|
||||
else:
|
||||
raise ValueError(f"Unknown ride status: {status}")
|
||||
# Return original value as fallback for unknown statuses
|
||||
return status.replace("_", " ").title()
|
||||
|
||||
def _get_rc_type_label(self, rc_type: str) -> str:
|
||||
"""Convert roller coaster type to human-readable label."""
|
||||
@@ -729,7 +731,8 @@ class SmartRideLoader:
|
||||
if rc_type in rc_type_labels:
|
||||
return rc_type_labels[rc_type]
|
||||
else:
|
||||
raise ValueError(f"Unknown roller coaster type: {rc_type}")
|
||||
# Return original value as fallback for unknown types
|
||||
return rc_type.replace("_", " ").title()
|
||||
|
||||
def _get_track_material_label(self, material: str) -> str:
|
||||
"""Convert track material to human-readable label."""
|
||||
@@ -741,7 +744,8 @@ class SmartRideLoader:
|
||||
if material in material_labels:
|
||||
return material_labels[material]
|
||||
else:
|
||||
raise ValueError(f"Unknown track material: {material}")
|
||||
# Return original value as fallback for unknown materials
|
||||
return material.replace("_", " ").title()
|
||||
|
||||
def _get_propulsion_system_label(self, propulsion_system: str) -> str:
|
||||
"""Convert propulsion system to human-readable label."""
|
||||
@@ -759,4 +763,6 @@ class SmartRideLoader:
|
||||
if propulsion_system in propulsion_labels:
|
||||
return propulsion_labels[propulsion_system]
|
||||
else:
|
||||
raise ValueError(f"Unknown propulsion system: {propulsion_system}")
|
||||
# Return original value as fallback for unknown propulsion systems
|
||||
return propulsion_system.replace("_", " ").title()
|
||||
|
||||
|
||||
Binary file not shown.
@@ -36,12 +36,13 @@
|
||||
- main_class: Additional classes for <main> tag
|
||||
|
||||
Usage Example:
|
||||
{% extends "base/base.html" %}
|
||||
{% block title %}My Page - ThrillWiki{% endblock %}
|
||||
{% block content %}
|
||||
<h1>My Page Content</h1>
|
||||
{% endblock %}
|
||||
{% templatetag openblock %} extends "base/base.html" {% templatetag closeblock %}
|
||||
{% templatetag openblock %} block title {% templatetag closeblock %}My Page - ThrillWiki{% templatetag openblock %} endblock {% templatetag closeblock %}
|
||||
{% templatetag openblock %} block content {% templatetag closeblock %}
|
||||
<h1>My Page Content</h1>
|
||||
{% templatetag openblock %} endblock {% templatetag closeblock %}
|
||||
============================================================================= #}
|
||||
|
||||
<!DOCTYPE html>
|
||||
<html lang="en" class="h-full">
|
||||
<head>
|
||||
|
||||
@@ -20,17 +20,18 @@ from rest_framework.test import APIClient
|
||||
|
||||
from tests.factories import (
|
||||
CoasterFactory,
|
||||
DesignerCompanyFactory,
|
||||
ManufacturerCompanyFactory,
|
||||
ParkFactory,
|
||||
RideFactory,
|
||||
RideModelFactory,
|
||||
RidesDesignerFactory,
|
||||
RidesManufacturerFactory,
|
||||
StaffUserFactory,
|
||||
UserFactory,
|
||||
)
|
||||
from tests.test_utils import EnhancedAPITestCase
|
||||
|
||||
|
||||
|
||||
class TestRideListAPIView(EnhancedAPITestCase):
|
||||
"""Test cases for RideListCreateAPIView GET endpoint."""
|
||||
|
||||
@@ -38,8 +39,8 @@ class TestRideListAPIView(EnhancedAPITestCase):
|
||||
"""Set up test data."""
|
||||
self.client = APIClient()
|
||||
self.park = ParkFactory()
|
||||
self.manufacturer = ManufacturerCompanyFactory()
|
||||
self.designer = DesignerCompanyFactory()
|
||||
self.manufacturer = RidesManufacturerFactory()
|
||||
self.designer = RidesDesignerFactory()
|
||||
self.rides = [
|
||||
RideFactory(
|
||||
park=self.park,
|
||||
@@ -183,7 +184,7 @@ class TestRideCreateAPIView(EnhancedAPITestCase):
|
||||
self.user = UserFactory()
|
||||
self.staff_user = StaffUserFactory()
|
||||
self.park = ParkFactory()
|
||||
self.manufacturer = ManufacturerCompanyFactory()
|
||||
self.manufacturer = RidesManufacturerFactory()
|
||||
self.url = "/api/v1/rides/"
|
||||
|
||||
self.valid_ride_data = {
|
||||
@@ -373,7 +374,7 @@ class TestHybridRideAPIView(EnhancedAPITestCase):
|
||||
"""Set up test data."""
|
||||
self.client = APIClient()
|
||||
self.park = ParkFactory()
|
||||
self.manufacturer = ManufacturerCompanyFactory()
|
||||
self.manufacturer = RidesManufacturerFactory()
|
||||
self.rides = [
|
||||
RideFactory(park=self.park, manufacturer=self.manufacturer, status="OPERATING", category="RC"),
|
||||
RideFactory(park=self.park, status="OPERATING", category="DR"),
|
||||
@@ -386,10 +387,9 @@ class TestHybridRideAPIView(EnhancedAPITestCase):
|
||||
response = self.client.get(self.url)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertTrue(response.data.get("success", False))
|
||||
self.assertIn("data", response.data)
|
||||
self.assertIn("rides", response.data["data"])
|
||||
self.assertIn("total_count", response.data["data"])
|
||||
# API returns rides directly, not wrapped in success/data
|
||||
self.assertIn("rides", response.data)
|
||||
self.assertIn("total_count", response.data)
|
||||
|
||||
def test__hybrid_ride__with_category_filter__returns_filtered_rides(self):
|
||||
"""Test filtering by category."""
|
||||
@@ -420,7 +420,8 @@ class TestHybridRideAPIView(EnhancedAPITestCase):
|
||||
response = self.client.get(self.url, {"offset": 0})
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertIn("has_more", response.data["data"])
|
||||
# API returns has_more directly at top level
|
||||
self.assertIn("has_more", response.data)
|
||||
|
||||
def test__hybrid_ride__with_invalid_offset__returns_400(self):
|
||||
"""Test invalid offset parameter."""
|
||||
@@ -465,15 +466,15 @@ class TestRideFilterMetadataAPIView(EnhancedAPITestCase):
|
||||
def setUp(self):
|
||||
"""Set up test data."""
|
||||
self.client = APIClient()
|
||||
self.url = "/api/v1/rides/filter-metadata/"
|
||||
self.url = "/api/v1/rides/hybrid/filter-metadata/"
|
||||
|
||||
def test__filter_metadata__unscoped__returns_all_metadata(self):
|
||||
"""Test getting unscoped filter metadata."""
|
||||
response = self.client.get(self.url)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertTrue(response.data.get("success", False))
|
||||
self.assertIn("data", response.data)
|
||||
# API returns metadata directly, not wrapped in success/data
|
||||
self.assertIsInstance(response.data, dict)
|
||||
|
||||
def test__filter_metadata__scoped__returns_filtered_metadata(self):
|
||||
"""Test getting scoped filter metadata."""
|
||||
@@ -488,7 +489,7 @@ class TestCompanySearchAPIView(EnhancedAPITestCase):
|
||||
def setUp(self):
|
||||
"""Set up test data."""
|
||||
self.client = APIClient()
|
||||
self.manufacturer = ManufacturerCompanyFactory(name="Bolliger & Mabillard")
|
||||
self.manufacturer = RidesManufacturerFactory(name="Bolliger & Mabillard")
|
||||
self.url = "/api/v1/rides/search/companies/"
|
||||
|
||||
def test__company_search__with_query__returns_matching_companies(self):
|
||||
@@ -520,7 +521,7 @@ class TestRideModelSearchAPIView(EnhancedAPITestCase):
|
||||
"""Set up test data."""
|
||||
self.client = APIClient()
|
||||
self.ride_model = RideModelFactory(name="Hyper Coaster")
|
||||
self.url = "/api/v1/rides/search-ride-models/"
|
||||
self.url = "/api/v1/rides/search/ride-models/"
|
||||
|
||||
def test__ride_model_search__with_query__returns_matching_models(self):
|
||||
"""Test searching for ride models."""
|
||||
|
||||
@@ -743,18 +743,11 @@ def bulk_operation_pending(db):
|
||||
# =============================================================================
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def live_server(live_server_url):
|
||||
"""Provide the live server URL for tests.
|
||||
|
||||
Note: This fixture is provided by pytest-django. The live_server_url
|
||||
fixture provides the URL as a string.
|
||||
"""
|
||||
|
||||
class LiveServer:
|
||||
url = live_server_url
|
||||
|
||||
return LiveServer()
|
||||
# NOTE: The live_server fixture is provided by pytest-django.
|
||||
# It has a .url attribute that provides the server URL.
|
||||
# We previously had a custom wrapper here, but it broke because
|
||||
# it depended on a non-existent 'live_server_url' fixture.
|
||||
# The built-in live_server fixture already works correctly.
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
|
||||
Reference in New Issue
Block a user