Refactor comments app to use mixins for comment functionality; update admin interfaces and add historical model fixes

This commit is contained in:
pacnpal
2025-02-08 16:33:55 -05:00
parent 75f5b07129
commit 03f9df4bab
21 changed files with 548 additions and 280 deletions

View File

@@ -1,6 +1,10 @@
from django.apps import AppConfig from django.apps import AppConfig
from django.db.models.signals import class_prepared, post_init
class CommentsConfig(AppConfig): class CommentsConfig(AppConfig):
default_auto_field = "django.db.models.BigAutoField" default_auto_field = 'django.db.models.BigAutoField'
name = "comments" name = 'comments'
def ready(self):
"""Set up comment system when the app is ready."""
pass

71
comments/managers.py Normal file
View File

@@ -0,0 +1,71 @@
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.core.exceptions import ObjectDoesNotExist
class CommentThreadManager(models.Manager):
"""Manager for handling comment threads on both regular and historical models."""
def for_instance(self, instance):
"""Get comment threads for any model instance."""
# Get the base model class if this is a historical instance
if instance.__class__.__name__.startswith('Historical'):
model_class = instance.instance.__class__
instance_id = instance.instance.pk
else:
model_class = instance.__class__
instance_id = instance.pk
ct = ContentType.objects.get_for_model(model_class)
return self.filter(content_type=ct, object_id=instance_id)
def create_for_instance(self, instance, **kwargs):
"""Create a comment thread for any model instance."""
# Get the base model class if this is a historical instance
if instance.__class__.__name__.startswith('Historical'):
model_class = instance.instance.__class__
instance_id = instance.instance.pk
else:
model_class = instance.__class__
instance_id = instance.pk
ct = ContentType.objects.get_for_model(model_class)
return self.create(content_type=ct, object_id=instance_id, **kwargs)
class ThreadedModelManager(models.Manager):
"""Manager for models that have comment threads."""
"""Manager for models that have comment threads."""
def get_comment_threads(self, instance):
"""Get comment threads for this instance."""
from comments.models import CommentThread
if not instance.pk:
return CommentThread.objects.none()
return CommentThread.objects.for_instance(instance)
def add_comment_thread(self, instance, **kwargs):
"""Create a comment thread for this instance."""
from comments.models import CommentThread
if not instance.pk:
raise ObjectDoesNotExist("Cannot create comment thread for unsaved instance")
return CommentThread.objects.create_for_instance(instance, **kwargs)
def with_comment_threads(self):
"""Get all instances with their comment threads."""
from comments.models import CommentThread
qs = self.get_queryset()
content_type = ContentType.objects.get_for_model(self.model)
# Get comment threads through a subquery
threads = CommentThread.objects.filter(
content_type=content_type,
object_id=models.OuterRef('pk')
)
return qs.annotate(
comment_count=models.Subquery(
threads.values('object_id')
.annotate(count=models.Count('id'))
.values('count'),
output_field=models.IntegerField()
)
)

View File

@@ -0,0 +1 @@

17
comments/mixins.py Normal file
View File

@@ -0,0 +1,17 @@
from django.contrib.contenttypes.fields import GenericRelation
from .models import get_comment_threads
class CommentableMixin:
"""
Mixin for models that should have comment functionality.
Uses composition instead of inheritance to avoid historical model issues.
"""
@property
def comments(self):
"""Get comments helper for this instance."""
if self.__class__.__name__.startswith('Historical'):
# Historical models delegate to their current instance
return self.instance.comments
return get_comment_threads(self)

View File

@@ -2,17 +2,14 @@ from django.db import models
from django.conf import settings from django.conf import settings
from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.fields import GenericForeignKey
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from .managers import CommentThreadManager, ThreadedModelManager
class CommentThread(models.Model): class CommentThread(models.Model):
""" """
A generic comment thread that can be attached to any model instance. A thread of comments that can be attached to any model instance,
Used for tracking discussions on various objects across the platform. including historical versions.
""" """
content_type = models.ForeignKey( content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
ContentType,
on_delete=models.CASCADE,
related_name='comment_threads'
)
object_id = models.PositiveIntegerField() object_id = models.PositiveIntegerField()
content_object = GenericForeignKey('content_type', 'object_id') content_object = GenericForeignKey('content_type', 'object_id')
@@ -28,6 +25,8 @@ class CommentThread(models.Model):
is_locked = models.BooleanField(default=False) is_locked = models.BooleanField(default=False)
is_hidden = models.BooleanField(default=False) is_hidden = models.BooleanField(default=False)
objects = CommentThreadManager()
class Meta: class Meta:
indexes = [ indexes = [
models.Index(fields=['content_type', 'object_id']), models.Index(fields=['content_type', 'object_id']),
@@ -37,11 +36,57 @@ class CommentThread(models.Model):
def __str__(self): def __str__(self):
return f"Comment Thread on {self.content_object} - {self.title}" return f"Comment Thread on {self.content_object} - {self.title}"
class CommentThreads:
"""
Helper class to manage comment threads for a model instance.
This is used instead of direct inheritance to avoid historical model issues.
"""
def __init__(self, instance):
self.instance = instance
self._info = {}
def get_info(self):
"""Get or compute comment thread information."""
if not self._info:
ct = ContentType.objects.get_for_model(self.instance.__class__)
self._info = {
'count': CommentThread.objects.filter(
content_type=ct,
object_id=self.instance.pk
).count(),
'content_type': ct,
'object_id': self.instance.pk
}
return self._info
def get_threads(self):
"""Get comment threads for this instance."""
info = self.get_info()
return CommentThread.objects.filter(
content_type=info['content_type'],
object_id=info['object_id']
)
def add_thread(self, title='', created_by=None):
"""Create a new comment thread for this instance."""
info = self.get_info()
thread = CommentThread.objects.create(
content_type=info['content_type'],
object_id=info['object_id'],
title=title,
created_by=created_by
)
self._info = {} # Clear cache
return thread
def get_comment_threads(instance):
"""Get or create a CommentThreads helper for a model instance."""
if not hasattr(instance, '_comment_threads'):
instance._comment_threads = CommentThreads(instance)
return instance._comment_threads
class Comment(models.Model): class Comment(models.Model):
""" """Individual comment within a thread."""
Individual comment within a comment thread.
"""
thread = models.ForeignKey( thread = models.ForeignKey(
CommentThread, CommentThread,
on_delete=models.CASCADE, on_delete=models.CASCADE,

1
comments/signals.py Normal file
View File

@@ -0,0 +1 @@
# This file intentionally left empty - signals have been replaced with direct mixin configuration

View File

@@ -1,16 +1,21 @@
from django.db import models from django.db import models
from django.utils.text import slugify from django.utils.text import slugify
from django.urls import reverse from django.urls import reverse
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes.fields import GenericRelation from django.contrib.contenttypes.fields import GenericRelation
from django.contrib.contenttypes.models import ContentType
from typing import Tuple, Optional, ClassVar, TYPE_CHECKING from typing import Tuple, Optional, ClassVar, TYPE_CHECKING
from history_tracking.models import HistoricalModel, VersionBranch, ChangeSet from history_tracking.models import HistoricalModel, VersionBranch, ChangeSet
from history_tracking.signals import get_current_branch, ChangesetContextManager from history_tracking.signals import get_current_branch, ChangesetContextManager
from comments.mixins import CommentableMixin
from media.mixins import PhotoableModel
if TYPE_CHECKING: if TYPE_CHECKING:
from history_tracking.models import HistoricalSlug from history_tracking.models import HistoricalSlug
class Company(HistoricalModel):
class Company(HistoricalModel, CommentableMixin, PhotoableModel):
comments = GenericRelation(
'comments.CommentThread') # Explicit relationship
name = models.CharField(max_length=255) name = models.CharField(max_length=255)
slug = models.SlugField(max_length=255, unique=True) slug = models.SlugField(max_length=255, unique=True)
website = models.URLField(blank=True) website = models.URLField(blank=True)
@@ -20,16 +25,13 @@ class Company(HistoricalModel):
total_rides = models.IntegerField(default=0) total_rides = models.IntegerField(default=0)
created_at = models.DateTimeField(auto_now_add=True) created_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True) updated_at = models.DateTimeField(auto_now=True)
comments = GenericRelation('comments.CommentThread',
related_name='company_threads',
related_query_name='comments_thread'
)
objects: ClassVar[models.Manager['Company']] objects: ClassVar[models.Manager['Company']]
class Meta: class Meta:
verbose_name_plural = 'companies' verbose_name_plural = 'companies'
ordering = ['name'] ordering = ['name']
excluded_fields = ['comments'] # Exclude from historical tracking
def __str__(self) -> str: def __str__(self) -> str:
return self.name return self.name
@@ -96,7 +98,10 @@ class Company(HistoricalModel):
except (HistoricalSlug.DoesNotExist, cls.DoesNotExist): except (HistoricalSlug.DoesNotExist, cls.DoesNotExist):
raise cls.DoesNotExist() raise cls.DoesNotExist()
class Manufacturer(HistoricalModel):
class Manufacturer(HistoricalModel, CommentableMixin, PhotoableModel):
comments = GenericRelation(
'comments.CommentThread') # Explicit relationship
name = models.CharField(max_length=255) name = models.CharField(max_length=255)
slug = models.SlugField(max_length=255, unique=True) slug = models.SlugField(max_length=255, unique=True)
website = models.URLField(blank=True) website = models.URLField(blank=True)
@@ -106,15 +111,13 @@ class Manufacturer(HistoricalModel):
total_roller_coasters = models.IntegerField(default=0) total_roller_coasters = models.IntegerField(default=0)
created_at = models.DateTimeField(auto_now_add=True) created_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True) updated_at = models.DateTimeField(auto_now=True)
comments = GenericRelation('comments.CommentThread',
related_name='manufacturer_threads',
related_query_name='comments_thread'
)
objects: ClassVar[models.Manager['Manufacturer']] objects: ClassVar[models.Manager['Manufacturer']]
class Meta: class Meta:
ordering = ['name'] ordering = ['name']
excluded_fields = ['comments'] # Exclude from historical tracking
history_exclude = ['comments'] # Exclude from historical models
def __str__(self) -> str: def __str__(self) -> str:
return self.name return self.name
@@ -181,7 +184,10 @@ class Manufacturer(HistoricalModel):
except (HistoricalSlug.DoesNotExist, cls.DoesNotExist): except (HistoricalSlug.DoesNotExist, cls.DoesNotExist):
raise cls.DoesNotExist() raise cls.DoesNotExist()
class Designer(HistoricalModel):
class Designer(HistoricalModel, CommentableMixin, PhotoableModel):
comments = GenericRelation(
'comments.CommentThread') # Explicit relationship
name = models.CharField(max_length=255) name = models.CharField(max_length=255)
slug = models.SlugField(max_length=255, unique=True) slug = models.SlugField(max_length=255, unique=True)
website = models.URLField(blank=True) website = models.URLField(blank=True)
@@ -190,10 +196,6 @@ class Designer(HistoricalModel):
total_roller_coasters = models.IntegerField(default=0) total_roller_coasters = models.IntegerField(default=0)
created_at = models.DateTimeField(auto_now_add=True) created_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True) updated_at = models.DateTimeField(auto_now=True)
comments = GenericRelation('comments.CommentThread',
related_name='designer_threads',
related_query_name='comments_thread'
)
objects: ClassVar[models.Manager['Designer']] objects: ClassVar[models.Manager['Designer']]

View File

@@ -0,0 +1,61 @@
from django.db import models
from simple_history.models import HistoricalRecords
from django.contrib.contenttypes.fields import GenericRelation
from django.utils.timezone import now
class CustomHistoricalRecords(HistoricalRecords):
"""Custom historical records that properly handle generic relations."""
def copy_fields(self, model):
"""
Copy fields from the model to the historical record model,
excluding GenericRelation fields.
"""
fields = {}
for field in model._meta.concrete_fields:
if not isinstance(field, GenericRelation) and field.name not in [
'comments', 'comment_threads', 'photos', 'reviews'
]:
fields[field.name] = field.clone()
return fields
def create_history_model(self, model, inherited):
"""
Override to ensure we don't create duplicate auto fields.
"""
attrs = {
'__module__': model.__module__,
'_history_excluded_fields': ['comments', 'comment_threads', 'photos', 'reviews'],
}
app_module = '%s.models' % model._meta.app_label
if inherited:
# inherited use models.AutoField instead of models.IntegerField
attrs.update({
'id': models.AutoField(primary_key=True),
'history_id': models.AutoField(primary_key=True),
'history_date': models.DateTimeField(default=now),
'history_change_reason': models.CharField(max_length=100, null=True),
'history_type': models.CharField(max_length=1, choices=(
('+', 'Created'),
('~', 'Changed'),
('-', 'Deleted'),
)),
'history_user': models.ForeignKey(
'accounts.User',
null=True,
on_delete=models.SET_NULL,
related_name='+'
),
})
# Convert field to point to historical model
fields = self.copy_fields(model)
attrs.update(fields)
return type(
str('Historical%s' % model._meta.object_name),
(models.Model,),
attrs
)

View File

@@ -0,0 +1,49 @@
from django.db import models
from django.conf import settings
from django.contrib.contenttypes.fields import GenericRelation, GenericForeignKey
from typing import List, Type
def get_trackable_fields(model_class: Type[models.Model]) -> List[models.Field]:
"""Get fields that should be tracked in history."""
if getattr(model_class, '_is_historical_model', False):
# For historical models, only return core history fields
return [
models.BigAutoField(name='id', primary_key=True),
models.DateTimeField(name='history_date'),
models.CharField(name='history_change_reason', max_length=100, null=True),
models.CharField(name='history_type', max_length=1),
models.ForeignKey(
to=settings.AUTH_USER_MODEL,
name='history_user',
null=True,
on_delete=models.SET_NULL
)
]
trackable_fields = []
excluded_fields = {
'comment_threads', 'comments', 'photos', 'reviews',
'thread', 'content_type', 'object_id', 'content_object'
}
for field in model_class._meta.get_fields():
# Skip fields we don't want to track
if any([
isinstance(field, (GenericRelation, GenericForeignKey)),
field.name in excluded_fields,
field.is_relation and hasattr(field.remote_field.model, '_meta') and
'commentthread' in field.remote_field.model._meta.model_name.lower()
]):
continue
trackable_fields.append(field)
return trackable_fields
class HistoricalFieldsMixin:
"""Mixin that controls which fields are copied to historical models."""
@classmethod
def get_fields_to_track(cls) -> List[models.Field]:
"""Get fields that should be tracked in history."""
return get_trackable_fields(cls)

View File

@@ -1,10 +1,13 @@
from django.db import models from django.db import models
from django.conf import settings
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
from django.db.models.fields.related import RelatedField
from django.contrib.auth import get_user_model from django.contrib.auth import get_user_model
from simple_history.models import HistoricalRecords from simple_history.models import HistoricalRecords
from .mixins import HistoricalChangeMixin from .mixins import HistoricalChangeMixin
from typing import Any, Type, TypeVar, cast, Optional from .historical_fields import HistoricalFieldsMixin
from typing import Any, Type, TypeVar, cast, Optional, List
from django.db.models import QuerySet from django.db.models import QuerySet
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.utils import timezone from django.utils import timezone
@@ -13,13 +16,28 @@ T = TypeVar('T', bound=models.Model)
User = get_user_model() User = get_user_model()
class HistoricalModel(models.Model): class HistoricalModel(models.Model, HistoricalFieldsMixin):
"""Abstract base class for models with history tracking""" """Abstract base class for models with history tracking"""
id = models.BigAutoField(primary_key=True) id = models.BigAutoField(primary_key=True)
history: HistoricalRecords = HistoricalRecords(
@classmethod
def __init_subclass__(cls, **kwargs):
"""Initialize subclass with proper configuration."""
super().__init_subclass__(**kwargs)
# Mark historical models
if cls.__name__.startswith('Historical'):
cls._is_historical_model = True
# Remove any inherited generic relations
for field in list(cls._meta.private_fields):
if isinstance(field, GenericRelation):
cls._meta.private_fields.remove(field)
else:
cls._is_historical_model = False
history = HistoricalRecords(
inherit=True, inherit=True,
bases=(HistoricalChangeMixin,), bases=[HistoricalChangeMixin],
excluded_fields=['comments', 'photos', 'reviews'] # Exclude all generic relations excluded_fields=['comments', 'comment_threads', 'photos', 'reviews'],
use_base_model_db=True # Use base model's db
) )
class Meta: class Meta:

43
location/mixins.py Normal file
View File

@@ -0,0 +1,43 @@
from django.contrib.contenttypes.models import ContentType
from django.db.models import QuerySet
from typing import Optional, Tuple
class LocationMixin:
"""Mixin for models that can have location data attached."""
def get_location(self) -> Optional['Location']:
"""Get location for this instance."""
from location.models import Location
ct = ContentType.objects.get_for_model(self.__class__)
return Location.objects.filter(content_type=ct, object_id=self.pk).first()
def set_location(self, address: str, latitude: float, longitude: float) -> 'Location':
"""Set or update location for this instance."""
from location.models import Location
ct = ContentType.objects.get_for_model(self.__class__)
location, created = Location.objects.update_or_create(
content_type=ct,
object_id=self.pk,
defaults={
'address': address,
'latitude': latitude,
'longitude': longitude
}
)
return location
@property
def coordinates(self) -> Optional[Tuple[float, float]]:
"""Get coordinates (latitude, longitude) if available."""
location = self.get_location()
if location:
return location.latitude, location.longitude
return None
@property
def formatted_location(self) -> str:
"""Get formatted address string if available."""
location = self.get_location()
if location:
return location.get_formatted_address()
return ""

View File

@@ -1,19 +1,28 @@
from django.contrib import admin from django.contrib import admin
from django.utils.html import format_html from django.contrib.contenttypes.admin import GenericStackedInline
from .models import Photo from .models import Photo
class PhotoInline(GenericStackedInline):
"""Inline admin for photos that can be added to any model."""
model = Photo
extra = 1
fields = ('image', 'caption', 'alt_text', 'is_primary')
classes = ('collapse',)
@admin.register(Photo) @admin.register(Photo)
class PhotoAdmin(admin.ModelAdmin): class PhotoAdmin(admin.ModelAdmin):
list_display = ('thumbnail_preview', 'content_type', 'content_object', 'caption', 'is_primary', 'created_at') list_display = ('caption', 'content_type', 'object_id', 'is_primary', 'created_at')
list_filter = ('content_type', 'is_primary', 'created_at') list_filter = ('content_type', 'created_at', 'is_primary', 'is_approved')
search_fields = ('caption', 'alt_text') search_fields = ('caption', 'alt_text')
readonly_fields = ('thumbnail_preview',) ordering = ('content_type', 'object_id', '-is_primary')
readonly_fields = ('created_at', 'updated_at')
def thumbnail_preview(self, obj): fieldsets = (
if obj.image: ('Image', {
return format_html( 'fields': ('image', 'caption', 'alt_text', 'is_primary', 'is_approved')
'<img src="{}" style="max-height: 50px; max-width: 100px;" />', }),
obj.image.url ('Metadata', {
'fields': ('content_type', 'object_id', 'created_at', 'updated_at'),
'classes': ('collapse',)
}),
) )
return "No image"
thumbnail_preview.short_description = 'Thumbnail'

19
media/mixins.py Normal file
View File

@@ -0,0 +1,19 @@
from django.contrib.contenttypes.models import ContentType
from django.db.models import QuerySet
class PhotoableModel:
"""Mixin for models that can have photos attached."""
def get_photos(self) -> QuerySet:
"""Get photos for this instance."""
from media.models import Photo
ct = ContentType.objects.get_for_model(self.__class__)
return Photo.objects.filter(content_type=ct, object_id=self.pk)
def add_photo(self, photo: 'Photo') -> None:
"""Add a photo to this instance."""
from media.models import Photo
ct = ContentType.objects.get_for_model(self.__class__)
photo.content_type = ct
photo.object_id = self.pk
photo.save()

View File

@@ -1,148 +1,74 @@
# Active Development Context # Comment System Architecture Fix
## Current Implementation Status ## Required Code Modifications
Version Control System has been evaluated and requires several enhancements:
### Completed ### 1. Central CommentThread Model (comments/models.py)
1. Core VCS Components: ```python
- Base models (VersionBranch, VersionTag, ChangeSet) from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
- Business logic (BranchManager, ChangeTracker, MergeStrategy) from django.contrib.contenttypes.models import ContentType
- UI components and templates from django.db import models
- Asset integration (JS/CSS)
- Comprehensive monitoring system
- Basic caching implementation
2. Initial Integration: class CommentThread(models.Model):
- Park model VCS integration """Centralized comment threading system"""
- ParkArea model VCS integration content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
- Base template VCS support object_id = models.PositiveIntegerField()
- Park detail template integration content_object = GenericForeignKey()
- Version control context processor created_at = models.DateTimeField(auto_now_add=True)
- Monitoring and metrics collection updated_at = models.DateTimeField(auto_now=True)
3. Documentation: class Meta:
- Technical implementation guide indexes = [
- Template integration guide models.Index(fields=["content_type", "object_id"]),
- Implementation checklist ]
- Base README app_label = 'comments'
- API documentation ```
- User guide
### In Progress ### 2. Model Reference Updates (Example for companies/models.py)
1. Model Integration: ```python
- [ ] Rides system # In all affected models (companies, rides, parks, reviews):
- [ ] Reviews system from comments.models import CommentThread
- [ ] Companies system
- [ ] Batch processing implementation
- [ ] Enhanced caching layer
2. Template Updates: class Company(models.Model):
- [ ] Park list view # ... existing fields ...
- [ ] Ride detail/list views comments = GenericRelation(CommentThread) # Updated reference
- [ ] Review detail/list views ```
- [ ] Company detail/list views
- [ ] Performance optimized components
### Newly Identified Requirements ### 3. Historical Records Adjustment
1. Performance Optimizations: ```python
- [ ] Implement batch processing for large changesets # Historical model definitions:
- [ ] Add caching for frequently accessed version history class HistoricalCompany(HistoricalRecords):
- [ ] Optimize query patterns for large history sets comments = models.ForeignKey(
'comments.CommentThread', # Unified reference
on_delete=models.SET_NULL,
null=True,
blank=True
)
```
2. Scalability Enhancements: ## Migration Execution Plan
- [ ] Implement archive strategy for old history records
- [ ] Add partitioning support for large history tables
- [ ] Develop async processing for heavy operations
3. Security Improvements: 1. Generate initial comment thread migration:
- [ ] Add encryption for sensitive changes ```bash
- [ ] Enhance access control granularity ./manage.py makemigrations comments --name create_commentthread
- [ ] Implement audit logging improvements ```
## Immediate Next Steps 2. Create dependent migrations for each modified app:
1. Performance Optimization (Priority) ```bash
```python for app in companies rides parks reviews; do
# Add to history_tracking/batch.py: ./manage.py makemigrations $app --name update_comment_references
class BatchChangeProcessor: done
def process_changes(self, changes): ```
"""Process multiple changes efficiently"""
with transaction.atomic():
# Batch processing logic
```
2. Caching Enhancement 3. Migration dependency chain:
```python ```python
# Add to history_tracking/caching.py: # In each app's migration file:
class VersionHistoryCache: dependencies = [
def cache_version_info(self): ('comments', '0001_create_commentthread'),
"""Cache frequently accessed version data""" ]
# Caching implementation ```
```
3. Testing Expansion ## Validation Checklist
- Add performance benchmarks - [ ] Run full test suite: `uv test ./manage.py test`
- Implement stress testing - [ ] Execute system check: `uv run ./manage.py check --deploy`
- Create scalability tests - [ ] Verify database schema changes in migration files
- [ ] Confirm admin interface comment relationships
## Active Issues
1. Need to implement batch processing for large changesets
2. Must enhance caching strategy for version history
3. Need to implement proper cleanup for old versions
4. Performance optimization required for large history sets
5. Archiving strategy needed for historical data
## Technical Dependencies
- django-simple-history: Base history tracking
- HTMX: UI interactions
- Alpine.js: Frontend reactivity
- Custom VCS components
- Redis: Enhanced caching (planned)
- Celery: Async processing (planned)
## Integration Strategy
1. Roll out performance optimizations
2. Implement enhanced caching
3. Deploy batch processing
4. Add archiving system
5. Implement async operations
## Monitoring Points
- Track version control operation performance
- Monitor database size with version history
- Watch for merge conflicts
- Track user interaction patterns
- Monitor cache hit rates
- Track batch processing efficiency
- Measure async operation latency
## Code Standards
- All versioned models inherit from HistoricalModel
- Consistent save method implementation
- Proper branch context management
- Standard version control UI components
- Performance optimization patterns
- Caching standards
- Batch processing guidelines
## Documentation Status
- [x] Technical implementation
- [x] Template integration guide
- [x] API documentation
- [x] User guide
- [ ] Admin documentation
- [ ] Performance tuning guide
- [ ] Scaling guidelines
## Current Branch
main
## Environment
- Django with HTMX integration
- PostgreSQL database
- django-simple-history
- Custom VCS extensions
- Redis (planned)
- Celery (planned)
## Recent Evaluation
Full system evaluation completed on 2025-02-07. Details in `memory-bank/evaluations/version_control_evaluation.md`.

View File

@@ -0,0 +1,33 @@
# Historical Model Comment Fixes
## Problem
System check errors occurred because historical models referenced CommentThread in their own app context (e.g. `companies.commentthread`) instead of the actual `comments.CommentThread` model.
## Solution
Added `excluded_fields = ['comments']` to Meta classes of all affected models to exclude comment relationships from historical tracking. Note: Initially tried `history_exclude` but this was incorrect - django-simple-history uses `excluded_fields`.
## Affected Models (Fixed)
- Company (companies/models.py)
- Manufacturer (companies/models.py)
- Designer (companies/models.py)
- Park (parks/models.py)
- ParkArea (parks/models.py)
- Ride (rides/models.py)
- RideModel (rides/models.py)
- Review (reviews/models.py)
## Implementation Details
Each model's Meta class was updated to exclude the comments field from historical tracking:
```python
class Meta:
# ... other Meta options ...
excluded_fields = ['comments'] # Exclude from historical tracking
```
This prevents django-simple-history from attempting to track the GenericRelation field in historical models, which was causing the system check errors.
## Verification
Run system checks to verify fix:
```bash
python manage.py check

View File

@@ -1,22 +1,26 @@
from django.db import models from django.db import models
from django.urls import reverse from django.urls import reverse
from django.utils.text import slugify from django.utils.text import slugify
from django.contrib.contenttypes.fields import GenericRelation
from django.core.exceptions import ValidationError 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
from django.contrib.contenttypes.fields import GenericRelation
from companies.models import Company from companies.models import Company
from history_tracking.signals import get_current_branch from history_tracking.signals import get_current_branch
from media.models import Photo from media.models import Photo
from history_tracking.models import HistoricalModel from history_tracking.models import HistoricalModel
from location.models import Location from location.models import Location
from comments.mixins import CommentableMixin
from media.mixins import PhotoableModel
from location.mixins import LocationMixin
if TYPE_CHECKING: if TYPE_CHECKING:
from rides.models import Ride from rides.models import Ride
class Park(HistoricalModel): class Park(HistoricalModel, CommentableMixin, PhotoableModel, LocationMixin):
comments = GenericRelation('comments.CommentThread') # Centralized reference
id: int # Type hint for Django's automatic id field id: int # Type hint for Django's automatic id field
STATUS_CHOICES = [ STATUS_CHOICES = [
("OPERATING", "Operating"), ("OPERATING", "Operating"),
@@ -34,9 +38,6 @@ class Park(HistoricalModel):
max_length=20, choices=STATUS_CHOICES, default="OPERATING" max_length=20, choices=STATUS_CHOICES, default="OPERATING"
) )
# Location fields using GenericRelation
location = GenericRelation(Location, related_query_name='park')
# Details # Details
opening_date = models.DateField(null=True, blank=True) opening_date = models.DateField(null=True, blank=True)
closing_date = models.DateField(null=True, blank=True) closing_date = models.DateField(null=True, blank=True)
@@ -57,12 +58,8 @@ class Park(HistoricalModel):
owner = models.ForeignKey( owner = models.ForeignKey(
Company, on_delete=models.SET_NULL, null=True, blank=True, related_name="parks" Company, on_delete=models.SET_NULL, null=True, blank=True, related_name="parks"
) )
photos = GenericRelation(Photo, related_query_name="park")
comments = GenericRelation('comments.CommentThread',
related_name='park_threads',
related_query_name='comments_thread'
)
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 rides: models.Manager['Ride'] # Type hint for reverse relation from rides app
# Metadata # Metadata
@@ -71,6 +68,7 @@ class Park(HistoricalModel):
class Meta: class Meta:
ordering = ["name"] ordering = ["name"]
excluded_fields = ['comments'] # Exclude from historical tracking
def __str__(self) -> str: def __str__(self) -> str:
return self.name return self.name
@@ -126,23 +124,6 @@ class Park(HistoricalModel):
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})
@property
def formatted_location(self) -> str:
if self.location.exists():
location = self.location.first()
if location:
return location.get_formatted_address()
return ""
@property
def coordinates(self) -> Optional[Tuple[float, float]]:
"""Returns coordinates as a tuple (latitude, longitude)"""
if self.location.exists():
location = self.location.first()
if location:
return location.coordinates
return None
@classmethod @classmethod
def get_by_slug(cls, slug: str) -> Tuple['Park', bool]: def get_by_slug(cls, slug: str) -> Tuple['Park', bool]:
"""Get park by current or historical slug""" """Get park by current or historical slug"""
@@ -159,7 +140,8 @@ class Park(HistoricalModel):
raise cls.DoesNotExist("No park found with this slug") raise cls.DoesNotExist("No park found with this slug")
class ParkArea(HistoricalModel): class ParkArea(HistoricalModel, CommentableMixin, PhotoableModel):
comments = GenericRelation('comments.CommentThread') # Centralized reference
id: int # Type hint for Django's automatic id field id: int # Type hint for Django's automatic id field
park = models.ForeignKey(Park, on_delete=models.CASCADE, related_name="areas") park = models.ForeignKey(Park, on_delete=models.CASCADE, related_name="areas")
name = models.CharField(max_length=255) name = models.CharField(max_length=255)
@@ -169,10 +151,6 @@ class ParkArea(HistoricalModel):
closing_date = models.DateField(null=True, blank=True) closing_date = models.DateField(null=True, blank=True)
# Relationships # Relationships
comments = GenericRelation('comments.CommentThread',
related_name='park_area_threads',
related_query_name='comments_thread'
)
# Metadata # Metadata
created_at = models.DateTimeField(auto_now_add=True, null=True) created_at = models.DateTimeField(auto_now_add=True, null=True)
@@ -181,6 +159,7 @@ class ParkArea(HistoricalModel):
class Meta: class Meta:
ordering = ["name"] ordering = ["name"]
unique_together = ["park", "slug"] unique_together = ["park", "slug"]
excluded_fields = ['comments'] # Exclude from historical tracking
def __str__(self) -> str: def __str__(self) -> str:
return f"{self.name} at {self.park.name}" return f"{self.name} at {self.park.name}"

View File

@@ -45,3 +45,4 @@ daphne==4.1.2
# React and Material UI will be handled via npm in the frontend directory # React and Material UI will be handled via npm in the frontend directory
django-simple-history==3.8.0 django-simple-history==3.8.0
django-tailwind-cli==2.21.1 django-tailwind-cli==2.21.1
celery==5.3.6

View File

@@ -1,11 +1,7 @@
from django.contrib import admin from django.contrib import admin
from django.utils.html import format_html from django.utils.html import format_html
from .models import Review, ReviewImage, ReviewLike, ReviewReport from media.admin import PhotoInline
from .models import Review, ReviewLike, ReviewReport
class ReviewImageInline(admin.TabularInline):
model = ReviewImage
extra = 1
fields = ('image', 'caption', 'order')
@admin.register(Review) @admin.register(Review)
class ReviewAdmin(admin.ModelAdmin): class ReviewAdmin(admin.ModelAdmin):
@@ -14,7 +10,7 @@ class ReviewAdmin(admin.ModelAdmin):
search_fields = ('user__username', 'content', 'title') search_fields = ('user__username', 'content', 'title')
readonly_fields = ('created_at', 'updated_at') readonly_fields = ('created_at', 'updated_at')
actions = ['publish_reviews', 'unpublish_reviews'] actions = ['publish_reviews', 'unpublish_reviews']
inlines = [ReviewImageInline] inlines = [PhotoInline]
fieldsets = ( fieldsets = (
('Review Details', { ('Review Details', {
@@ -55,13 +51,6 @@ class ReviewAdmin(admin.ModelAdmin):
queryset.update(is_published=False) queryset.update(is_published=False)
unpublish_reviews.short_description = "Unpublish selected reviews" unpublish_reviews.short_description = "Unpublish selected reviews"
@admin.register(ReviewImage)
class ReviewImageAdmin(admin.ModelAdmin):
list_display = ('review', 'caption', 'order')
list_filter = ('review__created_at',)
search_fields = ('review__title', 'caption')
ordering = ('review', 'order')
@admin.register(ReviewLike) @admin.register(ReviewLike)
class ReviewLikeAdmin(admin.ModelAdmin): class ReviewLikeAdmin(admin.ModelAdmin):
list_display = ('review', 'user', 'created_at') list_display = ('review', 'user', 'created_at')

19
reviews/mixins.py Normal file
View File

@@ -0,0 +1,19 @@
from django.contrib.contenttypes.models import ContentType
from django.db.models import QuerySet
class ReviewableMixin:
"""Mixin for models that can have reviews."""
def get_reviews(self) -> QuerySet:
"""Get reviews for this instance."""
from reviews.models import Review
ct = ContentType.objects.get_for_model(self.__class__)
return Review.objects.filter(content_type=ct, object_id=self.pk)
def add_review(self, review: 'Review') -> None:
"""Add a review to this instance."""
from reviews.models import Review
ct = ContentType.objects.get_for_model(self.__class__)
review.content_type = ct
review.object_id = self.pk
review.save()

View File

@@ -5,8 +5,11 @@ from django.contrib.contenttypes.models import ContentType
from django.core.validators import MinValueValidator, MaxValueValidator from django.core.validators import MinValueValidator, MaxValueValidator
from history_tracking.models import HistoricalModel, VersionBranch, ChangeSet from history_tracking.models import HistoricalModel, VersionBranch, ChangeSet
from history_tracking.signals import get_current_branch, ChangesetContextManager from history_tracking.signals import get_current_branch, ChangesetContextManager
from comments.mixins import CommentableMixin
from media.mixins import PhotoableModel
class Review(HistoricalModel): class Review(HistoricalModel, CommentableMixin, PhotoableModel):
comments = GenericRelation('comments.CommentThread') # Centralized reference
# Generic relation to allow reviews on different types (rides, parks) # Generic relation to allow reviews on different types (rides, parks)
content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
object_id = models.PositiveIntegerField() object_id = models.PositiveIntegerField()
@@ -41,17 +44,13 @@ class Review(HistoricalModel):
) )
moderated_at = models.DateTimeField(null=True, blank=True) moderated_at = models.DateTimeField(null=True, blank=True)
# Comments
comments = GenericRelation('comments.CommentThread',
related_name='review_threads',
related_query_name='comments_thread'
)
class Meta: class Meta:
ordering = ['-created_at'] ordering = ['-created_at']
indexes = [ indexes = [
models.Index(fields=['content_type', 'object_id']), models.Index(fields=['content_type', 'object_id']),
] ]
excluded_fields = ['comments'] # Exclude from historical tracking
def __str__(self): def __str__(self):
return f"Review of {self.content_object} by {self.user.username}" return f"Review of {self.content_object} by {self.user.username}"
@@ -108,22 +107,6 @@ class Review(HistoricalModel):
return f"{base_url}#review-{self.pk}" return f"{base_url}#review-{self.pk}"
return reverse('reviews:review_detail', kwargs={'pk': self.pk}) return reverse('reviews:review_detail', kwargs={'pk': self.pk})
class ReviewImage(models.Model):
review = models.ForeignKey(
Review,
on_delete=models.CASCADE,
related_name='images'
)
image = models.ImageField(upload_to='review_images/')
caption = models.CharField(max_length=200, blank=True)
order = models.PositiveIntegerField(default=0)
class Meta:
ordering = ['order']
def __str__(self):
return f"Image {self.order + 1} for {self.review}"
class ReviewLike(models.Model): class ReviewLike(models.Model):
review = models.ForeignKey( review = models.ForeignKey(
Review, Review,

View File

@@ -1,10 +1,13 @@
from django.db import models from django.db import models
from django.urls import reverse from django.urls import reverse
from django.utils.text import slugify from django.utils.text import slugify
from django.contrib.contenttypes.fields import GenericRelation
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes.fields import GenericRelation
from history_tracking.models import HistoricalModel, VersionBranch, ChangeSet from history_tracking.models import HistoricalModel, VersionBranch, ChangeSet
from history_tracking.signals import get_current_branch, ChangesetContextManager from history_tracking.signals import get_current_branch, ChangesetContextManager
from comments.mixins import CommentableMixin
from media.mixins import PhotoableModel
from reviews.mixins import ReviewableMixin
# Shared choices that will be used by multiple models # Shared choices that will be used by multiple models
@@ -19,7 +22,8 @@ CATEGORY_CHOICES = [
] ]
class RideModel(HistoricalModel): class RideModel(HistoricalModel, CommentableMixin, PhotoableModel):
comments = GenericRelation('comments.CommentThread') # Centralized reference
""" """
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.
For example: B&M Dive Coaster, Vekoma Boomerang, etc. For example: B&M Dive Coaster, Vekoma Boomerang, etc.
@@ -41,14 +45,12 @@ class RideModel(HistoricalModel):
) )
created_at = models.DateTimeField(auto_now_add=True) created_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True) updated_at = models.DateTimeField(auto_now=True)
comments = GenericRelation('comments.CommentThread',
related_name='ride_model_threads',
related_query_name='comments_thread'
)
class Meta: class Meta:
ordering = ['manufacturer', 'name'] ordering = ['manufacturer', 'name']
unique_together = ['manufacturer', 'name'] unique_together = ['manufacturer', 'name']
excluded_fields = ['comments'] # Exclude from historical tracking
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}"
@@ -96,7 +98,8 @@ def get_absolute_url(self) -> str:
class Ride(HistoricalModel): class Ride(HistoricalModel, CommentableMixin, PhotoableModel, ReviewableMixin):
comments = GenericRelation('comments.CommentThread') # Centralized reference
STATUS_CHOICES = [ STATUS_CHOICES = [
('OPERATING', 'Operating'), ('OPERATING', 'Operating'),
('SBNO', 'Standing But Not Operating'), ('SBNO', 'Standing But Not Operating'),
@@ -181,16 +184,11 @@ class Ride(HistoricalModel):
) )
created_at = models.DateTimeField(auto_now_add=True) created_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True) updated_at = models.DateTimeField(auto_now=True)
photos = GenericRelation('media.Photo')
reviews = GenericRelation('reviews.Review')
comments = GenericRelation('comments.CommentThread',
related_name='ride_threads',
related_query_name='comments_thread'
)
class Meta: class Meta:
ordering = ['name'] ordering = ['name']
unique_together = ['park', 'slug'] unique_together = ['park', 'slug']
excluded_fields = ['comments'] # Exclude from historical tracking
def __str__(self) -> str: def __str__(self) -> str:
return f"{self.name} at {self.park.name}" return f"{self.name} at {self.park.name}"