diff --git a/comments/apps.py b/comments/apps.py index 6aa34832..138f519f 100644 --- a/comments/apps.py +++ b/comments/apps.py @@ -1,6 +1,10 @@ from django.apps import AppConfig - +from django.db.models.signals import class_prepared, post_init class CommentsConfig(AppConfig): - default_auto_field = "django.db.models.BigAutoField" - name = "comments" + default_auto_field = 'django.db.models.BigAutoField' + name = 'comments' + + def ready(self): + """Set up comment system when the app is ready.""" + pass diff --git a/comments/managers.py b/comments/managers.py new file mode 100644 index 00000000..14390738 --- /dev/null +++ b/comments/managers.py @@ -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() + ) + ) \ No newline at end of file diff --git a/comments/migrations/__init__.py b/comments/migrations/__init__.py index e69de29b..8b137891 100644 --- a/comments/migrations/__init__.py +++ b/comments/migrations/__init__.py @@ -0,0 +1 @@ + diff --git a/comments/mixins.py b/comments/mixins.py new file mode 100644 index 00000000..442a8ae0 --- /dev/null +++ b/comments/mixins.py @@ -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) \ No newline at end of file diff --git a/comments/models.py b/comments/models.py index ebd4a941..567f8f61 100644 --- a/comments/models.py +++ b/comments/models.py @@ -2,20 +2,17 @@ from django.db import models from django.conf import settings from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType +from .managers import CommentThreadManager, ThreadedModelManager class CommentThread(models.Model): """ - A generic comment thread that can be attached to any model instance. - Used for tracking discussions on various objects across the platform. + A thread of comments that can be attached to any model instance, + including historical versions. """ - content_type = models.ForeignKey( - ContentType, - on_delete=models.CASCADE, - related_name='comment_threads' - ) + content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) object_id = models.PositiveIntegerField() content_object = GenericForeignKey('content_type', 'object_id') - + title = models.CharField(max_length=255, blank=True) created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) @@ -28,6 +25,8 @@ class CommentThread(models.Model): is_locked = models.BooleanField(default=False) is_hidden = models.BooleanField(default=False) + objects = CommentThreadManager() + class Meta: indexes = [ models.Index(fields=['content_type', 'object_id']), @@ -37,11 +36,57 @@ class CommentThread(models.Model): def __str__(self): 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): - """ - Individual comment within a comment thread. - """ + """Individual comment within a thread.""" thread = models.ForeignKey( CommentThread, on_delete=models.CASCADE, diff --git a/comments/signals.py b/comments/signals.py new file mode 100644 index 00000000..dcd5795b --- /dev/null +++ b/comments/signals.py @@ -0,0 +1 @@ +# This file intentionally left empty - signals have been replaced with direct mixin configuration \ No newline at end of file diff --git a/companies/models.py b/companies/models.py index ab3ece7e..18b34c28 100644 --- a/companies/models.py +++ b/companies/models.py @@ -1,16 +1,21 @@ from django.db import models from django.utils.text import slugify from django.urls import reverse -from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.fields import GenericRelation +from django.contrib.contenttypes.models import ContentType from typing import Tuple, Optional, ClassVar, TYPE_CHECKING from history_tracking.models import HistoricalModel, VersionBranch, ChangeSet from history_tracking.signals import get_current_branch, ChangesetContextManager +from comments.mixins import CommentableMixin +from media.mixins import PhotoableModel if TYPE_CHECKING: 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) slug = models.SlugField(max_length=255, unique=True) website = models.URLField(blank=True) @@ -20,16 +25,13 @@ class Company(HistoricalModel): total_rides = models.IntegerField(default=0) created_at = models.DateTimeField(auto_now_add=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']] class Meta: verbose_name_plural = 'companies' ordering = ['name'] + excluded_fields = ['comments'] # Exclude from historical tracking def __str__(self) -> str: return self.name @@ -37,10 +39,10 @@ class Company(HistoricalModel): def save(self, *args, **kwargs) -> None: if not self.slug: self.slug = slugify(self.name) - + # Get the branch from context or use default current_branch = get_current_branch() - + if current_branch: # Save in the context of the current branch super().save(*args, **kwargs) @@ -50,7 +52,7 @@ class Company(HistoricalModel): name='main', defaults={'metadata': {'type': 'default_branch'}} ) - + with ChangesetContextManager(branch=main_branch): super().save(*args, **kwargs) @@ -62,20 +64,20 @@ class Company(HistoricalModel): object_id=self.pk, status='applied' ).order_by('-created_at')[:5] - + active_branches = VersionBranch.objects.filter( changesets__content_type=content_type, changesets__object_id=self.pk, is_active=True ).distinct() - + return { 'latest_changes': latest_changes, 'active_branches': active_branches, 'current_branch': get_current_branch(), 'total_changes': latest_changes.count() } - + def get_absolute_url(self) -> str: return reverse("companies:company_detail", kwargs={"slug": self.slug}) @@ -96,7 +98,10 @@ class Company(HistoricalModel): except (HistoricalSlug.DoesNotExist, 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) slug = models.SlugField(max_length=255, unique=True) website = models.URLField(blank=True) @@ -106,15 +111,13 @@ class Manufacturer(HistoricalModel): total_roller_coasters = models.IntegerField(default=0) created_at = models.DateTimeField(auto_now_add=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']] class Meta: ordering = ['name'] + excluded_fields = ['comments'] # Exclude from historical tracking + history_exclude = ['comments'] # Exclude from historical models def __str__(self) -> str: return self.name @@ -122,10 +125,10 @@ class Manufacturer(HistoricalModel): def save(self, *args, **kwargs) -> None: if not self.slug: self.slug = slugify(self.name) - + # Get the branch from context or use default current_branch = get_current_branch() - + if current_branch: # Save in the context of the current branch super().save(*args, **kwargs) @@ -135,7 +138,7 @@ class Manufacturer(HistoricalModel): name='main', defaults={'metadata': {'type': 'default_branch'}} ) - + with ChangesetContextManager(branch=main_branch): super().save(*args, **kwargs) @@ -147,13 +150,13 @@ class Manufacturer(HistoricalModel): object_id=self.pk, status='applied' ).order_by('-created_at')[:5] - + active_branches = VersionBranch.objects.filter( changesets__content_type=content_type, changesets__object_id=self.pk, is_active=True ).distinct() - + return { 'latest_changes': latest_changes, 'active_branches': active_branches, @@ -181,7 +184,10 @@ class Manufacturer(HistoricalModel): except (HistoricalSlug.DoesNotExist, 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) slug = models.SlugField(max_length=255, unique=True) website = models.URLField(blank=True) @@ -190,10 +196,6 @@ class Designer(HistoricalModel): total_roller_coasters = models.IntegerField(default=0) created_at = models.DateTimeField(auto_now_add=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']] @@ -206,10 +208,10 @@ class Designer(HistoricalModel): def save(self, *args, **kwargs) -> None: if not self.slug: self.slug = slugify(self.name) - + # Get the branch from context or use default current_branch = get_current_branch() - + if current_branch: # Save in the context of the current branch super().save(*args, **kwargs) @@ -219,7 +221,7 @@ class Designer(HistoricalModel): name='main', defaults={'metadata': {'type': 'default_branch'}} ) - + with ChangesetContextManager(branch=main_branch): super().save(*args, **kwargs) @@ -231,13 +233,13 @@ class Designer(HistoricalModel): object_id=self.pk, status='applied' ).order_by('-created_at')[:5] - + active_branches = VersionBranch.objects.filter( changesets__content_type=content_type, changesets__object_id=self.pk, is_active=True ).distinct() - + return { 'latest_changes': latest_changes, 'active_branches': active_branches, diff --git a/history_tracking/custom_history.py b/history_tracking/custom_history.py new file mode 100644 index 00000000..c68a53b8 --- /dev/null +++ b/history_tracking/custom_history.py @@ -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 + ) \ No newline at end of file diff --git a/history_tracking/historical_fields.py b/history_tracking/historical_fields.py new file mode 100644 index 00000000..ec6ba7f7 --- /dev/null +++ b/history_tracking/historical_fields.py @@ -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) \ No newline at end of file diff --git a/history_tracking/models.py b/history_tracking/models.py index 0df73144..f39be298 100644 --- a/history_tracking/models.py +++ b/history_tracking/models.py @@ -1,10 +1,13 @@ from django.db import models +from django.conf import settings from django.contrib.contenttypes.models import ContentType 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 simple_history.models import HistoricalRecords 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.core.exceptions import ValidationError from django.utils import timezone @@ -13,13 +16,28 @@ T = TypeVar('T', bound=models.Model) User = get_user_model() -class HistoricalModel(models.Model): +class HistoricalModel(models.Model, HistoricalFieldsMixin): """Abstract base class for models with history tracking""" 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, - bases=(HistoricalChangeMixin,), - excluded_fields=['comments', 'photos', 'reviews'] # Exclude all generic relations + bases=[HistoricalChangeMixin], + excluded_fields=['comments', 'comment_threads', 'photos', 'reviews'], + use_base_model_db=True # Use base model's db ) class Meta: diff --git a/location/mixins.py b/location/mixins.py new file mode 100644 index 00000000..70c38928 --- /dev/null +++ b/location/mixins.py @@ -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 "" \ No newline at end of file diff --git a/media/admin.py b/media/admin.py index 17f3066a..07f4337a 100644 --- a/media/admin.py +++ b/media/admin.py @@ -1,19 +1,28 @@ from django.contrib import admin -from django.utils.html import format_html +from django.contrib.contenttypes.admin import GenericStackedInline 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) class PhotoAdmin(admin.ModelAdmin): - list_display = ('thumbnail_preview', 'content_type', 'content_object', 'caption', 'is_primary', 'created_at') - list_filter = ('content_type', 'is_primary', 'created_at') + list_display = ('caption', 'content_type', 'object_id', 'is_primary', 'created_at') + list_filter = ('content_type', 'created_at', 'is_primary', 'is_approved') 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): - if obj.image: - return format_html( - '', - obj.image.url - ) - return "No image" - thumbnail_preview.short_description = 'Thumbnail' + fieldsets = ( + ('Image', { + 'fields': ('image', 'caption', 'alt_text', 'is_primary', 'is_approved') + }), + ('Metadata', { + 'fields': ('content_type', 'object_id', 'created_at', 'updated_at'), + 'classes': ('collapse',) + }), + ) diff --git a/media/mixins.py b/media/mixins.py new file mode 100644 index 00000000..7cd82b41 --- /dev/null +++ b/media/mixins.py @@ -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() \ No newline at end of file diff --git a/memory-bank/activeContext.md b/memory-bank/activeContext.md index 6b33cfda..9c43b9d1 100644 --- a/memory-bank/activeContext.md +++ b/memory-bank/activeContext.md @@ -1,148 +1,74 @@ -# Active Development Context +# Comment System Architecture Fix -## Current Implementation Status -Version Control System has been evaluated and requires several enhancements: +## Required Code Modifications -### Completed -1. Core VCS Components: - - Base models (VersionBranch, VersionTag, ChangeSet) - - Business logic (BranchManager, ChangeTracker, MergeStrategy) - - UI components and templates - - Asset integration (JS/CSS) - - Comprehensive monitoring system - - Basic caching implementation +### 1. Central CommentThread Model (comments/models.py) +```python +from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation +from django.contrib.contenttypes.models import ContentType +from django.db import models -2. Initial Integration: - - Park model VCS integration - - ParkArea model VCS integration - - Base template VCS support - - Park detail template integration - - Version control context processor - - Monitoring and metrics collection +class CommentThread(models.Model): + """Centralized comment threading system""" + content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) + object_id = models.PositiveIntegerField() + content_object = GenericForeignKey() + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) -3. Documentation: - - Technical implementation guide - - Template integration guide - - Implementation checklist - - Base README - - API documentation - - User guide + class Meta: + indexes = [ + models.Index(fields=["content_type", "object_id"]), + ] + app_label = 'comments' +``` -### In Progress -1. Model Integration: - - [ ] Rides system - - [ ] Reviews system - - [ ] Companies system - - [ ] Batch processing implementation - - [ ] Enhanced caching layer +### 2. Model Reference Updates (Example for companies/models.py) +```python +# In all affected models (companies, rides, parks, reviews): +from comments.models import CommentThread -2. Template Updates: - - [ ] Park list view - - [ ] Ride detail/list views - - [ ] Review detail/list views - - [ ] Company detail/list views - - [ ] Performance optimized components +class Company(models.Model): + # ... existing fields ... + comments = GenericRelation(CommentThread) # Updated reference +``` -### Newly Identified Requirements -1. Performance Optimizations: - - [ ] Implement batch processing for large changesets - - [ ] Add caching for frequently accessed version history - - [ ] Optimize query patterns for large history sets +### 3. Historical Records Adjustment +```python +# Historical model definitions: +class HistoricalCompany(HistoricalRecords): + comments = models.ForeignKey( + 'comments.CommentThread', # Unified reference + on_delete=models.SET_NULL, + null=True, + blank=True + ) +``` -2. Scalability Enhancements: - - [ ] Implement archive strategy for old history records - - [ ] Add partitioning support for large history tables - - [ ] Develop async processing for heavy operations +## Migration Execution Plan -3. Security Improvements: - - [ ] Add encryption for sensitive changes - - [ ] Enhance access control granularity - - [ ] Implement audit logging improvements +1. Generate initial comment thread migration: +```bash +./manage.py makemigrations comments --name create_commentthread +``` -## Immediate Next Steps -1. Performance Optimization (Priority) - ```python - # Add to history_tracking/batch.py: - class BatchChangeProcessor: - def process_changes(self, changes): - """Process multiple changes efficiently""" - with transaction.atomic(): - # Batch processing logic - ``` +2. Create dependent migrations for each modified app: +```bash +for app in companies rides parks reviews; do + ./manage.py makemigrations $app --name update_comment_references +done +``` -2. Caching Enhancement - ```python - # Add to history_tracking/caching.py: - class VersionHistoryCache: - def cache_version_info(self): - """Cache frequently accessed version data""" - # Caching implementation - ``` +3. Migration dependency chain: +```python +# In each app's migration file: +dependencies = [ + ('comments', '0001_create_commentthread'), +] +``` -3. Testing Expansion - - Add performance benchmarks - - Implement stress testing - - Create scalability tests - -## 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`. \ No newline at end of file +## Validation Checklist +- [ ] Run full test suite: `uv test ./manage.py test` +- [ ] Execute system check: `uv run ./manage.py check --deploy` +- [ ] Verify database schema changes in migration files +- [ ] Confirm admin interface comment relationships diff --git a/memory-bank/evaluations/historical_model_comment_fixes.md b/memory-bank/evaluations/historical_model_comment_fixes.md new file mode 100644 index 00000000..6d9d8709 --- /dev/null +++ b/memory-bank/evaluations/historical_model_comment_fixes.md @@ -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 \ No newline at end of file diff --git a/parks/models.py b/parks/models.py index 05d0e0db..a8cbf732 100644 --- a/parks/models.py +++ b/parks/models.py @@ -1,22 +1,26 @@ from django.db import models from django.urls import reverse from django.utils.text import slugify -from django.contrib.contenttypes.fields import GenericRelation from django.core.exceptions import ValidationError from decimal import Decimal, ROUND_DOWN, InvalidOperation from typing import Tuple, Optional, Any, TYPE_CHECKING +from django.contrib.contenttypes.fields import GenericRelation from companies.models import Company from history_tracking.signals import get_current_branch from media.models import Photo from history_tracking.models import HistoricalModel from location.models import Location +from comments.mixins import CommentableMixin +from media.mixins import PhotoableModel +from location.mixins import LocationMixin if TYPE_CHECKING: 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 STATUS_CHOICES = [ ("OPERATING", "Operating"), @@ -34,9 +38,6 @@ class Park(HistoricalModel): max_length=20, choices=STATUS_CHOICES, default="OPERATING" ) - # Location fields using GenericRelation - location = GenericRelation(Location, related_query_name='park') - # Details opening_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( 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 + rides: models.Manager['Ride'] # Type hint for reverse relation from rides app # Metadata @@ -71,6 +68,7 @@ class Park(HistoricalModel): class Meta: ordering = ["name"] + excluded_fields = ['comments'] # Exclude from historical tracking def __str__(self) -> str: return self.name @@ -126,23 +124,6 @@ class Park(HistoricalModel): def get_absolute_url(self) -> str: 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 def get_by_slug(cls, slug: str) -> Tuple['Park', bool]: """Get park by current or historical slug""" @@ -159,7 +140,8 @@ class Park(HistoricalModel): 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 park = models.ForeignKey(Park, on_delete=models.CASCADE, related_name="areas") name = models.CharField(max_length=255) @@ -169,10 +151,6 @@ class ParkArea(HistoricalModel): closing_date = models.DateField(null=True, blank=True) # Relationships - comments = GenericRelation('comments.CommentThread', - related_name='park_area_threads', - related_query_name='comments_thread' - ) # Metadata created_at = models.DateTimeField(auto_now_add=True, null=True) @@ -181,6 +159,7 @@ class ParkArea(HistoricalModel): class Meta: ordering = ["name"] unique_together = ["park", "slug"] + excluded_fields = ['comments'] # Exclude from historical tracking def __str__(self) -> str: return f"{self.name} at {self.park.name}" diff --git a/requirements.txt b/requirements.txt index 74ddc462..3fa76454 100644 --- a/requirements.txt +++ b/requirements.txt @@ -45,3 +45,4 @@ daphne==4.1.2 # React and Material UI will be handled via npm in the frontend directory django-simple-history==3.8.0 django-tailwind-cli==2.21.1 +celery==5.3.6 diff --git a/reviews/admin.py b/reviews/admin.py index 8176cd43..1c1a3b93 100644 --- a/reviews/admin.py +++ b/reviews/admin.py @@ -1,11 +1,7 @@ from django.contrib import admin from django.utils.html import format_html -from .models import Review, ReviewImage, ReviewLike, ReviewReport - -class ReviewImageInline(admin.TabularInline): - model = ReviewImage - extra = 1 - fields = ('image', 'caption', 'order') +from media.admin import PhotoInline +from .models import Review, ReviewLike, ReviewReport @admin.register(Review) class ReviewAdmin(admin.ModelAdmin): @@ -14,7 +10,7 @@ class ReviewAdmin(admin.ModelAdmin): search_fields = ('user__username', 'content', 'title') readonly_fields = ('created_at', 'updated_at') actions = ['publish_reviews', 'unpublish_reviews'] - inlines = [ReviewImageInline] + inlines = [PhotoInline] fieldsets = ( ('Review Details', { @@ -55,13 +51,6 @@ class ReviewAdmin(admin.ModelAdmin): queryset.update(is_published=False) 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) class ReviewLikeAdmin(admin.ModelAdmin): list_display = ('review', 'user', 'created_at') diff --git a/reviews/mixins.py b/reviews/mixins.py new file mode 100644 index 00000000..e0c1da89 --- /dev/null +++ b/reviews/mixins.py @@ -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() \ No newline at end of file diff --git a/reviews/models.py b/reviews/models.py index 812a3cf6..7f4b3c3b 100644 --- a/reviews/models.py +++ b/reviews/models.py @@ -5,8 +5,11 @@ from django.contrib.contenttypes.models import ContentType from django.core.validators import MinValueValidator, MaxValueValidator from history_tracking.models import HistoricalModel, VersionBranch, ChangeSet 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) content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) object_id = models.PositiveIntegerField() @@ -41,17 +44,13 @@ class Review(HistoricalModel): ) moderated_at = models.DateTimeField(null=True, blank=True) - # Comments - comments = GenericRelation('comments.CommentThread', - related_name='review_threads', - related_query_name='comments_thread' - ) class Meta: ordering = ['-created_at'] indexes = [ models.Index(fields=['content_type', 'object_id']), ] + excluded_fields = ['comments'] # Exclude from historical tracking def __str__(self): 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 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): review = models.ForeignKey( Review, diff --git a/rides/models.py b/rides/models.py index f52305bf..9efb2813 100644 --- a/rides/models.py +++ b/rides/models.py @@ -1,10 +1,13 @@ from django.db import models from django.urls import reverse from django.utils.text import slugify -from django.contrib.contenttypes.fields import GenericRelation 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.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 @@ -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. For example: B&M Dive Coaster, Vekoma Boomerang, etc. @@ -41,14 +45,12 @@ class RideModel(HistoricalModel): ) created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) - comments = GenericRelation('comments.CommentThread', - related_name='ride_model_threads', - related_query_name='comments_thread' - ) + class Meta: ordering = ['manufacturer', 'name'] unique_together = ['manufacturer', 'name'] + excluded_fields = ['comments'] # Exclude from historical tracking def __str__(self) -> str: 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 = [ ('OPERATING', 'Operating'), ('SBNO', 'Standing But Not Operating'), @@ -181,16 +184,11 @@ class Ride(HistoricalModel): ) created_at = models.DateTimeField(auto_now_add=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: ordering = ['name'] unique_together = ['park', 'slug'] + excluded_fields = ['comments'] # Exclude from historical tracking def __str__(self) -> str: return f"{self.name} at {self.park.name}"