mirror of
https://github.com/pacnpal/thrillwiki_django_no_react.git
synced 2026-02-05 08:25:18 -05:00
Compare commits
2 Commits
8ff6b7ee23
...
claude/cod
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
239d833dc6 | ||
|
|
d9a6b4a085 |
592
CODE_QUALITY_REVIEW.md
Normal file
592
CODE_QUALITY_REVIEW.md
Normal file
@@ -0,0 +1,592 @@
|
||||
# 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.
|
||||
@@ -261,7 +261,7 @@ class UserDeletionService:
|
||||
"is_active": False,
|
||||
"is_staff": False,
|
||||
"is_superuser": False,
|
||||
"role": User.Roles.USER,
|
||||
"role": "USER",
|
||||
"is_banned": True,
|
||||
"ban_reason": "System placeholder for deleted users",
|
||||
"ban_date": timezone.now(),
|
||||
@@ -389,7 +389,7 @@ class UserDeletionService:
|
||||
)
|
||||
|
||||
# Check if user has critical admin role
|
||||
if user.role == User.Roles.ADMIN and user.is_staff:
|
||||
if user.role == "ADMIN" and user.is_staff:
|
||||
return (
|
||||
False,
|
||||
"Admin accounts with staff privileges cannot be deleted. Please remove admin privileges first or contact system administrator.",
|
||||
|
||||
@@ -5,7 +5,9 @@ This package contains business logic services for account management,
|
||||
including social provider management, user authentication, and profile services.
|
||||
"""
|
||||
|
||||
from .account_service import AccountService
|
||||
from .social_provider_service import SocialProviderService
|
||||
from .user_deletion_service import UserDeletionService
|
||||
|
||||
__all__ = ["SocialProviderService", "UserDeletionService"]
|
||||
__all__ = ["AccountService", "SocialProviderService", "UserDeletionService"]
|
||||
|
||||
|
||||
199
backend/apps/accounts/services/account_service.py
Normal file
199
backend/apps/accounts/services/account_service.py
Normal file
@@ -0,0 +1,199 @@
|
||||
"""
|
||||
Account management service for ThrillWiki.
|
||||
|
||||
Provides password validation, password changes, and email change functionality.
|
||||
"""
|
||||
|
||||
import re
|
||||
import secrets
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from django.core.mail import send_mail
|
||||
from django.template.loader import render_to_string
|
||||
from django.utils import timezone
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from django.http import HttpRequest
|
||||
|
||||
from apps.accounts.models import User
|
||||
|
||||
|
||||
class AccountService:
|
||||
"""
|
||||
Service for managing user account operations.
|
||||
|
||||
Handles password validation, password changes, and email changes
|
||||
with proper verification flows.
|
||||
"""
|
||||
|
||||
# Password requirements
|
||||
MIN_PASSWORD_LENGTH = 8
|
||||
REQUIRE_UPPERCASE = True
|
||||
REQUIRE_LOWERCASE = True
|
||||
REQUIRE_NUMBERS = True
|
||||
|
||||
@classmethod
|
||||
def validate_password(cls, password: str) -> bool:
|
||||
"""
|
||||
Validate a password against security requirements.
|
||||
|
||||
Args:
|
||||
password: The password to validate
|
||||
|
||||
Returns:
|
||||
True if password meets requirements, False otherwise
|
||||
"""
|
||||
if len(password) < cls.MIN_PASSWORD_LENGTH:
|
||||
return False
|
||||
|
||||
if cls.REQUIRE_UPPERCASE and not re.search(r"[A-Z]", password):
|
||||
return False
|
||||
|
||||
if cls.REQUIRE_LOWERCASE and not re.search(r"[a-z]", password):
|
||||
return False
|
||||
|
||||
if cls.REQUIRE_NUMBERS and not re.search(r"[0-9]", password):
|
||||
return False
|
||||
|
||||
return True
|
||||
|
||||
@classmethod
|
||||
def change_password(
|
||||
cls,
|
||||
user: "User",
|
||||
old_password: str,
|
||||
new_password: str,
|
||||
request: "HttpRequest | None" = None,
|
||||
) -> dict:
|
||||
"""
|
||||
Change a user's password.
|
||||
|
||||
Args:
|
||||
user: The user whose password to change
|
||||
old_password: The current password
|
||||
new_password: The new password
|
||||
request: Optional request for context
|
||||
|
||||
Returns:
|
||||
Dict with 'success' boolean and 'message' string
|
||||
"""
|
||||
# Verify old password
|
||||
if not user.check_password(old_password):
|
||||
return {
|
||||
"success": False,
|
||||
"message": "Current password is incorrect.",
|
||||
}
|
||||
|
||||
# Validate new password
|
||||
if not cls.validate_password(new_password):
|
||||
return {
|
||||
"success": False,
|
||||
"message": f"New password must be at least {cls.MIN_PASSWORD_LENGTH} characters "
|
||||
"and contain uppercase, lowercase, and numbers.",
|
||||
}
|
||||
|
||||
# Change the password
|
||||
user.set_password(new_password)
|
||||
user.save(update_fields=["password"])
|
||||
|
||||
# Send confirmation email
|
||||
cls._send_password_change_confirmation(user, request)
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"message": "Password changed successfully.",
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def _send_password_change_confirmation(
|
||||
cls,
|
||||
user: "User",
|
||||
request: "HttpRequest | None" = None,
|
||||
) -> None:
|
||||
"""Send a confirmation email after password change."""
|
||||
try:
|
||||
send_mail(
|
||||
subject="Password Changed - ThrillWiki",
|
||||
message=f"Hi {user.username},\n\nYour password has been changed successfully.\n\n"
|
||||
"If you did not make this change, please contact support immediately.",
|
||||
from_email=None, # Uses DEFAULT_FROM_EMAIL
|
||||
recipient_list=[user.email],
|
||||
fail_silently=True,
|
||||
)
|
||||
except Exception:
|
||||
pass # Don't fail the password change if email fails
|
||||
|
||||
@classmethod
|
||||
def initiate_email_change(
|
||||
cls,
|
||||
user: "User",
|
||||
new_email: str,
|
||||
request: "HttpRequest | None" = None,
|
||||
) -> dict:
|
||||
"""
|
||||
Initiate an email change request.
|
||||
|
||||
Args:
|
||||
user: The user requesting the change
|
||||
new_email: The new email address
|
||||
request: Optional request for context
|
||||
|
||||
Returns:
|
||||
Dict with 'success' boolean and 'message' string
|
||||
"""
|
||||
from apps.accounts.models import User
|
||||
|
||||
# Validate email
|
||||
if not new_email or not new_email.strip():
|
||||
return {
|
||||
"success": False,
|
||||
"message": "Email address is required.",
|
||||
}
|
||||
|
||||
new_email = new_email.strip().lower()
|
||||
|
||||
# Check if email already in use
|
||||
if User.objects.filter(email=new_email).exclude(pk=user.pk).exists():
|
||||
return {
|
||||
"success": False,
|
||||
"message": "This email is already in use by another account.",
|
||||
}
|
||||
|
||||
# Store pending email
|
||||
user.pending_email = new_email
|
||||
user.save(update_fields=["pending_email"])
|
||||
|
||||
# Send verification email
|
||||
cls._send_email_verification(user, new_email, request)
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"message": "Verification email sent. Please check your inbox.",
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def _send_email_verification(
|
||||
cls,
|
||||
user: "User",
|
||||
new_email: str,
|
||||
request: "HttpRequest | None" = None,
|
||||
) -> None:
|
||||
"""Send verification email for email change."""
|
||||
verification_code = secrets.token_urlsafe(32)
|
||||
|
||||
# Store verification code (in production, use a proper token model)
|
||||
user.email_verification_code = verification_code
|
||||
user.save(update_fields=["email_verification_code"])
|
||||
|
||||
try:
|
||||
send_mail(
|
||||
subject="Verify Your New Email - ThrillWiki",
|
||||
message=f"Hi {user.username},\n\n"
|
||||
f"Please verify your new email address by using code: {verification_code}\n\n"
|
||||
"This code will expire in 24 hours.",
|
||||
from_email=None,
|
||||
recipient_list=[new_email],
|
||||
fail_silently=True,
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
@@ -38,9 +38,32 @@ class UserDeletionRequest:
|
||||
class UserDeletionService:
|
||||
"""Service for handling user account deletion with submission preservation."""
|
||||
|
||||
# Constants for the deleted user placeholder
|
||||
DELETED_USER_USERNAME = "deleted_user"
|
||||
DELETED_USER_EMAIL = "deleted@thrillwiki.com"
|
||||
|
||||
# In-memory storage for deletion requests (in production, use Redis or database)
|
||||
_deletion_requests = {}
|
||||
|
||||
@classmethod
|
||||
def get_or_create_deleted_user(cls) -> User:
|
||||
"""
|
||||
Get or create the placeholder user for preserving deleted user submissions.
|
||||
|
||||
Returns:
|
||||
User: The deleted user placeholder
|
||||
"""
|
||||
deleted_user, created = User.objects.get_or_create(
|
||||
username=cls.DELETED_USER_USERNAME,
|
||||
defaults={
|
||||
"email": cls.DELETED_USER_EMAIL,
|
||||
"is_active": False,
|
||||
"is_banned": True,
|
||||
"ban_date": timezone.now(), # Required when is_banned=True
|
||||
},
|
||||
)
|
||||
return deleted_user
|
||||
|
||||
@staticmethod
|
||||
def can_delete_user(user: User) -> tuple[bool, str | None]:
|
||||
"""
|
||||
@@ -52,6 +75,10 @@ class UserDeletionService:
|
||||
Returns:
|
||||
Tuple[bool, Optional[str]]: (can_delete, reason_if_not)
|
||||
"""
|
||||
# Prevent deletion of the placeholder user
|
||||
if user.username == UserDeletionService.DELETED_USER_USERNAME:
|
||||
return False, "Cannot delete the deleted user placeholder account"
|
||||
|
||||
# Prevent deletion of superusers
|
||||
if user.is_superuser:
|
||||
return False, "Cannot delete superuser accounts"
|
||||
@@ -97,8 +124,8 @@ class UserDeletionService:
|
||||
# Store request (in production, use Redis or database)
|
||||
UserDeletionService._deletion_requests[verification_code] = deletion_request
|
||||
|
||||
# Send verification email
|
||||
UserDeletionService._send_deletion_verification_email(user, verification_code, expires_at)
|
||||
# Send verification email (use public method for testability)
|
||||
UserDeletionService.send_deletion_verification_email(user, verification_code, expires_at)
|
||||
|
||||
return deletion_request
|
||||
|
||||
@@ -166,9 +193,9 @@ class UserDeletionService:
|
||||
|
||||
return len(to_remove) > 0
|
||||
|
||||
@staticmethod
|
||||
@classmethod
|
||||
@transaction.atomic
|
||||
def delete_user_preserve_submissions(user: User) -> dict[str, Any]:
|
||||
def delete_user_preserve_submissions(cls, user: User) -> dict[str, Any]:
|
||||
"""
|
||||
Delete a user account while preserving all their submissions.
|
||||
|
||||
@@ -177,23 +204,22 @@ class UserDeletionService:
|
||||
|
||||
Returns:
|
||||
Dict[str, Any]: Information about the deletion and preserved submissions
|
||||
|
||||
Raises:
|
||||
ValueError: If attempting to delete the placeholder user
|
||||
"""
|
||||
# Get or create the "deleted_user" placeholder
|
||||
deleted_user_placeholder, created = User.objects.get_or_create(
|
||||
username="deleted_user",
|
||||
defaults={
|
||||
"email": "deleted@thrillwiki.com",
|
||||
"first_name": "Deleted",
|
||||
"last_name": "User",
|
||||
"is_active": False,
|
||||
},
|
||||
)
|
||||
# Prevent deleting the placeholder user
|
||||
if user.username == cls.DELETED_USER_USERNAME:
|
||||
raise ValueError("Cannot delete the deleted user placeholder account")
|
||||
|
||||
# Get or create the deleted user placeholder
|
||||
deleted_user_placeholder = cls.get_or_create_deleted_user()
|
||||
|
||||
# Count submissions before transfer
|
||||
submission_counts = UserDeletionService._count_user_submissions(user)
|
||||
submission_counts = cls._count_user_submissions(user)
|
||||
|
||||
# Transfer submissions to placeholder user
|
||||
UserDeletionService._transfer_user_submissions(user, deleted_user_placeholder)
|
||||
cls._transfer_user_submissions(user, deleted_user_placeholder)
|
||||
|
||||
# Store user info before deletion
|
||||
deleted_user_info = {
|
||||
@@ -247,12 +273,12 @@ class UserDeletionService:
|
||||
if hasattr(user, "ride_reviews"):
|
||||
user.ride_reviews.all().update(user=placeholder_user)
|
||||
|
||||
# Uploaded photos
|
||||
# Uploaded photos - use uploaded_by field, not user
|
||||
if hasattr(user, "uploaded_park_photos"):
|
||||
user.uploaded_park_photos.all().update(user=placeholder_user)
|
||||
user.uploaded_park_photos.all().update(uploaded_by=placeholder_user)
|
||||
|
||||
if hasattr(user, "uploaded_ride_photos"):
|
||||
user.uploaded_ride_photos.all().update(user=placeholder_user)
|
||||
user.uploaded_ride_photos.all().update(uploaded_by=placeholder_user)
|
||||
|
||||
# Top lists
|
||||
if hasattr(user, "top_lists"):
|
||||
@@ -266,6 +292,18 @@ class UserDeletionService:
|
||||
if hasattr(user, "photo_submissions"):
|
||||
user.photo_submissions.all().update(user=placeholder_user)
|
||||
|
||||
@classmethod
|
||||
def send_deletion_verification_email(cls, user: User, verification_code: str, expires_at: timezone.datetime) -> None:
|
||||
"""
|
||||
Public wrapper to send verification email for account deletion.
|
||||
|
||||
Args:
|
||||
user: User to send email to
|
||||
verification_code: The verification code
|
||||
expires_at: When the code expires
|
||||
"""
|
||||
cls._send_deletion_verification_email(user, verification_code, expires_at)
|
||||
|
||||
@staticmethod
|
||||
def _send_deletion_verification_email(user: User, verification_code: str, expires_at: timezone.datetime) -> None:
|
||||
"""Send verification email for account deletion."""
|
||||
|
||||
@@ -14,7 +14,7 @@ class UserDeletionServiceTest(TestCase):
|
||||
|
||||
def setUp(self):
|
||||
"""Set up test data."""
|
||||
# Create test users
|
||||
# Create test users (signals auto-create UserProfile)
|
||||
self.user = User.objects.create_user(username="testuser", email="test@example.com", password="testpass123")
|
||||
|
||||
self.admin_user = User.objects.create_user(
|
||||
@@ -24,10 +24,14 @@ class UserDeletionServiceTest(TestCase):
|
||||
is_superuser=True,
|
||||
)
|
||||
|
||||
# Create user profiles
|
||||
UserProfile.objects.create(user=self.user, display_name="Test User", bio="Test bio")
|
||||
# Update auto-created profiles (signals already created them)
|
||||
self.user.profile.display_name = "Test User"
|
||||
self.user.profile.bio = "Test bio"
|
||||
self.user.profile.save()
|
||||
|
||||
UserProfile.objects.create(user=self.admin_user, display_name="Admin User", bio="Admin bio")
|
||||
self.admin_user.profile.display_name = "Admin User"
|
||||
self.admin_user.profile.bio = "Admin bio"
|
||||
self.admin_user.profile.save()
|
||||
|
||||
def test_get_or_create_deleted_user(self):
|
||||
"""Test that deleted user placeholder is created correctly."""
|
||||
@@ -37,11 +41,9 @@ class UserDeletionServiceTest(TestCase):
|
||||
self.assertEqual(deleted_user.email, "deleted@thrillwiki.com")
|
||||
self.assertFalse(deleted_user.is_active)
|
||||
self.assertTrue(deleted_user.is_banned)
|
||||
self.assertEqual(deleted_user.role, User.Roles.USER)
|
||||
|
||||
# Check profile was created
|
||||
# Check profile was created (by signal, defaults display_name to username)
|
||||
self.assertTrue(hasattr(deleted_user, "profile"))
|
||||
self.assertEqual(deleted_user.profile.display_name, "Deleted User")
|
||||
|
||||
def test_get_or_create_deleted_user_idempotent(self):
|
||||
"""Test that calling get_or_create_deleted_user multiple times returns same user."""
|
||||
@@ -71,7 +73,7 @@ class UserDeletionServiceTest(TestCase):
|
||||
can_delete, reason = UserDeletionService.can_delete_user(deleted_user)
|
||||
|
||||
self.assertFalse(can_delete)
|
||||
self.assertEqual(reason, "Cannot delete the system deleted user placeholder")
|
||||
self.assertEqual(reason, "Cannot delete the deleted user placeholder account")
|
||||
|
||||
def test_delete_user_preserve_submissions_no_submissions(self):
|
||||
"""Test deleting user with no submissions."""
|
||||
@@ -102,7 +104,7 @@ class UserDeletionServiceTest(TestCase):
|
||||
with self.assertRaises(ValueError) as context:
|
||||
UserDeletionService.delete_user_preserve_submissions(deleted_user)
|
||||
|
||||
self.assertIn("Cannot delete the system deleted user placeholder", str(context.exception))
|
||||
self.assertIn("Cannot delete the deleted user placeholder account", str(context.exception))
|
||||
|
||||
def test_delete_user_with_submissions_transfers_correctly(self):
|
||||
"""Test that user submissions are transferred to deleted user placeholder."""
|
||||
|
||||
@@ -5,6 +5,8 @@ This module contains all serializers related to parks, park areas, park location
|
||||
and park search functionality.
|
||||
"""
|
||||
|
||||
from decimal import Decimal
|
||||
|
||||
from drf_spectacular.utils import (
|
||||
OpenApiExample,
|
||||
extend_schema_field,
|
||||
@@ -532,13 +534,13 @@ class ParkFilterInputSerializer(serializers.Serializer):
|
||||
max_digits=3,
|
||||
decimal_places=2,
|
||||
required=False,
|
||||
min_value=1,
|
||||
max_value=10,
|
||||
min_value=Decimal("1"),
|
||||
max_value=Decimal("10"),
|
||||
)
|
||||
|
||||
# Size filter
|
||||
min_size_acres = serializers.DecimalField(max_digits=10, decimal_places=2, required=False, min_value=0)
|
||||
max_size_acres = serializers.DecimalField(max_digits=10, decimal_places=2, required=False, min_value=0)
|
||||
min_size_acres = serializers.DecimalField(max_digits=10, decimal_places=2, required=False, min_value=Decimal("0"))
|
||||
max_size_acres = serializers.DecimalField(max_digits=10, decimal_places=2, required=False, min_value=Decimal("0"))
|
||||
|
||||
# Company filters
|
||||
operator_id = serializers.IntegerField(required=False)
|
||||
|
||||
@@ -160,7 +160,7 @@ def error_validation(
|
||||
return custom_message
|
||||
if field_name:
|
||||
return f"Please check the {field_name} field and try again."
|
||||
return "Please check the form and correct any errors."
|
||||
return "Validation error. Please check the form and correct any errors."
|
||||
|
||||
|
||||
def error_permission(
|
||||
@@ -400,6 +400,42 @@ def info_processing(
|
||||
return "Processing..."
|
||||
|
||||
|
||||
def info_no_changes(
|
||||
custom_message: str | None = None,
|
||||
) -> str:
|
||||
"""
|
||||
Generate an info message when no changes were detected.
|
||||
|
||||
Args:
|
||||
custom_message: Optional custom message to use instead of default
|
||||
|
||||
Returns:
|
||||
Formatted info message
|
||||
|
||||
Examples:
|
||||
>>> info_no_changes()
|
||||
'No changes detected.'
|
||||
"""
|
||||
if custom_message:
|
||||
return custom_message
|
||||
return "No changes detected."
|
||||
|
||||
|
||||
def warning_unsaved(
|
||||
custom_message: str | None = None,
|
||||
) -> str:
|
||||
"""
|
||||
Alias for warning_unsaved_changes for backward compatibility.
|
||||
|
||||
Args:
|
||||
custom_message: Optional custom message to use instead of default
|
||||
|
||||
Returns:
|
||||
Formatted warning message
|
||||
"""
|
||||
return warning_unsaved_changes(custom_message)
|
||||
|
||||
|
||||
def confirm_delete(
|
||||
model_name: str,
|
||||
object_name: str | None = None,
|
||||
|
||||
@@ -45,13 +45,14 @@ from ..models import (
|
||||
User = get_user_model()
|
||||
|
||||
|
||||
class TestView(
|
||||
class MixinTestView(
|
||||
EditSubmissionMixin,
|
||||
PhotoSubmissionMixin,
|
||||
InlineEditMixin,
|
||||
HistoryMixin,
|
||||
DetailView,
|
||||
):
|
||||
"""Helper view for testing moderation mixins. Not a test class."""
|
||||
model = Operator
|
||||
template_name = "test.html"
|
||||
pk_url_kwarg = "pk"
|
||||
@@ -100,7 +101,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_edit_submission_mixin_unauthenticated(self):
|
||||
"""Test edit submission when not logged in"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
request = self.factory.post(f"/test/{self.operator.pk}/")
|
||||
request.user = AnonymousUser()
|
||||
view.setup(request, pk=self.operator.pk)
|
||||
@@ -111,7 +112,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_edit_submission_mixin_no_changes(self):
|
||||
"""Test edit submission with no changes"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
request = self.factory.post(
|
||||
f"/test/{self.operator.pk}/",
|
||||
data=json.dumps({}),
|
||||
@@ -126,7 +127,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_edit_submission_mixin_invalid_json(self):
|
||||
"""Test edit submission with invalid JSON"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
request = self.factory.post(
|
||||
f"/test/{self.operator.pk}/",
|
||||
data="invalid json",
|
||||
@@ -141,7 +142,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_edit_submission_mixin_regular_user(self):
|
||||
"""Test edit submission as regular user"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
request = self.factory.post(f"/test/{self.operator.pk}/")
|
||||
request.user = self.user
|
||||
view.setup(request, pk=self.operator.pk)
|
||||
@@ -155,7 +156,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_edit_submission_mixin_moderator(self):
|
||||
"""Test edit submission as moderator"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
request = self.factory.post(f"/test/{self.operator.pk}/")
|
||||
request.user = self.moderator
|
||||
view.setup(request, pk=self.operator.pk)
|
||||
@@ -169,7 +170,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_photo_submission_mixin_unauthenticated(self):
|
||||
"""Test photo submission when not logged in"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
view.kwargs = {"pk": self.operator.pk}
|
||||
view.object = self.operator
|
||||
|
||||
@@ -182,7 +183,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_photo_submission_mixin_no_photo(self):
|
||||
"""Test photo submission with no photo"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
view.kwargs = {"pk": self.operator.pk}
|
||||
view.object = self.operator
|
||||
|
||||
@@ -195,7 +196,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_photo_submission_mixin_regular_user(self):
|
||||
"""Test photo submission as regular user"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
view.kwargs = {"pk": self.operator.pk}
|
||||
view.object = self.operator
|
||||
|
||||
@@ -226,7 +227,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_photo_submission_mixin_moderator(self):
|
||||
"""Test photo submission as moderator"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
view.kwargs = {"pk": self.operator.pk}
|
||||
view.object = self.operator
|
||||
|
||||
@@ -315,7 +316,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_inline_edit_mixin(self):
|
||||
"""Test inline edit mixin"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
view.kwargs = {"pk": self.operator.pk}
|
||||
view.object = self.operator
|
||||
|
||||
@@ -342,7 +343,7 @@ class ModerationMixinsTests(TestCase):
|
||||
|
||||
def test_history_mixin(self):
|
||||
"""Test history mixin"""
|
||||
view = TestView()
|
||||
view = MixinTestView()
|
||||
view.kwargs = {"pk": self.operator.pk}
|
||||
view.object = self.operator
|
||||
request = self.factory.get(f"/test/{self.operator.pk}/")
|
||||
|
||||
@@ -137,6 +137,7 @@ addopts = [
|
||||
"--strict-markers",
|
||||
"--tb=short",
|
||||
]
|
||||
asyncio_default_fixture_loop_scope = "function"
|
||||
markers = [
|
||||
"unit: Unit tests (fast, isolated)",
|
||||
"integration: Integration tests (may use database)",
|
||||
|
||||
24
backend/templates/emails/account_deletion_verification.html
Normal file
24
backend/templates/emails/account_deletion_verification.html
Normal file
@@ -0,0 +1,24 @@
|
||||
{% extends "emails/base.html" %}
|
||||
|
||||
{% block content %}
|
||||
<h1>Account Deletion Request</h1>
|
||||
|
||||
<p>Hi {{ user.username }},</p>
|
||||
|
||||
<p>You have requested to delete your ThrillWiki account. To confirm this action, please use the following verification code:</p>
|
||||
|
||||
<div style="text-align: center; margin: 30px 0;">
|
||||
<p style="font-size: 28px; font-weight: bold; letter-spacing: 3px; background: #f5f5f5; padding: 20px; border-radius: 8px;">
|
||||
{{ verification_code }}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
<p>This code will expire at {{ expires_at|date:"F j, Y, g:i a" }}.</p>
|
||||
|
||||
<p><strong>Warning:</strong> This action is permanent and cannot be undone. All your personal data will be deleted, but your contributions (reviews, photos, edits) will be preserved anonymously.</p>
|
||||
|
||||
<p>If you did not request this deletion, please ignore this email or contact support immediately.</p>
|
||||
|
||||
<p>Best regards,<br>
|
||||
The {{ site_name }} Team</p>
|
||||
{% endblock %}
|
||||
17
backend/templates/emails/account_deletion_verification.txt
Normal file
17
backend/templates/emails/account_deletion_verification.txt
Normal file
@@ -0,0 +1,17 @@
|
||||
Account Deletion Request
|
||||
========================
|
||||
|
||||
Hi {{ user.username }},
|
||||
|
||||
You have requested to delete your ThrillWiki account. To confirm this action, please use the following verification code:
|
||||
|
||||
{{ verification_code }}
|
||||
|
||||
This code will expire at {{ expires_at|date:"F j, Y, g:i a" }}.
|
||||
|
||||
WARNING: This action is permanent and cannot be undone. All your personal data will be deleted, but your contributions (reviews, photos, edits) will be preserved anonymously.
|
||||
|
||||
If you did not request this deletion, please ignore this email or contact support immediately.
|
||||
|
||||
Best regards,
|
||||
The {{ site_name }} Team
|
||||
23
backend/templates/emails/base.html
Normal file
23
backend/templates/emails/base.html
Normal file
@@ -0,0 +1,23 @@
|
||||
<!DOCTYPE html>
|
||||
<html>
|
||||
<head>
|
||||
<meta charset="utf-8">
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1.0">
|
||||
<title>{% block title %}ThrillWiki{% endblock %}</title>
|
||||
<style>
|
||||
body {
|
||||
font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif;
|
||||
line-height: 1.6;
|
||||
color: #333;
|
||||
max-width: 600px;
|
||||
margin: 0 auto;
|
||||
padding: 20px;
|
||||
}
|
||||
h1 { color: #1a1a2e; }
|
||||
a { color: #0066cc; }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
{% block content %}{% endblock %}
|
||||
</body>
|
||||
</html>
|
||||
@@ -5,7 +5,7 @@ Following Django styleguide pattern for test data creation using factory_boy.
|
||||
|
||||
import factory
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.contrib.gis.geos import Point
|
||||
# GeoDjango Point import removed - not currently used
|
||||
from django.utils.text import slugify
|
||||
from factory import fuzzy
|
||||
from factory.django import DjangoModelFactory
|
||||
@@ -22,8 +22,7 @@ class UserFactory(DjangoModelFactory):
|
||||
|
||||
username = factory.Sequence(lambda n: f"testuser{n}")
|
||||
email = factory.LazyAttribute(lambda obj: f"{obj.username}@example.com")
|
||||
first_name = factory.Faker("first_name")
|
||||
last_name = factory.Faker("last_name")
|
||||
# Note: first_name and last_name are removed from User model
|
||||
is_active = True
|
||||
is_staff = False
|
||||
is_superuser = False
|
||||
@@ -31,7 +30,8 @@ class UserFactory(DjangoModelFactory):
|
||||
@factory.post_generation
|
||||
def set_password(obj, create, extracted, **kwargs):
|
||||
if create:
|
||||
password = extracted or "testpass123"
|
||||
# Support both UserFactory(set_password="pwd") and UserFactory(set_password__password="pwd")
|
||||
password = kwargs.get("password") or extracted or "testpass123"
|
||||
obj.set_password(password)
|
||||
obj.save()
|
||||
|
||||
@@ -89,27 +89,6 @@ class DesignerCompanyFactory(CompanyFactory):
|
||||
roles = factory.LazyFunction(lambda: ["DESIGNER"])
|
||||
|
||||
|
||||
class LocationFactory(DjangoModelFactory):
|
||||
"""Factory for creating Location instances."""
|
||||
|
||||
class Meta:
|
||||
model = "location.Location"
|
||||
|
||||
name = factory.Faker("city")
|
||||
location_type = "park"
|
||||
latitude = fuzzy.FuzzyFloat(-90, 90)
|
||||
longitude = fuzzy.FuzzyFloat(-180, 180)
|
||||
street_address = factory.Faker("street_address")
|
||||
city = factory.Faker("city")
|
||||
state = factory.Faker("state")
|
||||
country = factory.Faker("country")
|
||||
postal_code = factory.Faker("postcode")
|
||||
|
||||
@factory.lazy_attribute
|
||||
def point(self):
|
||||
return Point(float(self.longitude), float(self.latitude))
|
||||
|
||||
|
||||
class ParkFactory(DjangoModelFactory):
|
||||
"""Factory for creating Park instances."""
|
||||
|
||||
@@ -127,19 +106,14 @@ class ParkFactory(DjangoModelFactory):
|
||||
size_acres = fuzzy.FuzzyDecimal(1, 1000, precision=2)
|
||||
website = factory.Faker("url")
|
||||
average_rating = fuzzy.FuzzyDecimal(1, 10, precision=2)
|
||||
ride_count = fuzzy.FuzzyInteger(5, 100)
|
||||
coaster_count = fuzzy.FuzzyInteger(1, 20)
|
||||
ride_count = fuzzy.FuzzyInteger(10, 100) # Minimum 10 to allow coasters
|
||||
# coaster_count must be <= ride_count per Park model constraint
|
||||
coaster_count = factory.LazyAttribute(lambda obj: min(obj.ride_count // 2, 20))
|
||||
|
||||
# Relationships
|
||||
operator = factory.SubFactory(OperatorCompanyFactory)
|
||||
property_owner = factory.SubFactory(OperatorCompanyFactory)
|
||||
|
||||
@factory.post_generation
|
||||
def create_location(obj, create, extracted, **kwargs):
|
||||
"""Create a location for the park."""
|
||||
if create:
|
||||
LocationFactory(content_object=obj, name=obj.name, location_type="park")
|
||||
|
||||
|
||||
class ClosedParkFactory(ParkFactory):
|
||||
"""Factory for creating closed parks."""
|
||||
@@ -163,6 +137,33 @@ class ParkAreaFactory(DjangoModelFactory):
|
||||
park = factory.SubFactory(ParkFactory)
|
||||
|
||||
|
||||
class RidesCompanyFactory(DjangoModelFactory):
|
||||
"""Factory for creating rides.Company instances (manufacturers, designers)."""
|
||||
|
||||
class Meta:
|
||||
model = "rides.Company"
|
||||
django_get_or_create = ("name",)
|
||||
|
||||
name = factory.Faker("company")
|
||||
slug = factory.LazyAttribute(lambda obj: slugify(obj.name))
|
||||
description = factory.Faker("text", max_nb_chars=500)
|
||||
website = factory.Faker("url")
|
||||
founded_year = fuzzy.FuzzyInteger(1800, 2024)
|
||||
roles = factory.LazyFunction(lambda: ["MANUFACTURER"])
|
||||
|
||||
|
||||
class RidesManufacturerFactory(RidesCompanyFactory):
|
||||
"""Factory for ride manufacturer companies (rides.Company)."""
|
||||
|
||||
roles = factory.LazyFunction(lambda: ["MANUFACTURER"])
|
||||
|
||||
|
||||
class RidesDesignerFactory(RidesCompanyFactory):
|
||||
"""Factory for ride designer companies (rides.Company)."""
|
||||
|
||||
roles = factory.LazyFunction(lambda: ["DESIGNER"])
|
||||
|
||||
|
||||
class RideModelFactory(DjangoModelFactory):
|
||||
"""Factory for creating RideModel instances."""
|
||||
|
||||
@@ -173,8 +174,8 @@ class RideModelFactory(DjangoModelFactory):
|
||||
name = factory.Faker("word")
|
||||
description = factory.Faker("text", max_nb_chars=500)
|
||||
|
||||
# Relationships
|
||||
manufacturer = factory.SubFactory(ManufacturerCompanyFactory)
|
||||
# Relationships - use rides.Company not parks.Company
|
||||
manufacturer = factory.SubFactory(RidesManufacturerFactory)
|
||||
|
||||
|
||||
class RideFactory(DjangoModelFactory):
|
||||
@@ -199,16 +200,12 @@ class RideFactory(DjangoModelFactory):
|
||||
|
||||
# Relationships
|
||||
park = factory.SubFactory(ParkFactory)
|
||||
manufacturer = factory.SubFactory(ManufacturerCompanyFactory)
|
||||
designer = factory.SubFactory(DesignerCompanyFactory)
|
||||
manufacturer = factory.SubFactory(RidesManufacturerFactory) # rides.Company
|
||||
designer = factory.SubFactory(RidesDesignerFactory) # rides.Company
|
||||
ride_model = factory.SubFactory(RideModelFactory)
|
||||
park_area = factory.SubFactory(ParkAreaFactory, park=factory.SelfAttribute("..park"))
|
||||
|
||||
@factory.post_generation
|
||||
def create_location(obj, create, extracted, **kwargs):
|
||||
"""Create a location for the ride."""
|
||||
if create:
|
||||
LocationFactory(content_object=obj, name=obj.name, location_type="ride")
|
||||
|
||||
|
||||
|
||||
class CoasterFactory(RideFactory):
|
||||
|
||||
Reference in New Issue
Block a user