From 4ebf126abc5e2fdf52f96d8cd9ff48c64a9ca337 Mon Sep 17 00:00:00 2001 From: ndu Date: Wed, 11 Dec 2024 14:08:56 +0100 Subject: [PATCH 1/3] chore(fix): add fix --- server/apps/research/models/article.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index f75f91c..7f33860 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -241,7 +241,7 @@ def save(self, *args, **kwargs): if self.from_article.pk: RelatedArticle.objects.select_for_update().filter( from_article=self.from_article - ) + ).exists() self.clean() super().save(*args, **kwargs) From 50264f0b6616ba660b13343462b52247a8869d63 Mon Sep 17 00:00:00 2001 From: ndu Date: Wed, 11 Dec 2024 15:26:44 +0100 Subject: [PATCH 2/3] Revert "chore(fix): fix related articles logic" --- server/apps/research/admin/article_admin.py | 34 ++++------ server/apps/research/models/article.py | 63 ++++--------------- .../serializers/article_serializer.py | 19 ++---- 3 files changed, 32 insertions(+), 84 deletions(-) diff --git a/server/apps/research/admin/article_admin.py b/server/apps/research/admin/article_admin.py index 689ec34..e4b13fa 100644 --- a/server/apps/research/admin/article_admin.py +++ b/server/apps/research/admin/article_admin.py @@ -18,23 +18,24 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): if db_field.name == 'to_article': # Get the parent object (Article) from the request obj_id = request.resolver_match.kwargs.get('object_id') - # For new articles (when obj_id is None), show all ready articles - base_queryset = Article.objects.filter(status='ready') + parent_obj = None if obj_id: try: parent_obj = Article.objects.get(pk=obj_id) - # Exclude self-reference and articles that already have a relationship - base_queryset = base_queryset.exclude( - id=parent_obj.id - ).exclude( - related_from__to_article=parent_obj - ) except Article.DoesNotExist: pass + base_queryset = Article.objects.filter(status='ready') + if parent_obj: + # Exclude self-reference and articles that already have a relationship + base_queryset = base_queryset.exclude( + id=parent_obj.id + ).exclude( + related_from__to_article=parent_obj + ) kwargs['queryset'] = base_queryset - return super().formfield_for_foreignkey(db_field, request, **kwargs) + return super().formfield_for_foreignkey(db_field, request, **kwargs) class ArticleForm(forms.ModelForm): class Meta: model = Article @@ -55,18 +56,9 @@ def current_slug_history(self, obj): current_slug_history.short_description = 'Slug Change History' def get_inlines(self, request, obj): - # Allow inlines for both new and existing articles - return [RelatedArticleInline] - - def save_related(self, request, form, formsets, change): - """Handle saving related articles for both new and existing articles.""" - super().save_related(request, form, formsets, change) - - # Process related articles from inline formsets - for formset in formsets: - if isinstance(formset, RelatedArticleInline): - # The related articles will be saved automatically through the formset - pass + if obj is None: + return [] + return super().get_inlines(request, obj) fieldsets = [ ('Article Details', {'fields': ['title', 'slug', 'authors', 'acknowledgement', 'categories', 'thumb', 'content', 'summary', 'status', 'scheduled_publish_time']}), diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index 7f33860..765a2be 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -119,14 +119,6 @@ def build_table_of_contents(self): def save(self, *args, **kwargs): """Override the save method to generate a unique slug and build table of contents.""" - is_new = self.pk is None - temp_related_articles = [] - - # If this is a new article and there are related articles in the form - if is_new and hasattr(self, "_temp_related_articles"): - temp_related_articles = self._temp_related_articles - delattr(self, "_temp_related_articles") - if not self.slug or self.title_update(): self.slug = self.generate_unique_slug() @@ -158,24 +150,7 @@ def save(self, *args, **kwargs): ): self.status = "ready" - with transaction.atomic(): - super().save(*args, **kwargs) - - # If this was a new article and we had temporary related articles - if is_new and temp_related_articles: - for related_article in temp_related_articles: - RelatedArticle.objects.create( - from_article=self, to_article=related_article - ) - - def set_temp_related_articles(self, related_articles): - """ - Store related articles temporarily before the initial save. - - Args: - related_articles: List of Article instances to be related - """ - self._temp_related_articles = related_articles + super().save(*args, **kwargs) def generate_unique_slug(self): """Generate a unique slug for the article.""" @@ -199,7 +174,7 @@ def title_update(self): class RelatedArticle(models.Model): """Through model for related articles to prevent circular references.""" - + from_article = models.ForeignKey( Article, on_delete=models.CASCADE, related_name="related_from" ) @@ -216,35 +191,23 @@ class Meta: name="prevent_self_reference", ) ] - + def clean(self): # Prevent direct circular references - if ( - self.from_article.pk - and RelatedArticle.objects.filter( - from_article=self.to_article, to_article=self.from_article - ).exists() - ): + if RelatedArticle.objects.filter( + from_article=self.to_article, to_article=self.from_article + ).exists(): raise ValidationError("Circular references detected.") - + # Maximum of 3 related articles - if ( - self.from_article.pk - and RelatedArticle.objects.filter(from_article=self.from_article).count() - >= 3 - ): + if RelatedArticle.objects.filter(from_article=self.from_article).count() >= 3: raise ValidationError("Maximum of 3 related articles allowed.") - + def save(self, *args, **kwargs): - with transaction.atomic(): - # Only acquire lock if from_article exists in database - if self.from_article.pk: - RelatedArticle.objects.select_for_update().filter( - from_article=self.from_article - ).exists() - self.clean() - super().save(*args, **kwargs) - + # Acquire a lock on related articles for this from_article + RelatedArticle.objects.select_for_update().filter(from_article=self.from_article) + self.clean() + super().save(*args, **kwargs) class ArticleSlugHistory(models.Model): """Model to track historical slugs for articles.""" diff --git a/server/apps/research/serializers/article_serializer.py b/server/apps/research/serializers/article_serializer.py index 451d60c..fd7373a 100644 --- a/server/apps/research/serializers/article_serializer.py +++ b/server/apps/research/serializers/article_serializer.py @@ -53,16 +53,10 @@ class Meta: model = Article fields = ['title', 'slug', 'categories', 'thumb', 'content', 'summary', 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', 'is_sponsored', 'sponsor_color', 'sponsor_text_color', 'related_article_ids'] - def validate_related_article_ids(self, value): - """Validate related articles.""" + def validate_related_articles_ids(self, value): + """Validate related articles""" if len(value) > 3: raise serializers.ValidationError("You can only have a maximum of 3 related articles.") - - # Check for self-reference - instance = self.instance - if instance and instance.id in [article.id for article in value]: - raise serializers.ValidationError("An article cannot be related to itself.") - return value def create(self, validated_data: dict) -> Article: @@ -78,19 +72,18 @@ def create(self, validated_data: dict) -> Article: if user_author: authors = [user_author] - # Create and save the article first - article = Article.objects.create(**validated_data) + article = Article(**validated_data) + article.save() if authors: article.authors.set(authors) if categories: article.categories.set(categories) - # Handle related articles after the article is created + # Handle related articles for related_article in related_article_ids: RelatedArticle.objects.create( - from_article=article, - to_article=related_article + from_article=article, to_article=related_article ) return article From c7bff5a1c7dcea8620efa7ef935ce8b5c1f443d9 Mon Sep 17 00:00:00 2001 From: ndu Date: Wed, 11 Dec 2024 15:27:27 +0100 Subject: [PATCH 3/3] Revert "chore(feat): add relatedArticles logic" --- server/apps/research/admin/article_admin.py | 39 +---- ...ticle_article_related_articles_and_more.py | 36 ----- server/apps/research/models/__init__.py | 2 +- server/apps/research/models/article.py | 151 ++++-------------- .../serializers/article_serializer.py | 54 +------ 5 files changed, 45 insertions(+), 237 deletions(-) delete mode 100644 server/apps/research/migrations/0015_relatedarticle_article_related_articles_and_more.py diff --git a/server/apps/research/admin/article_admin.py b/server/apps/research/admin/article_admin.py index e4b13fa..352ae59 100644 --- a/server/apps/research/admin/article_admin.py +++ b/server/apps/research/admin/article_admin.py @@ -1,41 +1,11 @@ from django.contrib import admin from django import forms -from apps.research.models import Article, ArticleSlugHistory, RelatedArticle +from apps.research.models import Article, ArticleSlugHistory from tinymce.widgets import TinyMCE from .slug_history import current_slug_history - -class RelatedArticleInline(admin.TabularInline): - model = RelatedArticle - fk_name = 'from_article' - extra = 1 - max_num = 3 - verbose_name = 'Related Article' - verbose_name_plural = 'Related Articles' - def formfield_for_foreignkey(self, db_field, request, **kwargs): - if db_field.name == 'to_article': - # Get the parent object (Article) from the request - obj_id = request.resolver_match.kwargs.get('object_id') - parent_obj = None - - if obj_id: - try: - parent_obj = Article.objects.get(pk=obj_id) - except Article.DoesNotExist: - pass - - base_queryset = Article.objects.filter(status='ready') - if parent_obj: - # Exclude self-reference and articles that already have a relationship - base_queryset = base_queryset.exclude( - id=parent_obj.id - ).exclude( - related_from__to_article=parent_obj - ) - kwargs['queryset'] = base_queryset - return super().formfield_for_foreignkey(db_field, request, **kwargs) class ArticleForm(forms.ModelForm): class Meta: model = Article @@ -49,17 +19,10 @@ def __init__(self, *args, **kwargs): class ArticleAdmin(admin.ModelAdmin): """Admin interface for the Article model.""" form = ArticleForm - inlines = [RelatedArticleInline] - def current_slug_history(self, obj): return current_slug_history(obj) current_slug_history.short_description = 'Slug Change History' - def get_inlines(self, request, obj): - if obj is None: - return [] - return super().get_inlines(request, obj) - fieldsets = [ ('Article Details', {'fields': ['title', 'slug', 'authors', 'acknowledgement', 'categories', 'thumb', 'content', 'summary', 'status', 'scheduled_publish_time']}), ('Sponsorship Details', {'fields': ['is_sponsored', 'sponsor_color', 'sponsor_text_color']}), diff --git a/server/apps/research/migrations/0015_relatedarticle_article_related_articles_and_more.py b/server/apps/research/migrations/0015_relatedarticle_article_related_articles_and_more.py deleted file mode 100644 index b7c0147..0000000 --- a/server/apps/research/migrations/0015_relatedarticle_article_related_articles_and_more.py +++ /dev/null @@ -1,36 +0,0 @@ -# Generated by Django 5.0.8 on 2024-12-11 05:54 - -import django.db.models.deletion -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('research', '0014_alter_article_authors'), - ] - - operations = [ - migrations.CreateModel( - name='RelatedArticle', - fields=[ - ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('created_at', models.DateTimeField(auto_now_add=True)), - ('from_article', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='related_from', to='research.article')), - ('to_article', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='related_to', to='research.article')), - ], - ), - migrations.AddField( - model_name='article', - name='related_articles', - field=models.ManyToManyField(blank=True, related_name='referenced_by', through='research.RelatedArticle', to='research.article'), - ), - migrations.AddConstraint( - model_name='relatedarticle', - constraint=models.CheckConstraint(check=models.Q(('from_article', models.F('to_article')), _negated=True), name='prevent_self_reference'), - ), - migrations.AlterUniqueTogether( - name='relatedarticle', - unique_together={('from_article', 'to_article')}, - ), - ] diff --git a/server/apps/research/models/__init__.py b/server/apps/research/models/__init__.py index 2cf7bc8..3737c10 100644 --- a/server/apps/research/models/__init__.py +++ b/server/apps/research/models/__init__.py @@ -1,3 +1,3 @@ from .category import Category from .author import Author -from .article import Article, ArticleSlugHistory, RelatedArticle \ No newline at end of file +from .article import Article, ArticleSlugHistory \ No newline at end of file diff --git a/server/apps/research/models/article.py b/server/apps/research/models/article.py index 765a2be..72cca9d 100644 --- a/server/apps/research/models/article.py +++ b/server/apps/research/models/article.py @@ -1,6 +1,5 @@ from django.db import models from django.utils.text import slugify -from django.core.exceptions import ValidationError from apps.common.models import BaseModel from apps.research.managers import ArticleObjects from .category import Category @@ -13,41 +12,28 @@ import uuid from django.db import transaction - def get_default_thumb(): return f"{settings.MEDIA_URL}images/2077-Collective.png" - class Article(BaseModel): """Model for articles.""" - + options = ( - ("draft", "Draft"), - ("ready", "Ready"), + ('draft', 'Draft'), + ('ready', 'Ready'), ) title = models.TextField() content = HTMLField(blank=True, null=True) summary = models.TextField(blank=True) acknowledgement = HTMLField(blank=True, null=True) - authors = models.ManyToManyField(Author, blank=True, related_name="articles") + authors = models.ManyToManyField(Author, blank=True, related_name='articles') slug = models.SlugField(max_length=255, blank=True, db_index=True) - categories = models.ManyToManyField(Category, blank=True, related_name="articles") - related_articles = models.ManyToManyField( - "self", - blank=True, - symmetrical=False, - related_name="referenced_by", - through="RelatedArticle", - ) - thumb = models.ImageField( - upload_to="images/", default=get_default_thumb, blank=True - ) + categories = models.ManyToManyField(Category, blank=True, related_name='articles') + thumb = models.ImageField(upload_to='images/', default=get_default_thumb, blank=True) views = models.PositiveBigIntegerField(default=0) - status = models.CharField( - max_length=10, choices=options, default="draft", db_index=True - ) - scheduled_publish_time = models.DateTimeField(null=True, blank=True, db_index=True) + status = models.CharField(max_length=10, choices=options, default='draft', db_index=True) + scheduled_publish_time = models.DateTimeField(null=True, blank=True, db_index=True) table_of_contents = models.JSONField(default=list, blank=True) is_sponsored = models.BooleanField(default=False) sponsor_color = models.CharField(max_length=7, default="#FF0420") @@ -57,31 +43,8 @@ class Article(BaseModel): post_objects = ArticleObjects() class Meta: - ordering = ("-scheduled_publish_time",) - - def get_related_articles(self): - """ - Get manually selected related articles for this article. - If none exist, fallback to default frontend logic - """ - - manual_related_articles = self.related_articles.filter(status="ready").order_by( - "-scheduled_publish_time" - )[:3] - - if manual_related_articles.exists(): - return manual_related_articles - - # Fallback to last 3 articles in the same category - - category_articles = ( - Article.objects.filter(categories__in=self.categories.all(), status="ready") - .exclude(id=self.id) - .order_by("-scheduled_publish_time") - .distinct()[:3] - ) - return category_articles - + ordering = ('-scheduled_publish_time',) + def calculate_min_read(self): word_count = len(self.content.split()) words_per_minute = 300 # Average reading speed (words per minute) @@ -95,24 +58,24 @@ def __str__(self): def build_table_of_contents(self): """Build the table of contents from the article content.""" - soup = BeautifulSoup(self.content, "html.parser") - headers = soup.find_all(["h1", "h2", "h3"]) - + soup = BeautifulSoup(self.content, 'html.parser') + headers = soup.find_all(['h1', 'h2', 'h3']) + toc = [] - stack = [{"level": 0, "children": toc}] + stack = [{'level': 0, 'children': toc}] for header in headers: level = int(header.name[1]) title = header.get_text() - header["id"] = slugify(title) + header['id'] = slugify(title) - while level <= stack[-1]["level"]: + while level <= stack[-1]['level']: stack.pop() - new_item = {"title": title, "id": header["id"], "children": []} - - stack[-1]["children"].append(new_item) - stack.append({"level": level, "children": new_item["children"]}) + new_item = {'title': title, 'id': header['id'], 'children': []} + + stack[-1]['children'].append(new_item) + stack.append({'level': level, 'children': new_item['children']}) self.table_of_contents = toc self.content = str(soup) @@ -129,26 +92,23 @@ def save(self, *args, **kwargs): # Generate new slug first if not self.slug or self.title_update(): self.slug = self.generate_unique_slug() - + # Then check if we need to create slug history if old_instance.slug and old_instance.slug != self.slug: # Only create history if the slug actually changed and isn't empty with transaction.atomic(): ArticleSlugHistory.objects.create( - article=self, old_slug=old_instance.slug + article=self, + old_slug=old_instance.slug ) except Article.DoesNotExist: - pass - + pass + if self.content: self.build_table_of_contents() - - if ( - self.scheduled_publish_time - and self.status == "draft" - and timezone.now() >= self.scheduled_publish_time - ): - self.status = "ready" + + if self.scheduled_publish_time and self.status == 'draft' and timezone.now() >= self.scheduled_publish_time: + self.status = 'ready' super().save(*args, **kwargs) @@ -162,69 +122,28 @@ def generate_unique_slug(self): slug = f"{base_slug}-{num}" num += 1 return slug - + def title_update(self): """Check if the title has changed.""" if self.pk: # Only check if the article exists - original = Article.objects.filter(pk=self.pk).only("title").first() + original = Article.objects.filter(pk=self.pk).only('title').first() if original: return original.title != self.title return False - -class RelatedArticle(models.Model): - """Through model for related articles to prevent circular references.""" - - from_article = models.ForeignKey( - Article, on_delete=models.CASCADE, related_name="related_from" - ) - to_article = models.ForeignKey( - Article, on_delete=models.CASCADE, related_name="related_to" - ) - created_at = models.DateTimeField(auto_now_add=True) - - class Meta: - unique_together = ("from_article", "to_article") - constraints = [ - models.CheckConstraint( - check=~models.Q(from_article=models.F("to_article")), - name="prevent_self_reference", - ) - ] - - def clean(self): - # Prevent direct circular references - if RelatedArticle.objects.filter( - from_article=self.to_article, to_article=self.from_article - ).exists(): - raise ValidationError("Circular references detected.") - - # Maximum of 3 related articles - if RelatedArticle.objects.filter(from_article=self.from_article).count() >= 3: - raise ValidationError("Maximum of 3 related articles allowed.") - - def save(self, *args, **kwargs): - # Acquire a lock on related articles for this from_article - RelatedArticle.objects.select_for_update().filter(from_article=self.from_article) - self.clean() - super().save(*args, **kwargs) - class ArticleSlugHistory(models.Model): """Model to track historical slugs for articles.""" - id = models.AutoField(primary_key=True) - article = models.ForeignKey( - "Article", on_delete=models.CASCADE, related_name="slug_history" - ) + article = models.ForeignKey('Article', on_delete=models.CASCADE, related_name='slug_history') old_slug = models.SlugField(max_length=255, db_index=True) created_at = models.DateTimeField(auto_now_add=True) class Meta: - unique_together = ("article", "old_slug") + unique_together = ('article', 'old_slug') indexes = [ - models.Index(fields=["old_slug"]), + models.Index(fields=['old_slug']), ] - db_table = "research_articleslughistory" # explicitly set table name + db_table = 'research_articleslughistory' # explicitly set table name def __str__(self): - return f"{self.old_slug} -> {self.article.slug}" + return f"{self.old_slug} -> {self.article.slug}" \ No newline at end of file diff --git a/server/apps/research/serializers/article_serializer.py b/server/apps/research/serializers/article_serializer.py index fd7373a..2bf8989 100644 --- a/server/apps/research/serializers/article_serializer.py +++ b/server/apps/research/serializers/article_serializer.py @@ -1,5 +1,5 @@ from rest_framework import serializers -from ..models import Article, Author, Category, RelatedArticle +from ..models import Article, Author, Category from .author_serializer import AuthorSerializer from .category_serializer import CategorySerializer @@ -9,17 +9,11 @@ class ArticleListSerializer(serializers.ModelSerializer): class Meta: model = Article - fields = ['id', 'title', 'slug', 'categories', 'authors'] - -class RelatedArticleSerializer(serializers.ModelSerializer): - id = serializers.PrimaryKeyRelatedField(source='to_article.id', read_only=True) - title = serializers.CharField(source='to_article.title', read_only=True) - slug = serializers.CharField(source='to_article.slug', read_only=True) - thumb = serializers.ImageField(source='to_article.thumb', read_only=True) - - class Meta: - model = RelatedArticle - fields = ['id', 'title', 'slug', 'thumb'] + include = ['categories' 'authors'] + exclude = [ + 'content', 'scheduled_publish_time', 'acknowledgement', + 'status', 'views', 'created_at', 'updated_at', 'table_of_contents' + ] class ArticleSerializer(serializers.ModelSerializer): """Serializer for the Article model.""" @@ -27,7 +21,6 @@ class ArticleSerializer(serializers.ModelSerializer): categories = CategorySerializer(many=True) views = serializers.ReadOnlyField() min_read = serializers.ReadOnlyField() - related_articles = serializers.SerializerMethodField() class Meta: model = Article @@ -35,36 +28,23 @@ class Meta: 'id', 'slug', 'title', 'authors', 'thumb', 'categories', 'summary', 'acknowledgement', 'content', 'min_read', 'status', 'views', 'created_at', 'updated_at', 'scheduled_publish_time', 'table_of_contents', - 'is_sponsored', 'sponsor_color', 'sponsor_text_color', 'related_articles' + 'is_sponsored', 'sponsor_color', 'sponsor_text_color' ] - - def get_related_articles(self, obj): - """Return the related articles for the article.""" - related_articles = obj.get_related_articles() - return ArticleListSerializer(related_articles, many=True).data class ArticleCreateUpdateSerializer(serializers.ModelSerializer): """Serializer for creating and updating articles.""" authors = serializers.PrimaryKeyRelatedField(queryset=Author.objects.all(), many=True, required=False) categories = serializers.PrimaryKeyRelatedField(queryset=Category.objects.all(), many=True, required=False) - related_article_ids = serializers.PrimaryKeyRelatedField(queryset=Article.objects.filter(status='ready'), many=True, required=False, write_only=True) class Meta: model = Article - fields = ['title', 'slug', 'categories', 'thumb', 'content', 'summary', 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', 'is_sponsored', 'sponsor_color', 'sponsor_text_color', 'related_article_ids'] - - def validate_related_articles_ids(self, value): - """Validate related articles""" - if len(value) > 3: - raise serializers.ValidationError("You can only have a maximum of 3 related articles.") - return value + fields = ['title', 'slug', 'categories', 'thumb', 'content', 'summary', 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', 'is_sponsored', 'sponsor_color', 'sponsor_text_color'] def create(self, validated_data: dict) -> Article: """Create a new article instance.""" request = self.context.get('request') authors = validated_data.pop('authors', []) categories = validated_data.pop('categories', []) - related_article_ids = validated_data.pop('related_article_ids', []) try: if not authors and request and hasattr(request, 'user'): @@ -79,12 +59,6 @@ def create(self, validated_data: dict) -> Article: article.authors.set(authors) if categories: article.categories.set(categories) - - # Handle related articles - for related_article in related_article_ids: - RelatedArticle.objects.create( - from_article=article, to_article=related_article - ) return article except Exception as e: @@ -95,8 +69,6 @@ def update(self, instance: Article, validated_data: dict) -> Article: """Update an existing article instance.""" authors = validated_data.pop('authors', []) categories = validated_data.pop('categories', []) - related_article_ids = validated_data.pop('related_article_ids', None) - try: instance = super().update(instance, validated_data) @@ -104,16 +76,6 @@ def update(self, instance: Article, validated_data: dict) -> Article: instance.authors.set(authors) if categories: instance.categories.set(categories) - - # Update related articles if provided - if related_article_ids is not None: - # Clear existing relations - RelatedArticle.objects.filter(from_article=instance).delete() - # Create new relations - for related_article in related_article_ids: - RelatedArticle.objects.create( - from_article=instance, to_article=related_article - ) return instance except Exception as e: