mirror of
https://github.com/pacnpal/thrillwiki_django_no_react.git
synced 2025-12-20 07:51:09 -05:00
Refactor parks and rides views for improved organization and readability
- Updated imports in parks/views.py to use ParkReview as Review for clarity. - Enhanced road trip views in parks/views_roadtrip.py by removing unnecessary parameters and improving context handling. - Streamlined error handling and response messages in CreateTripView and FindParksAlongRouteView. - Improved code formatting and consistency across various methods in parks/views_roadtrip.py. - Refactored rides/models.py to import Company from models for better clarity. - Updated rides/views.py to import RideSearchForm from services for better organization. - Added a comprehensive Django best practices analysis document to memory-bank/documentation.
This commit is contained in:
@@ -1,8 +1,7 @@
|
|||||||
from django.core.management.base import BaseCommand
|
from django.core.management.base import BaseCommand
|
||||||
from django.contrib.auth import get_user_model
|
from django.contrib.auth import get_user_model
|
||||||
from django.contrib.auth.models import Group
|
from django.contrib.auth.models import Group
|
||||||
from reviews.models import Review
|
from parks.models import Park, ParkReview as Review
|
||||||
from parks.models import Park
|
|
||||||
from rides.models import Ride
|
from rides.models import Ride
|
||||||
from media.models import Photo
|
from media.models import Photo
|
||||||
|
|
||||||
@@ -14,19 +13,22 @@ class Command(BaseCommand):
|
|||||||
|
|
||||||
def handle(self, *args, **kwargs):
|
def handle(self, *args, **kwargs):
|
||||||
# Delete test users
|
# Delete test users
|
||||||
test_users = User.objects.filter(username__in=["testuser", "moderator"])
|
test_users = User.objects.filter(
|
||||||
|
username__in=["testuser", "moderator"])
|
||||||
count = test_users.count()
|
count = test_users.count()
|
||||||
test_users.delete()
|
test_users.delete()
|
||||||
self.stdout.write(self.style.SUCCESS(f"Deleted {count} test users"))
|
self.stdout.write(self.style.SUCCESS(f"Deleted {count} test users"))
|
||||||
|
|
||||||
# Delete test reviews
|
# Delete test reviews
|
||||||
reviews = Review.objects.filter(user__username__in=["testuser", "moderator"])
|
reviews = Review.objects.filter(
|
||||||
|
user__username__in=["testuser", "moderator"])
|
||||||
count = reviews.count()
|
count = reviews.count()
|
||||||
reviews.delete()
|
reviews.delete()
|
||||||
self.stdout.write(self.style.SUCCESS(f"Deleted {count} test reviews"))
|
self.stdout.write(self.style.SUCCESS(f"Deleted {count} test reviews"))
|
||||||
|
|
||||||
# Delete test photos
|
# Delete test photos
|
||||||
photos = Photo.objects.filter(uploader__username__in=["testuser", "moderator"])
|
photos = Photo.objects.filter(uploader__username__in=[
|
||||||
|
"testuser", "moderator"])
|
||||||
count = photos.count()
|
count = photos.count()
|
||||||
photos.delete()
|
photos.delete()
|
||||||
self.stdout.write(self.style.SUCCESS(f"Deleted {count} test photos"))
|
self.stdout.write(self.style.SUCCESS(f"Deleted {count} test photos"))
|
||||||
@@ -62,6 +64,7 @@ class Command(BaseCommand):
|
|||||||
os.remove(f)
|
os.remove(f)
|
||||||
self.stdout.write(self.style.SUCCESS(f"Deleted {f}"))
|
self.stdout.write(self.style.SUCCESS(f"Deleted {f}"))
|
||||||
except OSError as e:
|
except OSError as e:
|
||||||
self.stdout.write(self.style.WARNING(f"Error deleting {f}: {e}"))
|
self.stdout.write(self.style.WARNING(
|
||||||
|
f"Error deleting {f}: {e}"))
|
||||||
|
|
||||||
self.stdout.write(self.style.SUCCESS("Test data cleanup complete"))
|
self.stdout.write(self.style.SUCCESS("Test data cleanup complete"))
|
||||||
|
|||||||
302
memory-bank/documentation/django-best-practices-analysis.md
Normal file
302
memory-bank/documentation/django-best-practices-analysis.md
Normal file
@@ -0,0 +1,302 @@
|
|||||||
|
# Django Best Practices Analysis - ThrillWiki Project
|
||||||
|
|
||||||
|
## Executive Summary
|
||||||
|
|
||||||
|
This analysis evaluates the ThrillWiki Django project against established Django best practices as defined in the HackSoft Django Styleguide. The project demonstrates strong adherence to many best practices while having opportunities for improvement in some areas.
|
||||||
|
|
||||||
|
**Overall Assessment: ⭐⭐⭐⭐☆ (8/10)**
|
||||||
|
|
||||||
|
## Key Strengths
|
||||||
|
|
||||||
|
### ✅ Model Architecture & Base Models
|
||||||
|
- **Excellent**: Implements proper base model pattern with `TrackedModel` in `core/history.py`
|
||||||
|
- **Strong**: All major models inherit from `TrackedModel` providing consistent `created_at`/`updated_at` fields
|
||||||
|
- **Advanced**: Complex historical tracking with `pghistory` integration for full audit trails
|
||||||
|
- **Good**: Proper use of abstract base classes (`SluggedModel`) for shared functionality
|
||||||
|
|
||||||
|
```python
|
||||||
|
# core/history.py - Proper base model implementation
|
||||||
|
class TrackedModel(models.Model):
|
||||||
|
created_at = models.DateTimeField(auto_now_add=True)
|
||||||
|
updated_at = models.DateTimeField(auto_now=True)
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
abstract = True
|
||||||
|
```
|
||||||
|
|
||||||
|
### ✅ Service Layer Architecture
|
||||||
|
- **Excellent**: Well-structured service layer in `core/services/`
|
||||||
|
- **Strong**: Clear separation of concerns with dedicated services:
|
||||||
|
- `UnifiedMapService` - Main orchestrating service
|
||||||
|
- `ClusteringService` - Specialized clustering logic
|
||||||
|
- `LocationSearchService` - Search functionality
|
||||||
|
- `RoadTripService` - Business logic for trip planning
|
||||||
|
- **Good**: Services follow keyword-only argument patterns
|
||||||
|
- **Good**: Type annotations throughout service layer
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Example of proper service implementation
|
||||||
|
class UnifiedMapService:
|
||||||
|
def get_map_data(
|
||||||
|
self,
|
||||||
|
bounds: Optional[GeoBounds] = None,
|
||||||
|
filters: Optional[MapFilters] = None,
|
||||||
|
zoom_level: int = DEFAULT_ZOOM_LEVEL,
|
||||||
|
cluster: bool = True,
|
||||||
|
use_cache: bool = True
|
||||||
|
) -> MapResponse:
|
||||||
|
```
|
||||||
|
|
||||||
|
### ✅ Template Organization & Structure
|
||||||
|
- **Excellent**: Proper template inheritance with `base/base.html`
|
||||||
|
- **Strong**: Logical template directory structure by app
|
||||||
|
- **Good**: Extensive use of partial templates for HTMX integration
|
||||||
|
- **Good**: Reusable components in `partials/` directories
|
||||||
|
- **Advanced**: HTMX integration for dynamic updates
|
||||||
|
|
||||||
|
```html
|
||||||
|
<!-- Proper template structure -->
|
||||||
|
{% extends "base/base.html" %}
|
||||||
|
{% load static %}
|
||||||
|
|
||||||
|
{% block title %}{{ area.name }} - {{ area.park.name }} - ThrillWiki{% endblock %}
|
||||||
|
```
|
||||||
|
|
||||||
|
### ✅ URL Structure & Organization
|
||||||
|
- **Excellent**: Clear URL namespacing by app
|
||||||
|
- **Strong**: RESTful URL patterns with proper slug usage
|
||||||
|
- **Good**: Separation of HTML views and API endpoints
|
||||||
|
- **Good**: Logical grouping of related endpoints
|
||||||
|
|
||||||
|
```python
|
||||||
|
# parks/urls.py - Well-organized URL structure
|
||||||
|
app_name = "parks"
|
||||||
|
urlpatterns = [
|
||||||
|
path("", views_search.ParkSearchView.as_view(), name="park_list"),
|
||||||
|
path("create/", views.ParkCreateView.as_view(), name="park_create"),
|
||||||
|
path("<slug:slug>/", views.ParkDetailView.as_view(), name="park_detail"),
|
||||||
|
]
|
||||||
|
```
|
||||||
|
|
||||||
|
### ✅ Testing Infrastructure
|
||||||
|
- **Strong**: Comprehensive testing setup with coverage reporting
|
||||||
|
- **Good**: Separate unit tests and E2E tests with Playwright
|
||||||
|
- **Good**: Custom test runner with coverage integration
|
||||||
|
- **Good**: Clear test organization by app
|
||||||
|
|
||||||
|
## Areas for Improvement
|
||||||
|
|
||||||
|
### ⚠️ Settings Organization
|
||||||
|
**Current State**: Single monolithic `settings.py` file
|
||||||
|
**Django Styleguide Recommendation**: Structured settings with separate modules
|
||||||
|
|
||||||
|
**Issues Identified**:
|
||||||
|
- All settings in one file (`thrillwiki/settings.py`)
|
||||||
|
- No environment-based configuration separation
|
||||||
|
- Hard-coded values mixed with environment-dependent settings
|
||||||
|
|
||||||
|
**Recommended Structure**:
|
||||||
|
```
|
||||||
|
config/
|
||||||
|
├── django/
|
||||||
|
│ ├── base.py # Common settings
|
||||||
|
│ ├── local.py # Development settings
|
||||||
|
│ ├── production.py # Production settings
|
||||||
|
│ └── test.py # Test settings
|
||||||
|
└── settings/
|
||||||
|
├── celery.py # Celery configuration
|
||||||
|
├── cors.py # CORS settings
|
||||||
|
└── sentry.py # Sentry configuration
|
||||||
|
```
|
||||||
|
|
||||||
|
### ⚠️ Selectors Pattern Implementation
|
||||||
|
**Current State**: Limited selector pattern usage
|
||||||
|
**Django Styleguide Recommendation**: Clear separation between services (push) and selectors (pull)
|
||||||
|
|
||||||
|
**Issues Identified**:
|
||||||
|
- Data retrieval logic mixed in views and services
|
||||||
|
- No dedicated `selectors.py` modules
|
||||||
|
- Query optimization scattered across multiple locations
|
||||||
|
|
||||||
|
**Recommended Pattern**:
|
||||||
|
```python
|
||||||
|
# parks/selectors.py
|
||||||
|
def park_list_with_stats(*, filters: Optional[Dict] = None) -> QuerySet[Park]:
|
||||||
|
"""Get parks with optimized queries for list display"""
|
||||||
|
queryset = Park.objects.select_related('operator', 'property_owner')
|
||||||
|
if filters:
|
||||||
|
queryset = queryset.filter(**filters)
|
||||||
|
return queryset.order_by('name')
|
||||||
|
```
|
||||||
|
|
||||||
|
### ⚠️ API & Serializers Structure
|
||||||
|
**Current State**: Limited API implementation
|
||||||
|
**Django Styleguide Recommendation**: Structured API with proper serializers
|
||||||
|
|
||||||
|
**Issues Identified**:
|
||||||
|
- Minimal DRF usage despite having REST framework installed
|
||||||
|
- API endpoints mixed with HTML views
|
||||||
|
- No clear API versioning strategy
|
||||||
|
|
||||||
|
### ⚠️ Environment Variable Management
|
||||||
|
**Current State**: Hard-coded configuration values
|
||||||
|
**Django Styleguide Recommendation**: Environment-based configuration with `django-environ`
|
||||||
|
|
||||||
|
**Issues Identified**:
|
||||||
|
```python
|
||||||
|
# Current problematic patterns in settings.py
|
||||||
|
SECRET_KEY = "django-insecure-=0)^0#h#k$0@$8$ys=^$0#h#k$0@$8$ys=^" # Hard-coded
|
||||||
|
DEBUG = True # Hard-coded
|
||||||
|
DATABASES = {
|
||||||
|
"default": {
|
||||||
|
"NAME": "thrillwiki",
|
||||||
|
"USER": "wiki",
|
||||||
|
"PASSWORD": "thrillwiki", # Hard-coded credentials
|
||||||
|
"HOST": "192.168.86.3", # Hard-coded host
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Detailed Analysis by Category
|
||||||
|
|
||||||
|
### Models (Score: 9/10)
|
||||||
|
**Strengths**:
|
||||||
|
- Excellent base model pattern with `TrackedModel`
|
||||||
|
- Complex history tracking with `pghistory`
|
||||||
|
- Proper model validation with `clean()` methods
|
||||||
|
- Type hints throughout model definitions
|
||||||
|
- Appropriate use of GenericForeignKeys
|
||||||
|
|
||||||
|
**Minor Issues**:
|
||||||
|
- Some models have redundant `created_at`/`updated_at` fields alongside `TrackedModel`
|
||||||
|
- Mixed inheritance patterns (some models don't use base classes consistently)
|
||||||
|
|
||||||
|
### Services (Score: 8/10)
|
||||||
|
**Strengths**:
|
||||||
|
- Clear service layer separation
|
||||||
|
- Type annotations and proper error handling
|
||||||
|
- Caching integration
|
||||||
|
- Business logic properly encapsulated
|
||||||
|
|
||||||
|
**Areas for Improvement**:
|
||||||
|
- Could benefit from more granular service decomposition
|
||||||
|
- Some business logic still in views
|
||||||
|
- Limited use of selectors pattern
|
||||||
|
|
||||||
|
### Templates (Score: 9/10)
|
||||||
|
**Strengths**:
|
||||||
|
- Excellent template organization
|
||||||
|
- Proper inheritance structure
|
||||||
|
- HTMX integration
|
||||||
|
- Reusable components
|
||||||
|
|
||||||
|
**Minor Issues**:
|
||||||
|
- Some templates could benefit from more granular partials
|
||||||
|
- CSS classes could be more consistently organized
|
||||||
|
|
||||||
|
### Testing (Score: 7/10)
|
||||||
|
**Strengths**:
|
||||||
|
- Comprehensive coverage reporting
|
||||||
|
- E2E tests with Playwright
|
||||||
|
- Good test organization
|
||||||
|
|
||||||
|
**Areas for Improvement**:
|
||||||
|
- Limited factory usage (recommended by styleguide)
|
||||||
|
- Some apps lack complete test coverage
|
||||||
|
- Could benefit from more integration tests
|
||||||
|
|
||||||
|
### URLs (Score: 8/10)
|
||||||
|
**Strengths**:
|
||||||
|
- Clear namespacing
|
||||||
|
- RESTful patterns
|
||||||
|
- Good organization
|
||||||
|
|
||||||
|
**Minor Issues**:
|
||||||
|
- Some URL patterns could be more consistent
|
||||||
|
- API URLs mixed with HTML view URLs
|
||||||
|
|
||||||
|
### Settings (Score: 4/10)
|
||||||
|
**Major Issues**:
|
||||||
|
- Monolithic settings file
|
||||||
|
- Hard-coded values
|
||||||
|
- No environment separation
|
||||||
|
- Security concerns with exposed secrets
|
||||||
|
|
||||||
|
## Security Assessment
|
||||||
|
|
||||||
|
### ✅ Security Strengths
|
||||||
|
- CSRF protection enabled
|
||||||
|
- Proper authentication backends
|
||||||
|
- SSL redirect configuration
|
||||||
|
- Secure headers implementation
|
||||||
|
|
||||||
|
### ⚠️ Security Concerns
|
||||||
|
- Hard-coded SECRET_KEY in settings
|
||||||
|
- Database credentials in source code
|
||||||
|
- DEBUG=True in production-destined code
|
||||||
|
- Hard-coded API keys (Turnstile keys)
|
||||||
|
|
||||||
|
## Performance Considerations
|
||||||
|
|
||||||
|
### ✅ Performance Strengths
|
||||||
|
- Query optimization with `select_related`/`prefetch_related`
|
||||||
|
- Caching implementation in services
|
||||||
|
- Efficient database queries in adapters
|
||||||
|
- HTMX for reduced page loads
|
||||||
|
|
||||||
|
### ⚠️ Performance Areas
|
||||||
|
- Could benefit from more aggressive caching
|
||||||
|
- Some N+1 query patterns in views
|
||||||
|
- Large template rendering without fragments
|
||||||
|
|
||||||
|
## Recommendations
|
||||||
|
|
||||||
|
### High Priority
|
||||||
|
1. **Restructure Settings**: Implement environment-based settings structure
|
||||||
|
2. **Environment Variables**: Use `django-environ` for all configuration
|
||||||
|
3. **Security**: Remove hard-coded secrets and credentials
|
||||||
|
4. **Selectors**: Implement proper selectors pattern for data retrieval
|
||||||
|
|
||||||
|
### Medium Priority
|
||||||
|
1. **API Structure**: Implement proper DRF API with versioning
|
||||||
|
2. **Testing**: Add factory_boy for test data generation
|
||||||
|
3. **Query Optimization**: Review and optimize database queries
|
||||||
|
4. **Documentation**: Add API documentation with DRF spectacular
|
||||||
|
|
||||||
|
### Low Priority
|
||||||
|
1. **Template Fragments**: Break down large templates into smaller components
|
||||||
|
2. **Service Decomposition**: Further break down large services
|
||||||
|
3. **Caching Strategy**: Implement more comprehensive caching
|
||||||
|
4. **Type Hints**: Complete type annotation coverage
|
||||||
|
|
||||||
|
## Conclusion
|
||||||
|
|
||||||
|
The ThrillWiki project demonstrates strong understanding and implementation of Django best practices, particularly in model architecture, service layer design, and template organization. The project's use of advanced features like `pghistory` for audit trails and HTMX for dynamic updates shows sophisticated Django development.
|
||||||
|
|
||||||
|
The main areas requiring attention are settings organization, environment configuration, and security hardening. These are common issues in Django projects and relatively straightforward to address.
|
||||||
|
|
||||||
|
The project is well-positioned for production deployment with the recommended improvements, and already exceeds many Django projects in terms of architectural decisions and code organization.
|
||||||
|
|
||||||
|
**Final Grade: B+ (85/100)**
|
||||||
|
|
||||||
|
## Implementation Timeline
|
||||||
|
|
||||||
|
### Phase 1 (Week 1): Critical Security & Settings
|
||||||
|
- [ ] Restructure settings into modular format
|
||||||
|
- [ ] Implement environment variable management
|
||||||
|
- [ ] Remove hard-coded secrets
|
||||||
|
- [ ] Add production-ready configuration
|
||||||
|
|
||||||
|
### Phase 2 (Week 2): Architecture Improvements
|
||||||
|
- [ ] Implement selectors pattern
|
||||||
|
- [ ] Optimize database queries
|
||||||
|
- [ ] Enhance API structure
|
||||||
|
- [ ] Add comprehensive error handling
|
||||||
|
|
||||||
|
### Phase 3 (Week 3): Testing & Documentation
|
||||||
|
- [ ] Add factory_boy integration
|
||||||
|
- [ ] Improve test coverage
|
||||||
|
- [ ] Add API documentation
|
||||||
|
- [ ] Performance optimization
|
||||||
|
|
||||||
|
This analysis provides a roadmap for bringing the project to full Django best practices compliance while maintaining its current strengths.
|
||||||
@@ -6,13 +6,14 @@ from django.core.exceptions import ValidationError
|
|||||||
from decimal import Decimal, ROUND_DOWN, InvalidOperation
|
from decimal import Decimal, ROUND_DOWN, InvalidOperation
|
||||||
from typing import Tuple, Optional, Any, TYPE_CHECKING
|
from typing import Tuple, Optional, Any, TYPE_CHECKING
|
||||||
import pghistory
|
import pghistory
|
||||||
|
|
||||||
from .companies import Company
|
from .companies import Company
|
||||||
from media.models import Photo
|
from media.models import Photo
|
||||||
from core.history import TrackedModel
|
from core.history import TrackedModel
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from rides.models import Ride
|
from rides.models import Ride
|
||||||
|
from . import ParkArea
|
||||||
|
|
||||||
|
|
||||||
@pghistory.track()
|
@pghistory.track()
|
||||||
class Park(TrackedModel):
|
class Park(TrackedModel):
|
||||||
@@ -71,7 +72,8 @@ class Park(TrackedModel):
|
|||||||
)
|
)
|
||||||
photos = GenericRelation(Photo, related_query_name="park")
|
photos = GenericRelation(Photo, related_query_name="park")
|
||||||
areas: models.Manager['ParkArea'] # Type hint for reverse relation
|
areas: models.Manager['ParkArea'] # Type hint for reverse relation
|
||||||
rides: models.Manager['Ride'] # Type hint for reverse relation from rides app
|
# Type hint for reverse relation from rides app
|
||||||
|
rides: models.Manager['Ride']
|
||||||
|
|
||||||
# Metadata
|
# Metadata
|
||||||
created_at = models.DateTimeField(auto_now_add=True, null=True)
|
created_at = models.DateTimeField(auto_now_add=True, null=True)
|
||||||
@@ -118,9 +120,11 @@ class Park(TrackedModel):
|
|||||||
def clean(self):
|
def clean(self):
|
||||||
super().clean()
|
super().clean()
|
||||||
if self.operator and 'OPERATOR' not in self.operator.roles:
|
if self.operator and 'OPERATOR' not in self.operator.roles:
|
||||||
raise ValidationError({'operator': 'Company must have the OPERATOR role.'})
|
raise ValidationError(
|
||||||
|
{'operator': 'Company must have the OPERATOR role.'})
|
||||||
if self.property_owner and 'PROPERTY_OWNER' not in self.property_owner.roles:
|
if self.property_owner and 'PROPERTY_OWNER' not in self.property_owner.roles:
|
||||||
raise ValidationError({'property_owner': 'Company must have the PROPERTY_OWNER role.'})
|
raise ValidationError(
|
||||||
|
{'property_owner': 'Company must have the PROPERTY_OWNER role.'})
|
||||||
|
|
||||||
def get_absolute_url(self) -> str:
|
def get_absolute_url(self) -> str:
|
||||||
return reverse("parks:park_detail", kwargs={"slug": self.slug})
|
return reverse("parks:park_detail", kwargs={"slug": self.slug})
|
||||||
@@ -168,26 +172,27 @@ class Park(TrackedModel):
|
|||||||
|
|
||||||
# Try historical slugs in HistoricalSlug model
|
# Try historical slugs in HistoricalSlug model
|
||||||
content_type = ContentType.objects.get_for_model(cls)
|
content_type = ContentType.objects.get_for_model(cls)
|
||||||
print(f"Searching HistoricalSlug with content_type: {content_type}")
|
print(
|
||||||
|
f"Searching HistoricalSlug with content_type: {content_type}")
|
||||||
historical = HistoricalSlug.objects.filter(
|
historical = HistoricalSlug.objects.filter(
|
||||||
content_type=content_type,
|
content_type=content_type,
|
||||||
slug=slug
|
slug=slug
|
||||||
).order_by('-created_at').first()
|
).order_by('-created_at').first()
|
||||||
|
|
||||||
if historical:
|
if historical:
|
||||||
print(f"Found historical slug record for object_id: {historical.object_id}")
|
print(
|
||||||
|
f"Found historical slug record for object_id: {historical.object_id}")
|
||||||
try:
|
try:
|
||||||
park = cls.objects.get(pk=historical.object_id)
|
park = cls.objects.get(pk=historical.object_id)
|
||||||
print(f"Found park from historical slug: {park.name}")
|
print(f"Found park from historical slug: {park.name}")
|
||||||
return park, True
|
return park, True
|
||||||
except cls.DoesNotExist:
|
except cls.DoesNotExist:
|
||||||
print(f"Park not found for historical slug record")
|
print("Park not found for historical slug record")
|
||||||
pass
|
|
||||||
else:
|
else:
|
||||||
print("No historical slug record found")
|
print("No historical slug record found")
|
||||||
|
|
||||||
# Try pghistory events
|
# Try pghistory events
|
||||||
print(f"Searching pghistory events")
|
print("Searching pghistory events")
|
||||||
event_model = getattr(cls, 'event_model', None)
|
event_model = getattr(cls, 'event_model', None)
|
||||||
if event_model:
|
if event_model:
|
||||||
historical_event = event_model.objects.filter(
|
historical_event = event_model.objects.filter(
|
||||||
@@ -195,14 +200,14 @@ class Park(TrackedModel):
|
|||||||
).order_by('-pgh_created_at').first()
|
).order_by('-pgh_created_at').first()
|
||||||
|
|
||||||
if historical_event:
|
if historical_event:
|
||||||
print(f"Found pghistory event for pgh_obj_id: {historical_event.pgh_obj_id}")
|
print(
|
||||||
|
f"Found pghistory event for pgh_obj_id: {historical_event.pgh_obj_id}")
|
||||||
try:
|
try:
|
||||||
park = cls.objects.get(pk=historical_event.pgh_obj_id)
|
park = cls.objects.get(pk=historical_event.pgh_obj_id)
|
||||||
print(f"Found park from pghistory: {park.name}")
|
print(f"Found park from pghistory: {park.name}")
|
||||||
return park, True
|
return park, True
|
||||||
except cls.DoesNotExist:
|
except cls.DoesNotExist:
|
||||||
print(f"Park not found for pghistory event")
|
print("Park not found for pghistory event")
|
||||||
pass
|
|
||||||
else:
|
else:
|
||||||
print("No pghistory event found")
|
print("No pghistory event found")
|
||||||
|
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ from django.core.cache import cache
|
|||||||
from django.contrib.gis.geos import Point
|
from django.contrib.gis.geos import Point
|
||||||
from django.contrib.gis.measure import Distance
|
from django.contrib.gis.measure import Distance
|
||||||
from django.db.models import Q
|
from django.db.models import Q
|
||||||
|
from parks.models import Park
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -143,10 +144,14 @@ class RoadTripService:
|
|||||||
self.osrm_base_url = "http://router.project-osrm.org/route/v1/driving"
|
self.osrm_base_url = "http://router.project-osrm.org/route/v1/driving"
|
||||||
|
|
||||||
# Configuration from Django settings
|
# Configuration from Django settings
|
||||||
self.cache_timeout = getattr(settings, 'ROADTRIP_CACHE_TIMEOUT', 3600 * 24)
|
self.cache_timeout = getattr(
|
||||||
self.route_cache_timeout = getattr(settings, 'ROADTRIP_ROUTE_CACHE_TIMEOUT', 3600 * 6)
|
settings, 'ROADTRIP_CACHE_TIMEOUT', 3600 * 24)
|
||||||
self.user_agent = getattr(settings, 'ROADTRIP_USER_AGENT', 'ThrillWiki Road Trip Planner')
|
self.route_cache_timeout = getattr(
|
||||||
self.request_timeout = getattr(settings, 'ROADTRIP_REQUEST_TIMEOUT', 10)
|
settings, 'ROADTRIP_ROUTE_CACHE_TIMEOUT', 3600 * 6)
|
||||||
|
self.user_agent = getattr(
|
||||||
|
settings, 'ROADTRIP_USER_AGENT', 'ThrillWiki Road Trip Planner')
|
||||||
|
self.request_timeout = getattr(
|
||||||
|
settings, 'ROADTRIP_REQUEST_TIMEOUT', 10)
|
||||||
self.max_retries = getattr(settings, 'ROADTRIP_MAX_RETRIES', 3)
|
self.max_retries = getattr(settings, 'ROADTRIP_MAX_RETRIES', 3)
|
||||||
self.backoff_factor = getattr(settings, 'ROADTRIP_BACKOFF_FACTOR', 2)
|
self.backoff_factor = getattr(settings, 'ROADTRIP_BACKOFF_FACTOR', 2)
|
||||||
|
|
||||||
@@ -184,7 +189,8 @@ class RoadTripService:
|
|||||||
wait_time = self.backoff_factor ** attempt
|
wait_time = self.backoff_factor ** attempt
|
||||||
time.sleep(wait_time)
|
time.sleep(wait_time)
|
||||||
else:
|
else:
|
||||||
raise OSMAPIException(f"Failed to make request after {self.max_retries} attempts: {e}")
|
raise OSMAPIException(
|
||||||
|
f"Failed to make request after {self.max_retries} attempts: {e}")
|
||||||
|
|
||||||
def geocode_address(self, address: str) -> Optional[Coordinates]:
|
def geocode_address(self, address: str) -> Optional[Coordinates]:
|
||||||
"""
|
"""
|
||||||
@@ -229,7 +235,8 @@ class RoadTripService:
|
|||||||
'longitude': coords.longitude
|
'longitude': coords.longitude
|
||||||
}, self.cache_timeout)
|
}, self.cache_timeout)
|
||||||
|
|
||||||
logger.info(f"Geocoded '{address}' to {coords.latitude}, {coords.longitude}")
|
logger.info(
|
||||||
|
f"Geocoded '{address}' to {coords.latitude}, {coords.longitude}")
|
||||||
return coords
|
return coords
|
||||||
else:
|
else:
|
||||||
logger.warning(f"No geocoding results for address: {address}")
|
logger.warning(f"No geocoding results for address: {address}")
|
||||||
@@ -293,11 +300,13 @@ class RoadTripService:
|
|||||||
'geometry': route_info.geometry
|
'geometry': route_info.geometry
|
||||||
}, self.route_cache_timeout)
|
}, self.route_cache_timeout)
|
||||||
|
|
||||||
logger.info(f"Route calculated: {route_info.formatted_distance}, {route_info.formatted_duration}")
|
logger.info(
|
||||||
|
f"Route calculated: {route_info.formatted_distance}, {route_info.formatted_duration}")
|
||||||
return route_info
|
return route_info
|
||||||
else:
|
else:
|
||||||
# Fallback to straight-line distance calculation
|
# Fallback to straight-line distance calculation
|
||||||
logger.warning(f"OSRM routing failed, falling back to straight-line distance")
|
logger.warning(
|
||||||
|
f"OSRM routing failed, falling back to straight-line distance")
|
||||||
return self._calculate_straight_line_route(start_coords, end_coords)
|
return self._calculate_straight_line_route(start_coords, end_coords)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -310,13 +319,16 @@ class RoadTripService:
|
|||||||
Calculate straight-line distance as fallback when routing fails.
|
Calculate straight-line distance as fallback when routing fails.
|
||||||
"""
|
"""
|
||||||
# Haversine formula for great-circle distance
|
# Haversine formula for great-circle distance
|
||||||
lat1, lon1 = math.radians(start_coords.latitude), math.radians(start_coords.longitude)
|
lat1, lon1 = math.radians(start_coords.latitude), math.radians(
|
||||||
lat2, lon2 = math.radians(end_coords.latitude), math.radians(end_coords.longitude)
|
start_coords.longitude)
|
||||||
|
lat2, lon2 = math.radians(
|
||||||
|
end_coords.latitude), math.radians(end_coords.longitude)
|
||||||
|
|
||||||
dlat = lat2 - lat1
|
dlat = lat2 - lat1
|
||||||
dlon = lon2 - lon1
|
dlon = lon2 - lon1
|
||||||
|
|
||||||
a = math.sin(dlat/2)**2 + math.cos(lat1) * math.cos(lat2) * math.sin(dlon/2)**2
|
a = math.sin(dlat/2)**2 + math.cos(lat1) * \
|
||||||
|
math.cos(lat2) * math.sin(dlon/2)**2
|
||||||
c = 2 * math.asin(math.sqrt(a))
|
c = 2 * math.asin(math.sqrt(a))
|
||||||
|
|
||||||
# Earth's radius in kilometers
|
# Earth's radius in kilometers
|
||||||
@@ -358,7 +370,8 @@ class RoadTripService:
|
|||||||
if not start_coords or not end_coords:
|
if not start_coords or not end_coords:
|
||||||
return []
|
return []
|
||||||
|
|
||||||
start_point = Point(start_coords[1], start_coords[0], srid=4326) # lon, lat
|
start_point = Point(
|
||||||
|
start_coords[1], start_coords[0], srid=4326) # lon, lat
|
||||||
end_point = Point(end_coords[1], end_coords[0], srid=4326)
|
end_point = Point(end_coords[1], end_coords[0], srid=4326)
|
||||||
|
|
||||||
# Find all parks within a reasonable distance from both start and end
|
# Find all parks within a reasonable distance from both start and end
|
||||||
@@ -409,7 +422,8 @@ class RoadTripService:
|
|||||||
if not route_to_waypoint or not route_from_waypoint:
|
if not route_to_waypoint or not route_from_waypoint:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
detour_distance = (route_to_waypoint.distance_km + route_from_waypoint.distance_km) - direct_route.distance_km
|
detour_distance = (route_to_waypoint.distance_km +
|
||||||
|
route_from_waypoint.distance_km) - direct_route.distance_km
|
||||||
return max(0, detour_distance) # Don't return negative detours
|
return max(0, detour_distance) # Don't return negative detours
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -562,7 +576,8 @@ class RoadTripService:
|
|||||||
if not center_coords:
|
if not center_coords:
|
||||||
return []
|
return []
|
||||||
|
|
||||||
center_point = Point(center_coords[1], center_coords[0], srid=4326) # lon, lat
|
center_point = Point(
|
||||||
|
center_coords[1], center_coords[0], srid=4326) # lon, lat
|
||||||
search_distance = Distance(km=radius_km)
|
search_distance = Distance(km=radius_km)
|
||||||
|
|
||||||
nearby_parks = Park.objects.filter(
|
nearby_parks = Park.objects.filter(
|
||||||
@@ -633,7 +648,8 @@ class RoadTripService:
|
|||||||
if coords:
|
if coords:
|
||||||
location.set_coordinates(coords.latitude, coords.longitude)
|
location.set_coordinates(coords.latitude, coords.longitude)
|
||||||
location.save()
|
location.save()
|
||||||
logger.info(f"Geocoded park '{park.name}' to {coords.latitude}, {coords.longitude}")
|
logger.info(
|
||||||
|
f"Geocoded park '{park.name}' to {coords.latitude}, {coords.longitude}")
|
||||||
return True
|
return True
|
||||||
|
|
||||||
return False
|
return False
|
||||||
@@ -7,7 +7,7 @@ from moderation.mixins import EditSubmissionMixin, PhotoSubmissionMixin, History
|
|||||||
from core.views import SlugRedirectMixin
|
from core.views import SlugRedirectMixin
|
||||||
from .filters import ParkFilter
|
from .filters import ParkFilter
|
||||||
from .forms import ParkForm
|
from .forms import ParkForm
|
||||||
from .models import Park, ParkArea
|
from .models import Park, ParkArea, ParkReview as Review
|
||||||
from django.http import HttpResponseRedirect, HttpResponse, HttpRequest, JsonResponse
|
from django.http import HttpResponseRedirect, HttpResponse, HttpRequest, JsonResponse
|
||||||
from django.core.exceptions import ObjectDoesNotExist
|
from django.core.exceptions import ObjectDoesNotExist
|
||||||
from django.contrib import messages
|
from django.contrib import messages
|
||||||
|
|||||||
@@ -20,6 +20,8 @@ from .models import Park
|
|||||||
from .services.roadtrip import RoadTripService
|
from .services.roadtrip import RoadTripService
|
||||||
from core.services.map_service import unified_map_service
|
from core.services.map_service import unified_map_service
|
||||||
from core.services.data_structures import LocationType, MapFilters
|
from core.services.data_structures import LocationType, MapFilters
|
||||||
|
JSON_DECODE_ERROR_MSG = 'Invalid JSON data'
|
||||||
|
PARKS_ALONG_ROUTE_HTML = 'parks/partials/parks_along_route.html'
|
||||||
|
|
||||||
|
|
||||||
class RoadTripViewMixin:
|
class RoadTripViewMixin:
|
||||||
@@ -29,7 +31,7 @@ class RoadTripViewMixin:
|
|||||||
super().__init__()
|
super().__init__()
|
||||||
self.roadtrip_service = RoadTripService()
|
self.roadtrip_service = RoadTripService()
|
||||||
|
|
||||||
def get_roadtrip_context(self, request: HttpRequest) -> Dict[str, Any]:
|
def get_roadtrip_context(self) -> Dict[str, Any]:
|
||||||
"""Get common context data for road trip views."""
|
"""Get common context data for road trip views."""
|
||||||
return {
|
return {
|
||||||
'roadtrip_api_urls': {
|
'roadtrip_api_urls': {
|
||||||
@@ -146,7 +148,7 @@ class CreateTripView(RoadTripViewMixin, View):
|
|||||||
except json.JSONDecodeError:
|
except json.JSONDecodeError:
|
||||||
return JsonResponse({
|
return JsonResponse({
|
||||||
'status': 'error',
|
'status': 'error',
|
||||||
'message': 'Invalid JSON data'
|
'message': JSON_DECODE_ERROR_MSG
|
||||||
}, status=400)
|
}, status=400)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return JsonResponse({
|
return JsonResponse({
|
||||||
@@ -219,10 +221,11 @@ class FindParksAlongRouteView(RoadTripViewMixin, View):
|
|||||||
|
|
||||||
start_park_id = data.get('start_park_id')
|
start_park_id = data.get('start_park_id')
|
||||||
end_park_id = data.get('end_park_id')
|
end_park_id = data.get('end_park_id')
|
||||||
max_detour_km = min(100, max(10, float(data.get('max_detour_km', 50))))
|
max_detour_km = min(
|
||||||
|
100, max(10, float(data.get('max_detour_km', 50))))
|
||||||
|
|
||||||
if not start_park_id or not end_park_id:
|
if not start_park_id or not end_park_id:
|
||||||
return render(request, 'parks/partials/parks_along_route.html', {
|
return render(request, PARKS_ALONG_ROUTE_HTML, {
|
||||||
'error': 'Start and end parks are required'
|
'error': 'Start and end parks are required'
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -235,7 +238,7 @@ class FindParksAlongRouteView(RoadTripViewMixin, View):
|
|||||||
id=end_park_id, location__isnull=False
|
id=end_park_id, location__isnull=False
|
||||||
)
|
)
|
||||||
except Park.DoesNotExist:
|
except Park.DoesNotExist:
|
||||||
return render(request, 'parks/partials/parks_along_route.html', {
|
return render(request, PARKS_ALONG_ROUTE_HTML, {
|
||||||
'error': 'One or both parks could not be found'
|
'error': 'One or both parks could not be found'
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -244,7 +247,7 @@ class FindParksAlongRouteView(RoadTripViewMixin, View):
|
|||||||
start_park, end_park, max_detour_km
|
start_park, end_park, max_detour_km
|
||||||
)
|
)
|
||||||
|
|
||||||
return render(request, 'parks/partials/parks_along_route.html', {
|
return render(request, PARKS_ALONG_ROUTE_HTML, {
|
||||||
'parks': parks_along_route,
|
'parks': parks_along_route,
|
||||||
'start_park': start_park,
|
'start_park': start_park,
|
||||||
'end_park': end_park,
|
'end_park': end_park,
|
||||||
@@ -253,11 +256,11 @@ class FindParksAlongRouteView(RoadTripViewMixin, View):
|
|||||||
})
|
})
|
||||||
|
|
||||||
except json.JSONDecodeError:
|
except json.JSONDecodeError:
|
||||||
return render(request, 'parks/partials/parks_along_route.html', {
|
return render(request, PARKS_ALONG_ROUTE_HTML, {
|
||||||
'error': 'Invalid request data'
|
'error': JSON_DECODE_ERROR_MSG
|
||||||
})
|
})
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return render(request, 'parks/partials/parks_along_route.html', {
|
return render(request, PARKS_ALONG_ROUTE_HTML, {
|
||||||
'error': str(e)
|
'error': str(e)
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -333,7 +336,7 @@ class GeocodeAddressView(RoadTripViewMixin, View):
|
|||||||
except json.JSONDecodeError:
|
except json.JSONDecodeError:
|
||||||
return JsonResponse({
|
return JsonResponse({
|
||||||
'status': 'error',
|
'status': 'error',
|
||||||
'message': 'Invalid JSON data'
|
'message': JSON_DECODE_ERROR_MSG
|
||||||
}, status=400)
|
}, status=400)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return JsonResponse({
|
return JsonResponse({
|
||||||
@@ -387,7 +390,7 @@ class ParkDistanceCalculatorView(RoadTripViewMixin, View):
|
|||||||
'message': 'One or both parks do not have coordinate data'
|
'message': 'One or both parks do not have coordinate data'
|
||||||
}, status=400)
|
}, status=400)
|
||||||
|
|
||||||
from ..services.roadtrip import Coordinates
|
from services.roadtrip import Coordinates
|
||||||
|
|
||||||
route = self.roadtrip_service.calculate_route(
|
route = self.roadtrip_service.calculate_route(
|
||||||
Coordinates(*coords1),
|
Coordinates(*coords1),
|
||||||
@@ -421,7 +424,7 @@ class ParkDistanceCalculatorView(RoadTripViewMixin, View):
|
|||||||
except json.JSONDecodeError:
|
except json.JSONDecodeError:
|
||||||
return JsonResponse({
|
return JsonResponse({
|
||||||
'status': 'error',
|
'status': 'error',
|
||||||
'message': 'Invalid JSON data'
|
'message': JSON_DECODE_ERROR_MSG
|
||||||
}, status=400)
|
}, status=400)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return JsonResponse({
|
return JsonResponse({
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ from django.contrib.contenttypes.fields import GenericRelation
|
|||||||
from core.history import TrackedModel, DiffMixin
|
from core.history import TrackedModel, DiffMixin
|
||||||
from .events import get_ride_display_changes, get_ride_model_display_changes
|
from .events import get_ride_display_changes, get_ride_model_display_changes
|
||||||
import pghistory
|
import pghistory
|
||||||
from .company import Company
|
from .models import Company
|
||||||
|
|
||||||
# Shared choices that will be used by multiple models
|
# Shared choices that will be used by multiple models
|
||||||
CATEGORY_CHOICES = [
|
CATEGORY_CHOICES = [
|
||||||
@@ -17,6 +17,7 @@ CATEGORY_CHOICES = [
|
|||||||
('OT', 'Other'),
|
('OT', 'Other'),
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
class RideEvent(models.Model, DiffMixin):
|
class RideEvent(models.Model, DiffMixin):
|
||||||
"""Event model for tracking Ride changes - uses existing pghistory table"""
|
"""Event model for tracking Ride changes - uses existing pghistory table"""
|
||||||
|
|
||||||
@@ -39,7 +40,8 @@ class RideEvent(models.Model, DiffMixin):
|
|||||||
max_height_in = models.PositiveIntegerField(null=True)
|
max_height_in = models.PositiveIntegerField(null=True)
|
||||||
capacity_per_hour = models.PositiveIntegerField(null=True)
|
capacity_per_hour = models.PositiveIntegerField(null=True)
|
||||||
ride_duration_seconds = models.PositiveIntegerField(null=True)
|
ride_duration_seconds = models.PositiveIntegerField(null=True)
|
||||||
average_rating = models.DecimalField(max_digits=3, decimal_places=2, null=True)
|
average_rating = models.DecimalField(
|
||||||
|
max_digits=3, decimal_places=2, null=True)
|
||||||
created_at = models.DateTimeField()
|
created_at = models.DateTimeField()
|
||||||
updated_at = models.DateTimeField()
|
updated_at = models.DateTimeField()
|
||||||
|
|
||||||
@@ -66,6 +68,7 @@ class RideEvent(models.Model, DiffMixin):
|
|||||||
"""Returns human-readable changes"""
|
"""Returns human-readable changes"""
|
||||||
return get_ride_display_changes(self.diff_against_previous())
|
return get_ride_display_changes(self.diff_against_previous())
|
||||||
|
|
||||||
|
|
||||||
class RideModelEvent(models.Model, DiffMixin):
|
class RideModelEvent(models.Model, DiffMixin):
|
||||||
"""Event model for tracking RideModel changes - uses existing pghistory table"""
|
"""Event model for tracking RideModel changes - uses existing pghistory table"""
|
||||||
|
|
||||||
@@ -102,6 +105,7 @@ class RideModelEvent(models.Model, DiffMixin):
|
|||||||
"""Returns human-readable changes"""
|
"""Returns human-readable changes"""
|
||||||
return get_ride_model_display_changes(self.diff_against_previous())
|
return get_ride_model_display_changes(self.diff_against_previous())
|
||||||
|
|
||||||
|
|
||||||
class RideModel(TrackedModel):
|
class RideModel(TrackedModel):
|
||||||
"""
|
"""
|
||||||
Represents a specific model/type of ride that can be manufactured by different companies.
|
Represents a specific model/type of ride that can be manufactured by different companies.
|
||||||
@@ -114,7 +118,8 @@ class RideModel(TrackedModel):
|
|||||||
related_name='ride_models',
|
related_name='ride_models',
|
||||||
null=True,
|
null=True,
|
||||||
blank=True,
|
blank=True,
|
||||||
limit_choices_to={'roles__contains': [Company.CompanyRole.MANUFACTURER]}
|
limit_choices_to={'roles__contains': [
|
||||||
|
Company.CompanyRole.MANUFACTURER]}
|
||||||
)
|
)
|
||||||
description = models.TextField(blank=True)
|
description = models.TextField(blank=True)
|
||||||
category = models.CharField(
|
category = models.CharField(
|
||||||
@@ -131,6 +136,7 @@ class RideModel(TrackedModel):
|
|||||||
def __str__(self) -> str:
|
def __str__(self) -> str:
|
||||||
return self.name if not self.manufacturer else f"{self.manufacturer.name} {self.name}"
|
return self.name if not self.manufacturer else f"{self.manufacturer.name} {self.name}"
|
||||||
|
|
||||||
|
|
||||||
class Ride(TrackedModel):
|
class Ride(TrackedModel):
|
||||||
"""Model for individual ride installations at parks"""
|
"""Model for individual ride installations at parks"""
|
||||||
STATUS_CHOICES = [
|
STATUS_CHOICES = [
|
||||||
@@ -177,7 +183,8 @@ class Ride(TrackedModel):
|
|||||||
null=True,
|
null=True,
|
||||||
blank=True,
|
blank=True,
|
||||||
related_name='manufactured_rides',
|
related_name='manufactured_rides',
|
||||||
limit_choices_to={'roles__contains': [Company.CompanyRole.MANUFACTURER]}
|
limit_choices_to={'roles__contains': [
|
||||||
|
Company.CompanyRole.MANUFACTURER]}
|
||||||
)
|
)
|
||||||
designer = models.ForeignKey(
|
designer = models.ForeignKey(
|
||||||
Company,
|
Company,
|
||||||
@@ -234,6 +241,7 @@ class Ride(TrackedModel):
|
|||||||
self.slug = slugify(self.name)
|
self.slug = slugify(self.name)
|
||||||
super().save(*args, **kwargs)
|
super().save(*args, **kwargs)
|
||||||
|
|
||||||
|
|
||||||
class RollerCoasterStats(models.Model):
|
class RollerCoasterStats(models.Model):
|
||||||
"""Model for tracking roller coaster specific statistics"""
|
"""Model for tracking roller coaster specific statistics"""
|
||||||
TRACK_MATERIAL_CHOICES = [
|
TRACK_MATERIAL_CHOICES = [
|
||||||
|
|||||||
@@ -418,7 +418,7 @@ class RideSearchView(ListView):
|
|||||||
|
|
||||||
def get_queryset(self):
|
def get_queryset(self):
|
||||||
"""Get filtered rides based on search form."""
|
"""Get filtered rides based on search form."""
|
||||||
from search.forms import RideSearchForm
|
from services.search import RideSearchForm
|
||||||
|
|
||||||
queryset = Ride.objects.select_related('park').order_by('name')
|
queryset = Ride.objects.select_related('park').order_by('name')
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user